Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/v1/mdb/mongodb_roles_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions api/v1/mdb/mongodb_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
kind: fix
date: 2025-10-27
---

* Fixed inability to specify cluster-wide privileges in custom roles.
Comment on lines +1 to +6
Copy link
Collaborator

Choose a reason for hiding this comment

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

LGTM!

46 changes: 40 additions & 6 deletions controllers/operator/common_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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.
Expand Down
68 changes: 49 additions & 19 deletions controllers/operator/common_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
Expand All @@ -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"},
},
Expand All @@ -470,6 +484,12 @@ func TestCheckEmptyStringsInPrivilegesEquivalentToNotPassingFields(t *testing.T)
},
Actions: []string{"find"},
},
{
Resource: mdbv1.Resource{
Cluster: ptr.To(true),
},
Actions: []string{"find"},
},
},
}

Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ spec:
collection: ""
actions:
- "find"
- resource:
cluster: true
actions:
- "bypassWriteBlockingMode"
authenticationRestrictions:
- clientSource:
- "127.0.0.0/8"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ spec:
- resource: {}
actions:
- "find"
- resource:
cluster: true
actions:
- "bypassWriteBlockingMode"
authenticationRestrictions:
- clientSource:
- "127.0.0.0/8"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,14 @@ def get_expected_role(role_name: str) -> dict:
"find"
]
},
{
"resource": {
"cluster": True
},
"actions": [
"bypassWriteBlockingMode"
]
}
],
"authenticationRestrictions": [
{
Expand Down
5 changes: 5 additions & 0 deletions scripts/evergreen/e2e/dump_diagnostic_information.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down