Skip to content

CLOUDP-314917 CLOUDP-314918 CLOUDP-314919 - Add ClusterMongoDBRole CRD #54

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
merged 20 commits into from
Jun 18, 2025

Conversation

lucian-tosa
Copy link
Contributor

@lucian-tosa lucian-tosa commented Apr 28, 2025

Summary

Introduced new CRD for MongoDB roles. The new CRD is cluster-scoped and can be reused in multiple MongoDB deployments even across-namespaces.

The CRD design can be found in the TD

Proof of Work

There are unit and E2E tests verifying that the custom roles are:

  • Roles are added in automation config
  • Removing a reference to the role removes the role from automation config
  • Updating the role triggers a reconciliation, and role is updated in AC
  • Deleting the role is blocked by the finalizer
  • Deleting the role after it is removed from resources is not blocked anymore

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • 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
  • 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

@lucian-tosa lucian-tosa changed the title Add MongoDBCustomRole CRD Add ClusterMongoDBRole CRD May 19, 2025
@lucian-tosa lucian-tosa changed the title Add ClusterMongoDBRole CRD CLOUDP-314917 CLOUDP-314918 CLOUDP-314919 CLOUDP-314920 - Add ClusterMongoDBRole CRD May 19, 2025
@lucian-tosa lucian-tosa marked this pull request as ready for review May 19, 2025 13:30
@lucian-tosa lucian-tosa requested review from dan-mckean, vinilage and a team as code owners May 19, 2025 13:30
@lucian-tosa lucian-tosa requested review from nammn and Julien-Ben May 19, 2025 13:30
@lucian-tosa lucian-tosa changed the title CLOUDP-314917 CLOUDP-314918 CLOUDP-314919 CLOUDP-314920 - Add ClusterMongoDBRole CRD CLOUDP-314917 CLOUDP-314918 CLOUDP-314919 - Add ClusterMongoDBRole CRD May 19, 2025
@anandsyncs anandsyncs requested a review from Copilot May 21, 2025 21:52
Copilot

This comment was marked as outdated.

Copy link
Collaborator

@Julien-Ben Julien-Ben left a comment

Choose a reason for hiding this comment

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

Great PR 👏 Very clean code
I just added a few nit, but overall LGTM

I approve the changes, but let's wait for a consensus on the finalizers before merging.
But on the implementation side, it follows what's described in the TD.

# Conflicts:
#	.evergreen.yml
#	api/v1/mdb/zz_generated.deepcopy.go
#	controllers/om/deployment.go
@lucian-tosa lucian-tosa requested a review from Julien-Ben June 6, 2025 10:26
@anandsyncs anandsyncs requested a review from Copilot June 6, 2025 11:44
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new cluster-scoped custom resource definition (ClusterMongoDBRole) for managing MongoDB roles across namespaces and updates related tooling/configuration to support it.

  • Add ClusterMongoDBRole to manager watches and CRD kustomization
  • Define and validate roleRefs alongside existing roles in multiple CRDs
  • Extend builders, deepcopy code, validation logic, and CI configs for custom roles

Reviewed Changes

Copilot reviewed 67 out of 67 changed files in this pull request and generated no comments.

Show a summary per file
File Description
config/manager/manager.yaml Watch clustermongodbroles in the operator manager
config/crd/kustomization.yaml Include new CRD in kustomization resources
config/crd/bases/mongodb.com_opsmanagers.yaml Add roleRefs array and XValidation for mutual exclusion
config/crd/bases/mongodb.com_mongodbmulticluster.yaml Add roleRefs array and XValidation
config/crd/bases/mongodb.com_mongodb.yaml Add roleRefs array and XValidation
config/crd/bases/mongodb.com_clustermongodbroles.yaml New ClusterMongoDBRole CRD definition
api/v1/role/zz_generated.deepcopy.go Generated deep-copy methods for ClusterMongoDBRole
api/v1/role/rolebuilder.go Builder for ClusterMongoDBRole
api/v1/role/groupversion_info.go Register API group version for role
api/v1/role/doc.go Package markers for deep-copy and versioning
api/v1/role/clustermongodbrole_types.go Define ClusterMongoDBRole spec and list types
api/v1/mdbmulti/mongodbmultibuilder.go MultiReplicaSet builder updated to support RoleRefs
api/v1/mdb/zz_generated.deepcopy.go Update deepcopy for MongoDBRole and add MongoDBRoleRef
api/v1/mdb/mongodbbuilder.go Add SetRoles and SetRoleRefs methods
api/v1/mdb/mongodb_validation.go Correct validation function name for roles attribute
api/v1/mdb/mongodb_types.go Extend Security type with roles/roleRefs fields
api/v1/mdb/mongodb_roles_validation.go Rename and update role-validation functions
PROJECT Scaffold project config for ClusterMongoDBRole
.evergreen.yml Update Evergreen task groups for custom roles
.evergreen-tasks.yml Add/rename Evergreen tasks for custom roles
Comments suppressed due to low confidence (4)

.evergreen.yml:731

  • The original e2e_replica_set_custom_roles test was renamed to e2e_replica_set_ldap_custom_roles without adding a matching test definition. Verify that this task name matches an existing test or update it accordingly.
+      - e2e_replica_set_ldap_custom_roles

.evergreen-tasks.yml:652

  • The task name was changed to e2e_replica_set_ldap_custom_roles but the matching function may not exist. Ensure the renamed task corresponds to a valid Evergreen task definition.
-  - name: e2e_replica_set_custom_roles

PROJECT:6

  • The layout field was changed from a string to a YAML list, which does not match the expected kubebuilder project config schema. It should remain a single string value (layout: go.kubebuilder.io/v3).
layout:

PROJECT:41

  • The new resource entry under resources: is indented and structured inconsistently with existing entries; this may break kubebuilder’s parsing of the project config. Align and format it as a map item matching the other list entries.
- api:

@lucian-tosa lucian-tosa requested review from a team and removed request for nammn and a team June 10, 2025 08:00
Co-authored-by: Łukasz Sierant <[email protected]>
if err := r.client.Update(ctx, user); err != nil {
return r.updateStatus(ctx, user, workflow.Failed(xerrors.Errorf("Failed to update the user with the removed finalizer: %w", err)), log)
}
return r.updateStatus(ctx, user, workflow.Pending("Finalizer will be removed. MongoDB resource not found"), log)
return r.updateStatus(ctx, user, workflow.Pending("UserFinalizer will be removed. MongoDB resource not found"), log)
Copy link
Contributor

@lsierant lsierant Jun 16, 2025

Choose a reason for hiding this comment

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

Should the message also be changed? The finalizer name wasn't changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

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.

Awesome changes! But some clarifications, suggestions required

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.

Overall the PR is awesome! Well done! 👏
I've left few minor and one blocking comment regarding checking in the operator if the cluster roles are enabled.

@lucian-tosa lucian-tosa requested review from nammn and lsierant June 17, 2025 07:58
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!
Please only update the error message.

@lucian-tosa lucian-tosa merged commit cc893f1 into master Jun 18, 2025
35 checks passed
@lucian-tosa lucian-tosa deleted the custom-roles branch June 18, 2025 09:03
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.

5 participants