CLOUDP-350197: allow to create a custom role with cluster: true (#553)

# Summary

This PR fixes a problem which was preventing creating cluster-wide
mongodb custom roles.

**Context**
As described in [MongoDB
docs](https://www.mongodb.com/docs/manual/reference/system-roles-collection/#mongodb-data-admin.system.roles.privileges-n-.resource)
the resource object is either:
```
{ db: <database>, collection: <collection> }
```
or
```
{ cluster : true }
```

So that means we cannot provide empty strings for `db` and `collection`
fields as empty string means "any db/collection".

At the same time our previous serialization rules were always sending
empty strings even if not set. This was the source of the problem - it
wasn't possible to specify `cluster: true` because the operator was also
sending empty db and collections.

**Backwards compatibility**
Making `db` and `collection` fields just as omitempty `*string` is not
sufficient, because that would change the semantics of the resource and
would be potentially a breaking change. In order to preserve backwards
compatibility we need additional logic (see
[normalizePrivilegeResource](https://github.com/mongodb/mongodb-kubernetes/pull/553/files#diff-457d90107d5cbfbfb07a276cc6d58bd1f53c710f58ce19d6ec54daa7ab8e08aaR174)
that will maintain the same behavior for non-cluster-wide resources
(sending empty strings even if the field is not set in yaml).



## Proof of Work

Tests passing.

<!-- start git-machete generated -->

# Based on PR #551

## Chain of upstream PRs as of 2025-10-25

* PR #551:
  `master` ← `lsierant/custom-roles-regression-tests`

  * **PR #553 (THIS ONE)**:
    `lsierant/custom-roles-regression-tests` ← `lsierant/custom-roles`

<!-- end git-machete generated -->

Author: lsierant

api/v1/mdb/mongodb_roles_validation.go

@@ -246,7 +246,7 @@ func validatePrivilege(privilege Privilege, mdbVersion string) v1.ValidationResu
 		if !*privilege.Resource.Cluster {
 			return v1.ValidationError("The only valid value for privilege.cluster, if set, is true")
 		}
-		if privilege.Resource.Collection != "" || privilege.Resource.Db != "" {
+		if privilege.Resource.Collection != nil || privilege.Resource.Db != nil {
 			return v1.ValidationError("Cluster: true is not compatible with setting db/collection")
 		}
 		if res := validateClusterPrivilegeActions(privilege.Actions, mdbVersion); res.Level == v1.ErrorLevel {

api/v1/mdb/mongodb_types.go

@@ -965,10 +965,10 @@ type AuthenticationRestriction struct {
 
 type Resource struct {
 	// +optional
-	Db string `json:"db"`
+	Db *string `json:"db,omitempty"`
 	// +optional
-	Collection string `json:"collection"`
-	Cluster    *bool  `json:"cluster,omitempty"`
+	Collection *string `json:"collection,omitempty"`
+	Cluster    *bool   `json:"cluster,omitempty"`
 }
 
 type Privilege struct {

changelog/20251027_fix_cloudp_350197_allow_to_create_a_custom_role_with.md

@@ -0,0 +1,6 @@
+---
+kind: fix
+date: 2025-10-27
+---
+
+* Fixed inability to specify cluster-wide privileges in custom roles.

controllers/operator/common_controller.go

@@ -15,6 +15,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -132,12 +133,24 @@ func (r *ReconcileCommonController) ensureRoles(ctx context.Context, db mdbv1.Db
 	if reflect.DeepEqual(dRoles, roles) {
 		return workflow.OK()
 	}
-	// HELP-20798: the agent deals correctly with a null value for
-	// privileges only when creating a role, not when updating
-	// we work around it by explicitly passing empty array
-	for i, role := range roles {
-		if role.Privileges == nil {
-			roles[i].Privileges = []mdbv1.Privilege{}
+
+	// clone roles list to avoid mutating the spec in normalizePrivilegeResource
+	newRoles := make([]mdbv1.MongoDBRole, len(roles))
+	for i := range roles {
+		newRoles[i] = *roles[i].DeepCopy()
+	}
+	roles = newRoles
+
+	for roleIdx := range roles {
+		// HELP-20798: the agent deals correctly with a null value for
+		// privileges only when creating a role, not when updating
+		// we work around it by explicitly passing empty array
+		if roles[roleIdx].Privileges == nil {
+			roles[roleIdx].Privileges = []mdbv1.Privilege{}
+		}
+
+		for privilegeIdx := range roles[roleIdx].Privileges {
+			roles[roleIdx].Privileges[privilegeIdx].Resource = normalizePrivilegeResource(roles[roleIdx].Privileges[privilegeIdx].Resource)
 		}
 	}
 
@@ -155,6 +168,27 @@ func (r *ReconcileCommonController) ensureRoles(ctx context.Context, db mdbv1.Db
 	return workflow.OK()
 }
 
+// normalizePrivilegeResource ensures that mutually exclusive fields are not passed at the same time and ensures backwards compatibility by
+// preserving empty strings for db and collection.
+// This function was introduced after we've changed db and collection fields to *string allowing to omit the field from serialization (CLOUDP-349078).
+func normalizePrivilegeResource(resource mdbv1.Resource) mdbv1.Resource {
+	if resource.Cluster != nil && *resource.Cluster {
+		// for cluster-wide privilege mongod is not accepting even empty strings in db and collection
+		resource.Db = nil
+		resource.Collection = nil
+	} else {
+		// for backwards compatibility we must convert "not specified" fields as empty strings
+		if resource.Db == nil {
+			resource.Db = ptr.To("")
+		}
+		if resource.Collection == nil {
+			resource.Collection = ptr.To("")
+		}
+	}
+
+	return resource
+}
+
 // getRoleRefs retrieves the roles from the referenced resources. It will return an error if any of the referenced resources are not found.
 // It will also add the referenced resources to the resource watcher, so that they are watched for changes.
 // The referenced resources are expected to be of kind ClusterMongoDBRole.

controllers/operator/common_controller_test.go

@@ -419,22 +419,36 @@ func TestCheckEmptyStringsInPrivilegesEquivalentToNotPassingFields(t *testing.T)
 		Privileges: []mdbv1.Privilege{
 			{
 				Resource: mdbv1.Resource{
-					Db:         "config",
-					Collection: "", // Explicit empty string
+					Db:         ptr.To("config"),
+					Collection: ptr.To(""), // Explicit empty string
 				},
 				Actions: []string{"find", "update", "insert", "remove"},
 			},
 			{
 				Resource: mdbv1.Resource{
-					Db:         "users",
-					Collection: "usersCollection",
+					Db:         ptr.To("users"),
+					Collection: ptr.To("usersCollection"),
 				},
 				Actions: []string{"update", "insert", "remove"},
 			},
 			{
 				Resource: mdbv1.Resource{
-					Db:         "", // Explicit empty string
-					Collection: "", // Explicit empty string
+					Db:         ptr.To(""), // Explicit empty string
+					Collection: ptr.To(""), // Explicit empty string
+				},
+				Actions: []string{"find"},
+			},
+			{
+				Resource: mdbv1.Resource{
+					Cluster: ptr.To(true),
+				},
+				Actions: []string{"find"},
+			},
+			{
+				Resource: mdbv1.Resource{
+					Cluster:    ptr.To(true),
+					Db:         ptr.To(""),
+					Collection: ptr.To(""),
 				},
 				Actions: []string{"find"},
 			},
@@ -452,15 +466,15 @@ func TestCheckEmptyStringsInPrivilegesEquivalentToNotPassingFields(t *testing.T)
 		Privileges: []mdbv1.Privilege{
 			{
 				Resource: mdbv1.Resource{
-					Db: "config",
+					Db: ptr.To("config"),
 					// field not set, should pass ""
 				},
 				Actions: []string{"find", "update", "insert", "remove"},
 			},
 			{
 				Resource: mdbv1.Resource{
-					Db:         "users",
-					Collection: "usersCollection",
+					Db:         ptr.To("users"),
+					Collection: ptr.To("usersCollection"),
 				},
 				Actions: []string{"update", "insert", "remove"},
 			},
@@ -470,6 +484,12 @@ func TestCheckEmptyStringsInPrivilegesEquivalentToNotPassingFields(t *testing.T)
 				},
 				Actions: []string{"find"},
 			},
+			{
+				Resource: mdbv1.Resource{
+					Cluster: ptr.To(true),
+				},
+				Actions: []string{"find"},
+			},
 		},
 	}
 
@@ -486,16 +506,26 @@ func TestCheckEmptyStringsInPrivilegesEquivalentToNotPassingFields(t *testing.T)
 	assert.True(t, ok)
 	require.Len(t, roles, 2)
 
-	assert.Equal(t, "config", roles[0].Privileges[0].Resource.Db)
-	assert.Equal(t, "", roles[0].Privileges[0].Resource.Collection)
-
-	assert.Equal(t, "users", roles[0].Privileges[1].Resource.Db)
-	assert.Equal(t, "usersCollection", roles[0].Privileges[1].Resource.Collection)
-
-	assert.Equal(t, "", roles[0].Privileges[2].Resource.Db)
-	assert.Equal(t, "", roles[0].Privileges[2].Resource.Collection)
-
-	assert.True(t, reflect.DeepEqual(roles[0].Privileges, roles[1].Privileges))
+	// we iterate over two created privileges because both should end with the same result
+	for i := range 2 {
+		assert.Nil(t, roles[i].Privileges[0].Resource.Cluster)
+		assert.Equal(t, ptr.To("config"), roles[i].Privileges[0].Resource.Db)
+		// even if the db or collection field is not passed it must result in empty string
+		assert.Equal(t, ptr.To(""), roles[i].Privileges[0].Resource.Collection)
+
+		assert.Nil(t, roles[i].Privileges[1].Resource.Cluster)
+		assert.Equal(t, ptr.To("users"), roles[i].Privileges[1].Resource.Db)
+		assert.Equal(t, ptr.To("usersCollection"), roles[i].Privileges[1].Resource.Collection)
+
+		assert.Nil(t, roles[i].Privileges[2].Resource.Cluster)
+		assert.Equal(t, ptr.To(""), roles[i].Privileges[2].Resource.Db)
+		assert.Equal(t, ptr.To(""), roles[i].Privileges[2].Resource.Collection)
+
+		require.NotNil(t, roles[i].Privileges[3].Resource.Cluster)
+		assert.True(t, *roles[i].Privileges[3].Resource.Cluster)
+		assert.Nil(t, roles[i].Privileges[3].Resource.Db)
+		assert.Nil(t, roles[i].Privileges[3].Resource.Collection)
+	}
 }
 
 func TestSecretWatcherWithAllResources(t *testing.T) {

docker/mongodb-kubernetes-tests/kubetester/mongotester.py

@@ -645,7 +645,7 @@ def run(self):
                 self.health_function(**self.health_function_params)
                 consecutive_failure = 0
             except Exception as e:
-                print(f"Error in {self.__class__.__name__}: {e})")
+                logging.error(f"Error in {self.__class__.__name__}: {e})")
                 self.last_exception = e
                 consecutive_failure = consecutive_failure + 1
                 self.max_consecutive_failure = max(self.max_consecutive_failure, consecutive_failure)

docker/mongodb-kubernetes-tests/tests/authentication/fixtures/cluster-mongodb-role-with-empty-strings.yaml

@@ -29,6 +29,10 @@ spec:
         collection: ""
       actions:
         - "find"
+    - resource:
+        cluster: true
+      actions:
+        - "bypassWriteBlockingMode"
   authenticationRestrictions:
     - clientSource:
         - "127.0.0.0/8"

docker/mongodb-kubernetes-tests/tests/authentication/fixtures/cluster-mongodb-role-without-empty-strings.yaml

@@ -26,6 +26,10 @@ spec:
     - resource: {}
       actions:
         - "find"
+    - resource:
+        cluster: true
+      actions:
+        - "bypassWriteBlockingMode"
   authenticationRestrictions:
     - clientSource:
         - "127.0.0.0/8"

docker/mongodb-kubernetes-tests/tests/authentication/mongodb_custom_roles.py

@@ -57,6 +57,14 @@ def get_expected_role(role_name: str) -> dict:
                     "find"
                 ]
             },
+            {
+                "resource": {
+                    "cluster": True
+                },
+                "actions": [
+                    "bypassWriteBlockingMode"
+                ]
+            }
         ],
         "authenticationRestrictions": [
             {

scripts/evergreen/e2e/dump_diagnostic_information.sh

@@ -206,6 +206,11 @@ dump_pod_readiness_state() {
     kubectl --context="${context}" cp -c "mongodb-agent" "${namespace}/${pod}:/var/log/mongodb-mms-automation/agent-health-status.json" "${agent_health_status}" &> /dev/null
     ([[ -f "${agent_health_status}" ]] && jq . < "${agent_health_status}" > tmpfile && mv tmpfile "${agent_health_status}")
 
+    if [[ ! -f "${agent_health_status}" ]]; then
+      kubectl --context="${context}" cp -c "mongodb-enterprise-database" "${namespace}/${pod}:/var/log/mongodb-mms-automation/agent-health-status.json" "${agent_health_status}" &> /dev/null
+      ([[ -f "${agent_health_status}" ]] && jq . < "${agent_health_status}" > tmpfile && mv tmpfile "${agent_health_status}")
+    fi
+
     if [[ ! -f "${agent_health_status}" ]]; then
       echo "Agent health status not found; trying community health status: "
       kubectl --context="${context}" cp -c "mongodb-agent" "${namespace}/${pod}:/var/log/mongodb-mms-automation/healthstatus/agent-health-status.json" "${agent_health_status}" &> /dev/null