]> review.fuel-infra Code Review - puppet-modules/puppetlabs-apt.git/commitdiff
Refine key validation vs content; port more tests to testmode switcher
authorDavid Schmitt <david.schmitt@puppet.com>
Sat, 23 Sep 2017 18:07:03 +0000 (19:07 +0100)
committerDavid Schmitt <david.schmitt@puppet.com>
Mon, 25 Sep 2017 12:30:43 +0000 (13:30 +0100)
lib/puppet/provider/apt_key2/apt_key2.rb
spec/acceptance/apt_key_provider_spec.rb
spec/unit/puppet/provider/apt_key2/apt_key2_spec.rb

index 2b21c0e51df510a97b37d30bbbc49bf6a7fbfe89..531fb83b657a77cb2a60c71b0c90b06458694a46 100644 (file)
@@ -186,19 +186,20 @@ class Puppet::Provider::AptKey2::AptKey2
     begin
       file.write content
       file.close
-      if title.size == 40
-        if File.executable? '/usr/bin/gpg'
-          extracted_key = `/usr/bin/gpg --with-fingerprint --with-colons #{file.path}`.each_line.find { |line| line =~ %r{^fpr:} }.split(':')[9]
-
-          if extracted_key =~ %r{^#{title}$}
-            context.debug('Fingerprint verified against extracted key')
-          else
-            raise ArgumentError, "The fingerprint in your manifest (#{title}) and the fingerprint from content/source (#{extracted_key}) do not match. "\
-              ' Please check there is not an error in the name or check the content/source is legitimate.'
-          end
+      if File.executable? '/usr/bin/gpg'
+        extracted_key = `/usr/bin/gpg --with-fingerprint --with-colons #{file.path}`.each_line.find { |line| line =~ %r{^fpr:} }.split(':')[9]
+
+        case extracted_key
+        when title
+          context.debug('Fingerprint verified against extracted key')
+        when %r{#{title}$}
+          context.debug('Fingerprint matches the extracted key')
         else
-          context.warning('/usr/bin/gpg cannot be found for verification of the fingerprint.')
+          raise ArgumentError, "The fingerprint in your manifest (#{title}) and the fingerprint from content/source (#{extracted_key}) do not match. "\
+            ' Please check there is not an error in the name or check the content/source is legitimate.'
         end
+      else
+        context.warning('/usr/bin/gpg cannot be found for verification of the fingerprint.')
       end
       yield file.path
     ensure
index 8e1f73e6c842471393e8855e3448c7aa0c714153..5df5e0aa3714f37c52886c38735bf6d183e120c7 100644 (file)
@@ -31,88 +31,74 @@ end
     fedora = {
       fingerprint: '128CF232A9371991C8A65695E08E7E629DB62FB1'.freeze,
       short: '9DB62FB1'.freeze,
+      long: 'E08E7E629DB62FB1'.freeze,
       content: my_fixture_read('fedora.txt').freeze,
     }.freeze
 
     after(:each) do
-      shell_ex("apt-key del #{fedora[:short]} > /dev/null")
       # Delete twice to make sure everything is cleaned
       # up after the short key collision
-      shell_ex("apt-key del #{fedora[:short]} > /dev/null")
+      shell_ex("apt-key del #{fedora[:fingerprint]} > /dev/null; apt-key del #{fedora[:fingerprint]} > /dev/null")
     end
 
-    # context 'with an already installed key' do
-    #   # run on :all hook to cooperate with `a puppet resource run` shared context
-    #   before(:all) do
-    #     # puts "apt-key add #{my_fixture('fedora.txt')}"
-    #     shell_ex("apt-key add #{my_fixture('fedora.txt')} >/dev/null 2>&1")
-    #   end
-
-    #   context 'when looked for using puppet resource' do
-    #     include_context 'a puppet resource run', typename, fedora[:fingerprint], trace: true
-    #     puppet_resource_should_show('ensure', 'present')
-    #     puppet_resource_should_show('fingerprint',fedora[:fingerprint])
-    #     puppet_resource_should_show('long', fedora[:fingerprint][-16..-1])
-    #     puppet_resource_should_show('short', fedora[:short])
-    #     # puppet_resource_should_show('created', '2017-08-14')
-    #     pending "Correctly calculate the created date"
-    #     puppet_resource_should_show('expired', 'false')
-    #     puppet_resource_should_show('size', '4096')
-    #     puppet_resource_should_show('type', ':?rsa')
-    #   end
-
-    # end
+    context 'with an already installed key' do
+      # run on :all hook to cooperate with `a puppet resource run` shared context
+      before(:all) do
+        # puts "apt-key add #{my_fixture('fedora.txt')}"
+        shell_ex("apt-key add #{my_fixture('fedora.txt')} >/dev/null 2>&1")
+      end
+
+      context 'when looked for using puppet resource' do
+        include_context 'a puppet resource run', typename, fedora[:fingerprint], trace: true
+        puppet_resource_should_show('ensure', 'present')
+        puppet_resource_should_show('fingerprint', fedora[:fingerprint])
+        puppet_resource_should_show('long', fedora[:fingerprint][-16..-1])
+        puppet_resource_should_show('short', fedora[:short])
+        # puppet_resource_should_show('created', '2017-08-14')
+        pending 'Correctly calculate the created date'
+        puppet_resource_should_show('expired', 'false')
+        puppet_resource_should_show('size', '4096')
+        puppet_resource_should_show('type', ':?rsa')
+      end
+    end
 
     describe 'default options' do
       key_versions = {
-        '32bit key id' => fedora[:short],
-        # '64bit key id'                        => PUPPETLABS_GPG_KEY_LONG_ID.to_s,
-        # '160bit key fingerprint'              => PUPPETLABS_GPG_KEY_FINGERPRINT.to_s,
-        # '32bit lowercase key id'              => PUPPETLABS_GPG_KEY_SHORT_ID.downcase.to_s,
-        # '64bit lowercase key id'              => PUPPETLABS_GPG_KEY_LONG_ID.downcase.to_s,
-        # '160bit lowercase key fingerprint'    => PUPPETLABS_GPG_KEY_FINGERPRINT.downcase.to_s,
-        # '0x formatted 32bit key id'           => "0x#{PUPPETLABS_GPG_KEY_SHORT_ID}",
-        # '0x formatted 64bit key id'           => "0x#{PUPPETLABS_GPG_KEY_LONG_ID}",
-        # '0x formatted 160bit key fingerprint' => "0x#{PUPPETLABS_GPG_KEY_FINGERPRINT}",
-        # '0x formatted 32bit lowercase key id' => "0x#{PUPPETLABS_GPG_KEY_SHORT_ID.downcase}",
-        # '0x formatted 64bit lowercase key id' => "0x#{PUPPETLABS_GPG_KEY_LONG_ID.downcase}",
-        # '0x formatted 160bit lowercase key fingerprint' => "0x#{PUPPETLABS_GPG_KEY_FINGERPRINT.downcase}",
+        '32bit key id'           => fedora[:short],
+        '64bit key id'           => fedora[:long],
+        '160bit key fingerprint' => fedora[:fingerprint],
       }
 
-      key_versions.each do |key, value|
-        context key.to_s do
-          it 'works' do
-            pp = <<-EOS
-              #{typename} { 'puppetlabs':
-                name    => '#{value}',
-                ensure  => 'present',
-                content => '#{fedora[:content]}',
-              }
-            EOS
-
-            puts 'Add'
-            execute_manifest(pp, trace: true, catch_failures: true)
-            puts 'Check 1'
-            check_key(fedora[:short])
-            puts 'Ensure'
-            execute_manifest(pp, trace: true, catch_changes: true)
-            puts 'Check 2'
-            check_key(fedora[:short])
-          end
+      key_versions.merge!(Hash[key_versions.map { |name, id| ["#{name}, lowercase", id.downcase] }])
+      key_versions.merge!(Hash[key_versions.map { |name, id| ["#{name}, 0x prefix", "0x#{id}"] }])
+
+      key_versions.each do |name, id|
+        it "works with #{name}: #{id}" do
+          pp = <<-EOS
+            #{typename} { 'test_key':
+              name    => '#{id}',
+              ensure  => 'present',
+              content => '#{fedora[:content]}',
+            }
+          EOS
+
+          execute_manifest(pp, trace: true, catch_failures: true)
+          check_key(fedora[:fingerprint])
+          execute_manifest(pp, trace: true, catch_changes: true)
+          check_key(fedora[:fingerprint])
         end
       end
 
