From 58b9c04a97ea5e6ed6a64e028b605475f59cfb26 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 24 May 2023 23:28:53 +0200 Subject: [PATCH] fix: restore backwards compatibility (#1915) * fix: clean-up unneeded matcher field * fix: restore backwards compatibility Note that the current implementation is flawed because custom ResourceUpdatePreProcessor should be registered to be found when attempting to use custom matching logic. See also #1914 for more details. --- .../KubernetesDependentResource.java | 6 +---- .../ResourceUpdatePreProcessor.java | 27 ++++++++++++++++++- .../GenericResourceUpdatePreProcessor.java | 26 ------------------ 3 files changed, 27 insertions(+), 32 deletions(-) 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 e1b67cf216..1b64d6b4ec 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 @@ -22,7 +22,6 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; import io.javaoperatorsdk.operator.processing.dependent.AbstractEventSourceHolderDependentResource; -import io.javaoperatorsdk.operator.processing.dependent.Matcher; import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors.GenericResourceUpdatePreProcessor; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -41,7 +40,6 @@ public abstract class KubernetesDependentResource matcher; private final ResourceUpdatePreProcessor processor; private final boolean garbageCollected = this instanceof GarbageCollected; private KubernetesDependentResourceConfig kubernetesDependentResourceConfig; @@ -49,8 +47,6 @@ public abstract class KubernetesDependentResource resourceType) { super(resourceType); - matcher = this instanceof Matcher ? (Matcher) this - : GenericKubernetesResourceMatcher.matcherFor(this); processor = this instanceof ResourceUpdatePreProcessor ? (ResourceUpdatePreProcessor) this @@ -141,7 +137,7 @@ public R update(R actual, R target, P primary, Context

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

context) { - return matcher.match(actualResource, primary, context); + return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context, false); } @SuppressWarnings("unused") diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceUpdatePreProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceUpdatePreProcessor.java index 54b16cfe0d..b3d5172bbc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceUpdatePreProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/ResourceUpdatePreProcessor.java @@ -1,11 +1,36 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.zjsonpatch.JsonDiff; +import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; public interface ResourceUpdatePreProcessor { R replaceSpecOnActual(R actual, R desired, Context context); - boolean matches(R actual, R desired, boolean equality); + default boolean matches(R actual, R desired, boolean equality) { + final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper(); + + // reflection will be replaced by this: + // https://github.com/fabric8io/kubernetes-client/issues/3816 + var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired)); + var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actual)); + var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode); + // In case of equality is set to true, no diffs are allowed, so we return early if diffs exist + // On contrary (if equality is false), "add" is allowed for cases when for some + // resources Kubernetes fills-in values into spec. + final var diffNumber = diffJsonPatch.size(); + if (equality && diffNumber > 0) { + return false; + } + for (int i = 0; i < diffNumber; i++) { + String operation = diffJsonPatch.get(i).get("op").asText(); + if (!operation.equals("add")) { + return false; + } + } + return true; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/processors/GenericResourceUpdatePreProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/processors/GenericResourceUpdatePreProcessor.java index dd835e7a94..d094f8573f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/processors/GenericResourceUpdatePreProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/processors/GenericResourceUpdatePreProcessor.java @@ -10,7 +10,6 @@ import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; import io.fabric8.kubernetes.api.model.rbac.Role; import io.fabric8.kubernetes.api.model.rbac.RoleBinding; -import io.fabric8.zjsonpatch.JsonDiff; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -50,29 +49,4 @@ protected void updateClonedActual(R actual, R desired) { var desiredSpec = ReconcilerUtils.getSpec(desired); ReconcilerUtils.setSpec(actual, desiredSpec); } - - @Override - public boolean matches(R actual, R desired, boolean equality) { - final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper(); - - // reflection will be replaced by this: - // https://github.com/fabric8io/kubernetes-client/issues/3816 - var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired)); - var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actual)); - var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode); - // In case of equality is set to true, no diffs are allowed, so we return early if diffs exist - // On contrary (if equality is false), "add" is allowed for cases when for some - // resources Kubernetes fills-in values into spec. - final var diffNumber = diffJsonPatch.size(); - if (equality && diffNumber > 0) { - return false; - } - for (int i = 0; i < diffNumber; i++) { - String operation = diffJsonPatch.get(i).get("op").asText(); - if (!operation.equals("add")) { - return false; - } - } - return true; - } }