Fix rabbitmq_plugin to correctly detect implicitly enabled plugins

lib/puppet/provider/rabbitmq_plugin/rabbitmqplugins.rb

@@ -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.select { |line| line =~ %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))
@@ -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

spec/unit/puppet/provider/rabbitmq_plugin/rabbitmqctl_spec.rb

@@ -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 existng 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