]> review.fuel-infra Code Review - puppet-modules/puppetlabs-apt.git/commitdiff
Use short id (8 chars) when deleting an apt-key
authorJames Stocks <james.stocks@puppet.com>
Tue, 26 Sep 2017 16:21:08 +0000 (16:21 +0000)
committerJames Stocks <james.stocks@puppet.com>
Tue, 26 Sep 2017 16:21:08 +0000 (16:21 +0000)
See https://bugs.launchpad.net/ubuntu/+source/apt/+bug/1481871 for why the full 40 characters cannot be used on some systems.

lib/puppet/provider/apt_key2/apt_key2.rb
spec/unit/puppet/provider/apt_key2/apt_key2_spec.rb

index 6ebdd7e042506790334f693c6c7d44e3188de98c..5612cbc8d36e9796df38f8726d3c38ef03546fa4 100644 (file)
@@ -176,7 +176,10 @@ class Puppet::Provider::AptKey2::AptKey2
 
   def delete(context, name, noop = false)
     context.deleting(name) do
-      @apt_key_cmd.run(context, 'del', name, noop: noop)
+      # Although canonicalize logs a warning NOT to use the short id instead of all 40 characters, `apt-key del` fails to delete
+      # on some systems unless the short id is used. Additionally, such systems will return 0 even though deletion failed.
+      # Ref: https://bugs.launchpad.net/ubuntu/+source/apt/+bug/1481871
+      @apt_key_cmd.run(context, 'del', name[-8..-1], noop: noop)
     end
   end
 
index 0b108c39b1bf3ebf5a0e5c0db82ea839cad2225a..a0d7a4c4f5a162d4f1707d87654ad6d34291b4e3 100644 (file)
@@ -130,6 +130,7 @@ EOS
     let(:process) { instance_double('ChildProcess::AbstractProcess') }
     let(:io) { instance_double('ChildProcess::AbstractIO') }
     let(:fingerprint) { 'A' * 40 }
+    let(:short) { 'A' * 8 }
 
     before(:each) do
       allow(Puppet::ResourceApi::Command).to receive(:new).and_return(apt_key_cmd)
@@ -216,7 +217,7 @@ EOS
     context 'when deleting a key' do
       it 'updates the system' do
         expect(context).to receive(:deleting).with(fingerprint).and_yield
-        expect(apt_key_cmd).to receive(:run).with(context, 'del', fingerprint, noop: false).and_return 0
+        expect(apt_key_cmd).to receive(:run).with(context, 'del', short, noop: false).and_return 0
         provider.set(context, fingerprint =>
         {
           is: {