Skip to content

Commit 02d34df

Browse files
J-MxJ-Mx
authored
Fix binding arguments parsing and uniqueness in rabbitmq_binding provider (#1054)
- Fix arguments handling and describe substitution - Fix arguments uniqueness - Add unit test for argument handling
1 parent 0968945 commit 02d34df

File tree

6 files changed

+175
-17
lines changed

6 files changed

+175
-17
lines changed

‎lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,13 @@ def self.instances
3737
if arguments.nil?
3838
arguments = '{}'
3939
else
40-
arguments = arguments.gsub(%r{^\[(.*)\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
40+
# Substitution : Removes the opening '[' and closing ']' brackets from the arguments string
41+
# Substitution 2 : Converts JSON-style "key", format to "key": format by replacing commas after quoted keys with colons
42+
# Substitution 3 : Merges multiple object definitions by replacing "},{" with "," to create a single valid JSON object
43+
arguments = arguments.gsub(%r{^\[|\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
4144
arguments = '{}' if arguments == ''
4245
end
43-
hashed_name = Digest::SHA256.hexdigest format('%s@%s@%s@%s', source_name, destination_name, vhost, routing_key)
46+
hashed_name = Digest::SHA256.hexdigest format('%s@%s@%s@%s@%s', source_name, destination_name, vhost, routing_key, arguments)
4447
next if source_name.empty?
4548

4649
binding = {
@@ -64,7 +67,7 @@ def self.instances
6467
def self.prefetch(resources)
6568
bindings = instances
6669
resources.each do |name, res|
67-
if (provider = bindings.find { |binding| binding.source == res[:source] && binding.destination == res[:destination] && binding.vhost == res[:vhost] && binding.routing_key == res[:routing_key] })
70+
if (provider = bindings.find { |binding| binding.source == res[:source] && binding.destination == res[:destination] && binding.vhost == res[:vhost] && binding.routing_key == res[:routing_key] && binding.arguments == res[:arguments] })
6871
resources[name].provider = provider
6972
end
7073
end

‎lib/puppet/provider/rabbitmq_exchange/rabbitmqadmin.rb‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def self.instances
2525
resources = []
2626
all_vhosts.each do |vhost|
2727
all_exchanges(vhost).each do |line|
28-
name, type, internal, durable, auto_delete, arguments = line.split
28+
name, type, internal, durable, auto_delete, arguments = line.split(%r{\t})
2929
if type.nil?
3030
# if name is empty, it will wrongly get the type's value.
3131
# This way type will get the correct value
@@ -36,7 +36,10 @@ def self.instances
3636
if arguments.nil?
3737
arguments = '{}'
3838
else
39-
arguments = arguments.gsub(%r{^\[(.*)\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
39+
# Substitution : Removes the opening '[' and closing ']' brackets from the arguments string
40+
# Substitution 2 : Converts JSON-style "key", format to "key": format by replacing commas after quoted keys with colons
41+
# Substitution 3 : Merges multiple object definitions by replacing "},{" with "," to create a single valid JSON object
42+
arguments = arguments.gsub(%r{^\[|\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
4043
arguments = '{}' if arguments == ''
4144
end
4245
exchange = {

‎lib/puppet/provider/rabbitmq_queue/rabbitmqadmin.rb‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,10 @@ def self.instances
3131
if arguments.nil?
3232
arguments = '{}'
3333
else
34-
arguments = arguments.gsub(%r{^\[(.*)\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
34+
# Substitution : Removes the opening '[' and closing ']' brackets from the arguments string
35+
# Substitution 2 : Converts JSON-style "key", format to "key": format by replacing commas after quoted keys with colons
36+
# Substitution 3 : Merges multiple object definitions by replacing "},{" with "," to create a single valid JSON object
37+
arguments = arguments.gsub(%r{^\[|\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
3538
arguments = '{}' if arguments == ''
3639
end
3740
queue = {

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,44 @@
7777
}
7878
])
7979
end
80+
81+
it 'returns multiple instances (with or without arguments)' do
82+
provider_class.expects(:rabbitmqctl_list).with('vhosts').returns <<~EOT
83+
/
84+
EOT
85+
provider_class.expects(:rabbitmqctl_list).with(
86+
'bindings', '-p', '/', 'source_name', 'destination_name', 'destination_kind', 'routing_key', 'arguments'
87+
).returns <<~EOT
88+
exchange\tdst_queue\tqueue\trouting_one\t[]
89+
exchange\tdst_queue\tqueue\trouting_two\t[{"header","value"},{"x-match","all"}]
90+
EOT
91+
instances = provider_class.instances
92+
expect(instances.size).to eq(2)
93+
expect(instances.map do |prov|
94+
{
95+
source: prov.get(:source),
96+
destination: prov.get(:destination),
97+
vhost: prov.get(:vhost),
98+
routing_key: prov.get(:routing_key),
99+
arguments: prov.get(:arguments)
100+
}
101+
end).to eq([
102+
{
103+
source: 'exchange',
104+
destination: 'dst_queue',
105+
vhost: '/',
106+
routing_key: 'routing_one',
107+
arguments: {}
108+
},
109+
{
110+
source: 'exchange',
111+
destination: 'dst_queue',
112+
vhost: '/',
113+
routing_key: 'routing_two',
114+
arguments: { 'header' => 'value', 'x-match' => 'all' }
115+
}
116+
])
117+
end
80118
end
81119

82120
describe 'Test for prefetch error' do

‎spec/unit/puppet/provider/rabbitmq_exchange/rabbitmqadmin_spec.rb‎

Lines changed: 92 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,101 @@
2323
/
2424
EOT
2525
provider_class.expects(:rabbitmqctl_list).with('exchanges', '-p', '/', 'name', 'type', 'internal', 'durable', 'auto_delete', 'arguments').returns <<~EOT
26-
direct false true false []
27-
amq.direct direct false true false []
28-
amq.fanout fanout false true false []
29-
amq.headers headers false true false []
30-
amq.match headers false true false []
31-
amq.rabbitmq.log topic true true false []
32-
amq.rabbitmq.trace topic true true false []
33-
amq.topic topic false true false []
34-
test.headers x-consistent-hash false true false [{"hash-header","message-distribution-hash"}]
26+
\tdirect\tfalse\ttrue\tfalse\t[]
27+
amq.direct\tdirect\tfalse\ttrue\tfalse\t[]
28+
amq.fanout\tfanout\tfalse\ttrue\tfalse\t[]
29+
amq.headers\theaders\tfalse\ttrue\tfalse\t[]
30+
amq.match\theaders\tfalse\ttrue\tfalse\t[]
31+
amq.rabbitmq.log\ttopic\ttrue\ttrue\tfalse\t[]
32+
amq.rabbitmq.trace\ttopic\ttrue\ttrue\tfalse\t[]
33+
amq.topic\ttopic\tfalse\ttrue\tfalse\t[]
34+
test.headers\tx-consistent-hash\tfalse\ttrue\tfalse\t[{"hash-header","message-distribution-hash"}]
3535
EOT
3636
instances = provider_class.instances
3737
expect(instances.size).to eq(9)
38+
expect(instances.map do |prov|
39+
{
40+
name: prov.get(:name),
41+
type: prov.get(:type),
42+
internal: prov.get(:internal),
43+
durable: prov.get(:durable),
44+
auto_delete: prov.get(:auto_delete),
45+
arguments: prov.get(:arguments)
46+
}
47+
end).to eq([
48+
{
49+
name: '@/',
50+
type: 'direct',
51+
internal: 'false',
52+
durable: 'true',
53+
auto_delete: 'false',
54+
arguments: {}
55+
},
56+
{
57+
name: 'amq.direct@/',
58+
type: 'direct',
59+
internal: 'false',
60+
durable: 'true',
61+
auto_delete: 'false',
62+
arguments: {}
63+
},
64+
{
65+
name: 'amq.fanout@/',
66+
type: 'fanout',
67+
internal: 'false',
68+
durable: 'true',
69+
auto_delete: 'false',
70+
arguments: {}
71+
},
72+
{
73+
name: 'amq.headers@/',
74+
type: 'headers',
75+
internal: 'false',
76+
durable: 'true',
77+
auto_delete: 'false',
78+
arguments: {}
79+
},
80+
{
81+
name: 'amq.match@/',
82+
type: 'headers',
83+
internal: 'false',
84+
durable: 'true',
85+
auto_delete: 'false',
86+
arguments: {}
87+
},
88+
{
89+
name: 'amq.rabbitmq.log@/',
90+
type: 'topic',
91+
internal: 'true',
92+
durable: 'true',
93+
auto_delete: 'false',
94+
arguments: {}
95+
},
96+
{
97+
name: 'amq.rabbitmq.trace@/',
98+
type: 'topic',
99+
internal: 'true',
100+
durable: 'true',
101+
auto_delete: 'false',
102+
arguments: {}
103+
},
104+
{
105+
name: 'amq.topic@/',
106+
type: 'topic',
107+
internal: 'false',
108+
durable: 'true',
109+
auto_delete: 'false',
110+
arguments: {}
111+
},
112+
{
113+
name: 'test.headers@/',
114+
type: 'x-consistent-hash',
115+
internal: 'false',
116+
durable: 'true',
117+
auto_delete: 'false',
118+
arguments: { 'hash-header' => 'message-distribution-hash' }
119+
}
120+
])
38121
end
39122

40123
it 'calls rabbitmqadmin to create as guest' do

‎spec/unit/puppet/provider/rabbitmq_queue/rabbitmqadmin_spec.rb‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,39 @@
1919
/
2020
EOT
2121
provider_class.expects(:rabbitmqctl_list).with('queues', '-p', '/', 'name', 'durable', 'auto_delete', 'arguments').returns <<~EOT
22-
test true false []
23-
test2 true false [{"x-message-ttl",342423},{"x-expires",53253232},{"x-max-length",2332},{"x-max-length-bytes",32563324242},{"x-dead-letter-exchange","amq.direct"},{"x-dead-letter-routing-key","test.routing"}]
22+
test\ttrue\tfalse\t[]
23+
test2\ttrue\tfalse\t[{"x-message-ttl",342423},{"x-expires",53253232},{"x-max-length",2332},{"x-max-length-bytes",32563324242},{"x-dead-letter-exchange","amq.direct"},{"x-dead-letter-routing-key","test.routing"}]
2424
EOT
2525
instances = provider_class.instances
2626
expect(instances.size).to eq(2)
27+
expect(instances.map do |prov|
28+
{
29+
name: prov.get(:name),
30+
durable: prov.get(:durable),
31+
auto_delete: prov.get(:auto_delete),
32+
arguments: prov.get(:arguments)
33+
}
34+
end).to eq([
35+
{
36+
name: 'test@/',
37+
durable: 'true',
38+
auto_delete: 'false',
39+
arguments: {}
40+
},
41+
{
42+
name: 'test2@/',
43+
durable: 'true',
44+
auto_delete: 'false',
45+
arguments: {
46+
'x-message-ttl' => 342_423,
47+
'x-expires' => 53_253_232,
48+
'x-max-length' => 2332,
49+
'x-max-length-bytes' => 32_563_324_242,
50+
'x-dead-letter-exchange' => 'amq.direct',
51+
'x-dead-letter-routing-key' => 'test.routing'
52+
}
53+
}
54+
])
2755
end
2856

2957
it 'calls rabbitmqadmin to create' do

0 commit comments

Comments
 (0)