Merge pull request #651 from wyardley/fix_binding_resource

Fix 'puppet resource rabbitmq_binding' and add tests (#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.

lib/puppet/type/rabbitmq_binding.rb

@@ -178,8 +178,12 @@ def validate_argument(argument)
   # 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.'
+    if !self[:source] && !defined? provider.source
+      raise ArgumentError, '`source` must be defined'
+    end
+
+    if !self[:destination] && !defined? provider.destination
+      raise ArgumentError, '`destination` must 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,77 @@
       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: {}
+  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'
       )
-    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
+      provider.destroy
     end
   end
 end

spec/unit/puppet/type/rabbitmq_binding_spec.rb

@@ -22,15 +22,15 @@
         name: 'test binding',
         destination: 'foobar'
       )
-    end.to raise_error(Puppet::Error, %r{Source and destination must both be defined})
+    end.to raise_error(Puppet::Error, %r{`source` must 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.to raise_error(Puppet::Error, %r{`destination` must be defined})
   end
   it 'accepts an binding destination_type' do
     binding[:destination_type] = :exchange