Skip to content
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

CLOUDP-196371: Delete users removed from the resource #1587

Merged
merged 17 commits into from
Jul 23, 2024

Conversation

mateigrigore
Copy link
Contributor

Summary:

This PR adds changes that will cleanup connection string secrets after a user is removed from the resource and ensure that it will also be removed from the database.

Connection string secrets are removed by comparing the lastAppliedSpec with the current spec to retrieve the users that have been removed and then their corresponding connection string secrets.

Users are deleted from the database by adding a corresponding DeletedUser object to the usersDeleted field in the auth object of the AutomationConfig. This ensures the users will be deleted according to the documentation

The PR adds:

  • 2 new methods in the controllers/mongodb_cleanup.go file:
    • getConnectionStringSecretsToDelete that compares the previous spec with the current one and returns a slice containing the secrets corresponding to removed users
    • cleanupConnectionStringSecrets that deletes the secrets returned by the previous method
  • The lastAppliedSpec parameter to r.deployMongoDBReplicaSet, r.deployAutomationConfig, r.ensureAutomationConfig, r.buildAutomationConfig to allow it to be used to retrieve removed users
  • 2 new methods in the pkg/authentication/authentication.go file:
    • getDeletedUsers that usses lastAppliedSpec to return a slice of DeletedUsers corresponding to the removed users from the spec
    • AddRemovedUsers that adds the users returned by the previous method to the usersDeleted field in the auth
  • The UsersDeleted field to the Auth type. This contains of a slice of objects of type DeletedUser, which contain 2 fields:
    • User: The user that will be deleted
    • Dbs: A list of databses from which the user should be deleted
  • 3 methods in the test/e2e/mongodbtests/mongodbtests.go file:
    • RemoveUserFromResource that sets the users field in the spec of the resource to an empty list
    • ConnectionStringSecretsAreCleanedUp that checks that a specific connection string secret is not found in the cluster anymore
    • AuthUsersDeletedIsUpdated that checks that the auth was updated to contain the specified user
  • An e2e test to check the behaviour of cleaning up users that:
    • Creates a MongoDBCommunity resource
    • Asserts basic functionality of that resource
    • Removed the users from it
    • Waits for the resource to be ready
    • Checks for the auth to be updated
    • Checks for the connection string secret to be deleted
    • Deletes the resource

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).

@mircea-cosbuc mircea-cosbuc added the safe-to-test Add this label to PRs from forks to trigger E2E tests label Jul 16, 2024
@mircea-cosbuc mircea-cosbuc added safe-to-test Add this label to PRs from forks to trigger E2E tests and removed safe-to-test Add this label to PRs from forks to trigger E2E tests labels Jul 16, 2024
@mircea-cosbuc mircea-cosbuc added safe-to-test Add this label to PRs from forks to trigger E2E tests and removed safe-to-test Add this label to PRs from forks to trigger E2E tests labels Jul 16, 2024
@mircea-cosbuc mircea-cosbuc added safe-to-test Add this label to PRs from forks to trigger E2E tests and removed safe-to-test Add this label to PRs from forks to trigger E2E tests labels Jul 18, 2024
@nammn nammn added safe-to-test Add this label to PRs from forks to trigger E2E tests and removed safe-to-test Add this label to PRs from forks to trigger E2E tests labels Jul 22, 2024
Copy link
Collaborator

@nammn nammn 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 your changes! They are looking good! Just some suggestions here and there

controllers/mongodb_cleanup.go Outdated Show resolved Hide resolved
controllers/mongodb_cleanup.go Outdated Show resolved Hide resolved
controllers/mongodb_cleanup.go Show resolved Hide resolved
controllers/mongodb_cleanup.go Outdated Show resolved Hide resolved
controllers/mongodb_cleanup.go Show resolved Hide resolved
pkg/authentication/authentication.go Outdated Show resolved Hide resolved
test/e2e/mongodbtests/mongodbtests.go Outdated Show resolved Hide resolved
test/e2e/mongodbtests/mongodbtests.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@nammn nammn 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 your contribution, LGTM! One small cleanup and we should be good to go once all tests are green

test/e2e/mongodbtests/mongodbtests.go Outdated Show resolved Hide resolved
@nammn nammn added safe-to-test Add this label to PRs from forks to trigger E2E tests and removed safe-to-test Add this label to PRs from forks to trigger E2E tests labels Jul 23, 2024
@nammn nammn merged commit 31acaf7 into mongodb:master Jul 23, 2024
46 checks passed
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.

3 participants