-
Notifications
You must be signed in to change notification settings - Fork 9
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
Conversation
There was a problem hiding this 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.
docker/mongodb-kubernetes-tests/tests/authentication/mongodb_custom_roles.py
Outdated
Show resolved
Hide resolved
# Conflicts: # .evergreen.yml # api/v1/mdb/zz_generated.deepcopy.go # controllers/om/deployment.go
There was a problem hiding this 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 existingroles
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 toe2e_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:
Co-authored-by: Łukasz Sierant <lukasz.sierant@mongodb.com>
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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
docker/mongodb-kubernetes-tests/tests/authentication/mongodb_custom_roles.py
Show resolved
Hide resolved
There was a problem hiding this 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
docker/mongodb-kubernetes-tests/tests/authentication/mongodb_custom_roles.py
Show resolved
Hide resolved
There was a problem hiding this 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.
There was a problem hiding this 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.
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:
Checklist
Reminder (Please remove this when merging)