opuppet/module-concat
4
0
Fork 0
Code Issues Pull requests Releases Wiki Activity

revert concat $warn/$warn_message param split + add deprecation warnings

Browse source 
Partially reverting the $warn/$warn_message param split from eaf8407 as this
change was unnecessarily API breaking.  Instead, we are adding string/bool type
validating to the $warn parameter and deprecation warnings for the usage of
stringified boolean values (eg, 'true', 'on', etc.). In a future major release,
the logic can be simplified to treating all string values as a warning message.
This commit is contained in:
Joshua Hoblitt 2013-12-23 13:23:49 -08:00
parent 63b5aba9fd
commit 8a500af5ee
4 changed files with 174 additions and 55 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
manifests/init.pp
View file

@ -21,8 +21,6 @@
# [*warn*]
# Adds a normal shell style comment top of the file indicating that it is
# built by puppet
# [*warn_message*]
# A custom message string that overides the default.
# [*force*]
# [*backup*]
# Controls the filebucketing behavior of the final file and see File type
@ -51,7 +49,7 @@
#
# * The exec can notified using Exec["concat_/path/to/file"] or
# Exec["concat_/path/to/directory"]
# * The final file can be referened as File["/path/to/file"] or
# * The final file can be referenced as File["/path/to/file"] or
# File["concat_/path/to/file"]
#
define concat(
@ -61,7 +59,6 @@ define concat(
$group = undef,
$mode = '0644',
$warn = false,
$warn_message = undef,
$force = false,
$backup = 'puppet',
$replace = true,
@ -74,8 +71,9 @@ define concat(
validate_string($owner)
validate_string($group)
validate_string($mode)
validate_bool($warn)
validate_string($warn_message)
if ! (is_string($warn) or $warn == true or $warn == false) {
fail('$warn is not a string or boolean')
}
validate_bool($force)
validate_string($backup)
validate_bool($replace)
@ -93,17 +91,29 @@ define concat(
$concat_name = 'fragments.concat.out'
$script_command = $concat::setup::script_command
$default_warn_message = '# This file is managed by Puppet. DO NOT EDIT.'
$bool_warn_message = 'Using stringified boolean values (\'true\', \'yes\', \'on\', \'false\', \'no\', \'off\') to represent boolean true/false as the $warn parameter to concat is deprecated and will be treated as the warning message in a future release'
if $warn == true {
$use_warn_message = $warn_message ? {
undef => $default_warn_message,
default => $warn_message,
case $warn {
true: {
$warn_message = $default_warn_message
}
'true', 'yes', 'on': {
warning($bool_warn_message)
$warn_message = $default_warn_message
}
false: {
$warn_message = ''
}
'false', 'no', 'off': {
warning($bool_warn_message)
$warn_message = ''
}
default: {
$warn_message = $warn
}
} else {
$use_warn_message = undef
}
$warnmsg_escaped = regsubst($use_warn_message, '\'', '\'\\\'\'', 'G')
$warnmsg_escaped = regsubst($warn_message, '\'', '\'\\\'\'', 'G')
$warnflag = $warnmsg_escaped ? {
'' => '',
default => "-w '${warnmsg_escaped}'"

48
spec/system/deprecation_warnings_spec.rb
View file

@ -27,6 +27,54 @@ describe 'deprecation warnings' do
it_behaves_like 'has_warning', pp, w
end
context 'concat warn parameter =>' do
['true', 'yes', 'on'].each do |warn|
context warn do
pp = <<-EOS
concat { '/tmp/concat/file':
warn => '#{warn}',
}
concat::fragment { 'foo':
target => '/tmp/concat/file',
content => 'bar',
}
EOS
w = 'Using stringified boolean values (\'true\', \'yes\', \'on\', \'false\', \'no\', \'off\') to represent boolean true/false as the $warn parameter to concat is deprecated and will be treated as the warning message in a future release'
it_behaves_like 'has_warning', pp, w
describe file('/tmp/concat/file') do
it { should be_file }
it { should contain '# This file is managed by Puppet. DO NOT EDIT.' }
it { should contain 'bar' }
end
end
end
['false', 'no', 'off'].each do |warn|
context warn do
pp = <<-EOS
concat { '/tmp/concat/file':
warn => '#{warn}',
}
concat::fragment { 'foo':
target => '/tmp/concat/file',
content => 'bar',
}
EOS
w = 'Using stringified boolean values (\'true\', \'yes\', \'on\', \'false\', \'no\', \'off\') to represent boolean true/false as the $warn parameter to concat is deprecated and will be treated as the warning message in a future release'
it_behaves_like 'has_warning', pp, w
describe file('/tmp/concat/file') do
it { should be_file }
it { should_not contain '# This file is managed by Puppet. DO NOT EDIT.' }
it { should contain 'bar' }
end
end
end
end
context 'concat::fragment ensure parameter' do
context 'target file exists' do
before(:all) do

75
spec/system/warn_spec.rb
View file

@ -1,12 +1,9 @@
require 'spec_helper_system'
describe 'basic concat test' do
context 'should run successfully' do
describe 'concat warn =>' do
context 'true should enable default warning message' do
pp = <<-EOS
concat { '/tmp/concat/file':
owner => root,
group => root,
mode => '0644',
warn => true,
}
@ -38,4 +35,72 @@ describe 'basic concat test' do
it { should contain '2' }
end
end
context 'false should not enable default warning message' do
pp = <<-EOS
concat { '/tmp/concat/file':
warn => false,
}
concat::fragment { '1':
target => '/tmp/concat/file',
content => '1',
order => '01',
}
concat::fragment { '2':
target => '/tmp/concat/file',
content => '2',
order => '02',
}
EOS
context puppet_apply(pp) do
its(:stderr) { should be_empty }
its(:exit_code) { should_not == 1 }
its(:refresh) { should be_nil }
its(:stderr) { should be_empty }
its(:exit_code) { should be_zero }
end
describe file('/tmp/concat/file') do
it { should be_file }
it { should_not contain '# This file is managed by Puppet. DO NOT EDIT.' }
it { should contain '1' }
it { should contain '2' }
end
end
context '# foo should overide default warning message' do
pp = <<-EOS
concat { '/tmp/concat/file':
warn => '# foo',
}
concat::fragment { '1':
target => '/tmp/concat/file',
content => '1',
order => '01',
}
concat::fragment { '2':
target => '/tmp/concat/file',
content => '2',
order => '02',
}
EOS
context puppet_apply(pp) do
its(:stderr) { should be_empty }
its(:exit_code) { should_not == 1 }
its(:refresh) { should be_nil }
its(:stderr) { should be_empty }
its(:exit_code) { should be_zero }
end
describe file('/tmp/concat/file') do
it { should be_file }
it { should contain '# foo' }
it { should contain '1' }
it { should contain '2' }
end
end
end

70
spec/unit/defines/concat_spec.rb
View file

@ -14,7 +14,6 @@ describe 'concat', :type => :define do
:group => nil,
:mode => '0644',
:warn => false,
:warn_message => nil,
:force => false,
:backup => 'puppet',
:replace => true,
@ -86,9 +85,25 @@ describe 'concat', :type => :define do
"-d #{concatdir}/#{safe_name}"
# flag order: fragdir, warnflag, forceflag, orderflag, newlineflag
if p[:warn]
message = p[:warn_message] || default_warn_message
cmd += " -w \'#{message}\'"
if p.has_key?(:warn)
case p[:warn]
when TrueClass
message = default_warn_message
when 'true', 'yes', 'on'
# should generate a stringified boolean warning
message = default_warn_message
when FalseClass
message = nil
when 'false', 'no', 'off'
# should generate a stringified boolean warning
message = nil
else
message = p[:warn]
end
unless message.nil?
cmd += " -w \'#{message}\'"
end
end
cmd += " -f" if p[:force]
@ -243,52 +258,33 @@ describe 'concat', :type => :define do
end # mode =>
context 'warn =>' do
[true, false].each do |warn|
[true, false, '# foo'].each do |warn|
context warn do
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 => warn }
it 'should create a warning' do
pending('rspec-puppet support for testing warning()')
end
end
end
end
context '123' do
let(:title) { '/etc/foo.bar' }
let(:params) {{ :warn => 123 }}
it 'should fail' do
expect { should }.to raise_error(Puppet::Error, /is not a boolean/)
expect { should }.to raise_error(Puppet::Error, /is not a string or boolean/)
end
end
end # warn =>
context 'warn_message =>' do
context '# ashp replaced your file' do
# should do nothing unless warn == true;
# but we can't presently test that because concatfragments.sh isn't run
# from rspec-puppet tests
context 'warn =>' do
context 'true' do
it_behaves_like 'concat', '/etc/foo.bar', {
:warn => true,
:warn_message => '# ashp replaced your file'
}
end
context 'false' do
it_behaves_like 'concat', '/etc/foo.bar', {
:warn => false,
:warn_message => '# ashp replaced your file'
}
end
end
end
context 'false' do
let(:title) { '/etc/foo.bar' }
let(:params) {{ :warn_message => false }}
it 'should fail' do
expect { should }.to raise_error(Puppet::Error, /is not a string/)
end
end
end # warn_message =>
context 'force =>' do
[true, false].each do |force|
context force do