Add support for multiple bindings with same source/dest/vhost but dif…

…ferent routing key (MODULES-3679)

lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb

@@ -19,7 +19,7 @@ def should_vhost
     if @should_vhost
       @should_vhost
     else
-      @should_vhost = resource[:name].split('@').last
+      @should_vhost = resource[:vhost]
     end
   end
 
@@ -51,11 +51,14 @@ def self.instances
         end
         unless(source_name.empty?)
           binding = {
+            :source           => source_name,
+            :dest             => destination_name,
+            :vhost            => vhost,
             :destination_type => destination_type,
             :routing_key      => routing_key,
             :arguments        => JSON.parse(arguments),
             :ensure           => :present,
-            :name             => "%s@%s@%s" % [source_name, destination_name, vhost],
+            :name             => "%s@%s@%s@%s" % [source_name, destination_name, vhost, routing_key],
           }
           resources << new(binding) if binding[:name]
         end
@@ -65,9 +68,9 @@ def self.instances
   end
 
   def self.prefetch(resources)
-    packages = instances
+    bindings = instances
     resources.keys.each do |name|
-      if provider = packages.find{ |pkg| pkg.name == name }
+      if provider = bindings.find{ |route| route.source == source && route.dest == dest && route.vhost == vhost && route.routing_key == routing_key }
         resources[name].provider = provider
       end
     end
@@ -79,8 +82,6 @@ def exists?
 
   def create
     vhost_opt = should_vhost ? "--vhost=#{should_vhost}" : ''
-    name = resource[:name].split('@').first
-    destination = resource[:name].split('@')[1]
     arguments = resource[:arguments]
     if arguments.nil?
       arguments = {}
@@ -92,8 +93,8 @@ def create
       "--password=#{resource[:password]}",
       '-c',
       '/etc/rabbitmq/rabbitmqadmin.conf',
-      "source=#{name}",
-      "destination=#{destination}",
+      "source=#{resource[:source]}",
+      "destination=#{resource[:dest]}",
       "arguments=#{arguments.to_json}",
       "routing_key=#{resource[:routing_key]}",
       "destination_type=#{resource[:destination_type]}"
@@ -103,9 +104,7 @@ def create
 
   def destroy
     vhost_opt = should_vhost ? "--vhost=#{should_vhost}" : ''
-    name = resource[:name].split('@').first
-    destination = resource[:name].split('@')[1]
-    rabbitmqadmin('delete', 'binding', vhost_opt, "--user=#{resource[:user]}", "--password=#{resource[:password]}", '-c', '/etc/rabbitmq/rabbitmqadmin.conf', "source=#{name}", "destination_type=#{resource[:destination_type]}", "destination=#{destination}", "properties_key=#{resource[:routing_key]}")
+    rabbitmqadmin('delete', 'binding', vhost_opt, "--user=#{resource[:user]}", "--password=#{resource[:password]}", '-c', '/etc/rabbitmq/rabbitmqadmin.conf', "source=#{resource[:source]}", "destination_type=#{resource[:destination_type]}", "destination=#{resource[:dest]}", "properties_key=#{resource[:routing_key]}")
     @property_hash[:ensure] = :absent
   end
 

lib/puppet/type/rabbitmq_binding.rb

@@ -11,21 +11,68 @@
     end
   end
 
-  newparam(:name, :namevar => true) do
-    desc 'source and destination of bind'
-    newvalues(/^\S*@\S+@\S+$/)
+  # Match patterns without '@' as arbitrary names; match patterns with
+  # src@dst@vhost to their named params for backwards compatibility.
+  def self.title_patterns
+    [
+      [
+        /(^([^@]*)$)/m,
+        [
+          [ :name ]
+        ]
+      ],
+      [
+        /^(\S+)@(\S+)@(\S+)$/m,
+        [
+          [ :source ],
+          [ :dest ],
+          [ :vhost ]
+        ]
+      ]
+    ]
+  end
+
+  newparam(:name) do
+    desc 'resource name, either source@dest@vhost or arbitrary name with params'
+
+    isnamevar
+  end
+
+  newparam(:source) do
+    desc 'source of binding'
+
+    newvalues(/^\S+$/)
+    isnamevar
+  end
+
+  newparam(:dest, :namevar => true) do
+    desc 'destination of binding'
+
+    newvalues(/^\S+$/)
+    isnamevar
+  end
+
+  newparam(:vhost, :namevar => true) do
+    desc 'vhost'
+
+    newvalues(/^\S+$/)
+    defaultto('/')
+    isnamevar
+  end
+
+  newparam(:routing_key, :namevar => true) do
+    desc 'binding routing_key'
+
+    newvalues(/^\S*$/)
+    defaultto('#')
+    isnamevar
   end
 
   newparam(:destination_type) do
     desc 'binding destination_type'
     newvalues(/queue|exchange/)
     defaultto('queue')
   end
