feat: sanitization of resources for matching

Signed-off-by: Attila Mészáros <[email protected]>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/DesiredResourceSanitizer.java

@@ -0,0 +1,50 @@
+package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.PersistentVolumeClaimStatus;
+import io.fabric8.kubernetes.api.model.Secret;
+import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+
+import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource.useSSA;
+
+public class DesiredResourceSanitizer {
+
+  private DesiredResourceSanitizer() {}
+
+  public static <R, P extends HasMetadata> void sanitizeDesired(R desired, R actual, P primary,
+      Context<P> context) {
+    if (useSSA(context)) {
+      if (desired instanceof StatefulSet) {
+        fillDefaultsOnVolumeClaimTemplate((StatefulSet) desired);
+      }
+      if (desired instanceof Secret) {
+        checkIfStringDataUsed((Secret) desired);
+      }
+    }
+  }
+
+  private static void checkIfStringDataUsed(Secret secret) {
+    if (secret.getStringData() != null && !secret.getStringData().isEmpty()) {
+      throw new IllegalStateException(
+          "There is a known issue using StringData with SSA. Use data instead.");
+    }
+  }
+
+  private static void fillDefaultsOnVolumeClaimTemplate(StatefulSet statefulSet) {
+    if (!statefulSet.getSpec().getVolumeClaimTemplates().isEmpty()) {
+      statefulSet.getSpec().getVolumeClaimTemplates().forEach(t -> {
+        if (t.getSpec().getVolumeMode() == null) {
+          t.getSpec().setVolumeMode("Filesystem");
+        }
+        if (t.getStatus() == null) {
+          t.setStatus(new PersistentVolumeClaimStatus());
+        }
+        if (t.getStatus().getPhase() == null) {
+          t.setStatus(new PersistentVolumeClaimStatus());
+          t.getStatus().setPhase("pending");
+        }
+      });
+    }
+  }
+}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

@@ -103,18 +103,19 @@ public void configureWith(InformerEventSource<R, P> informerEventSource) {
   }
 
   @SuppressWarnings("unused")
