Skip to content

fix: use correct namespace for password secret #1581

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 Jul 5, 2024

Summary:

Closes #1578.

I created this PR because, in our deployment of mongodb-kubernetes-operator at NIAEFEUP, we were having trouble creating the connection string secret in a namespace that was not the one the MDBC resource was deployed to. In particular, as described in the issue, one problem that we encountered was needing to have the password secret in both namespaces (the MDBC resource namespace and the connectionStringSecretNamespace namespace) for the connection string secret to be created. The logic was changed so that the password secret is always looked up in the namespace of the MDBC resource.

As a result, the password secret now only needs to be on the namespace of the MDBC resource (as was expected by the ensureUserResources function).

All Submissions:

  • Have you opened an Issue before filing this PR?
  • Have you signed our CLA?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if such).

@limwa
Copy link
Contributor Author

limwa commented Jul 5, 2024

Regarding the problem with the connection string secret being garbage-collected when the operator is not deployed cluster-wide, as described in #1578, this PR doesn't fix that problem, because it is unrelated to the logic changed in this PR. I can open another issue later for that problem.

EDIT: I've opened #1582 to mitigate this issue.

@legal90
Copy link

legal90 commented Jul 8, 2024

Thank you for submitting this fix, @limwa! Looking forward for MongoDB to review and release it soon :)

P.s. I run into the same issue recently, mentioned it here: f904443#r143621651

Copy link
Contributor

@slaskawi slaskawi left a comment

Choose a reason for hiding this comment

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

Thanks for the fix attempt. Could you please check my comment?

@slaskawi slaskawi self-assigned this Jul 9, 2024
@slaskawi slaskawi added the safe-to-test Add this label to PRs from forks to trigger E2E tests label Jul 10, 2024
Copy link
Contributor

@slaskawi slaskawi left a comment

Choose a reason for hiding this comment

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

LGTM. Scheduled a CI run for this one.

@slaskawi slaskawi merged commit 1322fd4 into mongodb:master Jul 12, 2024
53 of 54 checks passed
@slaskawi
Copy link
Contributor

Integrated! Thanks for contribution!

@realtimetodie
Copy link

@slaskawi Could you please make a release that includes this bugfix? This is urgently needed🙏 thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe-to-test Add this label to PRs from forks to trigger E2E tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't create a connection string secret in a different namespace
4 participants