]> review.fuel-infra Code Review - puppet-modules/puppetlabs-apt.git/commitdiff
(#12823) Add apt::key defined type and modify apt::source to use it
authorReid Vandewiele <marut@cat.pdx.edu>
Fri, 24 Feb 2012 18:10:03 +0000 (10:10 -0800)
committerReid Vandewiele <marut@cat.pdx.edu>
Thu, 1 Mar 2012 22:15:52 +0000 (14:15 -0800)
Adding this defined type allows puppet to add keys to the apt keystore without
needing to add a corresponding source; it also adds the "key_source" parameter
for wget'ing keys from arbitrary URLs, and allows for keys to be explicity
removed.

apt::key allows a key to be ensured present multiple times to account for
apt::source resources that all reference the same key. However, this means
that it is possible for a given key to be defined multiple times with
differing source parameters. e.g.

apt::key { "Add key: 4BD6EC30 from Apt::Source bunny":
  key        => "4BD6EC30",
  key_server => "pgp.mit.edu",
}

apt::key { "Add key: 4BD6EC30 from Apt::Source rabbit":
  key        => "4BD6EC30",
  key_server => "keyserver.ubuntu.com",
}

The defined type will accept both definitions and will create multiple exec
resources. This was deemed preferable to the alternative (creating only one
exec resource) in that one broken definition won't hose an entire catalog. If
one definition fails to install the key because of a bad "key_server", the
next apt::key that uses the key will get it done.

README.md
manifests/key.pp [new file with mode: 0644]
manifests/source.pp
spec/defines/key_spec.rb [new file with mode: 0644]
spec/defines/source_spec.rb

index 0c61866b0cec5a4ef4cf9da2e8a98c73d5cf3312..b60394f09520f9204c74ece8c9f7dc9ea24d954d 100644 (file)
--- a/README.md
+++ b/README.md
@@ -53,5 +53,24 @@ apt::source { "debian_unstable":
   key_server        => "subkeys.pgp.net",
   pin               => "-10",
   include_src       => true
+
+### apt::key
+Add a key to the list of keys used by apt to authenticate packages.
+<pre>
+apt::key { "puppetlabs":
+  key        => "4BD6EC30",
+  key_server => "pgp.mit.edu",
+}
+</pre>
+
+<pre>
+apt::key { "jenkins":
+  key        => "D50582E6",
+  key_source => "http://pkg.jenkins-ci.org/debian/jenkins-ci.org.key",
+}
+</pre>
+
+Note that use of the "key_source" parameter requires wget to be installed and working.
+
 }
 </pre>
diff --git a/manifests/key.pp b/manifests/key.pp
new file mode 100644 (file)
index 0000000..24eef9e
--- /dev/null
@@ -0,0 +1,68 @@
+define apt::key (
+  $key = $title,
+  $ensure = present,
+  $key_content = false,
+  $key_source = false,
+  $key_server = "keyserver.ubuntu.com"
+) {
+
+  include apt::params
+
+  if $key_content {
+    $method = "content"
+  } elsif $key_source {
+    $method = "source"
+  } elsif $key_server {
+    $method = "server"
+  }
+
+  # This is a hash of the parts of the key definition that we care about.
+  # It is used as a unique identifier for this instance of apt::key. It gets
+  # hashed to ensure that the resource name doesn't end up being pages and
+  # pages (e.g. in the situation where key_content is specified).
+  $digest = sha1("${key}/${key_content}/${key_source}/${key_server}/")
+
+  # Allow multiple ensure => present for the same key to account for many
+  # apt::source resources that all reference the same key.
+  case $ensure {
+    present: {
+      if defined(Exec["apt::key $key absent"]) {
+        fail ("Cannot ensure Apt::Key[$key] present; $key already ensured absent")
+      } elsif !defined(Exec["apt::key $key present"]) {
+        # this is a marker to ensure we don't simultaneously define a key
+        # ensure => absent AND ensure => present
+        exec { "apt::key $key present":
+          path   => "/",
+          onlyif => "/bin/false",
+          noop   => true;
+        }
+      }
+      if !defined(Exec[$digest]) {
+        exec { $digest:
+          path    => "/bin:/usr/bin",
+          unless  => "/usr/bin/apt-key list | /bin/grep '${key}'",
+          command => $method ? {
+            "content" => "echo '${key_content}' | /usr/bin/apt-key add -",
+            "source"  => "wget -q '${key_source}' -O- | apt-key add -",
+            "server"  => "apt-key adv --keyserver '${key_server}' --recv-keys '${key}'",
+          };
+        }
+      }
+    }
+    absent: {
+      if defined(Exec["apt::key $key present"]) {
+        fail ("Cannot ensure Apt::Key[$key] absent; $key already ensured present")
+      }
+      exec { "apt::key $key absent":
+        path    => "/bin:/usr/bin",
+        onlyif  => "apt-key list | grep '$key'",
+        command => "apt-key del '$key'",
+        user    => "root",
+        group   => "root",
+      }
+    }
+    default: {
+      fail "Invalid 'ensure' value '$ensure' for aptkey"
+    }
+  }
+}
index 9f31fe91308bbb911b5013a21227b17e4f0e11db..475bee3bf04c4e9a069eb615df8cd28916a91296 100644 (file)
@@ -9,8 +9,9 @@ define apt::source(
   $required_packages = false,
   $key = false,
   $key_server = 'keyserver.ubuntu.com',
-  $pin = false,
-  $key_content = false
+  $key_content = false,
+  $key_source  = false,
+  $pin = false
 ) {
 
   include apt::params
@@ -44,18 +45,13 @@ define apt::source(
   }
 
   if $key != false {
-    if $key_content {
-      exec { "Add key: ${key} from content for ${name}":
-        command => "/bin/echo '${key_content}' | /usr/bin/apt-key add -",
-        unless => "/usr/bin/apt-key list | /bin/grep '${key}'",
-        before => File["${name}.list"],
-      }
-    } else {
-      exec { "Add key: ${key} from ${key_server} for ${name}":
-        command => "/usr/bin/apt-key adv --keyserver ${key_server} --recv-keys ${key}",
-        unless => "/usr/bin/apt-key list | /bin/grep ${key}",
-        before => File["${name}.list"],
-      }
+    apt::key { "Add key: ${key} from Apt::Source ${title}":
+      key         => $key,
+      ensure      => present,
+      key_server  => $key_server,
+      key_content => $key_content,
+      key_source  => $key_source,
+      before      => File["${name}.list"],
     }
   }
 }
diff --git a/spec/defines/key_spec.rb b/spec/defines/key_spec.rb
new file mode 100644 (file)
index 0000000..88038d2
--- /dev/null
@@ -0,0 +1,114 @@
+require 'spec_helper'
+describe 'apt::key', :type => :define do
+  let :title do
+    '8347A27F'
+  end
+
+  let :default_params do
+    {
+      :key         => title,
+      :ensure      => 'present',
+      :key_server  => "keyserver.ubuntu.com",
+      :key_source  => false,
+      :key_content => false
+    }
+  end
+
+  [{},
+    {
+      :ensure  => 'absent'
+    },
+    {
+      :ensure  => 'random'
+    },
+    {
+      :key_source => 'ftp://ftp.example.org/key',
+    },
+    {
+      :key_content => 'deadbeef',
+    }
+  ].each do |param_set|
+
+    let :param_hash do
+      default_params.merge(param_set)
+    end
+
+    let :params do
+      param_set
+    end
+
+    let :digest do
+      str = String.new
+      str << param_hash[:key].to_s         << '/'
+      str << param_hash[:key_content].to_s << '/'
+      str << param_hash[:key_source].to_s  << '/'
+      str << param_hash[:key_server].to_s  << '/'
+      Digest::SHA1.hexdigest(str)
+    end
+
+    describe "when #{param_set == {} ? "using default" : "specifying"} define parameters" do
+
+      it {
+        if [:present, 'present', :absent, 'absent'].include? param_hash[:ensure]
+          should contain_apt__params
+        end
+      }
+
+      it {
+        if [:present, 'present'].include? param_hash[:ensure]
+          should_not contain_exec("apt::key #{param_hash[:key]} absent")
+          should contain_exec("apt::key #{param_hash[:key]} present")
+          should contain_exec(digest).with({
+            "path"    => "/bin:/usr/bin",
+            "unless"  => "/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'"
+          })
+        elsif [:absent, 'absent'].include? param_hash[:ensure]
+          should_not contain_exec("apt::key #{param_hash[:key]} present")
+          should contain_exec("apt::key #{param_hash[:key]} absent").with({
+            "path"    => "/bin:/usr/bin",
+            "onlyif"  => "apt-key list | grep '#{param_hash[:key]}'",
+            "command" => "apt-key del '#{param_hash[:key]}'"
+          })
+        else
+          expect { should raise_error(Puppet::Error) }
+        end
+      }
+
+      it {
+        if [:present, 'present'].include? param_hash[:ensure]
+          if param_hash[:key_content]
+            should contain_exec(digest).with({
+              "command" => "echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -"
+            })
+          elsif param_hash[:key_source]
+            should contain_exec(digest).with({
+              "command" => "wget -q '#{param_hash[:key_source]}' -O- | apt-key add -"
+            })
+          elsif param_hash[:key_server]
+            should contain_exec(digest).with({
+              "command" => "apt-key adv --keyserver '#{param_hash[:key_server]}' --recv-keys '#{param_hash[:key]}'"
+            })
+          end
+        end
+      }
+
+    end
+
+    describe "should correctly handle duplicate definitions" do
+      let :pre_condition do
+        "apt::key { 'duplicate': key => '#{params[:key]}'; }"
+      end
+
+      it {
+        if [:present, 'present'].include? param_hash[:ensure]
+          should contain_exec("apt::key #{param_hash[:key]} present")
+          should contain_apt__key("duplicate")
+          should contain_apt__key(title)
+        elsif [:absent, 'absent'].include? params[:ensure]
+          expect { should raise_error(Puppet::Error) }
+        end
+      }
+
+    end
+  end
+end
index 041175c1cee8db98d196ae4248e5e71b177955d0..3cafb5251bb93de44bc1a1e29b88e56f0683f567 100644 (file)
@@ -13,8 +13,9 @@ describe 'apt::source', :type => :define do
       :required_packages  => false,
       :key                => false,
       :key_server         => 'keyserver.ubuntu.com',
-      :pin                => false,
-      :key_content        => false
+      :key_content        => false,
+      :key_source         => false,
+      :pin                => false
     }
   end
 
@@ -110,48 +111,22 @@ describe 'apt::source', :type => :define do
 
       it {
         if param_hash[:key]
-          if param_hash[:key_content]
-            should contain_exec("Add key: #{param_hash[:key]} from content for #{title}").with({
-              "command" => "/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -",
-              "unless"  => "/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'",
-              "before"  => "File[#{title}.list]"
-            })
-          else
-            should_not contain_exec("Add key: #{param_hash[:key]} from content for #{title}").with({
-                "command" => "/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -",
-                "unless"  => "/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'",
-                "before"  => "File[#{title}.list]"
-            })
-          end
-        else
-          should_not contain_exec("Add key: #{param_hash[:key]} from content for #{title}").with({
-            "command"   => "/bin/echo '#{param_hash[:key_content]}' | /usr/bin/apt-key add -",
-            "unless"    => "/usr/bin/apt-key list | /bin/grep '#{param_hash[:key]}'",
-            "before"    => "File[#{title}.list]"
+          should contain_apt__key("Add key: #{param_hash[:key]} from Apt::Source #{title}").with({
+            "key"         => param_hash[:key],
+            "ensure"      => :present,
+            "key_server"  => param_hash[:key_server],
+            "key_content" => param_hash[:key_content],
+            "key_source"  => param_hash[:key_source],
+            "before"      => "File[#{title}.list]"
           })
-        end
-      }
-
-      it {
-        if param_hash[:key]
-          if param_hash[:key_content]
-            should_not contain_exec("Add key: #{param_hash[:key]} from #{param_hash[:key_server]} for #{title}").with({
-                "command" => "/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}",
-                "unless"  => "/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}",
-                "before"  => "File[#{title}.list]"
-            })
-          else
-            should contain_exec("Add key: #{param_hash[:key]} from #{param_hash[:key_server]} for #{title}").with({
-                "command" => "/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}",
-                "unless"  => "/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}",
-                "before"  => "File[#{title}.list]"
-            })
-          end
         else
-          should_not contain_exec("Add key: #{param_hash[:key]} from #{param_hash[:key_server]} for #{title}").with({
-              "command" => "/usr/bin/apt-key adv --keyserver #{param_hash[:key_server]} --recv-keys #{param_hash[:key]}",
-              "unless"  => "/usr/bin/apt-key list | /bin/grep #{param_hash[:key]}",
-              "before"  => "File[#{title}.list]"
+          should_not contain_apt__key("Add key: #{param_hash[:key]} from Apt::Source #{title}").with({
+            "key"         => param_hash[:key],
+            "ensure"      => :present,
+            "key_server"  => param_hash[:key_server],
+            "key_content" => param_hash[:key_content],
+            "key_source"  => param_hash[:key_source],
+            "before"      => "File[#{title}.list]"
           })
         end
       }