diff --git a/manifests/fragment.pp b/manifests/fragment.pp index 5a3e310..fabc359 100644 --- a/manifests/fragment.pp +++ b/manifests/fragment.pp @@ -1,49 +1,55 @@ # == Define: concat::fragment # # Creates a file_fragment in the catalogue # # === Options: # # [*target*] # The file that these fragments belong to # [*content*] # If present puts the content into the file # [*source*] # If content was not specified, use the source # [*order*] # By default all files gets a 10_ prefix in the directory you can set it to # anything else using this to influence the order of the content in the file # define concat::fragment( $target, + $ensure = undef, $content = undef, $source = undef, $order = '10', ) { validate_string($target) + + if $ensure != undef { + warning('The $ensure parameter to concat::fragment is deprecated and has no effect.') + } + validate_string($content) if !(is_string($source) or is_array($source)) { fail('$source is not a string or an Array.') } if !(is_string($order) or is_integer($order)) { fail('$order is not a string or integer.') } elsif (is_string($order) and $order =~ /[:\n\/]/) { fail("Order cannot contain '/', ':', or '\n'.") } if ! ($content or $source) { crit('No content, source or symlink specified') } elsif ($content and $source) { fail("Can't use 'source' and 'content' at the same time") } $safe_target_name = regsubst($target, '[/:\n\s]', '_', 'GM') file_fragment { $name: tag => $safe_target_name, order => $order, content => $content, source => $source, } } diff --git a/manifests/init.pp b/manifests/init.pp index 9c3c21d..981a5c2 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -1,118 +1,123 @@ # == Define: concat # # Sets up so that you can use fragments to build a final config file, # # === Options: # # [*ensure*] # Present/Absent # [*path*] # The path to the final file. Use this in case you want to differentiate # between the name of a resource and the file path. Note: Use the name you # provided in the target of your fragments. # [*owner*] # Who will own the file # [*group*] # Who will own the file # [*mode*] # The mode of the final file # [*warn_header*] # Adds a normal shell style comment top of the file indicating that it is # built by puppet # [*backup*] # Controls the filebucketing behavior of the final file and see File type # reference for its use. Defaults to 'puppet' # [*replace*] # Whether to replace a file that already exists on the local system # [*order*] # Select whether to order associated fragments by 'alpha' or 'numeric'. # Defaults to 'alpha'. # # === Actions: # * Creates a file_concat resource from the electrical/puppet-lib-file_concat library. # * Creates file_fragment resources from electrical/puppet-lib-file_concat # # === Aliases: # # * The exec can notified using Exec["concat_/path/to/file"] or # Exec["concat_/path/to/directory"] # * The final file can be referenced as File["/path/to/file"] or # File["concat_/path/to/file"] # define concat( $ensure = 'present', $path = $name, $owner = undef, $group = undef, $mode = '0644', - $warn_header = false, + $warn = false, + $force = undef, $backup = 'puppet', $replace = true, $order = 'alpha', $validate_cmd = undef, ) { validate_re($ensure, '^present$|^absent$') validate_absolute_path($path) validate_string($owner) validate_string($group) validate_string($mode) - if ! (is_string($warn_header) or $warn_header == true or $warn_header == false) { - fail('$warn_header is not a string or boolean') + if ! (is_string($warn) or $warn == true or $warn == false) { + fail('$warn is not a string or boolean') } if ! is_bool($backup) and ! is_string($backup) { fail('$backup must be string or bool!') } validate_bool($replace) validate_re($order, '^alpha$|^numeric$') if $validate_cmd and ! is_string($validate_cmd) { fail('$validate_cmd must be a string') } + if $force != undef { + warning('The $force parameter to concat is deprecated and has no effect.') + } + $safe_name = regsubst($name, '[/:\n\s]', '_', 'G') $default_warn_message = "# This file is managed by Puppet. DO NOT EDIT.\n" - case $warn_header { + case $warn { true: { $warn_message = $default_warn_message $append_header = true } false: { $warn_message = '' } default: { - $warn_message = $warn_header + $warn_message = $warn $append_header = true } } if $ensure == 'present' { file_concat { $name: tag => $safe_name, path => $path, owner => $owner, group => $group, mode => $mode, replace => $replace, backup => $backup, order => $order, validate_cmd => $validate_cmd, } if $append_header { file_fragment { "#{$name}_header": tag => $safe_name, content => $warn_message, order => '0', } } } else { file_concat { $name: ensure => $ensure, tag => $safe_name, path => $path, backup => $backup, } } } diff --git a/spec/acceptance/deprecation_warnings_spec.rb b/spec/acceptance/deprecation_warnings_spec.rb new file mode 100644 index 0000000..8a689a7 --- /dev/null +++ b/spec/acceptance/deprecation_warnings_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper_acceptance' + +describe 'deprecation warnings' do + basedir = default.tmpdir('concat') + + shared_examples 'has_warning' do |pp, w| + it 'applies the manifest twice with a stderr regex' do + expect(apply_manifest(pp, :catch_failures => true).stderr).to match(/#{Regexp.escape(w)}/m) + expect(apply_manifest(pp, :catch_changes => true).stderr).to match(/#{Regexp.escape(w)}/m) + end + end + + context 'concat force parameter' do + pp = <<-EOS + concat { '#{basedir}/file': + force => false, + } + concat::fragment { 'foo': + target => '#{basedir}/file', + content => 'bar', + } + EOS + w = 'The $force parameter to concat is deprecated and has no effect.' + + it_behaves_like 'has_warning', pp, w + end + + context 'concat::fragment ensure parameter' do + context 'target file exists' do + pp = <<-EOS + concat { '#{basedir}/file': + } + concat::fragment { 'foo': + target => '#{basedir}/file', + ensure => false, + content => 'bar', + } + EOS + w = 'The $ensure parameter to concat::fragment is deprecated and has no effect.' + + it_behaves_like 'has_warning', pp, w + end + end +end diff --git a/spec/acceptance/warn_header_spec.rb b/spec/acceptance/warn_header_spec.rb index 52615e1..b73414e 100644 --- a/spec/acceptance/warn_header_spec.rb +++ b/spec/acceptance/warn_header_spec.rb @@ -1,104 +1,104 @@ require 'spec_helper_acceptance' -describe 'concat warn_header =>' do +describe 'concat warn =>' do basedir = default.tmpdir('concat') context 'true should enable default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn_header => true, + warn => true, } concat::fragment { '1': target => '#{basedir}/file', content => '1', order => '01', } concat::fragment { '2': target => '#{basedir}/file', content => '2', order => '02', } EOS it 'applies the manifest twice with no stderr' do apply_manifest(pp, :catch_failures => true) apply_manifest(pp, :catch_changes => true) end describe file("#{basedir}/file") do it { should be_file } its(:content) { should match /# This file is managed by Puppet\. DO NOT EDIT\./ should match /1/ should match /2/ } end end context 'false should not enable default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn_header => false, + warn => false, } concat::fragment { '1': target => '#{basedir}/file', content => '1', order => '01', } concat::fragment { '2': target => '#{basedir}/file', content => '2', order => '02', } EOS it 'applies the manifest twice with no stderr' do apply_manifest(pp, :catch_failures => true) apply_manifest(pp, :catch_changes => true) end describe file("#{basedir}/file") do it { should be_file } its(:content) { should_not match /# This file is managed by Puppet\. DO NOT EDIT\./ should match /1/ should match /2/ } end end context '# foo should overide default warning message' do pp = <<-EOS concat { '#{basedir}/file': - warn_header => "# foo\n", + warn => "# foo\n", } concat::fragment { '1': target => '#{basedir}/file', content => '1', order => '01', } concat::fragment { '2': target => '#{basedir}/file', content => '2', order => '02', } EOS it 'applies the manifest twice with no stderr' do apply_manifest(pp, :catch_failures => true) apply_manifest(pp, :catch_changes => true) end describe file("#{basedir}/file") do it { should be_file } its(:content) { should match /# foo/ should match /1/ should match /2/ } end end end diff --git a/spec/unit/defines/concat_spec.rb b/spec/unit/defines/concat_spec.rb index 176f24a..35767ae 100644 --- a/spec/unit/defines/concat_spec.rb +++ b/spec/unit/defines/concat_spec.rb @@ -1,268 +1,251 @@ require 'spec_helper' describe 'concat', :type => :define do shared_examples 'concat' do |title, params, id| params = {} if params.nil? id = 'root' if id.nil? # default param values p = { :ensure => 'present', :path => title, :owner => nil, :group => nil, :mode => '0644', - :warn_header => false, - #:force => false, + :warn => false, :backup => 'puppet', :replace => true, }.merge(params) safe_name = title.gsub('/', '_') concat_name = 'fragments.concat.out' default_warn_message = "# This file is managed by Puppet. DO NOT EDIT.\n" file_defaults = { :backup => p[:backup], } let(:title) { title } let(:params) { params } let(:facts) do { :id => id, :osfamily => 'Debian', :path => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin', :kernel => 'Linux', :is_pe => false, } end if p[:ensure] == 'present' it do should contain_concat(title).with(file_defaults.merge({ :ensure => 'present', :owner => p[:owner], :group => p[:group], :mode => p[:mode], :path => p[:path], :backup => p[:backup], :replace => p[:replace], })) end else it do should contain_concat(title).with(file_defaults.merge({ :ensure => 'absent', :backup => p[:backup], })) end end end context 'title' do context 'without path param' do # title/name is the default value for the path param. therefore, the # title must be an absolute path unless path is specified ['/foo', '/foo/bar', '/foo/bar/baz'].each do |title| context title do it_behaves_like 'concat', '/etc/foo.bar' end end ['./foo', 'foo', 'foo/bar'].each do |title| context title do let(:title) { title } it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not an absolute path/) end end end end context 'with path param' do ['/foo', 'foo', 'foo/bar'].each do |title| context title do it_behaves_like 'concat', title, { :path => '/etc/foo.bar' } end end end end # title => context 'as non-root user' do it_behaves_like 'concat', '/etc/foo.bar', {}, 'bob' end context 'ensure =>' do ['present', 'absent'].each do |ens| context ens do it_behaves_like 'concat', '/etc/foo.bar', { :ensure => ens } end end context 'invalid' do let(:title) { '/etc/foo.bar' } let(:params) {{ :ensure => 'invalid' }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^present$|^absent$"')}/) end end end # ensure => context 'path =>' do context '/foo' do it_behaves_like 'concat', '/etc/foo.bar', { :path => '/foo' } end ['./foo', 'foo', 'foo/bar', false].each do |path| context path do let(:title) { '/etc/foo.bar' } let(:params) {{ :path => path }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not an absolute path/) end end end end # path => context 'owner =>' do context 'apenney' do it_behaves_like 'concat', '/etc/foo.bar', { :owner => 'apenny' } end context 'false' do let(:title) { '/etc/foo.bar' } let(:params) {{ :owner => false }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # owner => context 'group =>' do context 'apenney' do it_behaves_like 'concat', '/etc/foo.bar', { :group => 'apenny' } end context 'false' do let(:title) { '/etc/foo.bar' } let(:params) {{ :group => false }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # group => context 'mode =>' do context '1755' do it_behaves_like 'concat', '/etc/foo.bar', { :mode => '1755' } end context 'false' do let(:title) { '/etc/foo.bar' } let(:params) {{ :mode => false }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not a string/) end end end # mode => - context 'warn_header =>' do + context 'warn =>' do [true, false, '# foo'].each do |warn| context warn do - it_behaves_like 'concat', '/etc/foo.bar', { :warn_header => warn } + it_behaves_like 'concat', '/etc/foo.bar', { :warn => warn } end end context '(stringified boolean)' do ['true', 'yes', 'on', 'false', 'no', 'off'].each do |warn| context warn do - it_behaves_like 'concat', '/etc/foo.bar', { :warn_header => warn } + it_behaves_like 'concat', '/etc/foo.bar', { :warn => warn } it 'should create a warning' do skip('rspec-puppet support for testing warning()') end end end end context '123' do let(:title) { '/etc/foo.bar' } - let(:params) {{ :warn_header => 123 }} + let(:params) {{ :warn => 123 }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not a string or boolean/) end end end # warn => - #context 'force =>' do - # [true, false].each do |force| - # context force do - # it_behaves_like 'concat', '/etc/foo.bar', { :force => force } - # end - # end - - # context '123' do - # let(:title) { '/etc/foo.bar' } - # let(:params) {{ :force => 123 }} - # it 'should fail' do - # expect { catalogue }.to raise_error(Puppet::Error, /is not a boolean/) - # end - # end - #end # force => - context 'backup =>' do context 'reverse' do it_behaves_like 'concat', '/etc/foo.bar', { :backup => 'reverse' } end context 'false' do it_behaves_like 'concat', '/etc/foo.bar', { :backup => false } end context 'true' do it_behaves_like 'concat', '/etc/foo.bar', { :backup => true } end context 'true' do let(:title) { '/etc/foo.bar' } let(:params) {{ :backup => [] }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /backup must be string or bool/) end end end # backup => context 'replace =>' do [true, false].each do |replace| context replace do it_behaves_like 'concat', '/etc/foo.bar', { :replace => replace } end end context '123' do let(:title) { '/etc/foo.bar' } let(:params) {{ :replace => 123 }} it 'should fail' do expect { catalogue }.to raise_error(Puppet::Error, /is not a boolean/) end end end # replace => - #context 'order =>' do - # ['alpha', 'numeric'].each do |order| - # context order do - # it_behaves_like 'concat', '/etc/foo.bar', { :order => order } - # end - # end - - # context 'invalid' do - # let(:title) { '/etc/foo.bar' } - # let(:params) {{ :order => 'invalid' }} - # it 'should fail' do - # expect { catalogue }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^alpha$|^numeric$"')}/) - # end - # end - #end # order => + context 'order =>' do + ['alpha', 'numeric'].each do |order| + context order do + it_behaves_like 'concat', '/etc/foo.bar', { :order => order } + end + end + + context 'invalid' do + let(:title) { '/etc/foo.bar' } + let(:params) {{ :order => 'invalid' }} + it 'should fail' do + expect { catalogue }.to raise_error(Puppet::Error, /#{Regexp.escape('does not match "^alpha$|^numeric$"')}/) + end + end + end # order => end