Fix (and test for) 'puppet resource rabbitmq_binding) (#650)

lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb

@@ -3,17 +3,13 @@
 require 'digest'
 
 Puppet::Type.type(:rabbitmq_binding).provide(:rabbitmqadmin) do
-  if Puppet::PUPPETVERSION.to_f < 3
-    commands rabbitmqctl: 'rabbitmqctl'
-    commands rabbitmqadmin: '/usr/local/bin/rabbitmqadmin'
-  else
-    has_command(:rabbitmqctl, 'rabbitmqctl') do
-      environment HOME: '/tmp'
-    end
-    has_command(:rabbitmqadmin, '/usr/local/bin/rabbitmqadmin') do
-      environment HOME: '/tmp'
-    end
+  has_command(:rabbitmqctl, 'rabbitmqctl') do
+    environment HOME: '/tmp'
   end
+  has_command(:rabbitmqadmin, '/usr/local/bin/rabbitmqadmin') do
+    environment HOME: '/tmp'
+  end
+
   defaultfor feature: :posix
 
   # Without this, the composite namevar stuff doesn't work properly.
@@ -85,6 +81,9 @@ def exists?
   end
 
   def create
+    # Validate that we have both source and destination now that these are not
+    # necessarily only coming from the resource title.
+    raise Puppet::Error, "Source and destination must both be defined (binding: #{name})" if @resource[:source].nil? || @resource[:destination].nil?
     vhost_opt = should_vhost ? "--vhost=#{should_vhost}" : ''
     arguments = resource[:arguments]
     arguments = {} if arguments.nil?

lib/puppet/type/rabbitmq_binding.rb

@@ -174,12 +174,4 @@ def setup_autorequire(type)
   def validate_argument(argument)
     raise ArgumentError, 'Invalid argument' unless [Hash].include?(argument.class)
   end
-
-  # Validate that we have both source and destination now that these are not
-  # necessarily only coming from the resource title.
-  validate do
-    unless self[:source] && self[:destination]
-      raise ArgumentError, 'Source and destination must both be defined.'
-    end
-  end
 end

spec/acceptance/queue_spec.rb

