From b6754264b52be8d69ed8d008f84a9cba410192b0 Mon Sep 17 00:00:00 2001
From: Romain Acciari <romain.acciari+github@gmail.com>
Date: Wed, 25 Feb 2026 19:38:07 +0100
Subject: [PATCH] fix: use order-insensitive comparison for user roles in
 updateRoles

MongoDB does not guarantee the order of roles returned by usersInfo
command. The updateRoles function used reflect.DeepEqual to compare
the desired roles from the CR spec with the roles returned by MongoDB.
Since reflect.DeepEqual is order-sensitive, this caused an infinite
reconciliation loop where the operator would repeatedly detect a
"change" and call updateUser on every reconciliation cycle (~15s),
even when the roles were semantically identical.

This is the same class of bug that was fixed for custom roles
comparison in rolesChanged() (K8SPSMDB-1146 / PR #1679), which
correctly uses cmp.Equal with cmpopts.SortSlices. This commit applies
the same fix to updateRoles() for user roles.

Signed-off-by: Romain Acciari <romain.acciari+github@gmail.com>
---
 .../perconaservermongodb/custom_users.go      |  9 +-
 .../perconaservermongodb/custom_users_test.go | 99 +++++++++++++++++++
 2 files changed, 106 insertions(+), 2 deletions(-)

diff --git a/pkg/controller/perconaservermongodb/custom_users.go b/pkg/controller/perconaservermongodb/custom_users.go
index 05f8eb81c4..1198ade341 100644
--- a/pkg/controller/perconaservermongodb/custom_users.go
+++ b/pkg/controller/perconaservermongodb/custom_users.go
@@ -3,7 +3,6 @@ package perconaservermongodb
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"strings"
 
 	"github.com/google/go-cmp/cmp"
@@ -357,7 +356,13 @@ func updateRoles(ctx context.Context, mongoCli mongo.Client, user *api.User, use
 		roles = append(roles, mongo.Role{DB: role.DB, Role: role.Name})
 	}
 
-	if reflect.DeepEqual(userInfo.Roles, roles) {
+	sortRoles := cmpopts.SortSlices(func(a, b mongo.Role) bool {
+		if a.DB != b.DB {
+			return a.DB < b.DB
+		}
+		return a.Role < b.Role
+	})
+	if cmp.Equal(userInfo.Roles, roles, sortRoles) {
 		return nil
 	}
 
diff --git a/pkg/controller/perconaservermongodb/custom_users_test.go b/pkg/controller/perconaservermongodb/custom_users_test.go
index db055f7a42..6317595a99 100644
--- a/pkg/controller/perconaservermongodb/custom_users_test.go
+++ b/pkg/controller/perconaservermongodb/custom_users_test.go
@@ -16,6 +16,105 @@ import (
 	"github.com/percona/percona-server-mongodb-operator/pkg/psmdb/mongo"
 )
 
+// mockMongoClientForRoles is a minimal mock to test updateRoles behavior.
+type mockMongoClientForRoles struct {
+	mongo.Client
+	updateRolesCalled bool
+}
+
+func (m *mockMongoClientForRoles) UpdateUserRoles(ctx context.Context, db, username string, roles []mongo.Role) error {
+	m.updateRolesCalled = true
+	return nil
+}
+
+func TestUpdateRoles(t *testing.T) {
+	tests := []struct {
+		name              string
+		user              *api.User
+		userInfo          *mongo.User
+		expectUpdateCalled bool
+	}{
+		{
+			name: "same roles same order - no update",
+			user: &api.User{
+				Name: "testuser",
+				DB:   "testdb",
+				Roles: []api.UserRole{
+					{Name: "readWrite", DB: "db1"},
+					{Name: "read", DB: "db2"},
+				},
+			},
+			userInfo: &mongo.User{
+				Roles: []mongo.Role{
+					{Role: "readWrite", DB: "db1"},
+					{Role: "read", DB: "db2"},
+				},
+			},
+			expectUpdateCalled: false,
+		},
+		{
+			name: "same roles different order - no update",
+			user: &api.User{
+				Name: "testuser",
+				DB:   "testdb",
+				Roles: []api.UserRole{
+					{Name: "readWrite", DB: "db1"},
+					{Name: "clusterMonitor", DB: "admin"},
+					{Name: "readWrite", DB: "db2"},
+				},
+			},
+			userInfo: &mongo.User{
+				Roles: []mongo.Role{
+					{Role: "clusterMonitor", DB: "admin"},
+					{Role: "readWrite", DB: "db2"},
+					{Role: "readWrite", DB: "db1"},
+				},
+			},
+			expectUpdateCalled: false,
+		},
+		{
+			name: "different roles - update called",
+			user: &api.User{
+				Name: "testuser",
+				DB:   "testdb",
+				Roles: []api.UserRole{
+					{Name: "readWrite", DB: "db1"},
+					{Name: "read", DB: "db2"},
+				},
+			},
+			userInfo: &mongo.User{
+				Roles: []mongo.Role{
+					{Role: "readWrite", DB: "db1"},
+					{Role: "readWrite", DB: "db3"},
+				},
+			},
+			expectUpdateCalled: true,
+		},
+		{
+			name: "nil userInfo - no update",
+			user: &api.User{
+				Name: "testuser",
+				DB:   "testdb",
+				Roles: []api.UserRole{
+					{Name: "readWrite", DB: "db1"},
+				},
+			},
+			userInfo:          nil,
+			expectUpdateCalled: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mock := &mockMongoClientForRoles{}
+			err := updateRoles(t.Context(), mock, tt.user, tt.userInfo)
+			assert.NoError(t, err)
+			assert.Equal(t, tt.expectUpdateCalled, mock.updateRolesCalled,
+				"UpdateUserRoles called = %v, want %v", mock.updateRolesCalled, tt.expectUpdateCalled)
+		})
+	}
+}
+
 func TestRolesChanged(t *testing.T) {
 	r2 := &mongo.Role{
 		Privileges: []mongo.RolePrivilege{