Skip to content

Commit

Permalink
Fix rabbitmq_plugin to correctly detect implicitly enabled plugins
Browse files Browse the repository at this point in the history
Rabbitmq_plugin now correctly detects implicitly enabled plugins to
preserve idempotency regardless of plugin install order

Cherry-picked from #844
Fixes #930

Signed-off-by: William Yardley <[email protected]>
  • Loading branch information
nmaludy authored and wyardley committed May 11, 2023
1 parent 99b08eb commit b610fca
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 17 deletions.
29 changes: 23 additions & 6 deletions lib/puppet/provider/rabbitmq_plugin/rabbitmqplugins.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,30 @@
Puppet::Type.type(:rabbitmq_plugin).provide(:rabbitmqplugins, parent: Puppet::Provider::RabbitmqCli) do
confine feature: :posix

def self.instances
plugin_list = run_with_retries do
rabbitmqplugins('list', '-E', '-m')
def self.plugin_list
list_str = run_with_retries do
# Pass in -e to list both implicitly and explicitly enabled plugins.
# If you pass in -E instead, then only explicitly enabled plugins are listed.
# Implicitly enabled plugins are those that were enabled as a dependency of another plugin/
# If we do not pass in -e then the order if plugin installation matters within the puppet
# code. Example, if Plugin A depends on Plugin B and we install Plugin B first it will
# implicitly enable Plugin A. Then when we go to run Puppet a second time without the
# -e parameter, we won't see Plugin A as being enabled so we'll try to install it again.
# To preserve idempotency we should get all enabled plugins regardless of implicitly or
# explicitly enabled.
rabbitmqplugins('list', '-e', '-m')
end
# Split by newline.
lines = list_str.split(%r{\n})
# Return only lines that are single words because sometimes RabbitMQ likes to output
# information messages. Suppressing those messages via CLI flags is inconsistent between
# versions, so this this regex removes those message without having to use painful
# version switches.
lines.grep(%r{^(\S+)$})
end

plugin_list.split(%r{\n}).map do |line|
next if line.start_with?('Listing plugins')
def self.instances
plugin_list.map do |line|
raise Puppet::Error, "Cannot parse invalid plugins line: #{line}" unless line =~ %r{^(\S+)$}

new(name: Regexp.last_match(1))
Expand All @@ -36,6 +53,6 @@ def destroy
end

def exists?
run_with_retries { rabbitmqplugins('list', '-E', '-m') }.split(%r{\n}).include? resource[:name]
self.class.plugin_list.include? resource[:name]
end
end
63 changes: 52 additions & 11 deletions spec/unit/puppet/provider/rabbitmq_plugin/rabbitmqctl_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,25 +11,66 @@
end
let(:provider) { provider_class.new(resource) }

it 'matches plugins' do
provider.expects(:rabbitmqplugins).with('list', '-E', '-m').returns("foo\n")
expect(provider.exists?).to eq(true)
end

it 'calls rabbitmqplugins to enable when node not running' do
provider.class.expects(:rabbitmq_running).returns false
provider.expects(:rabbitmqplugins).with('enable', 'foo')
provider.create
end

context 'with RabbitMQ version >=3.10.0' do
it 'matches plugins' do
provider.expects(:rabbitmqplugins).with('list', '-E', '-m').returns <<~EOT
Listing plugins with pattern ".*" ...
foo
EOT
describe '#instances' do
it 'exists' do
expect(provider_class).to respond_to :instances
end

it 'retrieves instances' do
provider.class.expects(:plugin_list).returns(%w[foo bar])
instances = provider_class.instances
instances_cmp = instances.map { |prov| { name: prov.get(:name) } }
expect(instances_cmp).to eq(
[
{ name: 'foo' },
{ name: 'bar' }
]
)
end

it 'raises error on invalid line' do
provider_class.expects(:plugin_list).returns([' '])
expect { provider_class.instances }.to raise_error Puppet::Error, %r{Cannot parse invalid plugins line}
end
end

describe '#plugin_list' do
it 'exists' do
expect(provider_class).to respond_to :instances
end

it 'returns a list of plugins' do
provider.class.expects(:rabbitmqplugins).with('list', '-e', '-m').returns("foo\nbar\nbaz\n")
expect(provider.class.plugin_list).to eq(%w[foo bar baz])
end

it 'handles no training newline properly' do
provider.class.expects(:rabbitmqplugins).with('list', '-e', '-m').returns("foo\nbar")
expect(provider.class.plugin_list).to eq(%w[foo bar])
end

it 'handles lines that are not plugins' do
provider.class.expects(:rabbitmqplugins).with('list', '-e', '-m').returns("Listing plugins with pattern \".*\" ...\nfoo\nbar")
expect(provider.class.plugin_list).to eq(%w[foo bar])
end
end

describe '#exists?' do
it 'matches existing plugins' do
provider_class.expects(:plugin_list).returns(%w[foo])
expect(provider.exists?).to eq(true)
end

it 'returns false for missing plugins' do
provider_class.expects(:plugin_list).returns(%w[bar])
expect(provider.exists?).to eq(false)
end
end

context 'with RabbitMQ version >=3.4.0' do
Expand Down

0 comments on commit b610fca

Please sign in to comment.