Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop disabling management tcp listener when tls enabled #487

Merged
merged 1 commit into from
Nov 23, 2020
Merged

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?

system_tests/system_tests.go Show resolved Hide resolved
internal/resource/configmap.go Outdated Show resolved Hide resolved
- 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?

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
3 participants