From: Ryan Coleman <ryan@puppetlabs.com>
Date: Thu, 1 Mar 2012 22:40:44 +0000 (-0800)
Subject: Merge pull request #26 from pdxcat/ticket_12823_aptkey_defined_type
X-Git-Tag: 0.0.1~3
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=76dbf992e04bbbaa909cbe779adac0045cee5a7f;hp=8cdaf855a1d0dd24fed02fc3ee9941e0a1e6849b;p=puppet-modules%2Fpuppetlabs-apt.git

Merge pull request #26 from pdxcat/ticket_12823_aptkey_defined_type

(#12823) Add apt::key defined type and modify apt::source to use it

Reviewed and tested by Ryan Coleman (ryan@puppetlabs.com)
---

diff --git a/manifests/params.pp b/manifests/params.pp
index 3c39288..ca5d655 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -1,4 +1,5 @@
 class apt::params {
-  $root = '/etc/apt'
-  $provider = '/usr/bin/apt-get'
+  $root           = '/etc/apt'
+  $provider       = '/usr/bin/apt-get'
+  $sources_list_d = "${root}/sources.list.d"
 }
diff --git a/manifests/ppa.pp b/manifests/ppa.pp
index a41c814..712f425 100644
--- a/manifests/ppa.pp
+++ b/manifests/ppa.pp
@@ -1,21 +1,36 @@
 # ppa.pp
 
-define apt::ppa() {
+define apt::ppa(
+  $release = $lsbdistcodename
+) {
 
   Class['apt'] -> Apt::Ppa[$title]
 
+  include apt::params
+
+  if ! $release {
+    fail("lsbdistcodename fact not available: release parameter required")
+  }
+
   exec { "apt-update-${name}":
     command     => "/usr/bin/aptitude update",
     refreshonly => true,
   }
 
+  $filename_without_slashes = regsubst($name,'/','-','G')
+  $filename_without_ppa     = regsubst($filename_without_slashes, '^ppa:','','G')
+  $sources_list_d_filename   = "${filename_without_ppa}-${release}.list"
+
   exec { "add-apt-repository-${name}":
     command => "/usr/bin/add-apt-repository ${name}",
     notify  => Exec["apt-update-${name}"],
-    unless => $name? {
-      /ppa:(.*)/ => "/bin/cat /etc/apt/sources.list /etc/apt/sources.list.d/* | /bin/egrep '^[^#].*ppa.*$1.*$'",
-      default    => "/bin/cat /etc/apt/sources.list /etc/apt/sources.list.d/* | /bin/egrep '^[^#].*${title}.*$'",
-    }
+    creates => "${apt::params::sources_list_d}/${sources_list_d_filename}",
   }
+
+  file { "${apt::params::sources_list_d}/${sources_list_d_filename}":
+    ensure  => file,
+    require => Exec["add-apt-repository-${name}"];
+  }
+
 }
 
diff --git a/manifests/source.pp b/manifests/source.pp
index 475bee3..4c03412 100644
--- a/manifests/source.pp
+++ b/manifests/source.pp
@@ -3,7 +3,7 @@
 
 define apt::source(
   $location = '',
-  $release = 'karmic',
+  $release = $lsbdistcodename,
   $repos = 'main',
   $include_src = true,
   $required_packages = false,
@@ -16,6 +16,10 @@ define apt::source(
 
   include apt::params
 
+  if ! $release {
+    fail("lsbdistcodename fact not available: release parameter required")
+  }
+
   file { "${name}.list":
     path => "${apt::params::root}/sources.list.d/${name}.list",
     ensure => file,
diff --git a/spec/defines/ppa_spec.rb b/spec/defines/ppa_spec.rb
index c2ce264..9d4750b 100644
--- a/spec/defines/ppa_spec.rb
+++ b/spec/defines/ppa_spec.rb
@@ -1,37 +1,53 @@
 require 'spec_helper'
 describe 'apt::ppa', :type => :define do
-  ['ppa:dans_ppa', 'dans_ppa'].each do |t|
+  ['ppa:dans_ppa', 'dans_ppa','ppa:dans-daily/ubuntu'].each do |t|
     describe "with title #{t}" do
       let :pre_condition do
         'class { "apt": }'
       end
+      let :facts do
+        {:lsbdistcodename => 'natty'}
+      end
       let :title do
         t
       end
-      let :unless_statement do
-        if t =~ /ppa:(.*)/
-          /^[^#].*ppa.*#{$1}.*$/
-        else
-          /^[^#].*#{t}.*$/
-        end
+      let :release do
+        "natty"
+      end
+      let :filename do
+        t.sub(/^ppa:/,'').gsub('/','-') << "-" << "#{release}.list"
       end
+
+      it { should contain_exec("apt-update-#{t}").with(
+        'command'     => '/usr/bin/aptitude update',
+        'refreshonly' => true
+        )
+      }
+
       it { should contain_exec("add-apt-repository-#{t}").with(
         'command' => "/usr/bin/add-apt-repository #{t}",
-        'notify'  => "Exec[apt-update-#{t}]"
+        'notify'  => "Exec[apt-update-#{t}]",
+        'creates' => "/etc/apt/sources.list.d/#{filename}"
         )
       }
-      it { should contain_exec("add-apt-repository-#{t}").with({"unless" => unless_statement}) }
-      it { should contain_exec("apt-update-#{t}").with(
-        'command'     => '/usr/bin/aptitude update',
-        'refreshonly' => true
+
+      it { should create_file("/etc/apt/sources.list.d/#{filename}").with(
+        'ensure'  => 'file',
+        'require' => "Exec[add-apt-repository-#{t}]"
         )
       }
-      it { should contain_exec("apt-update-#{t}").without_unless }
     end
   end
 
   describe "without Class[apt] should raise a Puppet::Error" do
+    let(:release) { "natty" }
     let(:title) { "ppa" }
     it { expect { should contain_apt__ppa(title) }.to raise_error(Puppet::Error) }
   end
+
+  describe "without release should raise a Puppet::Error" do
+    let(:title) { "ppa:" }
+    it { expect { should contain_apt__ppa(:release) }.to raise_error(Puppet::Error) }
+  end
+
 end
diff --git a/spec/defines/source_spec.rb b/spec/defines/source_spec.rb
index 3cafb52..903468e 100644
--- a/spec/defines/source_spec.rb
+++ b/spec/defines/source_spec.rb
@@ -42,6 +42,10 @@ describe 'apt::source', :type => :define do
         default_params.merge(param_set)
       end
 
+      let :facts do
+        {:lsbdistcodename => 'karmic'}
+      end
+
       let :params do
         param_set
       end
@@ -132,5 +136,8 @@ describe 'apt::source', :type => :define do
       }
     end
   end
+    describe "without release should raise a Puppet::Error" do
+      it { expect { should contain_apt__source(:release) }.to raise_error(Puppet::Error) }
+    end
 end