From: David Schmitt <david.schmitt@puppet.com>
Date: Thu, 21 Sep 2017 10:24:55 +0000 (+0100)
Subject: Add basic create/delete handling
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a34622291a79dd63ea4afa07f10e6ea94c6345de;p=puppet-modules%2Fpuppetlabs-apt.git

Add basic create/delete handling

This still requires more work to match up to the original implementation,
but suffices for basic acceptance testing.
---

diff --git a/lib/puppet/provider/apt_key2/apt_key2.rb b/lib/puppet/provider/apt_key2/apt_key2.rb
index 3b6cd91..c4cf9de 100644
--- a/lib/puppet/provider/apt_key2/apt_key2.rb
+++ b/lib/puppet/provider/apt_key2/apt_key2.rb
@@ -89,42 +89,55 @@ class Puppet::Provider::AptKey2::AptKey2
     }
   end
 
-  def set(current_state, target_state, noop = false)
-    target_state.each do |title, resource|
-      logger.warning(title, 'The id should be a full fingerprint (40 characters) to avoid collision attacks, see the README for details.') if title.length < 40
-      if resource[:source] && resource[:content]
-        logger.fail(title, 'The properties content and source are mutually exclusive')
-        next
-      end
-
-      current = current_state[title]
-      if current && resource[:ensure].to_s == 'absent'
-        logger.deleting(title) do
-          begin
-            apt_key('del', resource[:short], noop: noop)
-            r = execute(["#{command(:apt_key)} list | grep '/#{resource[:short]}\s'"], failonfail: false)
-          end while r.exitstatus.zero?
-        end
-      elsif current && resource[:ensure].to_s == 'present'
-        logger.warning(title, 'No updating implemented')
-        # update(key, noop: noop)
-      elsif !current && resource[:ensure].to_s == 'present'
-        create(title, resource, noop: noop)
+  def set(context, changes)
+    changes.each do |name, change|
+      is = change.key?(:is) ? change[:is] : get_single(name)
+      should = change[:should]
+      context.warning(name, 'The id should be a full fingerprint (40 characters) to avoid collision attacks, see the README for details.') if name.length < 40
+
+      is = { id: name, ensure: 'absent' } if is.nil?
+      should = { id: name, ensure: 'absent' } if should.nil?
+
+      if is[:ensure].to_s == 'absent' && should[:ensure].to_s == 'present'
+        create(context, name, should)
+      elsif is[:ensure].to_s == 'present' && should[:ensure].to_s == 'absent'
+        delete(context, name)
       end
     end
+    # target_state.each do |title, resource|
+    #   if resource[:source] && resource[:content]
+    #     logger.fail(title, 'The properties content and source are mutually exclusive')
+    #     next
+    #   end
+
+    #   current = current_state[title]
+    #   if current && resource[:ensure].to_s == 'absent'
+    #     logger.deleting(title) do
+    #       begin
+    #         apt_key('del', resource[:short], noop: noop)
+    #         r = execute(["#{command(:apt_key)} list | grep '/#{resource[:short]}\s'"], failonfail: false)
+    #       end while r.exitstatus.zero?
+    #     end
+    #   elsif current && resource[:ensure].to_s == 'present'
+    #     logger.warning(title, 'No updating implemented')
+    #     # update(key, noop: noop)
+    #   elsif !current && resource[:ensure].to_s == 'present'
+    #     create(title, resource, noop: noop)
+    #   end
+    # end
   end
 
-  def create(title, resource, noop = false)
-    logger.creating(title) do |logger|
+  def create(global_context, title, resource, noop = false)
+    global_context.creating(title) do |context|
       if resource[:source].nil? && resource[:content].nil?
         # Breaking up the command like this is needed because it blows up
         # if --recv-keys isn't the last argument.
-        args = ['adv', '--keyserver', resource[:server]]
+        args = ['adv', '--keyserver', resource[:server].to_s]
         if resource[:options]
           args.push('--keyserver-options', resource[:options])
         end
         args.push('--recv-keys', resource[:id])
