fix: add drift detection to ProtocolMapper, Role and ClientScope

internal/controller/keycloakclientscope_controller.go

@@ -141,12 +141,17 @@ func (r *KeycloakClientScopeReconciler) Reconcile(ctx context.Context, req ctrl.
 		scopeID = *existingScope.ID
 		definition = mergeIDIntoDefinition(definition, existingScope.ID)
 
-		log.Info("updating client scope", "name", scopeDef.Name, "realm", realmName)
-		if err := kc.UpdateClientScope(ctx, realmName, scopeID, definition); err != nil {
-			RecordError(controllerName, "keycloak_api_error")
-			return r.updateStatus(ctx, clientScope, false, "UpdateFailed", fmt.Sprintf("Failed to update client scope: %v", err), scopeID)
+		currentRaw, fetchErr := kc.GetClientScopeRaw(ctx, realmName, scopeID)
+		if fetchErr == nil && definitionsMatch(definition, currentRaw) {
+			log.V(1).Info("client scope already in sync, skipping update", "name", scopeDef.Name)
+		} else {
+			log.Info("updating client scope", "name", scopeDef.Name, "realm", realmName)
+			if err := kc.UpdateClientScope(ctx, realmName, scopeID, definition); err != nil {
+				RecordError(controllerName, "keycloak_api_error")
+				return r.updateStatus(ctx, clientScope, false, "UpdateFailed", fmt.Sprintf("Failed to update client scope: %v", err), scopeID)
+			}
+			log.Info("client scope updated successfully", "name", scopeDef.Name)
 		}
-		log.Info("client scope updated successfully", "name", scopeDef.Name)
 	}
 
 	// Update status

internal/controller/keycloakprotocolmapper_controller.go

@@ -122,28 +122,19 @@ func (r *KeycloakProtocolMapperReconciler) Reconcile(ctx context.Context, req ct
 		return r.updateStatus(ctx, mapper, false, "ConfigSecretError", err.Error(), "", "", "", "")
 	}
 
-	// Find existing mapper by name
+	// Find existing mapper by name. The raw representation is kept for drift
+	// detection so that an unchanged mapper is not re-PUT on every sync.
 	var mapperID string
+	var currentRaw json.RawMessage
+	var existingMappers []json.RawMessage
+	var listErr error
 	if parentType == "client" {
-		existingMappers, err := kc.GetClientProtocolMappers(ctx, realmName, parentID)
-		if err == nil {
-			for _, m := range existingMappers {
-				if m.Name != nil && *m.Name == mapperName {
-					mapperID = *m.ID
-					break
-				}
-			}
-		}
+		existingMappers, listErr = kc.GetClientProtocolMappersRaw(ctx, realmName, parentID)
 	} else {
-		existingMappers, err := kc.GetClientScopeProtocolMappers(ctx, realmName, parentID)
-		if err == nil {
-			for _, m := range existingMappers {
-				if m.Name != nil && *m.Name == mapperName {
-					mapperID = *m.ID
-					break
-				}
-			}
-		}
+		existingMappers, listErr = kc.GetClientScopeProtocolMappersRaw(ctx, realmName, parentID)
+	}
+	if listErr == nil {
+		mapperID, currentRaw = findMapperByName(existingMappers, mapperName)
 	}
 
 	if mapperID == "" {
@@ -160,6 +151,11 @@ func (r *KeycloakProtocolMapperReconciler) Reconcile(ctx context.Context, req ct
 			return r.updateStatus(ctx, mapper, false, "CreateFailed", fmt.Sprintf("Failed to create protocol mapper: %v", err), "", "", parentType, parentID)
 		}
 		log.Info("protocol mapper created successfully", "name", mapperName, "id", mapperID)
+	} else if definitionsMatch(definition, currentRaw) {
+		// Every PUT on a protocol mapper invalidates the owning client in
+		// Keycloak's realm cache on all nodes, so skipping no-op updates keeps
+		// token endpoint latency flat in steady state.
+		log.V(1).Info("protocol mapper already in sync, skipping update", "name", mapperName)
 	} else {
 		// Update mapper
 		definition = mergeIDIntoDefinition(definition, &mapperID)
@@ -345,3 +341,21 @@ func (r *KeycloakProtocolMapperReconciler) findMappersForSecret(ctx context.Cont
 		return o.(*keycloakv1beta1.KeycloakProtocolMapper).Spec.ConfigSecretRef
 	})
 }
