diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java index 05ee05b036..00f68f36a7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java @@ -4,17 +4,14 @@ import java.util.Collections; import java.util.List; -import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.zjsonpatch.JsonDiff; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.Matcher; import com.fasterxml.jackson.databind.JsonNode; -public class GenericKubernetesResourceMatcher - implements Matcher { +public class GenericKubernetesResourceMatcher { private static final String SPEC = "/spec"; private static final String METADATA = "/metadata"; @@ -26,40 +23,7 @@ public class GenericKubernetesResourceMatcher dependentResource; - private GenericKubernetesResourceMatcher(KubernetesDependentResource dependentResource) { - this.dependentResource = dependentResource; - } - - @SuppressWarnings({"unchecked", "rawtypes", "unused"}) - static Matcher matcherFor( - KubernetesDependentResource dependentResource) { - return new GenericKubernetesResourceMatcher(dependentResource); - } - - /** - * {@inheritDoc} - *

- * This implementation attempts to cover most common cases out of the box by considering - * non-additive changes to the resource's spec (if the resource in question has a {@code spec} - * field), making special provisions for {@link ConfigMap} and {@link Secret} resources. Additive - * changes (i.e. a field is added that previously didn't exist) are not considered as triggering a - * mismatch by default to account for validating webhooks that might add default values - * automatically when not present or some other controller adding labels and/or annotations. - *

- *

- * It should be noted that this implementation is potentially intensive because it generically - * attempts to cover common use cases by performing diffs on the JSON representation of objects. - * If performance is a concern, it might be easier / simpler to provide a {@link Matcher} - * implementation optimized for your use case. - *

- */ - @Override - public Result match(R actualResource, P primary, Context

context) { - var desired = dependentResource.desired(primary, context); - return match(desired, actualResource, false, false, context); - } /** * Determines whether the specified actual resource matches the specified desired resource, @@ -84,7 +48,7 @@ public Result match(R actualResource, P primary, Context

context) { * @param resource * @return results of matching */ - public static Result match(R desired, + public static Matcher.Result match(R desired, R actualResource, boolean labelsAndAnnotationsEquality, boolean valuesEquality, Context

context) { @@ -92,6 +56,12 @@ public static Result match(R d labelsAndAnnotationsEquality, valuesEquality, context, EMPTY_ARRAY); } + public static Matcher.Result match(R desired, + R actualResource, Context

context) { + return match(desired, actualResource, + false, false, context, EMPTY_ARRAY); + } + /** * Determines whether the specified actual resource matches the specified desired resource, * possibly considering metadata and deeper equality checks. @@ -108,7 +78,7 @@ public static Result match(R d * @param resource * @return results of matching */ - public static Result match(R desired, + public static Matcher.Result match(R desired, R actualResource, boolean labelsAndAnnotationsEquality, Context

context, String... ignorePaths) { @@ -139,7 +109,7 @@ public static Result match(R d * match fails. * @return a {@link io.javaoperatorsdk.operator.processing.dependent.Matcher.Result} object */ - public static Result match( + public static Matcher.Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean labelsAndAnnotationsEquality, @@ -150,7 +120,7 @@ public static Result match( ignorePaths); } - public static Result match( + public static Matcher.Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean specEquality, @@ -161,7 +131,7 @@ public static Result match( labelsAndAnnotationsEquality, specEquality, context, ignorePaths); } - public static Result match(R desired, + public static Matcher.Result match(R desired, R actualResource, boolean labelsAndAnnotationsEquality, boolean valuesEquality, Context

context, String... ignoredPaths) { @@ -194,7 +164,7 @@ public static Result match(R d } } - return Result.computed(matched, desired); + return Matcher.Result.computed(matched, desired); } private static boolean match(boolean equality, JsonNode diff, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/GenericResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdater.java similarity index 62% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/GenericResourceUpdaterMatcher.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdater.java index 418a5dc964..4a8fef01e9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/GenericResourceUpdaterMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdater.java @@ -1,29 +1,17 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; +package io.javaoperatorsdk.operator.processing.dependent.kubernetes; import java.util.Map; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceUpdaterMatcher; -public class GenericResourceUpdaterMatcher implements - ResourceUpdaterMatcher { +public class GenericResourceUpdater { private static final String METADATA = "metadata"; - private static final ResourceUpdaterMatcher INSTANCE = new GenericResourceUpdaterMatcher<>(); - - protected GenericResourceUpdaterMatcher() {} - - @SuppressWarnings("unchecked") - public static ResourceUpdaterMatcher updaterMatcherFor() { - return (ResourceUpdaterMatcher) INSTANCE; - } @SuppressWarnings("unchecked") - @Override - public R updateResource(R actual, R desired, Context context) { + public static R updateResource(R actual, R desired, Context context) { KubernetesSerialization kubernetesSerialization = context.getClient().getKubernetesSerialization(); Map actualMap = kubernetesSerialization.convertValue(actual, Map.class); @@ -40,12 +28,6 @@ public R updateResource(R actual, R desired, Context context) { return clonedActual; } - @Override - public boolean matches(R actual, R desired, Context context) { - return GenericKubernetesResourceMatcher.match(desired, actual, - false, false, context).matched(); - } - public static void updateLabelsAndAnnotation(K actual, K desired) { actual.getMetadata().getLabels().putAll(desired.getMetadata().getLabels()); actual.getMetadata().getAnnotations().putAll(desired.getMetadata().getAnnotations()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 8663081460..ae4c2d1d6b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -21,7 +21,6 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ConfiguredDependentResource; import io.javaoperatorsdk.operator.processing.dependent.AbstractEventSourceHolderDependentResource; import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher.GenericResourceUpdaterMatcher; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; @@ -38,10 +37,7 @@ public abstract class KubernetesDependentResource updaterMatcher = usingCustomResourceUpdateMatcher - ? (ResourceUpdaterMatcher) this - : GenericResourceUpdaterMatcher.updaterMatcherFor(); + private KubernetesDependentResourceConfig kubernetesDependentResourceConfig; public KubernetesDependentResource(Class resourceType) { @@ -57,7 +53,6 @@ public void configureWith(KubernetesDependentResourceConfig config) { this.kubernetesDependentResourceConfig = config; } - @SuppressWarnings("unused") public R create(R desired, P primary, Context

context) { if (useSSA(context)) { @@ -91,7 +86,7 @@ public R update(R actual, R desired, P primary, Context

context) { .fieldManager(context.getControllerConfiguration().fieldManager()) .forceConflicts().serverSideApply(); } else { - var updatedActual = updaterMatcher.updateResource(actual, desired, context); + var updatedActual = GenericResourceUpdater.updateResource(actual, desired, context); updatedResource = prepare(context, updatedActual, primary, "Updating").update(); } log.debug("Resource version after update: {}", @@ -102,17 +97,10 @@ public R update(R actual, R desired, P primary, Context

context) { @Override public Result match(R actualResource, P primary, Context

context) { final var desired = desired(primary, context); - return match(actualResource, desired, primary, updaterMatcher, context); - } - - @SuppressWarnings({"unused"}) - public Result match(R actualResource, R desired, P primary, Context

context) { - return match(actualResource, desired, primary, - GenericResourceUpdaterMatcher.updaterMatcherFor(), - context); + return match(actualResource, desired, primary, context); } - public Result match(R actualResource, R desired, P primary, ResourceUpdaterMatcher matcher, + public Result match(R actualResource, R desired, P primary, Context

context) { final boolean matches; addMetadata(true, actualResource, desired, primary, context); @@ -120,7 +108,8 @@ public Result match(R actualResource, R desired, P primary, ResourceUpdaterMa matches = SSABasedGenericKubernetesResourceMatcher.getInstance() .matches(actualResource, desired, context); } else { - matches = matcher.matches(actualResource, desired, context); + matches = GenericKubernetesResourceMatcher.match(desired, actualResource, + false, false, context).matched(); } return Result.computed(matches, desired); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java index 997f7688e8..2663657157 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java @@ -13,15 +13,13 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.processing.dependent.Matcher; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher.GenericResourceUpdaterMatcher; import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher.match; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -@SuppressWarnings({"unchecked", "rawtypes"}) +@SuppressWarnings({"unchecked"}) class GenericKubernetesResourceMatcherTest { private static final Context context = mock(Context.class); @@ -29,7 +27,7 @@ class GenericKubernetesResourceMatcherTest { Deployment actual = createDeployment(); Deployment desired = createDeployment(); TestDependentResource dependentResource = new TestDependentResource(desired); - Matcher matcher = GenericKubernetesResourceMatcher.matcherFor(dependentResource); + @BeforeAll static void setUp() { @@ -39,15 +37,17 @@ static void setUp() { @Test void matchesTrivialCases() { - assertThat(matcher.match(actual, null, context).matched()).isTrue(); - assertThat(matcher.match(actual, null, context).computedDesired()).isPresent(); - assertThat(matcher.match(actual, null, context).computedDesired()).contains(desired); + assertThat(GenericKubernetesResourceMatcher.match(desired, actual, context).matched()).isTrue(); + assertThat(GenericKubernetesResourceMatcher.match(desired, actual, context).computedDesired()) + .isPresent(); + assertThat(GenericKubernetesResourceMatcher.match(desired, actual, context).computedDesired()) + .contains(desired); } @Test void matchesAdditiveOnlyChanges() { actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val"); - assertThat(matcher.match(actual, null, context).matched()) + assertThat(GenericKubernetesResourceMatcher.match(desired, actual, context).matched()) .withFailMessage("Additive changes should not cause a mismatch by default") .isTrue(); } @@ -64,7 +64,9 @@ void matchesWithStrongSpecEquality() { @Test void doesNotMatchRemovedValues() { actual = createDeployment(); - assertThat(matcher.match(actual, createPrimary("removed"), context).matched()) + assertThat(GenericKubernetesResourceMatcher + .match(dependentResource.desired(createPrimary("removed"), null), actual, context) + .matched()) .withFailMessage("Removing values in metadata should lead to a mismatch") .isFalse(); } @@ -73,7 +75,7 @@ void doesNotMatchRemovedValues() { void doesNotMatchChangedValues() { actual = createDeployment(); actual.getSpec().setReplicas(2); - assertThat(matcher.match(actual, null, context).matched()) + assertThat(GenericKubernetesResourceMatcher.match(desired, actual, context).matched()) .withFailMessage("Should not have matched because values have changed") .isFalse(); } @@ -82,7 +84,7 @@ void doesNotMatchChangedValues() { void ignoreStatus() { actual = createDeployment(); actual.setStatus(new DeploymentStatusBuilder().withReadyReplicas(1).build()); - assertThat(matcher.match(actual, null, context).matched()) + assertThat(GenericKubernetesResourceMatcher.match(desired, actual, context).matched()) .withFailMessage("Should ignore status in actual") .isTrue(); } @@ -146,8 +148,8 @@ void checkServiceAccount() { .addNewImagePullSecret("imagePullSecret3") .build(); - final var matcher = GenericResourceUpdaterMatcher.updaterMatcherFor(); - assertThat(matcher.matches(actual, desired, context)).isTrue(); + assertThat(GenericKubernetesResourceMatcher.match(desired, actual, false, false, context) + .matched()).isTrue(); } @Test diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterTest.java similarity index 81% rename from operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java rename to operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterTest.java index bbdb4811fe..313520b89e 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterTest.java @@ -14,14 +14,13 @@ import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher.GenericResourceUpdaterMatcher; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @SuppressWarnings("rawtypes") -class GenericResourceUpdaterMatcherTest { +class GenericResourceUpdaterTest { private static final Context context = mock(Context.class); @@ -40,7 +39,6 @@ static void setUp() { @Test void preservesValues() { - var processor = GenericResourceUpdaterMatcher.updaterMatcherFor(); var desired = createDeployment(); var actual = createDeployment(); actual.getMetadata().setLabels(new HashMap<>()); @@ -48,7 +46,7 @@ void preservesValues() { actual.getMetadata().setResourceVersion("1234"); actual.getSpec().setRevisionHistoryLimit(5); - var result = processor.updateResource(actual, desired, context); + var result = GenericResourceUpdater.updateResource(actual, desired, context); assertThat(result.getMetadata().getLabels().get("additionalActualKey")).isEqualTo("value"); assertThat(result.getMetadata().getResourceVersion()).isEqualTo("1234"); @@ -57,27 +55,26 @@ void preservesValues() { @Test void checkNamespaces() { - var processor = GenericResourceUpdaterMatcher.updaterMatcherFor(); var desired = new NamespaceBuilder().withNewMetadata().withName("foo").endMetadata().build(); var actual = new NamespaceBuilder().withNewMetadata().withName("foo").endMetadata().build(); actual.getMetadata().setLabels(new HashMap<>()); actual.getMetadata().getLabels().put("additionalActualKey", "value"); actual.getMetadata().setResourceVersion("1234"); - var result = processor.updateResource(actual, desired, context); + var result = GenericResourceUpdater.updateResource(actual, desired, context); assertThat(result.getMetadata().getLabels().get("additionalActualKey")).isEqualTo("value"); assertThat(result.getMetadata().getResourceVersion()).isEqualTo("1234"); desired.setSpec(new NamespaceSpec(List.of("halkyon.io/finalizer"))); - result = processor.updateResource(actual, desired, context); + result = GenericResourceUpdater.updateResource(actual, desired, context); assertThat(result.getMetadata().getLabels().get("additionalActualKey")).isEqualTo("value"); assertThat(result.getMetadata().getResourceVersion()).isEqualTo("1234"); assertThat(result.getSpec().getFinalizers()).containsExactly("halkyon.io/finalizer"); desired = new NamespaceBuilder().withNewMetadata().withName("foo").endMetadata().build(); - result = processor.updateResource(actual, desired, context); + result = GenericResourceUpdater.updateResource(actual, desired, context); assertThat(result.getMetadata().getLabels().get("additionalActualKey")).isEqualTo("value"); assertThat(result.getMetadata().getResourceVersion()).isEqualTo("1234"); assertThat(result.getSpec()).isNull(); @@ -85,7 +82,6 @@ void checkNamespaces() { @Test void checkSecret() { - var processor = GenericResourceUpdaterMatcher.updaterMatcherFor(); var desired = new SecretBuilder() .withMetadata(new ObjectMeta()) @@ -94,15 +90,14 @@ void checkSecret() { .withMetadata(new ObjectMeta()) .build(); - final var secret = processor.updateResource(actual, desired, context); + final var secret = GenericResourceUpdater.updateResource(actual, desired, context); assertThat(secret.getImmutable()).isTrue(); assertThat(secret.getType()).isEqualTo("Opaque"); assertThat(secret.getData()).containsOnlyKeys("foo"); } @Test - void checkSeviceAccount() { - var processor = GenericResourceUpdaterMatcher.updaterMatcherFor(); + void checkServiceAccount() { var desired = new ServiceAccountBuilder() .withMetadata(new ObjectMetaBuilder().addToLabels("new", "label").build()) .build(); @@ -111,7 +106,7 @@ void checkSeviceAccount() { .withImagePullSecrets(new LocalObjectReferenceBuilder().withName("secret").build()) .build(); - final var serviceAccount = processor.updateResource(actual, desired, context); + final var serviceAccount = GenericResourceUpdater.updateResource(actual, desired, context); assertThat(serviceAccount.getMetadata().getLabels()) .isEqualTo(Map.of("a", "label", "new", "label")); assertThat(serviceAccount.getImagePullSecrets()).isNullOrEmpty(); @@ -119,7 +114,7 @@ void checkSeviceAccount() { Deployment createDeployment() { return ReconcilerUtils.loadYaml( - Deployment.class, GenericResourceUpdaterMatcherTest.class, "nginx-deployment.yaml"); + Deployment.class, GenericResourceUpdaterTest.class, "nginx-deployment.yaml"); } }