Repository navigation
docs(rabbitmq): say that deleting the application leaves the TLS Secrets behind - #4143
Merged
Aleksei Sviridkin (lexfrei) merged 1 commit intoSep 8, 2026
Conversation
…ets behind The README covered only the disable path. Deleting the application hits the same cert-manager behaviour: the chart renders Certificate and Issuer objects and never the Secrets, and cert-manager does not own what it issued, so the CA and server private keys stay in the tenant namespace with nothing referencing them. Name the two Secrets and the cleanup. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe RabbitMQ README adds instructions to delete cert-manager-generated CA and TLS Secrets after uninstalling a TLS-enabled application. ChangesRabbitMQ documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to The RabbitMQ README now documents manual cleanup of retained TLS Secrets after uninstall. The documented Secret names match the chart’s TLS naming contract, with no remaining merge-readiness risk. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
area/uncategorized
Aleksei Sviridkin (lexfrei)
marked this pull request as ready for review
September 7, 2026 22:33
Aleksei Sviridkin (lexfrei)
requested review from
Andrei Kvapil (kvaps) and
Timofei Larkin (lllamnyp)
as code owners
September 7, 2026 22:33
Aleksei Sviridkin (lexfrei)
requested review from
IvanHunters,
Matthieu ROBIN (matthieu-robin),
myasnikovdaniil and
scooby87
as code owners
September 7, 2026 22:33
IvanHunters
approved these changes
Sep 8, 2026
IvanHunters
left a comment
IvanHunters
left a comment
Collaborator
There was a problem hiding this comment.
Verdict
LGTM with non-blocking notes
The added claim is accurate and complete; the only issue is where the paragraph is placed.
Findings
- [MINOR]
packages/apps/rabbitmq/README.md:118, Uninstall-time Secret leftovers documented without their own heading
Caveats
- The two-Secret claim holds.
templates/certmanager.yamlrenders exactly twoCertificateobjects ({{ .Release.Name }}-ca,{{ .Release.Name }}-tls), the chart never renders the Secrets itself, andrelease.prefix: rabbitmq-atpackages/system/rabbitmq-rd/cozyrds/rabbitmq.yaml:13makes the rendered names match therabbitmq-<name>-ca/-tlsthe doc quotes. enableCertificateOwnerRefisfalseatpackages/system/cert-manager/charts/cert-manager/values.yaml:268and no cozystack wrapper orvalues-isp-*.yamlpreset overrides it, so on every shipped variant those Secrets really do carry no owner reference back to theirCertificate.- I went looking for a third survivor the doc misses. The operator's
<name>-default-userand<name>-erlang-cookieSecrets are excluded from tenant visibility in the same cozyrds file, so they were the obvious candidate, but the pinnedrabbitmq/[email protected]sets a controller reference on both (internal/resource/default_user_secret.go,internal/resource/erlang_cookie.go), so GC takes them when Helm deletes the CR. "The same two Secrets" is complete, not just correct about the two it names. make generatereproduces the file byte for byte, so the new prose sits in the hand-authored half of the README and needed no regeneration.
|
|
||
| Disabling TLS on a release that had it needs one manual step. Helm removes the `TenantProjection` sentinel along with the `Certificate` objects, and Kubernetes garbage-collects `rabbitmq-<name>.tenant-ca` with it, so the trust anchor stops being projected as soon as TLS goes off — no stale anchor is left for an endpoint that has gone back to plaintext. What Helm cannot clean up is the raw CA material: cert-manager does not delete the Secrets it produced (`enableCertificateOwnerRef` is off), so `rabbitmq-<name>-ca` and `rabbitmq-<name>-tls` stick around, unused, holding private keys nobody references any more. Delete them by hand after disabling TLS. | ||
|
|
||
| Deleting the application leaves the same two Secrets behind. The chart renders the `Certificate` and `Issuer` objects and never the Secrets, and cert-manager does not own what it issued, so a Helm uninstall removes neither `rabbitmq-<name>-ca`, which holds the CA private key, nor `rabbitmq-<name>-tls`. After removing a RabbitMQ that had TLS on, delete both from the tenant namespace by hand: |
Collaborator
There was a problem hiding this comment.
[MINOR] Uninstall-time Secret leftovers documented without their own heading
This paragraph describes what survives deleting the whole RabbitMQ application, a different lifecycle event from disabling TLS on a running one, but it is appended under "### Turning TLS off again" (line 114) with no heading of its own. An operator scanning headings for uninstall/deletion behaviour has nothing to jump to. Give it its own "### Deleting the application" subheading next to the existing one.
Aleksei Sviridkin (lexfrei)
deleted the
docs/rabbitmq-tls-secrets-outlive-the-application
branch
September 8, 2026 15:45
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
packages/apps/rabbitmq/README.mddocumented the key material that survives turning TLS off, but not the same outcome on deleting the application. The chart rendersCertificateandIssuerobjects and never the Secrets, and cert-manager runs withenableCertificateOwnerRef: false, so a Helm uninstall leavesrabbitmq-<name>-ca(the CA private key) andrabbitmq-<name>-tlsin the tenant namespace with nothing referencing them. One paragraph names the two Secrets and the cleanup command. Documentation only.Screenshots
Not a UI change.
Downstream repositories
A README paragraph in one chart; the generated reference page is built from
values.yamland does not move.Release note
Summary by CodeRabbit