Skip to content

feat: add connection string secret annotations #119

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

Merged

Conversation

limwa
Copy link
Contributor

@limwa limwa commented May 14, 2025

Summary

Fixes mongodb/mongodb-kubernetes-operator#1522.

This is a port of mongodb/mongodb-kubernetes-operator#1582.

In this PR, I've added the ability to add custom annotations to the generated connection string secrets in MongoDB Community Operator.

This is useful to handle more deployment scenarios, in particular, scenarios where the operator is not deployed cluster-wide, but to a specific namespace. In these scenarios, the connectionStringSecretNamespace property becomes useless because, as stated in the Kubernetes docs, cross-namespace owner references are disallowed, thus allowing for the secrets to be immediately garbage-collected, as stated in mongodb/mongodb-kubernetes-operator#1578. For the owner references to be valid, the secrets need to be generated in the namespace of the MDBC resource. However, if the user needs the secrets to be present in other namespaces, they can use reflector, for instance, which allows for the secrets to be copied to other namespaces. The problem is that reflector and other similar controllers require the source secrets to be annotated with specific properties.

As such, I've implemented a connectionStringSecretAnnotations property that allows MongoDB Community Operator users to specify per-user connection string secret annotations.

Proof of Work

I've added a unit test and an e2e test. The unit test is passing. Regarding the e2e test, it was passing in the mongodb-kubernetes-operator repository, but I couldn't figure out how to run the e2e tests in this repository.

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
    • No, should I create an issue in this repository for that?
  • Have you checked whether your jira ticket required DOCSP changes?
    • What does this mean?
  • Have you checked for release_note changes?

Reminder (Please remove this when merging)

  • Please try to Approve or Reject Changes the PR, keep PRs in review as short as possible
  • Our Short Guide for PRs: Link
    • I can't access this link
  • Remember the following Communication Standards - use comment prefixes for clarity:
    • blocking: Must be addressed before approval.
    • follow-up: Can be addressed in a later PR or ticket.
    • q: Clarifying question.
    • nit: Non-blocking suggestions.
    • note: Side-note, non-actionable. Example: Praise
    • --> no prefix is considered a question

@limwa limwa requested review from dan-mckean, vinilage and a team as code owners May 14, 2025 14:17
@limwa limwa requested review from MaciejKaras and SimonBaeumer May 14, 2025 14:17
@MaciejKaras
Copy link
Collaborator

@limwa thanks for the contribution and migrating the change to MCK repository.

For the e2e tests to run MongoDB engineer needs to approve the CI pipeline. I did that and it seems PR needs to be updated with master and require running pre-commit again.

@limwa limwa force-pushed the feat/connection-string-secret-annotations branch 3 times, most recently from bc8f179 to e87cef8 Compare May 15, 2025 09:47
@limwa
Copy link
Contributor Author

limwa commented May 15, 2025

@MaciejKaras thanks! I've updated the branch with master and everything should be okay now.

I've also changed the LC_ALL variable in my Linux computer so that sort properly sorts the files, and the PR diff is now much smaller and comprehensible.

Copy link
Collaborator

@MaciejKaras MaciejKaras left a comment

Choose a reason for hiding this comment

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

From my side this is a complete feature with all necessary docs and testing. Great work!

@limwa limwa force-pushed the feat/connection-string-secret-annotations branch 2 times, most recently from 412aec0 to c918353 Compare May 24, 2025 01:14
@limwa limwa force-pushed the feat/connection-string-secret-annotations branch from c918353 to 201ef1b Compare May 26, 2025 08:56
@MaciejKaras
Copy link
Collaborator

evergreen retry

@limwa
Copy link
Contributor Author

limwa commented May 26, 2025

Any chance I could get the logs for the failing e2e_mco_tests job?

@MaciejKaras
Copy link
Collaborator

@limwa It is fine, just flaky test

Copy link
Contributor

@lsierant lsierant left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for contributing it!

@vinilage
Copy link
Collaborator

Thanks for your contribution @limwa.

@limwa
Copy link
Contributor Author

limwa commented May 28, 2025

Thanks to everyone for approving this PR 🙏🏻

@MaciejKaras
Copy link
Collaborator

Thanks to everyone for approving this PR 🙏🏻

@limwa np :) feel free to merge the PR (squash and merge option please).

@limwa
Copy link
Contributor Author

limwa commented May 28, 2025

I don't have write access to merge the PR haha, so I'll leave that to you! 😁

@MaciejKaras MaciejKaras merged commit 308ca3c into mongodb:master May 28, 2025
35 checks passed
@limwa limwa deleted the feat/connection-string-secret-annotations branch May 28, 2025 11:36
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.

[Feature Request] Enable Copying/Annotation of Generated Secrets to Additional Namespaces
5 participants