Skip to content

Fix binding arguments parsing and uniqueness in rabbitmq_binding provider #1054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 20, 2025
Merged

Fix binding arguments parsing and uniqueness in rabbitmq_binding provider #1054

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e812e5
(#1053) Fix arguments handling and describe substitution
J-Mx Jun 19, 2025
741e93a
(#1053) Fix arguments uniqueness
J-Mx Jun 19, 2025
4c84aff
(#1053) Add unit test for argument handling
J-Mx Jun 19, 2025
95d1e29
(#1053) Fix: properly parse lines from rabbitmqctl output
J-Mx Jun 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions lib/puppet/provider/rabbitmq_binding/rabbitmqadmin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,13 @@ def self.instances
if arguments.nil?
arguments = '{}'
else
arguments = arguments.gsub(%r{^\[(.*)\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
# Substitution : Removes the opening '[' and closing ']' brackets from the arguments string
# Substitution 2 : Converts JSON-style "key", format to "key": format by replacing commas after quoted keys with colons
# Substitution 3 : Merges multiple object definitions by replacing "},{" with "," to create a single valid JSON object
arguments = arguments.gsub(%r{^\[|\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
arguments = '{}' if arguments == ''
end
hashed_name = Digest::SHA256.hexdigest format('%s@%s@%s@%s', source_name, destination_name, vhost, routing_key)
hashed_name = Digest::SHA256.hexdigest format('%s@%s@%s@%s@%s', source_name, destination_name, vhost, routing_key, arguments)
next if source_name.empty?

binding = {
Expand All @@ -64,7 +67,7 @@ def self.instances
def self.prefetch(resources)
bindings = instances
resources.each do |name, res|
if (provider = bindings.find { |binding| binding.source == res[:source] && binding.destination == res[:destination] && binding.vhost == res[:vhost] && binding.routing_key == res[:routing_key] })
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] })
resources[name].provider = provider
end
end
Expand Down
7 changes: 5 additions & 2 deletions lib/puppet/provider/rabbitmq_exchange/rabbitmqadmin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def self.instances
resources = []
all_vhosts.each do |vhost|
all_exchanges(vhost).each do |line|
name, type, internal, durable, auto_delete, arguments = line.split
name, type, internal, durable, auto_delete, arguments = line.split(%r{\t})
if type.nil?
# if name is empty, it will wrongly get the type's value.
# This way type will get the correct value
Expand All @@ -36,7 +36,10 @@ def self.instances
if arguments.nil?
arguments = '{}'
else
arguments = arguments.gsub(%r{^\[(.*)\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
# Substitution : Removes the opening '[' and closing ']' brackets from the arguments string
# Substitution 2 : Converts JSON-style "key", format to "key": format by replacing commas after quoted keys with colons
# Substitution 3 : Merges multiple object definitions by replacing "},{" with "," to create a single valid JSON object
arguments = arguments.gsub(%r{^\[|\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
arguments = '{}' if arguments == ''
end
exchange = {
Expand Down
5 changes: 4 additions & 1 deletion lib/puppet/provider/rabbitmq_queue/rabbitmqadmin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ def self.instances
if arguments.nil?
arguments = '{}'
else
arguments = arguments.gsub(%r{^\[(.*)\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
# Substitution : Removes the opening '[' and closing ']' brackets from the arguments string
# Substitution 2 : Converts JSON-style "key", format to "key": format by replacing commas after quoted keys with colons
# Substitution 3 : Merges multiple object definitions by replacing "},{" with "," to create a single valid JSON object
arguments = arguments.gsub(%r{^\[|\]$}, '').gsub(%r{\{("(?:.|\\")*?"),}, '{\1:').gsub(%r{\},\{}, ',')
arguments = '{}' if arguments == ''
end
queue = {
Expand Down
38 changes: 38 additions & 0 deletions spec/unit/puppet/provider/rabbitmq_binding/rabbitmqadmin_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,44 @@
}
])
end

it 'returns multiple instances (with or without arguments)' do
provider_class.expects(:rabbitmqctl_list).with('vhosts').returns <<~EOT
/
EOT
provider_class.expects(:rabbitmqctl_list).with(
'bindings', '-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[{"header","value"},{"x-match","all"}]
EOT
instances = provider_class.instances
expect(instances.size).to eq(2)
expect(instances.map do |prov|
{
source: prov.get(:source),
destination: prov.get(:destination),
vhost: prov.get(:vhost),
routing_key: prov.get(:routing_key),
arguments: prov.get(:arguments)
}
end).to eq([
{
source: 'exchange',
destination: 'dst_queue',
vhost: '/',
routing_key: 'routing_one',
arguments: {}
},
{
source: 'exchange',
destination: 'dst_queue',
vhost: '/',
routing_key: 'routing_two',
arguments: { 'header' => 'value', 'x-match' => 'all' }
}
])
end
end

describe 'Test for prefetch error' do
Expand Down
101 changes: 92 additions & 9 deletions spec/unit/puppet/provider/rabbitmq_exchange/rabbitmqadmin_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,101 @@
/
EOT
provider_class.expects(:rabbitmqctl_list).with('exchanges', '-p', '/', 'name', 'type', 'internal', 'durable', 'auto_delete', 'arguments').returns <<~EOT
direct false true false []
amq.direct direct false true false []
amq.fanout fanout false true false []
amq.headers headers false true false []
amq.match headers false true false []
amq.rabbitmq.log topic true true false []
amq.rabbitmq.trace topic true true false []
amq.topic topic false true false []
test.headers x-consistent-hash false true false [{"hash-header","message-distribution-hash"}]
\tdirect\tfalse\ttrue\tfalse\t[]
amq.direct\tdirect\tfalse\ttrue\tfalse\t[]
amq.fanout\tfanout\tfalse\ttrue\tfalse\t[]
amq.headers\theaders\tfalse\ttrue\tfalse\t[]
amq.match\theaders\tfalse\ttrue\tfalse\t[]
amq.rabbitmq.log\ttopic\ttrue\ttrue\tfalse\t[]
amq.rabbitmq.trace\ttopic\ttrue\ttrue\tfalse\t[]
amq.topic\ttopic\tfalse\ttrue\tfalse\t[]
test.headers\tx-consistent-hash\tfalse\ttrue\tfalse\t[{"hash-header","message-distribution-hash"}]
EOT
instances = provider_class.instances
expect(instances.size).to eq(9)
expect(instances.map do |prov|
{
name: prov.get(:name),
type: prov.get(:type),
internal: prov.get(:internal),
durable: prov.get(:durable),
auto_delete: prov.get(:auto_delete),
arguments: prov.get(:arguments)
}
end).to eq([
{
name: '@/',
type: 'direct',
internal: 'false',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'amq.direct@/',
type: 'direct',
internal: 'false',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'amq.fanout@/',
type: 'fanout',
internal: 'false',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'amq.headers@/',
type: 'headers',
internal: 'false',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'amq.match@/',
type: 'headers',
internal: 'false',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'amq.rabbitmq.log@/',
type: 'topic',
internal: 'true',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'amq.rabbitmq.trace@/',
type: 'topic',
internal: 'true',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'amq.topic@/',
type: 'topic',
internal: 'false',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'test.headers@/',
type: 'x-consistent-hash',
internal: 'false',
durable: 'true',
auto_delete: 'false',
arguments: { 'hash-header' => 'message-distribution-hash' }
}
])
end

it 'calls rabbitmqadmin to create as guest' do
Expand Down
32 changes: 30 additions & 2 deletions spec/unit/puppet/provider/rabbitmq_queue/rabbitmqadmin_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,39 @@
/
EOT
provider_class.expects(:rabbitmqctl_list).with('queues', '-p', '/', 'name', 'durable', 'auto_delete', 'arguments').returns <<~EOT
test true false []
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"}]
test\ttrue\tfalse\t[]
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"}]
EOT
instances = provider_class.instances
expect(instances.size).to eq(2)
expect(instances.map do |prov|
{
name: prov.get(:name),
durable: prov.get(:durable),
auto_delete: prov.get(:auto_delete),
arguments: prov.get(:arguments)
}
end).to eq([
{
name: 'test@/',
durable: 'true',
auto_delete: 'false',
arguments: {}
},
{
name: 'test2@/',
durable: 'true',
auto_delete: 'false',
arguments: {
'x-message-ttl' => 342_423,
'x-expires' => 53_253_232,
'x-max-length' => 2332,
'x-max-length-bytes' => 32_563_324_242,
'x-dead-letter-exchange' => 'amq.direct',
'x-dead-letter-routing-key' => 'test.routing'
}
}
])
end

it 'calls rabbitmqadmin to create' do
Expand Down
Loading