+
+// findMapperByName returns the ID and raw representation of the mapper with
+// the given name, or empty values if none matches.
+func findMapperByName(mappers []json.RawMessage, name string) (string, json.RawMessage) {
+	for _, raw := range mappers {
+		var m struct {
+			ID   string `json:"id"`
+			Name string `json:"name"`
+		}
+		if err := json.Unmarshal(raw, &m); err != nil {
+			continue
+		}
+		if m.Name == name && m.ID != "" {
+			return m.ID, raw
+		}
+	}
+	return "", nil
+}

internal/controller/keycloakprotocolmapper_drift_test.go

@@ -0,0 +1,64 @@
+package controller
+
+import (
+	"encoding/json"
+	"testing"
+)
+
+// Shape taken from a live Keycloak 26 GET .../protocol-mappers/models: the
+// server adds "id" and reorders keys, which must not count as drift.
+const liveMapperList = `[
+	{"id":"eb2a593f-8dc6-4791-8d7d-5c576a203fd4","name":"realm roles","protocol":"openid-connect","protocolMapper":"oidc-usermodel-realm-role-mapper","consentRequired":false,"config":{"access.token.claim":"true","introspection.token.claim":"true","claim.name":"realm_access.roles","jsonType.label":"String","multivalued":"true"}},
+	{"id":"9544248a-c141-43fd-97a7-b5a918fcf9c9","name":"email","protocol":"openid-connect","protocolMapper":"oidc-usermodel-property-mapper","consentRequired":false,"config":{"user.attribute":"email","claim.name":"email"}}
+]`
+
+func liveMappers(t *testing.T) []json.RawMessage {
+	t.Helper()
+	var items []json.RawMessage
+	if err := json.Unmarshal([]byte(liveMapperList), &items); err != nil {
+		t.Fatal(err)
+	}
+	return items
+}
+
+func TestFindMapperByName(t *testing.T) {
+	id, raw := findMapperByName(liveMappers(t), "realm roles")
+	if id != "eb2a593f-8dc6-4791-8d7d-5c576a203fd4" || raw == nil {
+		t.Fatalf("got id=%q raw=%s", id, raw)
+	}
+	if id, raw := findMapperByName(liveMappers(t), "missing"); id != "" || raw != nil {
+		t.Fatalf("expected no match, got id=%q", id)
+	}
+}
+
+func TestProtocolMapperDrift_InSyncSkipsUpdate(t *testing.T) {
+	desired := json.RawMessage(`{"config":{"access.token.claim":"true","claim.name":"realm_access.roles","introspection.token.claim":"true","jsonType.label":"String","multivalued":"true"},"consentRequired":false,"name":"realm roles","protocol":"openid-connect","protocolMapper":"oidc-usermodel-realm-role-mapper"}`)
+	_, current := findMapperByName(liveMappers(t), "realm roles")
+	if !definitionsMatch(desired, current) {
+		t.Fatal("unchanged mapper reported as drift; would re-PUT on every sync")
+	}
+}
+
+func TestProtocolMapperDrift_ConfigChangeDetected(t *testing.T) {
+	desired := json.RawMessage(`{"name":"realm roles","protocolMapper":"oidc-usermodel-realm-role-mapper","config":{"claim.name":"roles"}}`)
+	_, current := findMapperByName(liveMappers(t), "realm roles")
+	if definitionsMatch(desired, current) {
+		t.Fatal("changed config.claim.name not detected")
+	}
+}
+
+func TestProtocolMapperDrift_NoCurrentForcesUpdate(t *testing.T) {
+	if definitionsMatch(json.RawMessage(`{"name":"x"}`), nil) {
+		t.Fatal("missing current representation must fall through to update")
+	}
+}
+
+func TestRoleDrift_CompositesStrippedBeforeCompare(t *testing.T) {
+	// Role GET does not return composites; the controller strips them from the
+	// definition and syncs them via the composites endpoint instead.
+	desired := removeFieldFromDefinition(json.RawMessage(`{"name":"default-roles-x","composite":true,"composites":{"realm":["offline_access"]}}`), "composites")
+	current := json.RawMessage(`{"id":"1","name":"default-roles-x","composite":true,"clientRole":false,"containerId":"x","attributes":{}}`)
+	if !definitionsMatch(desired, current) {
+		t.Fatal("unchanged role reported as drift")
+	}
+}