-  public R create(R target, P primary, Context<P> context) {
+  public R create(R desired, P primary, Context<P> context) {
     if (useSSA(context)) {
       // setting resource version for SSA so only created if it doesn't exist already
       var createIfNotExisting = kubernetesDependentResourceConfig == null
           ? KubernetesDependentResourceConfig.DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA
           : kubernetesDependentResourceConfig.createResourceOnlyIfNotExistingWithSSA();
       if (createIfNotExisting) {
-        target.getMetadata().setResourceVersion("1");
+        desired.getMetadata().setResourceVersion("1");
       }
     }
-    addMetadata(false, null, target, primary);
-    final var resource = prepare(target, primary, "Creating");
+    addMetadata(false, null, desired, primary);
+    sanitizeDesired(desired, null, primary, context);
+    final var resource = prepare(desired, primary, "Creating");
     return useSSA(context)
         ? resource
             .fieldManager(context.getControllerConfiguration().fieldManager())
@@ -123,19 +124,20 @@ public R create(R target, P primary, Context<P> context) {
         : resource.create();
   }
 
-  public R update(R actual, R target, P primary, Context<P> context) {
+  public R update(R actual, R desired, P primary, Context<P> context) {
     if (log.isDebugEnabled()) {
       log.debug("Updating actual resource: {} version: {}", ResourceID.fromResource(actual),
           actual.getMetadata().getResourceVersion());
     }
     R updatedResource;
-    addMetadata(false, actual, target, primary);
+    addMetadata(false, actual, desired, primary);
+    sanitizeDesired(desired, actual, primary, context);
     if (useSSA(context)) {
-      updatedResource = prepare(target, primary, "Updating")
+      updatedResource = prepare(desired, primary, "Updating")
           .fieldManager(context.getControllerConfiguration().fieldManager())
           .forceConflicts().serverSideApply();
     } else {
-      var updatedActual = updaterMatcher.updateResource(actual, target, context);
+      var updatedActual = updaterMatcher.updateResource(actual, desired, context);
       updatedResource = prepare(updatedActual, primary, "Updating").update();
     }
     log.debug("Resource version after update: {}",
@@ -146,6 +148,7 @@ public R update(R actual, R target, P primary, Context<P> context) {
   @Override
   public Result<R> match(R actualResource, P primary, Context<P> context) {
     final var desired = desired(primary, context);
+    sanitizeDesired(desired, actualResource, primary, context);
     return match(actualResource, desired, primary, updaterMatcher, context);
   }
 
@@ -189,7 +192,11 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P
     addReferenceHandlingMetadata(target, primary);
   }
 
-  private boolean useSSA(Context<P> context) {
+  protected void sanitizeDesired(R desired, R actual, P primary, Context<P> context) {
+    DesiredResourceSanitizer.sanitizeDesired(desired, actual, primary, context);
+  }
+
+  public static <P extends HasMetadata> boolean useSSA(Context<P> context) {
     return context.getControllerConfiguration().getConfigurationService()
         .ssaBasedCreateUpdateMatchForDependentResources();
   }

operator-framework/src/test/java/io/javaoperatorsdk/operator/SSAStatefulSetMatcherIssueIT.java

@@ -0,0 +1,57 @@
+package io.javaoperatorsdk.operator;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerCustomResource;
+import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerDependentResource;
+import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerReconciler;
+import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerSpec;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class StatefulSetDesiredSanitizerIT {
+
+  public static final String TEST_1 = "test1";
+
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(new StatefulSetDesiredSanitizerReconciler())
+          .build();
+
+  @Test
+  void testSSAMatcher() {
+    var resource = extension.create(testResource());
+
+    await().pollDelay(Duration.ofMillis(200)).untilAsserted(() -> {
+      var statefulSet = extension.get(StatefulSet.class, TEST_1);
+      assertThat(statefulSet).isNotNull();
+    });
+    // make sure reconciliation happens at least once more
+    resource.getSpec().setValue("changed value");
+    extension.replace(resource);
+
+    await().untilAsserted(
+        () -> assertThat(StatefulSetDesiredSanitizerDependentResource.nonMatchedAtLeastOnce)
+            .isFalse());
+  }
+
+  StatefulSetDesiredSanitizerCustomResource testResource() {
+    var res = new StatefulSetDesiredSanitizerCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(TEST_1)
+        .build());
+    res.setSpec(new StatefulSetDesiredSanitizerSpec());
+    res.getSpec().setValue("initial value");
+
+    return res;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/StatefulSetDesiredSanitizerIT.java

@@ -33,10 +33,10 @@ protected ConfigMap desired(DependnetSSACustomResource primary,
   }
 
   @Override
-  public ConfigMap update(ConfigMap actual, ConfigMap target,
+  public ConfigMap update(ConfigMap actual, ConfigMap desired,
       DependnetSSACustomResource primary,
       Context<DependnetSSACustomResource> context) {
     NUMBER_OF_UPDATES.incrementAndGet();
-    return super.update(actual, target, primary, context);
+    return super.update(actual, desired, primary, context);
   }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java

@@ -53,10 +53,10 @@ public Matcher.Result<Service> match(Service actualResource,
   }
 
   @Override
-  public Service update(Service actual, Service target,
+  public Service update(Service actual, Service desired,
       ServiceStrictMatcherTestCustomResource primary,
       Context<ServiceStrictMatcherTestCustomResource> context) {
     updated.addAndGet(1);
-    return super.update(actual, target, primary, context);
+    return super.update(actual, desired, primary, context);
   }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/servicestrictmatcher/ServiceDependentResource.java

@@ -45,17 +45,17 @@ public Result<Service> match(Service actualResource, SSALegacyMatcherCustomResou
 
   // override just to check the exec count
   @Override
-  public Service update(Service actual, Service target, SSALegacyMatcherCustomResource primary,
+  public Service update(Service actual, Service desired, SSALegacyMatcherCustomResource primary,
       Context<SSALegacyMatcherCustomResource> context) {
     createUpdateCount.addAndGet(1);
-    return super.update(actual, target, primary, context);
+    return super.update(actual, desired, primary, context);
   }
 
   // override just to check the exec count
   @Override
-  public Service create(Service target, SSALegacyMatcherCustomResource primary,
+  public Service create(Service desired, SSALegacyMatcherCustomResource primary,
       Context<SSALegacyMatcherCustomResource> context) {
     createUpdateCount.addAndGet(1);
-    return super.create(target, primary, context);
+    return super.create(desired, primary, context);
   }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ssalegacymatcher/ServiceDependentResource.java

@@ -1,4 +1,4 @@
-package io.javaoperatorsdk.operator.sample.ssastatefulsetmatcherissue;
+package io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer;
 
 import io.fabric8.kubernetes.api.model.Namespaced;
 import io.fabric8.kubernetes.client.CustomResource;
@@ -7,8 +7,8 @@
 
 @Group("sample.javaoperatorsdk")
 @Version("v1")
-public class SSAStatefulSetMatcherIssueCustomResource
-    extends CustomResource<Void, Void>
+public class StatefulSetDesiredSanitizerCustomResource
+    extends CustomResource<StatefulSetDesiredSanitizerSpec, Void>
     implements Namespaced {
 
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ssastatefulsetmatcherissue/SSAStatefulSetMatcherIssueReconciler.java

@@ -0,0 +1,43 @@
+package io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.javaoperatorsdk.operator.ReconcilerUtils;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
+
+public class StatefulSetDesiredSanitizerDependentResource
+    extends
+    CRUDKubernetesDependentResource<StatefulSet, StatefulSetDesiredSanitizerCustomResource> {
+
+  public static volatile Boolean nonMatchedAtLeastOnce;
+
+  public StatefulSetDesiredSanitizerDependentResource() {
+    super(StatefulSet.class);
+  }
+
+  @Override
+  protected StatefulSet desired(StatefulSetDesiredSanitizerCustomResource primary,
+      Context<StatefulSetDesiredSanitizerCustomResource> context) {
+    var template =
+        ReconcilerUtils.loadYaml(StatefulSet.class, getClass(), "statefulset.yaml");
+    template.setMetadata(new ObjectMetaBuilder()
+        .withName(primary.getMetadata().getName())
+        .withNamespace(primary.getMetadata().getNamespace())
+        .build());
+    return template;
+  }
+
+  @Override
+  public Result<StatefulSet> match(StatefulSet actualResource,
+      StatefulSetDesiredSanitizerCustomResource primary,
+      Context<StatefulSetDesiredSanitizerCustomResource> context) {
+    var res = super.match(actualResource, primary, context);
+    if (!res.matched()) {
+      nonMatchedAtLeastOnce = true;
+    } else if (nonMatchedAtLeastOnce == null) {
+      nonMatchedAtLeastOnce = false;
+    }
+    return res;
+  }
+}