Skip to content

Commit

Permalink
generalization of the update matcher
Browse files Browse the repository at this point in the history
  • Loading branch information
shawkins committed Aug 11, 2023
1 parent 4110c8b commit 243b320
Show file tree
Hide file tree
Showing 12 changed files with 49 additions and 257 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -24,6 +25,9 @@ public class GenericKubernetesResourceMatcher<R extends HasMetadata, P extends H
private static final String OP = "op";
public static final String METADATA_LABELS = "/metadata/labels";
public static final String METADATA_ANNOTATIONS = "/metadata/annotations";
// without knowing the CRD we cannot ignore status as it may not be a subresource, so if it's
// included we expect it to match
private static Set<String> NOT_OTHER_FIELDS = Set.of(SPEC, "/metadata", "/apiVersion", "/kind");

private static final String PATH = "path";
private static final String[] EMPTY_ARRAY = {};
Expand Down Expand Up @@ -192,11 +196,11 @@ public static <R extends HasMetadata, P extends HasMetadata> Result<R> 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 <R extends HasMetadata> boolean matchSpec(R actual, R desired, boolean equality,
private static <R extends HasMetadata> boolean match(R actual, R desired, boolean equality,
Context<?> context,
String[] ignoredPaths) {

Expand All @@ -208,25 +212,38 @@ private static <R extends HasMetadata> boolean matchSpec(R actual, R desired, bo
final List<String> 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<String> 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<String> 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<JsonNode> metadataJSonDiffs,
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,49 +1,39 @@
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 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;

import java.util.Map;

public class GenericResourceUpdaterMatcher<R extends HasMetadata> implements
ResourceUpdaterMatcher<R> {

private static final String METADATA = "metadata";
private static final ResourceUpdaterMatcher<?> INSTANCE = new GenericResourceUpdaterMatcher<>();

@SuppressWarnings("rawtypes")
private static final Map<Class, ResourceUpdaterMatcher> 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());

protected GenericResourceUpdaterMatcher() {}

@SuppressWarnings("unchecked")
public static <R extends HasMetadata> ResourceUpdaterMatcher<R> updaterMatcherFor(
Class<R> resourceType) {
final var processor = processors.get(resourceType);
return processor != null ? processor : (ResourceUpdaterMatcher<R>) INSTANCE;
return (ResourceUpdaterMatcher<R>) INSTANCE;
}

@SuppressWarnings("unchecked")
@Override
public R updateResource(R actual, R desired, Context<?> context) {
var clonedActual = context.getControllerConfiguration().getConfigurationService()
.getResourceCloner().clone(actual);
KubernetesSerialization kubernetesSerialization = context.getClient().getKubernetesSerialization();
Map<String, Object> actualMap = kubernetesSerialization.convertValue(actual, Map.class);
Map<String, Object> desiredMap = kubernetesSerialization.convertValue(desired, Map.class);
// replace all top level fields from actual with desired, but merge metadata separately
actualMap.replaceAll((k, v) -> METADATA.equals(k)?v:desiredMap.get(k));
desiredMap.remove(METADATA);
actualMap.putAll(desiredMap);
var clonedActual = (R) kubernetesSerialization.convertValue(actualMap, desired.getClass());
updateLabelsAndAnnotation(clonedActual, desired);
updateClonedActual(clonedActual, desired);
return clonedActual;
}

Expand All @@ -53,15 +43,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 <K extends HasMetadata> void updateSpec(K actual, K desired) {
var desiredSpec = ReconcilerUtils.getSpec(desired);
ReconcilerUtils.setSpec(actual, desiredSpec);
}

public static <K extends HasMetadata> void updateLabelsAndAnnotation(K actual, K desired) {
actual.getMetadata().getLabels().putAll(desired.getMetadata().getLabels());
actual.getMetadata().getAnnotations().putAll(desired.getMetadata().getAnnotations());
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ static void setUp() {
final var client = MockKubernetesClient.client(HasMetadata.class);
when(configService.getKubernetesClient()).thenReturn(client);
when(configService.getResourceCloner()).thenCallRealMethod();

when(context.getClient()).thenReturn(client);
when(context.getControllerConfiguration()).thenReturn(controllerConfiguration);
}

Expand Down

0 comments on commit 243b320

Please sign in to comment.