internal/controller/keycloakrole_controller.go

@@ -130,12 +130,17 @@ func (r *KeycloakRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		} else {
 			roleID = *existingRole.ID
 			definition = mergeIDIntoDefinition(definition, existingRole.ID)
-			log.Info("updating client role", "name", roleName, "realm", realmName, "client", clientUUID)
-			if err := kc.UpdateClientRole(ctx, realmName, clientUUID, roleName, definition); err != nil {
-				RecordError(controllerName, "keycloak_api_error")
-				return r.updateStatus(ctx, role, false, "UpdateFailed", fmt.Sprintf("Failed to update client role: %v", err), roleID, roleName, true, clientUUID)
+			currentRaw, fetchErr := kc.GetClientRoleRaw(ctx, realmName, clientUUID, roleName)
+			if fetchErr == nil && definitionsMatch(definition, currentRaw) {
+				log.V(1).Info("client role already in sync, skipping update", "name", roleName)
+			} else {
+				log.Info("updating client role", "name", roleName, "realm", realmName, "client", clientUUID)
+				if err := kc.UpdateClientRole(ctx, realmName, clientUUID, roleName, definition); err != nil {
+					RecordError(controllerName, "keycloak_api_error")
+					return r.updateStatus(ctx, role, false, "UpdateFailed", fmt.Sprintf("Failed to update client role: %v", err), roleID, roleName, true, clientUUID)
+				}
+				log.Info("client role updated successfully", "name", roleName)
 			}
-			log.Info("client role updated successfully", "name", roleName)
 		}
 	} else {
 		existingRole, err := kc.GetRealmRole(ctx, realmName, roleName)
@@ -150,12 +155,17 @@ func (r *KeycloakRoleReconciler) Reconcile(ctx context.Context, req ctrl.Request
 		} else {
 			roleID = *existingRole.ID
 			definition = mergeIDIntoDefinition(definition, existingRole.ID)
-			log.Info("updating realm role", "name", roleName, "realm", realmName)
-			if err := kc.UpdateRealmRole(ctx, realmName, roleName, definition); err != nil {
-				RecordError(controllerName, "keycloak_api_error")
-				return r.updateStatus(ctx, role, false, "UpdateFailed", fmt.Sprintf("Failed to update realm role: %v", err), roleID, roleName, false, "")
+			currentRaw, fetchErr := kc.GetRealmRoleRaw(ctx, realmName, roleName)
+			if fetchErr == nil && definitionsMatch(definition, currentRaw) {
+				log.V(1).Info("realm role already in sync, skipping update", "name", roleName)
+			} else {
+				log.Info("updating realm role", "name", roleName, "realm", realmName)
+				if err := kc.UpdateRealmRole(ctx, realmName, roleName, definition); err != nil {
+					RecordError(controllerName, "keycloak_api_error")
+					return r.updateStatus(ctx, role, false, "UpdateFailed", fmt.Sprintf("Failed to update realm role: %v", err), roleID, roleName, false, "")
+				}
+				log.Info("realm role updated successfully", "name", roleName)
 			}
-			log.Info("realm role updated successfully", "name", roleName)
 		}
 	}