From 41a2725683bbaacfc827f8ebfe1fc1044d51df80 Mon Sep 17 00:00:00 2001 From: Daniele Sluijters Date: Thu, 5 Mar 2015 20:35:02 +0100 Subject: [PATCH] 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. --- manifests/source.pp | 6 ++++- spec/defines/source_spec.rb | 44 +++++++++++++++++++++++++++---------- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/manifests/source.pp b/manifests/source.pp index 96c174c..163a411 100644 --- 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 d5e146a..7fd86b5 100644 --- 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 { -- 2.32.3