voxpupuli / puppet-rabbitmq

RabbitMQ Puppet Module
http://forge.puppetlabs.com/puppet/rabbitmq
Apache License 2.0
172 stars 503 forks source link

rabbitmqctl 'broken' in 3.7.9 #740

Closed ralfbosz closed 5 years ago

ralfbosz commented 5 years ago

Affected Puppet, Ruby, OS and module versions/distributions

How to reproduce (e.g Puppet code you use)

class { 'rabbitmq':
  delete_guest_user => true,
}

What are you seeing

Error: Failed to apply catalog: Cannot parse invalid user line: user tags

What behaviour did you expect instead

Notice: /Stage[main]/Rabbitmq::Management/Rabbitmq_user[guest]/ensure: removed

Output log

Debug: Executing: '/usr/sbin/rabbitmqctl -q list_users'

This is generating since 3.7.9:

user    tags
guest   [administrator]

The first line should be removed...

Any additional information you'd like to impart

See: https://github.com/rabbitmq/rabbitmq-cli/issues/273

Seems a '--no-table-headers' needs to be added...

Too bad this parameter is not supported in 3.7.8-1 and (likely) lower...

ralfbosz commented 5 years ago

Trying it:

https://github.com/ralfbosz/puppet-rabbitmq/blob/list_users/lib/puppet/provider/rabbitmq_user/rabbitmqctl.rb

But not deepdived in the rspec so this is also done...

s10 commented 5 years ago

This is an issue not only for list_users. list_policies, list_vhosts, list_user_permissions and others are also affected.

lukebakken commented 5 years ago

Too bad this parameter is not supported in 3.7.8-1 and (likely) lower...

To clarify, versions 3.7.0 - 3.7.7 do not have the header row (by mistake), there is no way to disable the header row in 3.7.8, and version 3.7.9 adds support for --no-table-headers. Version 3.7.10 will add --silent which removes the informational banner and the header row.

michaelklishin commented 5 years ago

All 3.7.x versions should support

rabbitmqctl list_queues --formatter=csv

which would produce output in the CSV format with a header.

HTTP API can list most things and hasn't changed in years. Consider using it instead of parsing CLI tools output. rabbitmqadmin output also hasn't changed in a long time but we can't guarantee that it will remain the case as CLI output by default is meant to be used by humans, not deployment tools.

michaelklishin commented 5 years ago

Piping rabbitmqctl -q list_users through grep -v ^user or similar might be sufficient (assuming users are not named user* which might be the case in some environments).

wyardley commented 5 years ago

There was a WIP to convert at least one of the providers to use the http API. While this would add some Ruby dependencies it’s probably worth doing if someone wants to take it on.

michaelklishin commented 5 years ago

There's also rabbitmqctl list_users --formatter=json (should work equally fine for all list_* commands). Obviously we expect the CSV and JSON formatters of said commands to remain stable as long as their input rows do so.

wyardley commented 5 years ago

I think #741 seems similar. Note that 3.7.9 isn't officially supported (https://github.com/voxpupuli/puppet-rabbitmq#module-description)

I don't have the time / inclination to do these fixes, but would be happy to help review.

@michaelklishin do you know off-hand if there are any old versions that don't support --formatter=json? i.e., if someone reworked the provider to use this, would we need to drop support for older versions (not that this would be a bad idea, necessarily).

wyardley commented 5 years ago

Just confirmed in the acceptance testing that there's no json / csv formatter on older versions. Using the API might be the most stable, and might be easier than managing code and tests for two different output formats.

[root@centos-7-x64 ~]# rpm -qa | grep rabbitmq
rabbitmq-server-3.3.5-34.el7.noarch
[root@centos-7-x64 ~]# rabbitmqctl -q list_users --formatter json
Error: invalid command 'list_users --formatter json'

FWIW, a lot of good work was done in #529 that might be able to be used if someone wanted to take on the project. IMHO, if possible, it would be better to do it without pulling in (m)any third party gems, though.

FWIW, for now, the default is to use the (older) distro packages, and the acceptance tests don't currently test the path using the official RMQ packages. Also, we still need to improve the docs and handling around satisfying the Erlang docs.