Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#puppethack] Adding replace attribute to file_line #494

Merged
merged 1 commit into from
Jul 30, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ All parameters are optional, unless otherwise noted.
* `multiple`: Determines if `match` and/or `after` can change multiple lines. If set to false, an exception will be raised if more than one line matches. Valid options: 'true', 'false'. Default: Undefined.
* `name`: Sets the name to use as the identity of the resource. This is necessary if you want the resource namevar to differ from the supplied `title` of the resource. Valid options: String. Default: Undefined.
* `path`: **Required.** Defines the file in which Puppet will ensure the line specified by `line`. Must be an absolute path to the file.
* `replace`: Defines whether the resource will overwrite an existing line that matches the `match` parameter. If set to false and a line is found matching the `match` param, the line will not be placed in the file. Valid options: true, false, yes, no. Default: true


### Functions
Expand Down
31 changes: 20 additions & 11 deletions lib/puppet/provider/file_line/ruby.rb
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
Puppet::Type.type(:file_line).provide(:ruby) do
def exists?
lines.find do |line|
line.chomp == resource[:line].chomp
if !resource[:replace] and count_matches(match_regex) > 0
true
else
lines.find do |line|
line.chomp == resource[:line].chomp
end
end
end

def create
if resource[:match]
handle_create_with_match
elsif resource[:after]
handle_create_with_after
else
append_line
unless !resource[:replace] and count_matches(match_regex) > 0
if resource[:match]
handle_create_with_match
elsif resource[:after]
handle_create_with_after
else
append_line
end
end
end

Expand All @@ -32,18 +38,21 @@ def lines
@lines ||= File.readlines(resource[:path])
end

def match_regex
resource[:match] ? Regexp.new(resource[:match]) : nil
end

def handle_create_with_match()
regex = resource[:match] ? Regexp.new(resource[:match]) : nil
regex_after = resource[:after] ? Regexp.new(resource[:after]) : nil
match_count = count_matches(regex)
match_count = count_matches(match_regex)

if match_count > 1 && resource[:multiple].to_s != 'true'
raise Puppet::Error, "More than one line in file '#{resource[:path]}' matches pattern '#{resource[:match]}'"
end

File.open(resource[:path], 'w') do |fh|
lines.each do |l|
fh.puts(regex.match(l) ? resource[:line] : l)
fh.puts(match_regex.match(l) ? resource[:line] : l)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If match_regex returns nil... this will result in a NoMethodError

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second look, given this code path, you should be able to assume that match_regex is not nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the handle_create_with_match function will not be used to write the line unless resource[:match] is not nil. So the creation of the regex that match_regex returns should not fail in this block.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it also isn't a new behavior with this patch. The line only changes because where the Regexp is created is moved and renamed from regex to match_regex

if (match_count == 0 and regex_after)
if regex_after.match(l)
fh.puts(resource[:line])
Expand Down
6 changes: 6 additions & 0 deletions lib/puppet/type/file_line.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
require 'puppet/parameter/boolean'
Puppet::Type.newtype(:file_line) do

desc <<-EOT
Expand Down Expand Up @@ -78,6 +79,11 @@
end
end

newparam(:replace, :boolean => true, :parent => Puppet::Parameter::Boolean) do
desc 'If true, replace line that matches. If false, do not write line if a match is found'
defaultto true
end

# Autorequire the file resource if it's being managed
autorequire(:file) do
self[:path]
Expand Down
51 changes: 51 additions & 0 deletions spec/unit/puppet/provider/file_line/ruby_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,58 @@
expect(File.read(tmpfile).chomp).to eq('foo')
end
end
context 'when using replace' do
before :each do
# TODO: these should be ported over to use the PuppetLabs spec_helper
# file fixtures once the following pull request has been merged:
# https://github.com/puppetlabs/puppetlabs-stdlib/pull/73/files
tmp = Tempfile.new('tmp')
@tmpfile = tmp.path
tmp.close!
@resource = Puppet::Type::File_line.new(
{
:name => 'foo',
:path => @tmpfile,
:line => 'foo = bar',
:match => '^foo\s*=.*$',
:replace => false,
}
)
@provider = provider_class.new(@resource)
end

it 'should not replace the matching line' do
File.open(@tmpfile, 'w') do |fh|
fh.write("foo1\nfoo=blah\nfoo2\nfoo3")
end
expect(@provider.exists?).to be_truthy
@provider.create
expect(File.read(@tmpfile).chomp).to eql("foo1\nfoo=blah\nfoo2\nfoo3")
end

it 'should append the line if no matches are found' do
File.open(@tmpfile, 'w') do |fh|
fh.write("foo1\nfoo2")
end
expect(@provider.exists?).to be_nil
@provider.create
expect(File.read(@tmpfile).chomp).to eql("foo1\nfoo2\nfoo = bar")
end

it 'should raise an error with invalid values' do
expect {
@resource = Puppet::Type::File_line.new(
{
:name => 'foo',
:path => @tmpfile,
:line => 'foo = bar',
:match => '^foo\s*=.*$',
:replace => 'asgadga',
}
)
}.to raise_error(Puppet::Error, /Invalid value "asgadga"\. Valid values are true, false, yes, no\./)
end
end
context "when matching" do
before :each do
# TODO: these should be ported over to use the PuppetLabs spec_helper
Expand Down
3 changes: 3 additions & 0 deletions spec/unit/puppet/type/file_line_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@
it 'should default to ensure => present' do
expect(file_line[:ensure]).to eq :present
end
it 'should default to replace => true' do
expect(file_line[:replace]).to eq true
end

it "should autorequire the file it manages" do
catalog = Puppet::Resource::Catalog.new
Expand Down