Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: use processors for matching as well since usually both are related #1911

Merged
merged 3 commits into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,10 @@

import java.util.Objects;

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.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.dependent.Matcher;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors.GenericResourceUpdatePreProcessor;

public class GenericKubernetesResourceMatcher<R extends HasMetadata, P extends HasMetadata>
implements Matcher<R, P> {
Expand All @@ -22,7 +18,7 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource<R, P> depen

@SuppressWarnings({"unchecked", "rawtypes"})
static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
Class<R> resourceType, KubernetesDependentResource<R, P> dependentResource) {
KubernetesDependentResource<R, P> dependentResource) {
return new GenericKubernetesResourceMatcher(dependentResource);
}

Expand Down Expand Up @@ -61,6 +57,7 @@ public static <R extends HasMetadata> Result<R> match(R desired, R actualResourc
* @return results of matching
* @param <R> resource
*/
@SuppressWarnings("unchecked")
public static <R extends HasMetadata> Result<R> match(R desired, R actualResource,
boolean considerMetadata, boolean equality) {
if (considerMetadata) {
Expand All @@ -74,36 +71,10 @@ public static <R extends HasMetadata> Result<R> match(R desired, R actualResourc
}
}

if (desired instanceof ConfigMap) {
return Result.computed(
ResourceComparators.compareConfigMapData((ConfigMap) desired, (ConfigMap) actualResource),
desired);
} else if (desired instanceof Secret) {
return Result.computed(
ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource),
desired);
} else {
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(actualResource));
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.
if (equality && diffJsonPatch.size() > 0) {
return Result.computed(false, desired);
}
for (int i = 0; i < diffJsonPatch.size(); i++) {
String operation = diffJsonPatch.get(i).get("op").asText();
if (!operation.equals("add")) {
return Result.computed(false, desired);
}
}
return Result.computed(true, desired);
}
final ResourceUpdatePreProcessor<R> processor =
GenericResourceUpdatePreProcessor.processorFor((Class<R>) desired.getClass());
final var matched = processor.matches(actualResource, desired, equality);
return Result.computed(matched, desired);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
public KubernetesDependentResource(Class<R> resourceType) {
super(resourceType);
matcher = this instanceof Matcher ? (Matcher<R, P>) this
: GenericKubernetesResourceMatcher.matcherFor(resourceType, this);
: GenericKubernetesResourceMatcher.matcherFor(this);

processor = this instanceof ResourceUpdatePreProcessor
? (ResourceUpdatePreProcessor<R>) this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@
public interface ResourceUpdatePreProcessor<R extends HasMetadata> {

R replaceSpecOnActual(R actual, R desired, Context<?> context);

boolean matches(R actual, R desired, boolean equality);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors;

import java.util.Objects;

import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;

public class ClusterRoleBindingResourceUpdatePreProcessor
Expand All @@ -10,4 +12,10 @@ protected void updateClonedActual(ClusterRoleBinding actual, ClusterRoleBinding
actual.setRoleRef(desired.getRoleRef());
actual.setSubjects(desired.getSubjects());
}

@Override
public boolean matches(ClusterRoleBinding actual, ClusterRoleBinding desired, boolean equality) {
return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) &&
Objects.equals(actual.getSubjects(), desired.getSubjects());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors;

import java.util.Objects;

import io.fabric8.kubernetes.api.model.rbac.ClusterRole;

public class ClusterRoleResourceUpdatePreProcessor
Expand All @@ -10,4 +12,10 @@ protected void updateClonedActual(ClusterRole actual, ClusterRole desired) {
actual.setAggregationRule(desired.getAggregationRule());
actual.setRules(desired.getRules());
}

@Override
public boolean matches(ClusterRole actual, ClusterRole desired, boolean equality) {
return Objects.equals(actual.getRules(), desired.getRules()) &&
Objects.equals(actual.getAggregationRule(), desired.getAggregationRule());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors;

import java.util.Objects;

import io.fabric8.kubernetes.api.model.ConfigMap;

public class ConfigMapResourceUpdatePreProcessor
Expand All @@ -11,4 +13,11 @@ protected void updateClonedActual(ConfigMap actual, ConfigMap desired) {
actual.setBinaryData((desired.getBinaryData()));
actual.setImmutable(desired.getImmutable());
}

@Override
public boolean matches(ConfigMap actual, ConfigMap desired, boolean equality) {
return Objects.equals(actual.getImmutable(), desired.getImmutable()) &&
Objects.equals(actual.getData(), desired.getData()) &&
Objects.equals(actual.getBinaryData(), desired.getBinaryData());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import io.fabric8.kubernetes.api.model.rbac.ClusterRole;
import io.fabric8.kubernetes.api.model.rbac.ClusterRoleBinding;
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,4 +51,29 @@ 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;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors;

import java.util.Objects;

import io.fabric8.kubernetes.api.model.rbac.RoleBinding;

public class RoleBindingResourceUpdatePreProcessor
Expand All @@ -10,4 +12,10 @@ protected void updateClonedActual(RoleBinding actual, RoleBinding desired) {
actual.setRoleRef(desired.getRoleRef());
actual.setSubjects(desired.getSubjects());
}

@Override
public boolean matches(RoleBinding actual, RoleBinding desired, boolean equality) {
return Objects.equals(actual.getRoleRef(), desired.getRoleRef()) &&
Objects.equals(actual.getSubjects(), desired.getSubjects());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors;

import java.util.Objects;

import io.fabric8.kubernetes.api.model.rbac.Role;

public class RoleResourceUpdatePreProcessor extends GenericResourceUpdatePreProcessor<Role> {
Expand All @@ -8,4 +10,9 @@ public class RoleResourceUpdatePreProcessor extends GenericResourceUpdatePreProc
protected void updateClonedActual(Role actual, Role desired) {
actual.setRules(desired.getRules());
}

@Override
public boolean matches(Role actual, Role desired, boolean equality) {
return Objects.equals(actual.getRules(), desired.getRules());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors;

import java.util.Objects;

import io.fabric8.kubernetes.api.model.Secret;

public class SecretResourceUpdatePreProcessor extends GenericResourceUpdatePreProcessor<Secret> {
Expand All @@ -11,4 +13,12 @@ protected void updateClonedActual(Secret actual, Secret desired) {
actual.setImmutable(desired.getImmutable());
actual.setType(desired.getType());
}

@Override
public boolean matches(Secret actual, Secret desired, boolean equality) {
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());
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.processing.dependent.kubernetes.processors;

import java.util.Objects;

import io.fabric8.kubernetes.api.model.ServiceAccount;

public class ServiceAccountResourceUpdateProcessor
Expand All @@ -11,4 +13,12 @@ protected void updateClonedActual(ServiceAccount actual, ServiceAccount desired)
actual.setImagePullSecrets(desired.getImagePullSecrets());
actual.setSecrets(desired.getSecrets());
}

@Override
public boolean matches(ServiceAccount actual, ServiceAccount desired, boolean equality) {
return Objects.equals(actual.getAutomountServiceAccountToken(),
desired.getAutomountServiceAccountToken()) &&
Objects.equals(actual.getImagePullSecrets(), desired.getImagePullSecrets()) &&
Objects.equals(actual.getSecrets(), desired.getSecrets());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import org.junit.jupiter.api.Test;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.ServiceAccount;
import io.fabric8.kubernetes.api.model.ServiceAccountBuilder;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
import io.javaoperatorsdk.operator.ReconcilerUtils;
Expand All @@ -32,8 +34,7 @@ void checksIfDesiredValuesAreTheSame() {
var actual = createDeployment();
final var desired = createDeployment();
final var dependentResource = new TestDependentResource(desired);
final var matcher =
GenericKubernetesResourceMatcher.matcherFor(Deployment.class, dependentResource);
final var matcher = GenericKubernetesResourceMatcher.matcherFor(dependentResource);
assertThat(matcher.match(actual, null, context).matched()).isTrue();
assertThat(matcher.match(actual, null, context).computedDesired().isPresent()).isTrue();
assertThat(matcher.match(actual, null, context).computedDesired().get()).isEqualTo(desired);
Expand Down Expand Up @@ -75,6 +76,19 @@ void checksIfDesiredValuesAreTheSame() {
.isFalse();
}

@Test
void checkServiceAccount() {
final var serviceAccountDR = new ServiceAccountDR();

final var desired = serviceAccountDR.desired(null, context);
var actual = new ServiceAccountBuilder(desired)
.addNewImagePullSecret("imagePullSecret3")
.build();

final var matcher = GenericKubernetesResourceMatcher.matcherFor(serviceAccountDR);
assertThat(matcher.match(actual, null, context).matched()).isFalse();
}

Deployment createDeployment() {
return ReconcilerUtils.loadYaml(
Deployment.class, GenericKubernetesResourceMatcherTest.class, "nginx-deployment.yaml");
Expand All @@ -88,6 +102,24 @@ HasMetadata createPrimary(String caseName) {
.build();
}

private static class ServiceAccountDR
extends KubernetesDependentResource<ServiceAccount, HasMetadata> {

public ServiceAccountDR() {
super(ServiceAccount.class);
}

@Override
protected ServiceAccount desired(HasMetadata primary, Context<HasMetadata> context) {
return new ServiceAccountBuilder()
.withNewMetadata().withName("foo").endMetadata()
.withAutomountServiceAccountToken()
.addNewImagePullSecret("imagePullSecret1")
.addNewImagePullSecret("imagePullSecret2")
.build();
}
}

private class TestDependentResource extends KubernetesDependentResource<Deployment, HasMetadata> {

private final Deployment desired;
Expand Down