From 1472dadd25d6779ce33613475db877677a51bd8e Mon Sep 17 00:00:00 2001 From: William Yardley Date: Fri, 13 Oct 2017 11:59:59 -0700 Subject: [PATCH] Fix (and test for) 'puppet resource rabbitmq_binding) (#650) --- .../rabbitmq_binding/rabbitmqadmin.rb | 16 +-- lib/puppet/type/rabbitmq_binding.rb | 8 +- spec/acceptance/queue_spec.rb | 6 + .../rabbitmq_binding/rabbitmqadmin_spec.rb | 106 +++++++++++------- .../unit/puppet/type/rabbitmq_binding_spec.rb | 4 +- 5 files changed, 85 insertions(+), 55 deletions(-) diff --git a/lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb b/lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb index a9ff40604..57e63411c 100644 --- a/lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb +++ b/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. diff --git a/lib/puppet/type/rabbitmq_binding.rb b/lib/puppet/type/rabbitmq_binding.rb index 656978129..1165fd955 100644 --- a/lib/puppet/type/rabbitmq_binding.rb +++ b/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 diff --git a/spec/acceptance/queue_spec.rb b/spec/acceptance/queue_spec.rb index c94785d42..a5ed1d109 100644 --- a/spec/acceptance/queue_spec.rb +++ b/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 diff --git a/spec/unit/puppet/provider/rabbitmq_binding/rabbitmqadmin_spec.rb b/spec/unit/puppet/provider/rabbitmq_binding/rabbitmqadmin_spec.rb index feb46a9ca..04015a33d 100644 --- a/spec/unit/puppet/provider/rabbitmq_binding/rabbitmqadmin_spec.rb +++ b/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,7 +111,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 @@ -113,51 +121,67 @@ 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 diff --git a/spec/unit/puppet/type/rabbitmq_binding_spec.rb b/spec/unit/puppet/type/rabbitmq_binding_spec.rb index f9ee963e6..14b882487 100644 --- a/spec/unit/puppet/type/rabbitmq_binding_spec.rb +++ b/spec/unit/puppet/type/rabbitmq_binding_spec.rb @@ -22,7 +22,7 @@ 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 @@ -30,7 +30,7 @@ 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