From e789e3213f088911ac6b0e7611c2ca455d5022b1 Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Mon, 22 Sep 2025 13:03:06 -0400 Subject: [PATCH] fix: limiting what fields are hashed to identify compatible update jobs (#42623) closes: #41014 Signed-off-by: Steve Hawkins --- .github/workflows/ci.yml | 2 +- ...ycloakServiceMonitorDependentResource.java | 4 +- .../KeycloakUpdateJobDependentResource.java | 9 +++- ...eycloakUpdateJobDependentResourceTest.java | 49 +++++++++++++++++++ 4 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 operator/src/test/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResourceTest.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 753378a5639..bfe8bb0bc59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -103,7 +103,7 @@ jobs: run: | SEP="" PROJECTS="" - for i in `find -name '*Test.java' -type f | egrep -v './(testsuite|quarkus|docs|tests|test-framework)/' | sed 's|/src/test/java/.*||' | sort | uniq | sed 's|./||'`; do + for i in `find -name '*Test.java' -type f | egrep -v './(operator|testsuite|quarkus|docs|tests|test-framework)/' | sed 's|/src/test/java/.*||' | sort | uniq | sed 's|./||'`; do PROJECTS="$PROJECTS$SEP$i" SEP="," done diff --git a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakServiceMonitorDependentResource.java b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakServiceMonitorDependentResource.java index 8d7f622ee4f..587cde735a6 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakServiceMonitorDependentResource.java +++ b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakServiceMonitorDependentResource.java @@ -30,8 +30,9 @@ public class KeycloakServiceMonitorDependentResource extends CRUDKubernetesDepen public boolean isMet(DependentResource dependentResource, Keycloak primary, Context context) { var opts = configuredOptions(primary); - if (Boolean.parseBoolean(opts.get(LEGACY_MANAGEMENT_ENABLED)) || !Boolean.parseBoolean(opts.getOrDefault(METRICS_ENABLED, "false"))) + if (Boolean.parseBoolean(opts.get(LEGACY_MANAGEMENT_ENABLED)) || !Boolean.parseBoolean(opts.getOrDefault(METRICS_ENABLED, "false"))) { return false; + } return ServiceMonitorSpec.get(primary).isEnabled(); } @@ -46,6 +47,7 @@ public class KeycloakServiceMonitorDependentResource extends CRUDKubernetesDepen .withNewMetadata() .withName(meta.getName()) .withNamespace(meta.getNamespace()) + .withLabels(Utils.allInstanceLabels(primary)) .endMetadata() .withNewSpec() .withNewNamespaceSelector() diff --git a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResource.java b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResource.java index 39eede01a1c..5a2c982417b 100644 --- a/operator/src/main/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResource.java +++ b/operator/src/main/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResource.java @@ -32,6 +32,7 @@ import org.keycloak.operator.ContextUtils; import org.keycloak.operator.Utils; import org.keycloak.operator.crds.v2alpha1.CRDUtils; import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak; +import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakSpecBuilder; import org.keycloak.operator.crds.v2alpha1.deployment.spec.UpdateSpec; import io.fabric8.kubernetes.api.model.ContainerFluent; @@ -218,8 +219,12 @@ public class KeycloakUpdateJobDependentResource extends CRUDKubernetesDependentR return Stream.concat(updateArgs.stream(), currentArgs.stream().filter(arg -> !arg.equals("start"))).toList(); } - private static String keycloakHash(Keycloak keycloak) { - return Utils.hash(List.of(keycloak.getSpec())); + static String keycloakHash(Keycloak keycloak) { + return Utils.hash( + List.of(new KeycloakSpecBuilder(keycloak.getSpec()).withInstances(null).withLivenessProbeSpec(null) + .withStartupProbeSpec(null).withReadinessProbeSpec(null).withResourceRequirements(null) + .withSchedulingSpec(null).withNetworkPolicySpec(null).withIngressSpec(null) + .withImagePullSecrets().withImportSpec(null).withServiceMonitorSpec(null).build())); } private static Map getLabels(HasMetadata keycloak) { diff --git a/operator/src/test/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResourceTest.java b/operator/src/test/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResourceTest.java new file mode 100644 index 00000000000..6fc9c811085 --- /dev/null +++ b/operator/src/test/java/org/keycloak/operator/controllers/KeycloakUpdateJobDependentResourceTest.java @@ -0,0 +1,49 @@ +/* + * Copyright 2025 Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.operator.controllers; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +import org.junit.jupiter.api.Test; +import org.keycloak.operator.crds.v2alpha1.deployment.Keycloak; +import org.keycloak.operator.crds.v2alpha1.deployment.KeycloakBuilder; + +public class KeycloakUpdateJobDependentResourceTest { + + @Test + void testKeycloakHashConsistency() { + Keycloak keycloak = new KeycloakBuilder().withNewSpec().withInstances(2).endSpec().build(); + + String hash1 = KeycloakUpdateJobDependentResource.keycloakHash(keycloak); + String hash2 = KeycloakUpdateJobDependentResource.keycloakHash(new KeycloakBuilder(keycloak).editSpec().withInstances(1).endSpec().build()); + + assertEquals(hash1, hash2, "Hashes should be equal for identical specs"); + } + + @Test + void testKeycloakHashDifference() { + Keycloak keycloak = new KeycloakBuilder().withNewSpec().withInstances(2).endSpec().build(); + + String hash1 = KeycloakUpdateJobDependentResource.keycloakHash(keycloak); + String hash2 = KeycloakUpdateJobDependentResource.keycloakHash(new KeycloakBuilder(keycloak).editSpec().withNewFeatureSpec().withEnabledFeatures("new-feature").endFeatureSpec().withInstances(1).endSpec().build()); + + assertNotEquals(hash1, hash2, "Hashes should be different"); + } + +}