From 4d66d23becb51efd0eb0766352ce5f53dfacd7ba Mon Sep 17 00:00:00 2001 From: Philipp Wagner Date: Wed, 3 Feb 2016 17:09:40 +0100 Subject: [PATCH 1/2] Ensure PPAs in tests have valid form "man apt-add-repository" notes: "REPOSITORY can be either a line that can be added directly to sources.list(5), in the form ppa:/ for adding Personal Package Archives". Fix the tests to always use the format ppa:/ when adding PPAs. --- spec/defines/ppa_spec.rb | 56 ++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb index 5080a87..1e7e23a 100644 --- a/spec/defines/ppa_spec.rb +++ b/spec/defines/ppa_spec.rb @@ -75,7 +75,7 @@ describe 'apt::ppa' do describe 'package_manage => true, multiple ppas, MODULES-2873' do let :pre_condition do 'class { "apt": } - apt::ppa {"ppa:foo": + apt::ppa {"ppa:user/foo": package_manage => true }' end @@ -95,18 +95,18 @@ describe 'apt::ppa' do } end - let(:title) { 'ppa:bar' } + let(:title) { 'ppa:user/bar' } it { is_expected.to contain_package('python-software-properties') } - it { is_expected.to contain_exec('add-apt-repository-ppa:bar').that_notifies('Class[Apt::Update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:user/bar').that_notifies('Class[Apt::Update]').with({ 'environment' => [], - 'command' => '/usr/bin/add-apt-repository -y ppa:bar', - 'unless' => '/usr/bin/test -s /etc/apt/sources.list.d/bar-natty.list', + 'command' => '/usr/bin/add-apt-repository -y ppa:user/bar', + 'unless' => '/usr/bin/test -s /etc/apt/sources.list.d/user-bar-natty.list', 'user' => 'root', 'logoutput' => 'on_failure', }) } - it { is_expected.to contain_file('/etc/apt/sources.list.d/bar-natty.list').that_requires('Exec[add-apt-repository-ppa:bar]').with({ + it { is_expected.to contain_file('/etc/apt/sources.list.d/user-bar-natty.list').that_requires('Exec[add-apt-repository-ppa:user/bar]').with({ 'ensure' => 'file', }) } @@ -152,7 +152,7 @@ describe 'apt::ppa' do describe 'apt included, no proxy' do let :pre_condition do 'class { "apt": } - apt::ppa { "ppa:foo2": } + apt::ppa { "ppa:user/foo2": } ' end let :facts do @@ -169,16 +169,16 @@ describe 'apt::ppa' do { :options => '', :package_manage => true, - :require => 'Apt::Ppa[ppa:foo2]', + :require => 'Apt::Ppa[ppa:user/foo2]', } end - let(:title) { 'ppa:foo' } + let(:title) { 'ppa:user/foo' } it { is_expected.to compile.with_all_deps } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => [], - :command => '/usr/bin/add-apt-repository ppa:foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', + :command => '/usr/bin/add-apt-repository ppa:user/foo', + :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -207,12 +207,12 @@ describe 'apt::ppa' do 'package_manage' => true, } end - let(:title) { 'ppa:foo' } + let(:title) { 'ppa:user/foo' } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8080'], - :command => '/usr/bin/add-apt-repository ppa:foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', + :command => '/usr/bin/add-apt-repository ppa:user/foo', + :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -241,12 +241,12 @@ describe 'apt::ppa' do :package_manage => true, } end - let(:title) { 'ppa:foo' } + let(:title) { 'ppa:user/foo' } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8180'], - :command => '/usr/bin/add-apt-repository ppa:foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', + :command => '/usr/bin/add-apt-repository ppa:user/foo', + :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -275,12 +275,12 @@ describe 'apt::ppa' do :package_manage => true, } end - let(:title) { 'ppa:foo' } + let(:title) { 'ppa:user/foo' } it { is_expected.to contain_package('software-properties-common') } - it { is_expected.to contain_exec('add-apt-repository-ppa:foo').that_notifies('Class[Apt::Update]').with({ + it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ :environment => ['http_proxy=http://localhost:8180', 'https_proxy=https://localhost:8180'], - :command => '/usr/bin/add-apt-repository ppa:foo', - :unless => '/usr/bin/test -s /etc/apt/sources.list.d/foo-trusty.list', + :command => '/usr/bin/add-apt-repository ppa:user/foo', + :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-foo-trusty.list', :user => 'root', :logoutput => 'on_failure', }) @@ -301,13 +301,13 @@ describe 'apt::ppa' do :puppetversion => Puppet.version, } end - let(:title) { 'ppa:foo' } + let(:title) { 'ppa:user/foo' } let :params do { :ensure => 'absent' } end - it { is_expected.to contain_file('/etc/apt/sources.list.d/foo-trusty.list').that_notifies('Class[Apt::Update]').with({ + it { is_expected.to contain_file('/etc/apt/sources.list.d/user-foo-trusty.list').that_notifies('Class[Apt::Update]').with({ :ensure => 'absent', }) } @@ -325,7 +325,7 @@ describe 'apt::ppa' do :puppetversion => Puppet.version, } end - let(:title) { 'ppa:foo' } + let(:title) { 'ppa:user/foo' } it do expect { subject.call @@ -344,7 +344,7 @@ describe 'apt::ppa' do :puppetversion => Puppet.version, } end - let(:title) { 'ppa:foo' } + let(:title) { 'ppa:user/foo' } it do expect { subject.call From a02654e36ca0cec2fa8360d0f52d34801df9229a Mon Sep 17 00:00:00 2001 From: Philipp Wagner Date: Wed, 3 Feb 2016 17:13:46 +0100 Subject: [PATCH 2/2] Look for correct sources.list.d file for apt::ppa In Ubuntu 15.10 the path of the apt sources file, which is generated by apt-add-repository, changed to include the distid. This breaks apt::ppa idempotency, since it does not recognize the repository is already added. Reported on puppet-users as well: https://groups.google.com/forum/#!topic/puppet-users/YzeMyZYUo98 --- manifests/ppa.pp | 18 +++++++++++++----- spec/classes/apt_spec.rb | 1 + spec/defines/ppa_spec.rb | 23 +++++++++++++++++++++++ 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/manifests/ppa.pp b/manifests/ppa.pp index b83500b..7069e2e 100644 --- a/manifests/ppa.pp +++ b/manifests/ppa.pp @@ -14,11 +14,19 @@ define apt::ppa( fail('apt::ppa is not currently supported on Debian.') } - $filename_without_slashes = regsubst($name, '/', '-', 'G') - $filename_without_dots = regsubst($filename_without_slashes, '\.', '_', 'G') - $filename_without_pluses = regsubst($filename_without_dots, '\+', '_', 'G') - $filename_without_ppa = regsubst($filename_without_pluses, '^ppa:', '', 'G') - $sources_list_d_filename = "${filename_without_ppa}-${release}.list" + $ubuntu_release_year = regsubst($::apt::xfacts['lsbdistrelease'], '\.\d+$', '', 'G') + 0 + $ubuntu_release_month = regsubst($::apt::xfacts['lsbdistrelease'], '^\d+\.', '', 'G') + 0 + + if $ubuntu_release_year >= 15 and $ubuntu_release_month >= 10 { + $distid = downcase($::apt::xfacts['lsbdistid']) + $filename = regsubst($name, '^ppa:([^/]+)/(.+)$', "\\1-${distid}-\\2-${release}") + } else { + $filename = regsubst($name, '^ppa:([^/]+)/(.+)$', "\\1-\\2-${release}") + } + + $filename_no_slashes = regsubst($filename, '/', '-', 'G') + $filename_no_specialchars = regsubst($filename_no_slashes, '[\.\+]', '_', 'G') + $sources_list_d_filename = "${filename_no_specialchars}.list" if $ensure == 'present' { if $package_manage { diff --git a/spec/classes/apt_spec.rb b/spec/classes/apt_spec.rb index 5aded56..cc2264b 100644 --- a/spec/classes/apt_spec.rb +++ b/spec/classes/apt_spec.rb @@ -208,6 +208,7 @@ describe 'apt' do { :osfamily => 'Debian', :lsbdistcodename => 'precise', :lsbdistid => 'ubuntu', + :lsbdistrelease => '12.04', :puppetversion => Puppet.version, } end diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb index 1e7e23a..74b52ea 100644 --- a/spec/defines/ppa_spec.rb +++ b/spec/defines/ppa_spec.rb @@ -28,6 +28,29 @@ describe 'apt::ppa' do } end + describe 'Ubuntu 15.10 sources.list filename' do + let :facts do + { + :lsbdistrelease => '15.10', + :lsbdistcodename => 'wily', + :operatingsystem => 'Ubuntu', + :osfamily => 'Debian', + :lsbdistid => 'Ubuntu', + :puppetversion => Puppet.version, + } + end + + let(:title) { 'ppa:user/foo' } + it { is_expected.to contain_exec('add-apt-repository-ppa:user/foo').that_notifies('Class[Apt::Update]').with({ + :environment => [], + :command => '/usr/bin/add-apt-repository -y ppa:user/foo', + :unless => '/usr/bin/test -s /etc/apt/sources.list.d/user-ubuntu-foo-wily.list', + :user => 'root', + :logoutput => 'on_failure', + }) + } + end + describe 'ppa depending on ppa, MODULES-1156' do let :pre_condition do 'class { "apt": }'