From d261d8f11b85407c149924a8b03b0e681777f65a Mon Sep 17 00:00:00 2001 From: Daniele Sluijters Date: Thu, 26 Feb 2015 19:12:53 +0100 Subject: [PATCH] apt::setting: Parse type and name from title. Instead of having two additional parameters, `base_name` and `setting_type` simply parse it from `title`. We need to prefix most resources with `list-`, `conf-`, or `pref-` any way to avoid duplicate resources so we might as well leverage that. --- manifests/conf.pp | 8 +++---- manifests/init.pp | 6 ++--- manifests/pin.pp | 8 +++---- manifests/setting.pp | 12 ++++++---- manifests/source.pp | 8 +++---- spec/defines/pin_spec.rb | 20 +++------------- spec/defines/setting_spec.rb | 44 ++++++++++++------------------------ 7 files changed, 35 insertions(+), 71 deletions(-) diff --git a/manifests/conf.pp b/manifests/conf.pp index 49e32ab..2850205 100644 --- a/manifests/conf.pp +++ b/manifests/conf.pp @@ -4,10 +4,8 @@ define apt::conf ( $priority = '50', ) { apt::setting { "conf-${name}": - ensure => $ensure, - base_name => $name, - setting_type => 'conf', - priority => $priority, - content => template('apt/_header.erb', 'apt/conf.erb'), + ensure => $ensure, + priority => $priority, + content => template('apt/_header.erb', 'apt/conf.erb'), } } diff --git a/manifests/init.pp b/manifests/init.pp index 59a7fe4..0b4a0bc 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -31,10 +31,8 @@ class apt( } apt::setting { 'conf-update-stamp': - base_name => 'update-stamp', - setting_type => 'conf', - priority => 15, - content => template('apt/_header.erb', 'apt/15update-stamp.erb'), + priority => 15, + content => template('apt/_header.erb', 'apt/15update-stamp.erb'), } file { 'sources.list': diff --git a/manifests/pin.pp b/manifests/pin.pp index 214fa99..b27ed8e 100644 --- a/manifests/pin.pp +++ b/manifests/pin.pp @@ -62,10 +62,8 @@ define apt::pin( $file_name = regsubst($title, '[^0-9a-z\-_\.]', '_', 'IG') apt::setting { "pref-${file_name}": - ensure => $ensure, - base_name => $file_name, - setting_type => 'pref', - priority => $order, - content => template('apt/_header.erb', 'apt/pin.pref.erb'), + ensure => $ensure, + priority => $order, + content => template('apt/_header.erb', 'apt/pin.pref.erb'), } } diff --git a/manifests/setting.pp b/manifests/setting.pp index 78f007f..f6f0bc0 100644 --- a/manifests/setting.pp +++ b/manifests/setting.pp @@ -1,6 +1,4 @@ define apt::setting ( - $setting_type, - $base_name = $title, $priority = 50, $ensure = file, $source = undef, @@ -18,13 +16,17 @@ define apt::setting ( fail('apt::setting needs either of content or source') } - validate_re($setting_type, ['conf', 'pref', 'list']) validate_re($ensure, ['file', 'present', 'absent']) - validate_string($base_name) + + $title_array = split($title, '-') + $setting_type = $title_array[0] + $base_name = join(delete_at($title_array, 0), '-') + + validate_re($setting_type, ['\Aconf\z', '\Apref\z', '\Alist\z'], "apt::setting resource name/title must start with either 'conf-', 'pref-' or 'list-'") unless is_integer($priority) { # need this to allow zero-padded priority. - validate_re($priority, '^\d+$', 'apt::setting priority must be an integer or a zero-padded integer.') + validate_re($priority, '^\d+$', 'apt::setting priority must be an integer or a zero-padded integer') } if $source { diff --git a/manifests/source.pp b/manifests/source.pp index e4e4aa3..9a3a791 100644 --- a/manifests/source.pp +++ b/manifests/source.pp @@ -24,11 +24,9 @@ define apt::source( } apt::setting { "list-${name}": - ensure => $ensure, - base_name => $name, - setting_type => 'list', - content => template('apt/_header.erb', 'apt/source.list.erb'), - notify => Exec['apt_update'], + ensure => $ensure, + content => template('apt/_header.erb', 'apt/source.list.erb'), + notify => Exec['apt_update'], } if ($pin != false) { diff --git a/spec/defines/pin_spec.rb b/spec/defines/pin_spec.rb index 1421d68..e4d5d0a 100644 --- a/spec/defines/pin_spec.rb +++ b/spec/defines/pin_spec.rb @@ -8,11 +8,7 @@ describe 'apt::pin', :type => :define do context 'defaults' do it { is_expected.to contain_apt__setting("pref-my_pin").with_content(/Explanation: : my_pin\nPackage: \*\nPin: release a=my_pin\nPin-Priority: 0\n/)} - it { is_expected.to contain_apt__setting("pref-my_pin").with({ - 'setting_type' => 'pref', - 'base_name' => 'my_pin', - }) - } + it { is_expected.to contain_apt__setting("pref-my_pin") } end context 'set version' do @@ -23,11 +19,7 @@ describe 'apt::pin', :type => :define do } end it { is_expected.to contain_apt__setting("pref-my_pin").with_content(/Explanation: : my_pin\nPackage: vim\nPin: version 1\nPin-Priority: 0\n/)} - it { is_expected.to contain_apt__setting("pref-my_pin").with({ - 'setting_type' => 'pref', - 'base_name' => 'my_pin', - }) - } + it { is_expected.to contain_apt__setting("pref-my_pin") } end context 'set origin' do @@ -38,11 +30,7 @@ describe 'apt::pin', :type => :define do } end it { is_expected.to contain_apt__setting("pref-my_pin").with_content(/Explanation: : my_pin\nPackage: vim\nPin: origin test\nPin-Priority: 0\n/)} - it { is_expected.to contain_apt__setting("pref-my_pin").with({ - 'setting_type' => 'pref', - 'base_name' => 'my_pin', - }) - } + it { is_expected.to contain_apt__setting("pref-my_pin") } end context 'not defaults' do @@ -61,8 +49,6 @@ describe 'apt::pin', :type => :define do end it { is_expected.to contain_apt__setting("pref-my_pin").with_content(/Explanation: foo\nPackage: \*\nPin: release a=1, n=bar, v=2, c=baz, o=foobar, l=foobaz\nPin-Priority: 10\n/) } it { is_expected.to contain_apt__setting("pref-my_pin").with({ - 'setting_type' => 'pref', - 'base_name' => 'my_pin', 'priority' => 99, }) } diff --git a/spec/defines/setting_spec.rb b/spec/defines/setting_spec.rb index 7b7c54e..5e88eea 100644 --- a/spec/defines/setting_spec.rb +++ b/spec/defines/setting_spec.rb @@ -3,41 +3,36 @@ require 'spec_helper' describe 'apt::setting' do let(:pre_condition) { 'class { "apt": }' } let(:facts) { { :lsbdistid => 'Debian', :osfamily => 'Debian' } } - let(:title) { 'teddybear' } + let(:title) { 'conf-teddybear' } - let(:default_params) { { :setting_type => 'conf', :content => 'di' } } + let(:default_params) { { :content => 'di' } } describe 'when using the defaults' do - context 'without setting_type' do - it do - expect { is_expected.to compile }.to raise_error(Puppet::Error, /Must pass setting_type /) - end - end - context 'without source or content' do - let(:params) { { :setting_type => 'conf' } } it do expect { is_expected.to compile }.to raise_error(Puppet::Error, /needs either of /) end end - context 'with setting_type=conf' do + context 'with title=conf-teddybear ' do let(:params) { default_params } it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear') } end - context 'with setting_type=pref' do - let(:params) { { :setting_type => 'pref', :content => 'di' } } + context 'with title=pref-teddybear' do + let(:title) { 'pref-teddybear' } + let(:params) { default_params } it { is_expected.to contain_file('/etc/apt/preferences.d/50teddybear') } end - context 'with setting_type=list' do - let(:params) { { :setting_type => 'list', :content => 'di' } } + context 'with title=list-teddybear' do + let(:title) { 'list-teddybear' } + let(:params) { default_params } it { is_expected.to contain_file('/etc/apt/sources.list.d/teddybear.list') } end context 'with source' do - let(:params) { { :setting_type => 'conf', :source => 'puppet:///la/die/dah' } } + let(:params) { { :source => 'puppet:///la/die/dah' } } it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').with({ :ensure => 'file', @@ -68,10 +63,11 @@ describe 'apt::setting' do end end - context 'with setting_type=ext' do - let(:params) { default_params.merge({ :setting_type => 'ext' }) } + context 'with title=ext-teddybear' do + let(:title) { 'ext-teddybear' } + let(:params) { default_params } it do - expect { is_expected.to compile }.to raise_error(Puppet::Error, /"ext" does not /) + expect { is_expected.to compile }.to raise_error(Puppet::Error, /must start with /) end end @@ -95,18 +91,6 @@ describe 'apt::setting' do it { is_expected.to contain_file('/etc/apt/apt.conf.d/100teddybear') } end - describe 'with base_name=puppy' do - let(:params) { default_params.merge({ :base_name => 'puppy' }) } - it { should contain_file('/etc/apt/apt.conf.d/50puppy') } - end - - describe 'with base_name=true' do - let(:params) { default_params.merge({ :base_name => true }) } - it do - expect { should compile }.to raise_error(Puppet::Error, /not a string/) - end - end - describe 'with ensure=absent' do let(:params) { default_params.merge({ :ensure => 'absent' }) } it { is_expected.to contain_file('/etc/apt/apt.conf.d/50teddybear').with({ -- 2.32.3