apt::source: Make location mostly required.

In what universe does it make sense to create a `sources.list.d` entry
for a repository **without** specifying where this repository is?
:confounded:

:disappointed: :weary: :anguished: :scream:

Only when removing the resource should a location not be required.

diff --git a/manifests/source.pp b/manifests/source.pp
index 96c174c05e2539e25066198f3a931e36ac22e6af..163a411bb21f426f1ae1f43aeeffc4cd4c188464 100644 (file)
--- a/manifests/source.pp
+++ b/manifests/source.pp
@@ -1,9 +1,9 @@
 # source.pp
 # add an apt source
 define apt::source(
+  $location       = undef,
   $comment        = $name,
   $ensure         = present,
-  $location       = '',
   $release        = $::apt::xfacts['lsbdistcodename'],
   $repos          = 'main',
   $include        = {},
@@ -20,6 +20,10 @@ define apt::source(
     fail('lsbdistcodename fact not available: release parameter required')
   }
 
+  if $ensure == 'present' and ! $location {
+    fail('cannot create a source entry without specifying a location')
+  }
+
   $_before = Apt::Setting["list-${title}"]
   $_include = merge($::apt::include_defaults, $include)
 

diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb
index d5e146a2f449d231fcbd11981ba6a51cb14e65d7..7fd86b56ff0ff35e98ff8a4e5abd90fdfb31c13b 100644 (file)
--- a/spec/defines/source_spec.rb
+++ b/spec/defines/source_spec.rb
@@ -12,18 +12,35 @@ describe 'apt::source' do
   end
 
   context 'defaults' do
-    let :facts do
-      {
-        :lsbdistid       => 'Debian',
-        :lsbdistcodename => 'wheezy',
-        :osfamily        => 'Debian'
-      }
+    context 'without location' do
+      let :facts do
+        {
+          :lsbdistid       => 'Debian',
+          :lsbdistcodename => 'wheezy',
+          :osfamily        => 'Debian'
+        }
+      end
+      it do
+        expect {
+          is_expected.to compile
+        }.to raise_error(Puppet::Error, /source entry without specifying a location/)
+      end
     end
+    context 'with location' do
+      let :facts do
+        {
+          :lsbdistid       => 'Debian',
+          :lsbdistcodename => 'wheezy',
+          :osfamily        => 'Debian'
+        }
+      end
+      let(:params) { { :location => 'hello.there', } }
 
-    it { is_expected.to contain_apt__setting('list-my_source').with({
-      :ensure  => 'present',
-    }).without_content(/# my_source\ndeb-src  wheezy main\n/)
-    }
+      it { is_expected.to contain_apt__setting('list-my_source').with({
+        :ensure  => 'present',
+      }).without_content(/# my_source\ndeb-src hello.there wheezy main\n/)
+      }
+    end
   end
 
   describe 'no defaults' do
@@ -149,13 +166,14 @@ describe 'apt::source' do
     end
     let :params do
       {
+        :location       => 'hello.there',
         :allow_unsigned => true,
       }
     end
 
     it { is_expected.to contain_apt__setting('list-my_source').with({
       :ensure => 'present',
-    }).with_content(/# my_source\ndeb \[trusted=yes\]  wheezy main\n/)
+    }).with_content(/# my_source\ndeb \[trusted=yes\] hello.there wheezy main\n/)
     }
   end
 
@@ -169,6 +187,7 @@ describe 'apt::source' do
     end
     let :params do
       {
+        :location     => 'hello.there',
         :include      => {'deb' => false, 'src' => true,},
         :architecture => 'x86_64',
       }
@@ -176,7 +195,7 @@ describe 'apt::source' do
 
     it { is_expected.to contain_apt__setting('list-my_source').with({
       :ensure => 'present',
-    }).with_content(/# my_source\ndeb-src \[arch=x86_64 \]  wheezy main\n/)
+    }).with_content(/# my_source\ndeb-src \[arch=x86_64 \] hello.there wheezy main\n/)
     }
   end
 
@@ -208,6 +227,7 @@ describe 'apt::source' do
           :osfamily        => 'Debian'
         }
       end
+      let(:params) { { :location => 'hello.there', } }
 
       it do
         expect {