-
-  newparam(:routing_key) do
-    desc 'binding routing_key'
-    newvalues(/^\S*$/)
-  end
 
   newparam(:arguments) do
     desc 'binding arguments'
@@ -48,7 +95,7 @@
   end
 
   autorequire(:rabbitmq_vhost) do
-    [self[:name].split('@')[2]]
+    setup_autorequire('vhost')
   end
 
   autorequire(:rabbitmq_exchange) do
@@ -65,21 +112,21 @@
 
   autorequire(:rabbitmq_user_permissions) do
     [
-      "#{self[:user]}@#{self[:name].split('@')[1]}",
-      "#{self[:user]}@#{self[:name].split('@')[0]}"
+      "#{self[:user]}@#{self[:source]}",
+      "#{self[:user]}@#{self[:dest]}"
     ]
   end
 
   def setup_autorequire(type)
     destination_type = value(:destination_type)
     if type == 'exchange'
-      rval = ["#{self[:name].split('@')[0]}@#{self[:name].split('@')[2]}"]
+      rval = ["#{self[:source]}@#{self[:vhost]}"]
       if destination_type == type
-        rval.push("#{self[:name].split('@')[1]}@#{self[:name].split('@')[2]}")
+        rval.push("#{self[:dest]}@#{self[:vhost]}")
       end
     else
       if destination_type == type
-        rval = ["#{self[:name].split('@')[1]}@#{self[:name].split('@')[2]}"]
+        rval = ["#{self[:dest]}@#{self[:vhost]}"]
       else
         rval = []
       end

spec/acceptance/queue_spec.rb

@@ -2,8 +2,7 @@
 
 describe 'rabbitmq binding:' do
 