-      context 'invalid length key id' do
-        it 'fails' do
+      context 'when specifying a key id with invalid length' do
+        it 'reports an error' do
           pp = <<-EOS
-        #{typename} { 'puppetlabs':
-          id => '8280EF8D349F',
-        }
-        EOS
+            #{typename} { 'puppetlabs':
+              id => '8280EF8D349F',
+            }
+          EOS
 
-          apply_manifest(pp, expect_failures: true) do |r|
-            expect(r.stderr).to match(%r{Valid values match})
-          end
+          result = execute_manifest(pp, expect_failures: true)
+          expect(result.stderr).to match(%r{Valid values match})
         end
       end
     end
index 0ccba40e6bcd2c7eda40cf6b205be6cf680574e2..1b1b378f5a9b974fc2a92cfc181174d0f9203edd 100644 (file)
@@ -29,7 +29,7 @@ EOS
 
   describe '#canonicalize(resources)' do
     before(:each) do
-      allow(provider).to receive(:`).with('apt-key adv --list-keys --with-colons --fingerprint --fixed-list-mode 2>/dev/null').and_return(key_list)
+      allow(provider).to receive(:`).with('apt-key adv --list-keys --with-colons --fingerprint --fixed-list-mode 2>/dev/null').and_return(key_list) # rubocop:disable RSpec/SubjectStub
       allow(context).to receive(:warning)
     end
 
@@ -196,11 +196,11 @@ EOS
         allow(key_tempfile).to receive(:path).with(no_args).and_return('tempfilename')
         allow(key_tempfile).to receive(:close)
         expect(key_tempfile).to receive(:unlink)
-        expect(provider).to receive(:`).with('/usr/bin/gpg --with-fingerprint --with-colons tempfilename').and_return("\nfpr:::::::::#{fingerprint}:\n")
+        expect(provider).to receive(:`).with('/usr/bin/gpg --with-fingerprint --with-colons tempfilename').and_return("\nfpr:::::::::#{fingerprint}:\n") # rubocop:disable RSpec/SubjectStub
         expect(creating_ctx).to receive(:debug).with('Fingerprint verified against extracted key')
 
         # expect(apt_key_cmd).to receive(:run).with(creating_ctx, 'add', 'tempfilename', noop: false).and_return 0
-        expect(provider).to receive(:system).with('apt-key add tempfilename')
+        expect(provider).to receive(:system).with('apt-key add tempfilename') # rubocop:disable RSpec/SubjectStub
         provider.set(context, fingerprint =>
         {
           is: nil,