fix(rabbitmq): use SASL EXTERNAL for RabbitMQ AMQP TLS without credentials#7606
fix(rabbitmq): use SASL EXTERNAL for RabbitMQ AMQP TLS without credentials#7606rickbrouwer merged 11 commits intokedacore:mainfrom
Conversation
…tials Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
Thank you for your contribution! 🙏 Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected. While you are waiting, make sure to:
Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient. Learn more about our contribution guide. |
|
/run-e2e rabbit |
dttung2905
left a comment
There was a problem hiding this comment.
Thanks @rickbrouwer for the PR. We might also need some documentation about the expected behavior. something like
"with TLS and no username/password parameters, SASL EXTERNAL is used; put credentials in trigger auth if you need PLAIN"
Thanks, can you check if my note is clear enough? kedacore/keda-docs#1727 |
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
|
/run-e2e rabbit |
There was a problem hiding this comment.
Pull request overview
This PR updates the RabbitMQ scaler’s AMQP/TLS connection setup to support certificate-based authentication by explicitly selecting SASL EXTERNAL when TLS is enabled and no credentials are provided, preventing amqp091-go from falling back to PLAIN auth from the URI.
Changes:
- Refactors AMQP connection config creation into a new
buildAMQPConfighelper. - Sets
amqp.Config.SASLtoExternalAuthfor TLS connections without credentials (to avoid unintended PLAIN fallback). - Adds unit tests for
buildAMQPConfigSASL selection behavior and updates the changelog.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pkg/scalers/rabbitmq_scaler.go | Builds AMQP config via helper and conditionally enables SASL EXTERNAL for TLS/no-credential scenarios |
| pkg/scalers/rabbitmq_scaler_test.go | Adds unit tests validating the new SASL-selection logic |
| CHANGELOG.md | Documents the RabbitMQ TLS/SASL EXTERNAL behavior change |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
|
@zroubalik can you run a new copilot review? thanks! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Rick Brouwer <rickbrouwer@gmail.com>
|
/run-e2e rabbit |
|
/run-e2e rabbit |
The report in issue 6840 mentions that when RabbitMQ is configured for certificate-based authentication (SASL EXTERNAL via the
rabbitmq_auth_mechanism_sslplugin) KEDA would fail to connect with a 403 "username or password not allowed" error.This happened because
amqp091-gofalls back to PLAIN auth using credentials from the host URL whenConfig.SASLis not set, even when TLS is enabled and no explicit credentials are provided.So, the fix is that when TLS is enabled without credentials, explicitly set
Config.SASLto ExternalAuth so it uses SASL EXTERNAL instead of falling back to PLAIN auth.Checklist
make generate-scalers-schemahas been run to update any outdated generated filesFixes #6840
Docs: kedacore/keda-docs#1727