Skip to content

Commit

Permalink
fix: restore backwards compatibility (#1915)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
metacosm authored May 24, 2023
1 parent 1c1783e commit 58b9c04
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,16 +40,13 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
private static final Logger log = LoggerFactory.getLogger(KubernetesDependentResource.class);

protected KubernetesClient client;
private final Matcher<R, P> matcher;
private final ResourceUpdatePreProcessor<R> processor;
private final boolean garbageCollected = this instanceof GarbageCollected;
private KubernetesDependentResourceConfig<R> kubernetesDependentResourceConfig;

@SuppressWarnings("unchecked")
public KubernetesDependentResource(Class<R> resourceType) {
super(resourceType);
matcher = this instanceof Matcher ? (Matcher<R, P>) this
: GenericKubernetesResourceMatcher.matcherFor(this);

processor = this instanceof ResourceUpdatePreProcessor
? (ResourceUpdatePreProcessor<R>) this
Expand Down Expand Up @@ -141,7 +137,7 @@ public R update(R actual, R target, P primary, Context<P> context) {
}

public Result<R> match(R actualResource, P primary, Context<P> context) {
return matcher.match(actualResource, primary, context);
return GenericKubernetesResourceMatcher.match(this, actualResource, primary, context, false);
}

@SuppressWarnings("unused")
Expand Down
Original file line number Diff line number Diff line change
@@ -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 extends HasMetadata> {

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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}

0 comments on commit 58b9c04

Please sign in to comment.