From 69d004853e7a990be3e5cfd0ad6a27ba02885a48 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Fri, 11 Aug 2023 07:00:17 -0400 Subject: [PATCH] draft of a generalization of the update matcher --- .../GenericKubernetesResourceMatcher.java | 45 ++++++++---- ...sterRoleBindingResourceUpdaterMatcher.java | 23 ------ .../ClusterRoleResourceUpdaterMatcher.java | 22 ------ .../ConfigMapResourceUpdaterMatcher.java | 24 ------- .../EndpointSliceResourceUpdateMatcher.java | 25 ------- .../EndpointsResourceUpdaterMatcher.java | 20 ------ .../GenericResourceUpdaterMatcher.java | 71 ++++++++++--------- .../RoleBindingResourceUpdaterMatcher.java | 23 ------ .../RoleResourceUpdaterMatcher.java | 19 ----- .../SecretResourceUpdaterMatcher.java | 25 ------- .../ServiceAccountResourceUpdaterMatcher.java | 25 ------- 11 files changed, 67 insertions(+), 255 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleBindingResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java 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 36935e08a0..651d5d243d 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 @@ -6,6 +6,7 @@ import java.util.List; import java.util.Objects; import java.util.Optional; +import java.util.Set; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -24,6 +25,8 @@ public class GenericKubernetesResourceMatcher NOT_OTHER_FIELDS = Set.of(SPEC, "/metadata", "/apiVersion", "/kind"); private static final String PATH = "path"; private static final String[] EMPTY_ARRAY = {}; @@ -192,11 +195,11 @@ public static Result match(R d } } - final var matched = matchSpec(actualResource, desired, specEquality, context, ignoredPaths); + final var matched = match(actualResource, desired, specEquality, context, ignoredPaths); return Result.computed(matched, desired); } - private static boolean matchSpec(R actual, R desired, boolean equality, + private static boolean match(R actual, R desired, boolean equality, Context context, String[] ignoredPaths) { @@ -208,25 +211,37 @@ private static boolean matchSpec(R actual, R desired, bo final List ignoreList = ignoredPaths != null && ignoredPaths.length > 0 ? Arrays.asList(ignoredPaths) : Collections.emptyList(); - // reflection will be replaced by this: - // https://github.com/fabric8io/kubernetes-client/issues/3816 - var specDiffJsonPatch = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, SPEC); + boolean specMatch = match(equality, wholeDiffJsonPatch, ignoreList, SPEC); + if (!specMatch) { + return false; + } + // expect everything else to be equal + var names = desiredNode.fieldNames(); + List prefixes = new ArrayList<>(); + while (names.hasNext()) { + String prefix = "/" + names.next(); + if (!NOT_OTHER_FIELDS.contains(prefix)) { + prefixes.add(prefix); + } + } + return match(true, wholeDiffJsonPatch, ignoreList, prefixes.toArray(String[]::new)); + } + + private static boolean match(boolean equality, JsonNode wholeDiffJsonPatch, final List ignoreList, String... prefixes) { + var diffJsonPatch = getDiffsImpactingPathsWithPrefixes(wholeDiffJsonPatch, prefixes); // 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. - if (equality && !specDiffJsonPatch.isEmpty()) { + if (diffJsonPatch.isEmpty()) { + return true; + } + if (equality) { return false; } - if (!equality && !ignoreList.isEmpty()) { - if (!allDiffsOnIgnoreList(specDiffJsonPatch, ignoreList)) { - return false; - } - } else { - if (!allDiffsAreAddOps(specDiffJsonPatch)) { - return false; - } + if (!ignoreList.isEmpty() && allDiffsOnIgnoreList(diffJsonPatch, ignoreList)) { + return true; } - return true; + return allDiffsAreAddOps(diffJsonPatch); } private static boolean allDiffsOnIgnoreList(List metadataJSonDiffs, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java deleted file mode 100644 index f6f6d1ef54..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleBindingResourceUpdaterMatcher.java +++ /dev/null @@ -1,23 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ClusterRoleBindingResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ClusterRoleBinding actual, ClusterRoleBinding desired) { - actual.setRoleRef(desired.getRoleRef()); - actual.setSubjects(desired.getSubjects()); - } - - @Override - public boolean matches(ClusterRoleBinding actual, ClusterRoleBinding desired, - Context context) { - return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) && - Objects.equals(actual.getSubjects(), desired.getSubjects()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java deleted file mode 100644 index da7997c040..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ClusterRoleResourceUpdaterMatcher.java +++ /dev/null @@ -1,22 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.ClusterRole; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ClusterRoleResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ClusterRole actual, ClusterRole desired) { - actual.setAggregationRule(desired.getAggregationRule()); - actual.setRules(desired.getRules()); - } - - @Override - public boolean matches(ClusterRole actual, ClusterRole desired, Context context) { - return Objects.equals(actual.getRules(), desired.getRules()) && - Objects.equals(actual.getAggregationRule(), desired.getAggregationRule()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java deleted file mode 100644 index 7f89d45ff5..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ConfigMapResourceUpdaterMatcher.java +++ /dev/null @@ -1,24 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.ConfigMap; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ConfigMapResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ConfigMap actual, ConfigMap desired) { - actual.setData(desired.getData()); - actual.setBinaryData((desired.getBinaryData())); - actual.setImmutable(desired.getImmutable()); - } - - @Override - public boolean matches(ConfigMap actual, ConfigMap desired, Context context) { - return Objects.equals(actual.getImmutable(), desired.getImmutable()) && - Objects.equals(actual.getData(), desired.getData()) && - Objects.equals(actual.getBinaryData(), desired.getBinaryData()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java deleted file mode 100644 index dc83c86dbf..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointSliceResourceUpdateMatcher.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.discovery.v1.EndpointSlice; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class EndpointSliceResourceUpdateMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(EndpointSlice actual, EndpointSlice desired) { - actual.setEndpoints(desired.getEndpoints()); - actual.setAddressType(desired.getAddressType()); - actual.setPorts(desired.getPorts()); - } - - @Override - public boolean matches(EndpointSlice actual, EndpointSlice desired, Context context) { - return Objects.equals(actual.getEndpoints(), desired.getEndpoints()) && - Objects.equals(actual.getAddressType(), desired.getAddressType()) && - Objects.equals(actual.getPorts(), desired.getPorts()); - } - -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java deleted file mode 100644 index 3dcfabde28..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/EndpointsResourceUpdaterMatcher.java +++ /dev/null @@ -1,20 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.Endpoints; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class EndpointsResourceUpdaterMatcher extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(Endpoints actual, Endpoints desired) { - actual.setSubsets(desired.getSubsets()); - } - - @Override - public boolean matches(Endpoints actual, Endpoints desired, Context context) { - return Objects.equals(actual.getSubsets(), desired.getSubsets()); - } - -} 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/updatermatcher/GenericResourceUpdaterMatcher.java index cd9fbe2b16..743d9fde0f 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/updatermatcher/GenericResourceUpdaterMatcher.java @@ -1,49 +1,61 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; -import java.util.Map; - -import io.fabric8.kubernetes.api.model.*; -import io.fabric8.kubernetes.api.model.discovery.v1.EndpointSlice; -import io.fabric8.kubernetes.api.model.rbac.ClusterRole; -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.javaoperatorsdk.operator.ReconcilerUtils; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.introspect.ClassIntrospector.MixInResolver; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.KubernetesResource; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceUpdaterMatcher; +import java.util.Map; + public class GenericResourceUpdaterMatcher implements ResourceUpdaterMatcher { private static final ResourceUpdaterMatcher INSTANCE = new GenericResourceUpdaterMatcher<>(); - @SuppressWarnings("rawtypes") - private static final Map processors = Map.of( - Secret.class, new SecretResourceUpdaterMatcher(), - ConfigMap.class, new ConfigMapResourceUpdaterMatcher(), - ServiceAccount.class, new ServiceAccountResourceUpdaterMatcher(), - Role.class, new RoleResourceUpdaterMatcher(), - ClusterRole.class, new ClusterRoleResourceUpdaterMatcher(), - RoleBinding.class, new RoleBindingResourceUpdaterMatcher(), - ClusterRoleBinding.class, new ClusterRoleBindingResourceUpdaterMatcher(), - Endpoints.class, new EndpointsResourceUpdaterMatcher(), - EndpointSlice.class, new EndpointSliceResourceUpdateMatcher()); + @JsonInclude(JsonInclude.Include.ALWAYS) + private static class NullIncludingMixIn { + } + + // we need a specialized mapper so that all fields, including the null ones, can be gleaning from the desired state + private static ObjectMapper MAPPER = new ObjectMapper(); + static { + MAPPER.setMixInResolver(new MixInResolver() { + + @Override + public Class findMixInClassFor(Class cls) { + if (KubernetesResource.class.isAssignableFrom(cls)) { + return NullIncludingMixIn.class; + } + return null; + } + + @Override + public MixInResolver copy() { + return this; + } + }); + } protected GenericResourceUpdaterMatcher() {} @SuppressWarnings("unchecked") public static ResourceUpdaterMatcher updaterMatcherFor( Class resourceType) { - final var processor = processors.get(resourceType); - return processor != null ? processor : (ResourceUpdaterMatcher) INSTANCE; + return (ResourceUpdaterMatcher) INSTANCE; } + @Override public R updateResource(R actual, R desired, Context context) { - var clonedActual = context.getControllerConfiguration().getConfigurationService() - .getResourceCloner().clone(actual); + Map actualMap = MAPPER.convertValue(actual, Map.class); + Map desiredMap = MAPPER.convertValue(desired, Map.class); + desiredMap.remove("metadata"); // handled separately below + actualMap.putAll(desiredMap); // will also preserve additionalProperties + var clonedActual = (R) MAPPER.convertValue(actualMap, desired.getClass()); updateLabelsAndAnnotation(clonedActual, desired); - updateClonedActual(clonedActual, desired); return clonedActual; } @@ -53,15 +65,6 @@ public boolean matches(R actual, R desired, Context context) { false, false, context).matched(); } - protected void updateClonedActual(R actual, R desired) { - updateSpec(actual, desired); - } - - public static void updateSpec(K actual, K desired) { - var desiredSpec = ReconcilerUtils.getSpec(desired); - ReconcilerUtils.setSpec(actual, desiredSpec); - } - 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/updatermatcher/RoleBindingResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleBindingResourceUpdaterMatcher.java deleted file mode 100644 index c6a87cdae3..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleBindingResourceUpdaterMatcher.java +++ /dev/null @@ -1,23 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.RoleBinding; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class RoleBindingResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(RoleBinding actual, RoleBinding desired) { - actual.setRoleRef(desired.getRoleRef()); - actual.setSubjects(desired.getSubjects()); - } - - @Override - public boolean matches(RoleBinding actual, RoleBinding desired, - Context context) { - return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) && - Objects.equals(actual.getSubjects(), desired.getSubjects()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java deleted file mode 100644 index f02d946db8..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/RoleResourceUpdaterMatcher.java +++ /dev/null @@ -1,19 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.rbac.Role; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class RoleResourceUpdaterMatcher extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(Role actual, Role desired) { - actual.setRules(desired.getRules()); - } - - @Override - public boolean matches(Role actual, Role desired, Context context) { - return Objects.equals(actual.getRules(), desired.getRules()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java deleted file mode 100644 index 14e8696704..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/SecretResourceUpdaterMatcher.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.Secret; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class SecretResourceUpdaterMatcher extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(Secret actual, Secret desired) { - actual.setData(desired.getData()); - actual.setStringData(desired.getStringData()); - actual.setImmutable(desired.getImmutable()); - actual.setType(desired.getType()); - } - - @Override - public boolean matches(Secret actual, Secret desired, Context context) { - return Objects.equals(actual.getImmutable(), desired.getImmutable()) && - Objects.equals(actual.getType(), desired.getType()) && - Objects.equals(actual.getData(), desired.getData()) && - Objects.equals(actual.getStringData(), desired.getStringData()); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java deleted file mode 100644 index f3d625c778..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/updatermatcher/ServiceAccountResourceUpdaterMatcher.java +++ /dev/null @@ -1,25 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent.kubernetes.updatermatcher; - -import java.util.Objects; - -import io.fabric8.kubernetes.api.model.ServiceAccount; -import io.javaoperatorsdk.operator.api.reconciler.Context; - -public class ServiceAccountResourceUpdaterMatcher - extends GenericResourceUpdaterMatcher { - - @Override - protected void updateClonedActual(ServiceAccount actual, ServiceAccount desired) { - actual.setAutomountServiceAccountToken(desired.getAutomountServiceAccountToken()); - actual.setImagePullSecrets(desired.getImagePullSecrets()); - actual.setSecrets(desired.getSecrets()); - } - - @Override - public boolean matches(ServiceAccount actual, ServiceAccount desired, Context context) { - return Objects.equals(actual.getAutomountServiceAccountToken(), - desired.getAutomountServiceAccountToken()) && - Objects.equals(actual.getImagePullSecrets(), desired.getImagePullSecrets()) && - Objects.equals(actual.getSecrets(), desired.getSecrets()); - } -}