ADR: Optional RabbitMQ mutual TLS support#3946
Conversation
darrenschwarz
requested review from
DevJonny,
holytshirt,
iancooper and
preardon
as code owners
|
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Removed redundant 'Proposed' line from the document.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
No application code in the PR — skipped Code Health checks.
See analysis details in CodeScene
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
|
Hi @darrenschwarz - Thanks for the ADR. A few comments, mostly giving you situational awareness of things that will help you. Any questions, do shout. |
Hey, This is the only comment I can see? |
lillo42
left a comment
lillo42
left a comment
There was a problem hiding this comment.
What is the scope of this change? Will it introduce any breaking changes? If not, an ADR might not be necessary, and you can proceed directly with the implementation.
|
No,an ADR is always good and in our guidelines. Some of you, no names, should be learning from this ;-) ADRs are not only good to record design decisions, but we can point agents to them for docs or building a plan. |
|
|
||
| ### Negative | ||
|
|
||
| - Increases configuration surface area for RabbitMQ gateways |
There was a problem hiding this comment.
I think this is alright, as its opt in, so only you pay the cost if you take the benefit
|
|
||
| The implementation will be supported by: | ||
|
|
||
| - **Unit tests** verifying correct propagation of mTLS configuration into the RabbitMQ connection factory |
There was a problem hiding this comment.
Generally we tend to think about developer tests and acceptance tests, so the unit/integration distinction is a little spurious for us. For a transport, we tend to test against the actual transport, configured in Docker. (The core engine is tested against in-memory versions). So you really only need to extend the RMQ tests for this, and you can use Docker containers. Your issue might be that you need to configure Docker differently for mTLS, so other tests would fail. In that case, use an attibute for tests that use mTLS so that we can run you in GHA against a different container more easily.
| 4. Support will be implemented with **parity across synchronous and asynchronous RabbitMQ gateways**. | ||
| 5. No changes will be made to existing message wire formats, headers, or CloudEvents propagation. | ||
|
|
||
| The primary mechanism for enabling mTLS will be the ability to supply an `X509Certificate2` (and associated trust configuration) to the RabbitMQ connection factory in a controlled, backwards-compatible manner. |
There was a problem hiding this comment.
One note. We continue to support both RMQ v6 as it is sync, alongside RMQ v7, which is async. For now, we should try to support pushing this option to both versions.
4 participants
Proposes an ADR for adding optional, opt-in mutual TLS (mTLS) support to RabbitMQ messaging gateways.
This PR is intentionally ADR-only to align on scope, constraints, and non-goals before implementation.
Related issue: #3902.