Skip to content

Conversation

@ChunyiLyu
Copy link
Contributor

This closes #135

Note to reviewers: remember to look at the commits in this PR and consider if they can be squashed
Note to contributors: remember to re-generate client set if there are any API changes

Summary Of Changes

This adds TLS support for resources that are using spec.rabbitmqClusterReference.connectionSecret.
Certificates are provided in the same way as spec.rabbitmqClusterReference.name use cases, user needs to either configure their RabbitMQ with public trusted certs, or mount CAs to the topology operator deployment.

To use secure TLS connection, users need to set scheme in the URI to https in provided connection secrets. When scheme is not provided, operator defaults to non-tls http scheme for connection.

Follow up

Need to add docs to rabbitmq.com

Copy link
Member

@Zerpet Zerpet left a comment

Choose a reason for hiding this comment

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

Looking at the TLS system test in system_tests/tls_system_test.go makes we wonder if we should move this test at integration level, given that it doesn't need the running RabbitMQ to perform its assertions. This test is creating objects in Kubernetes, and asserting on other objects created/updated by the Top-Op, and this could be accomplished at integration level.

Do you know of any limitation/blocker that makes this test a must-be system level?

Everything else looks good 👍

@ChunyiLyu
Copy link
Contributor Author

ChunyiLyu commented Feb 10, 2022

@Zerpet IMO having the tls tests in the e2e system levels test the actual user workflow of mounting non public trusted certs to topology operator and ensure tls connection works, which we can't cover in an integration tests. I think this provides important coverage that otherwise would be missed in integration.
In terms of technical blockers, we are currently using a faked RabbitMQClient with fixed returned values with no mocked RMQ server in integration test. I think there will be difficulties for us to make the integration tests fake reconcilers to be able to connect to a mocked RabbitMQ tls server.

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.

Be able to use existing RabbitMQ clusters that are not created/managed by https://github.com/rabbitmq/cluster-operator

4 participants