Metricbeat: Set guest as default user in RabbitMQ module #7107
Conversation
jsoriano
added
in progress
jsoriano
force-pushed
the
rabbitmq-default-user-password
branch
from
e69969e to
fb74904
Compare
jsoriano
added
review
and removed
in progress
jsoriano
force-pushed
the
rabbitmq-default-user-password
branch
from
fb74904 to
e910764
Compare
| @@ -37,13 +39,17 @@ func (b URLHostParserBuilder) Build() mb.HostParser { | |||
| if !ok { | |||
| return mb.HostData{}, errors.Errorf("'username' config for module %v is not a string", module.Name()) | |||
| } | |||
| } else { | |||
| user = b.DefaultUsername | |||
There was a problem hiding this comment.
I'm wondering why map[string]interface{} is used here instead of a tagged struct. That would ensure types and allows defaults. This method was not introduced by this PR I would say we can live with this as it is, unless you want to improve it.
There was a problem hiding this comment.
By now I'd prefer to leave it as is, but I agree that some improvements are possible in this package.
CHANGELOG.asciidoc
Outdated
| @@ -236,6 +236,7 @@ https://github.com/elastic/beats/compare/v6.2.3...master[Check the HEAD diff] | |||
| - Add Elasticsearch index_summary metricset. {pull}6918[6918] | |||
| - Add config option `management_path_prefix` for RabbitMQ module to configure management plugin path prefix {issue}6875[6875] {pull}7074[7074] | |||
| - Add shard metricset to Elasticsearch module. {pull}7006[7006] | |||
| - Set guest as default user in RabbitMQ module. {pull}7107[7107] | |||
There was a problem hiding this comment.
Sounds a bit like a breaking change. Or was it before a bug?
There was a problem hiding this comment.
I think it is an enhancement for the out-of-the-box case, it can be breaking if someone has managed to setup the RabbitMQ endpoint without authentication, a very corner case if even possible.
If there are other authentication mechanisms configured in RabbitMQ, probably plain authentication headers are ignored, but not sure. I can try to test if this breaks this case.
|
Please don't merge it by now, I want to test if we break other authentication methods by having these defaults. |
|
Data generation is broken here, I'll solve it after #7106 |
jsoriano
force-pushed
the
rabbitmq-default-user-password
branch
from
e910764 to
10a67e7
Compare
jsoriano
force-pushed
the
rabbitmq-default-user-password
branch
from
10a67e7 to
ffe620f
Compare
|
Ok, data generation works now as before. |
jsoriano
added
review
and removed
in progress
Follow-up of #6908 (comment)