-
-  context "create binding and queue resources when rabbit using default management port" do
+  context "create binding and queue resources when using default management port" do
     it 'should run successfully' do
       pp = <<-EOS
       if $::osfamily == 'RedHat' {
@@ -22,7 +21,7 @@ class { '::rabbitmq':
         password => 'bar',
         tags     => ['monitoring', 'tag1'],
       } ->
-      
+
       rabbitmq_user_permissions { 'dan@host1':
         configure_permission => '.*',
         read_permission      => '.*',
@@ -55,7 +54,7 @@ class { '::rabbitmq':
         routing_key      => '#',
         ensure           => present,
       }
-      
+
       EOS
 
       apply_manifest(pp, :catch_failures => true)
@@ -68,7 +67,7 @@ class { '::rabbitmq':
         expect(r.exit_code).to be_zero
       end
     end
-    
+
     it 'should have the queue' do
       shell('rabbitmqctl list_queues -q -p host1') do |r|
         expect(r.stdout).to match(/queue1/)
@@ -78,7 +77,87 @@ class { '::rabbitmq':
 
   end
 
-  context "create binding and queue resources when rabbit using a non-default management port" do
+
+  context "create multiple bindings when same source / dest / vhost but different routing keys" do
+    it 'should run successfully' do
+      pp = <<-EOS
+      if $::osfamily == 'RedHat' {
+        class { 'erlang': epel_enable => true }
+        Class['erlang'] -> Class['::rabbitmq']
+      }
+      class { '::rabbitmq':
+        service_manage    => true,
+        port              => '5672',
+        delete_guest_user => true,
+        admin_enable      => true,
+      } ->
+
+      rabbitmq_user { 'dan':
+        admin    => true,
+        password => 'bar',
+        tags     => ['monitoring', 'tag1'],
+      } ->
+
+      rabbitmq_user_permissions { 'dan@host1':
+        configure_permission => '.*',
+        read_permission      => '.*',
+        write_permission     => '.*',
+      }
+
+      rabbitmq_vhost { 'host1':
+        ensure => present,
+      } ->
+
+      rabbitmq_exchange { 'exchange1@host1':
+        user     => 'dan',
+        password => 'bar',
+        type     => 'topic',
+        ensure   => present,
+      } ->
+
+      rabbitmq_queue { 'queue1@host1':
+        user        => 'dan',
+        password    => 'bar',
+        durable     => true,
+        auto_delete => false,
+        ensure      => present,
+      } ->
+
+      rabbitmq_binding { 'binding 1':
+        source           => 'exchange1',
+        user             => 'dan',
+        password         => 'bar',
+        destination_type => 'queue',
+        routing_key      => 'test1',
+        ensure           => present,
+      } ->
+
+      rabbitmq_binding { 'binding 2':
+        source           => 'exchange1',
+        user             => 'dan',
+        password         => 'bar',
+        destination_type => 'queue',
+        routing_key      => 'test2',
+        ensure           => present,
+      }
+
+      EOS
+
+      apply_manifest(pp, :catch_failures => true)
+      apply_manifest(pp, :catch_changes => true)
+    end
+
+    it 'should have the bindings' do
+      shell('rabbitmqctl list_bindings -q -p host1') do |r|
+        expect(r.stdout).to match(/exchange1\sexchange\squeue1\squeue\stest1/)
+        expect(r.stdout).to match(/exchange1\sexchange\squeue1\squeue\stest2/)
+        expect(r.exit_code).to be_zero
+      end
+    end
+
+  end
+
+  context "create binding and queue resources when using a non-default management port" do
     it 'should run successfully' do
       pp = <<-EOS
       if $::osfamily == 'RedHat' {
@@ -98,7 +177,7 @@ class { '::rabbitmq':
         password => 'bar',
         tags     => ['monitoring', 'tag1'],
       } ->
-      
+
       rabbitmq_user_permissions { 'dan@host2':
         configure_permission => '.*',
         read_permission      => '.*',
@@ -131,7 +210,7 @@ class { '::rabbitmq':
         routing_key      => '#',
         ensure           => present,
       }
-     
+
       EOS
 
       apply_manifest(pp, :catch_failures => true)
@@ -144,7 +223,7 @@ class { '::rabbitmq':
         expect(r.exit_code).to be_zero
       end
     end
-    
+
     it 'should have the queue' do
       shell('rabbitmqctl list_queues -q -p host2') do |r|
         expect(r.stdout).to match(/queue2/)
@@ -153,5 +232,5 @@ class { '::rabbitmq':
     end
 
   end
-  
+
 end

spec/spec_helper_acceptance.rb

@@ -21,7 +21,6 @@
                          %{"exec { 'setenforce 0': path   => '/bin:/sbin:/usr/bin:/usr/sbin', onlyif => 'which setenforce && getenforce | grep Enforcing', }"})
       end
 
-      on host, "/bin/touch #{default['puppetpath']}/hiera.yaml"
       on host, puppet('module', 'install', 'puppetlabs-stdlib'), { :acceptable_exit_codes => [0,1] }
       on host, puppet('module', 'install', 'puppet-staging'), { :acceptable_exit_codes => [0,1] }
 

spec/unit/puppet/type/rabbitmq_binding_spec.rb

@@ -15,23 +15,6 @@
       Puppet::Type.type(:rabbitmq_binding).new({})
     }.to raise_error(Puppet::Error, 'Title or name must be provided')
   end
-  it 'should not allow whitespace in the name' do
-    expect {
-      @binding[:name] = 'b r'
-    }.to raise_error(Puppet::Error, /Valid values match/)
-  end
-  it 'should not allow names without one @' do
-    expect {
-      @binding[:name] = 'b_r'
-    }.to raise_error(Puppet::Error, /Valid values match/)
-  end
-
-  it 'should not allow names without two @' do
-    expect {
-      @binding[:name] = 'b@r'
-    }.to raise_error(Puppet::Error, /Valid values match/)
-  end
-
   it 'should accept an binding destination_type' do
     @binding[:destination_type] = :exchange
     @binding[:destination_type].should == :exchange