@@ -159,6 +159,12 @@ class { 'rabbitmq':
       end
     end
     # rubocop:enable RSpec/MultipleExpectations
+
+    it 'puppet resource shows a binding' do
+      shell('puppet resource rabbitmq_binding') do |r|
+        expect(r.stdout).to match(%r{source\s+=>\s+'exchange1',})
+      end
+    end
   end
 
   context 'create binding and queue resources when using a non-default management port' do

spec/unit/puppet/provider/rabbitmq_binding/rabbitmqadmin_spec.rb

@@ -18,7 +18,9 @@
       provider_class.expects(:rabbitmqctl).with('list_vhosts', '-q').returns <<-EOT
 /
 EOT
-      provider_class.expects(:rabbitmqctl).with('list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments').returns <<-EOT
+      provider_class.expects(:rabbitmqctl).with(
+        'list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments'
+      ).returns <<-EOT
 exchange\tdst_queue\tqueue\t*\t[]
 EOT
       instances = provider_class.instances
@@ -46,7 +48,9 @@
       provider_class.expects(:rabbitmqctl).with('list_vhosts', '-q').returns <<-EOT
 /
 EOT
-      provider_class.expects(:rabbitmqctl).with('list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments').returns <<-EOT
+      provider_class.expects(:rabbitmqctl).with(
+        'list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments'
+      ).returns <<-EOT
 exchange\tdst_queue\tqueue\trouting_one\t[]
 exchange\tdst_queue\tqueue\trouting_two\t[]
 EOT
@@ -93,7 +97,9 @@
       provider_class.expects(:rabbitmqctl).with('list_vhosts', '-q').returns <<-EOT
 /
 EOT
-      provider_class.expects(:rabbitmqctl).with('list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments').returns <<-EOT
+      provider_class.expects(:rabbitmqctl).with(
+        'list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments'
+      ).returns <<-EOT
 exchange\tdst_queue\tqueue\t*\t[]
 EOT
 
@@ -105,59 +111,107 @@
       provider_class.expects(:rabbitmqctl).with('list_vhosts', '-q').returns <<-EOT
 /
 EOT
-      provider_class.expects(:rabbitmqctl).with('list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments').returns <<-EOT
+      provider_class.expects(:rabbitmqctl).with(
+        'list_bindings', '-q', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments'
+      ).returns <<-EOT
 exchange\tdst_queue\tqueue\t*\t[]
 EOT
 
       provider_class.prefetch('binding1' => resource)
     end
   end
 
-  it 'calls rabbitmqadmin to create' do
-    provider.expects(:rabbitmqadmin).with('declare', 'binding', '--vhost=/', '--user=guest', '--password=guest', '-c', '/etc/rabbitmq/rabbitmqadmin.conf', 'source=source', 'destination=target', 'arguments={}', 'routing_key=blablub', 'destination_type=queue')
-    provider.create
-  end
+  describe '#create' do
+    it 'calls rabbitmqadmin to create' do
+      provider.expects(:rabbitmqadmin).with(
+        'declare', 'binding', '--vhost=/', '--user=guest', '--password=guest', '-c', '/etc/rabbitmq/rabbitmqadmin.conf',
+        'source=source', 'destination=target', 'arguments={}', 'routing_key=blablub', 'destination_type=queue'
+      )
+      provider.create
+    end
 
-  it 'calls rabbitmqadmin to destroy' do
-    provider.expects(:rabbitmqadmin).with('delete', 'binding', '--vhost=/', '--user=guest', '--password=guest', '-c', '/etc/rabbitmq/rabbitmqadmin.conf', 'source=source', 'destination_type=queue', 'destination=target', 'properties_key=blablub')
-    provider.destroy
-  end
+    context 'specifying credentials' do
+      let(:resource) do
+        Puppet::Type::Rabbitmq_binding.new(
+          name: 'source@test2@/',
+          destination_type: :queue,
+          routing_key: 'blablubd',
+          arguments: {},
+          user: 'colin',
+          password: 'secret'
+        )
+      end
+      let(:provider) { provider_class.new(resource) }
 
-  context 'specifying credentials' do
-    let(:resource) do
-      Puppet::Type::Rabbitmq_binding.new(
-        name: 'source@test2@/',
-        destination_type: :queue,
-        routing_key: 'blablubd',
-        arguments: {},
-        user: 'colin',
-        password: 'secret'
-      )
+      it 'calls rabbitmqadmin to create' do
+        provider.expects(:rabbitmqadmin).with(
+          'declare', 'binding', '--vhost=/', '--user=colin', '--password=secret', '-c', '/etc/rabbitmq/rabbitmqadmin.conf',
+          'source=source', 'destination=test2', 'arguments={}', 'routing_key=blablubd', 'destination_type=queue'
+        )
+        provider.create
+      end
     end
-    let(:provider) { provider_class.new(resource) }
 
-    it 'calls rabbitmqadmin to create' do
-      provider.expects(:rabbitmqadmin).with('declare', 'binding', '--vhost=/', '--user=colin', '--password=secret', '-c', '/etc/rabbitmq/rabbitmqadmin.conf', 'source=source', 'destination=test2', 'arguments={}', 'routing_key=blablubd', 'destination_type=queue')
-      provider.create
+    context 'new queue_bindings' do
+      let(:resource) do
+        Puppet::Type::Rabbitmq_binding.new(
+          name: 'binding1',
+          source: 'exchange1',
+          destination: 'destqueue',
+          destination_type: :queue,
+          routing_key: 'blablubd',
+          arguments: {}
+        )
+      end
+      let(:provider) { provider_class.new(resource) }
+
+      it 'calls rabbitmqadmin to create' do
+        provider.expects(:rabbitmqadmin).with(
+          'declare', 'binding', '--vhost=/', '--user=guest', '--password=guest', '-c', '/etc/rabbitmq/rabbitmqadmin.conf',
+          'source=exchange1', 'destination=destqueue', 'arguments={}', 'routing_key=blablubd', 'destination_type=queue'
+        )
+        provider.create
+      end
     end
-  end
 
-  context 'new queue_bindings' do
-    let(:resource) do
-      Puppet::Type::Rabbitmq_binding.new(
-        name: 'binding1',
-        source: 'exchange1',
-        destination: 'destqueue',
-        destination_type: :queue,
-        routing_key: 'blablubd',
-        arguments: {}
-      )
+    context 'no source' do
+      let(:resource) do
+        Puppet::Type::Rabbitmq_binding.new(
+          name: 'binding1',
+          destination: 'destqueue'
+        )
+      end
+
+      it 'raises an error' do
+        expect do
+          provider.create
+        end.to raise_error(Puppet::Error, 'Source and destination must both be defined (binding: binding1)')
+      end
     end
-    let(:provider) { provider_class.new(resource) }
 
-    it 'calls rabbitmqadmin to create' do
-      provider.expects(:rabbitmqadmin).with('declare', 'binding', '--vhost=/', '--user=guest', '--password=guest', '-c', '/etc/rabbitmq/rabbitmqadmin.conf', 'source=exchange1', 'destination=destqueue', 'arguments={}', 'routing_key=blablubd', 'destination_type=queue')
-      provider.create
+    context 'no destination' do
+      let(:resource) do
+        Puppet::Type::Rabbitmq_binding.new(
+          name: 'binding1',
+          source: 'exchange1'
+        )
+      end
+
+      it 'raises an error' do
+        expect do
+          provider.create
+        end.to raise_error(Puppet::Error, 'Source and destination must both be defined (binding: binding1)')
+      end
+    end
+  end
+
+  describe '#destroy' do
+    it 'calls rabbitmqadmin to destroy' do
+      provider.expects(:rabbitmqadmin).with(
+        'delete', 'binding', '--vhost=/', '--user=guest', '--password=guest', '-c', '/etc/rabbitmq/rabbitmqadmin.conf',
+        'source=source', 'destination_type=queue', 'destination=target', 'properties_key=blablub'
+      )
+      provider.destroy
     end
   end
 end

spec/unit/puppet/type/rabbitmq_binding_spec.rb

@@ -16,22 +16,6 @@
       Puppet::Type.type(:rabbitmq_binding).new({})
     end.to raise_error(Puppet::Error, 'Title or name must be provided')
   end
-  it 'errors when missing source' do
-    expect do
-      Puppet::Type.type(:rabbitmq_binding).new(
-        name: 'test binding',
-        destination: 'foobar'
-      )
-    end.to raise_error(Puppet::Error, %r{Source and destination must both be defined})
-  end
-  it 'errors when missing destination' do
-    expect do
-      Puppet::Type.type(:rabbitmq_binding).new(
-        name: 'test binding',
-        source: 'foobar'
-      )
-    end.to raise_error(Puppet::Error, %r{Source and destination must both be defined})
-  end
   it 'accepts an binding destination_type' do
     binding[:destination_type] = :exchange
     expect(binding[:destination_type]).to eq(:exchange)