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.

diff --git a/manifests/conf.pp b/manifests/conf.pp
index 49e32abaaa256254fd0d144d0bfc0eeed9187cef..28502052814b73a238d7fc6704a05ae70811cd47 100644 (file)
--- 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 59a7fe4e3a9c179352afe776a11c3abaa1e9c557..0b4a0bc6267aad080422786c4dea95c0f9fffecb 100644 (file)
--- 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 214fa997e8d9a2c112b883c8abf6605dd58eab07..b27ed8ea2ab699678ece5524ac8af4377f836333 100644 (file)
--- 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 78f007fdbc6ff9815c5287fc67dab8c2b1501ec6..f6f0bc0c2275821a3656d9e14cb73b30f1202f88 100644 (file)
--- 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 e4e4aa3dea1c08e71b834f4019d1342a28c8a1fe..9a3a7912aa0c8159925c304ab9807bcb6d7ac292 100644 (file)
--- 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 1421d68fbdefc96c05b968a71e57e3ed197e30bd..e4d5d0ae1af79538f76b85b6f16946911ecc7af3 100644 (file)
--- 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 7b7c54efc452c4744072c901bbdbb722260532ac..5e88eea16ed90b72a6778f9c8c9b1da60d9eb442 100644 (file)
--- 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({