-        apt_key(*args, noop: noop)
+        @apt_key_cmd.run(context, *args, noop: noop)
       elsif resource[:content]
         temp_key_file(resource[:content], logger) do |key_file|
           apt_key('add', key_file, noop: noop)
@@ -134,11 +147,17 @@ class Puppet::Provider::AptKey2::AptKey2
         apt_key('add', key_file.path, noop: noop)
         # In case we really screwed up, better safe than sorry.
       else
-        logger.fail("an unexpected condition occurred while trying to add the key: #{title} (content: #{resource[:content].inspect}, source: #{resource[:source].inspect})")
+        context.fail("an unexpected condition occurred while trying to add the key: #{title} (content: #{resource[:content].inspect}, source: #{resource[:source].inspect})")
       end
     end
   end
 
+  def delete(global_context, title, noop = false)
+    global_context.deleting(title) do |context|
+      @apt_key_cmd.run(context, 'del', title, noop: noop)
+    end
+  end
+
   # This method writes out the specified contents to a temporary file and
   # confirms that the fingerprint from the file, matches the long key that is in the manifest
   def temp_key_file(resource, logger)
diff --git a/spec/unit/puppet/provider/apt_key2/apt_key2_spec.rb b/spec/unit/puppet/provider/apt_key2/apt_key2_spec.rb
index f98e031..9b1c744 100644
--- a/spec/unit/puppet/provider/apt_key2/apt_key2_spec.rb
+++ b/spec/unit/puppet/provider/apt_key2/apt_key2_spec.rb
@@ -108,4 +108,88 @@ EOS
       ]
     end
   end
+
+  describe '#set(context, changes)' do
+    let(:apt_key_cmd) { instance_double('Puppet::ResourceApi::Command') }
+    let(:process) { instance_double('ChildProcess::AbstractProcess') }
+    let(:io) { instance_double('ChildProcess::AbstractIO') }
+    let(:id) { 'A' * 40 }
+
+    before(:each) do
+      allow(Puppet::ResourceApi::Command).to receive(:new).and_return(apt_key_cmd)
+    end
+
+    context 'when passing in empty changes' do
+      it 'does nothing' do
+        expect { provider.set(context, {}) }.not_to raise_error
+      end
+    end
+
+    context 'when managing a up-to-date key' do
+      it 'does nothing' do
+        expect {
+          provider.set(context, id => {
+                         is: {
+                           id: id, ensure: :present
+                         },
+                         should: {
+                           id: id, ensure: :present
+                         },
+                       })
+        }.not_to raise_error
+      end
+    end
+
+    context 'when managing an absent key' do
+      it 'does nothing' do
+        provider.set(context, id =>
+        {
+          is: nil,
+          should: {
+            id: id,
+            ensure: :absent,
+          },
+        })
+      end
+    end
+
+    context 'when fetching a key from the keyserver' do
+      let(:creating_ctx) { instance_double('Puppet::ResourceApi::BaseContext', 'creating_ctx') }
+
+      it 'updates the system' do
+        expect(context).to receive(:creating).with(id).and_yield(creating_ctx)
+        expect(apt_key_cmd).to receive(:run).with(creating_ctx, 'adv', '--keyserver', 'keyserver.example.com', '--recv-keys', id, noop: false).and_return 0
+        provider.set(context, id =>
+        {
+          is: nil,
+          should: {
+            id: id,
+            ensure: :present,
+            server: :'keyserver.example.com',
+          },
+        })
+      end
+    end
+
+    context 'when deleting a key' do
+      let(:deleting_ctx) { instance_double('Puppet::ResourceApi::BaseContext', 'deleting_ctx') }
+
+      it 'updates the system' do
+        expect(context).to receive(:deleting).with(id).and_yield(deleting_ctx)
+        expect(apt_key_cmd).to receive(:run).with(deleting_ctx, 'del', id, noop: false).and_return 0
+        provider.set(context, id =>
+        {
+          is: {
+            id: id,
+            ensure: :present,
+            server: :'keyserver.ubuntu.com',
+          },
+          should: {
+            id: id,
+            ensure: :absent,
+          },
+        })
+      end
+    end
+  end
 end