Skip to content

Conversation

@ChunyiLyu
Copy link
Contributor

This closes #482

Note to reviewers: remember to look at the commits in this PR and consider if they can be squashed

Summary Of Changes

  • fix a bug where management tcp listener no longer works when tls is enabled and disableNonTLSListeners is set to false
  • management tcp listener can be disabled by setting management.ssl.port without setting management.tcp.port
  • management.tcp.port is set to 15672 when tls is enabled but disableNonTLSListeners is false
  • do not set management.tcp.port when tls is enabled and disableNonTLSListeners is true
  • increate timeout in one integration test case( it was flaking due to the short timeout)

I have thought about including management.tcp.port = 15672 as the default configuration and deleting the key if disableNonTLSListeners is set to true. However, that will unfortunately change all clusters's rabbitmq.conf (no matter its tls configurations), that would be a breaking behavior. Here, management.tcp.port = 15672 is only set if people enable tls and do not set disableNonTLSListeners to true.

Additional Context

Local Testing

Added unit and system tests for the case.

Copy link
Member

@ansd ansd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to your PR, but I'm just noticing we should also update

open "http://localhost:15672/"
) &
kubectl port-forward "service/${service}" 15672
and use port 15671 if TLS is enabled.
Do you want to fix it or create an issue with label bug?

- fix a bug where management tcp listener no longer
works when tls is enabled and disableNonTLSListeners is set to false
- management tcp listener can be disabled by setting
management.ssl.port without setting management.tcp.port
- ensure that management.tcp.port is set to 15672 when tls is enabled
but disableNonTLSListeners is false
- do not set management.tcp.port when tls is enabled and
disableNonTLSListeners is true
@ChunyiLyu
Copy link
Contributor Author

@ansd maybe a separate PR? When tls is enabled, port 15672 would still work unless people configure the disableNonTLSListeners property to true, so I don't think this is urgent. Similarly, we should also look into the perf-test command. Would the perf test fail to connect if disableNonTLSListeners is true?

@ChunyiLyu ChunyiLyu merged commit 57e7c6f into main Nov 23, 2020
@ChunyiLyu ChunyiLyu deleted the tlsbug branch November 23, 2020 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Management TCP listener should still work when TLS is enabled

4 participants