feat: name uniqueness validation (#79)

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

src/main/java/io/csviri/operator/glue/customresource/AbstractStatus.java

@@ -0,0 +1,17 @@
+package io.csviri.operator.glue.customresource;
+
+import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
+
+public class AbstractStatus extends ObservedGenerationAwareStatus {
+
+  private String errorMessage;
+
+  public String getErrorMessage() {
+    return errorMessage;
+  }
+
+  public void setErrorMessage(String errorMessage) {
+    this.errorMessage = errorMessage;
+  }
+
+}

src/main/java/io/csviri/operator/glue/customresource/glue/GlueStatus.java

@@ -1,7 +1,7 @@
 package io.csviri.operator.glue.customresource.glue;
 
-import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
+import io.csviri.operator.glue.customresource.AbstractStatus;
 
-public class GlueStatus extends ObservedGenerationAwareStatus {
+public class GlueStatus extends AbstractStatus {
 
 }

src/main/java/io/csviri/operator/glue/customresource/operator/ResourceFlowOperatorStatus.java

@@ -1,7 +1,8 @@
 package io.csviri.operator.glue.customresource.operator;
 
-import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
+import io.csviri.operator.glue.customresource.AbstractStatus;
+
+public class ResourceFlowOperatorStatus extends AbstractStatus {
 
-public class ResourceFlowOperatorStatus extends ObservedGenerationAwareStatus {
 
 }

src/main/java/io/csviri/operator/glue/reconciler/ValidationAndErrorHandler.java

@@ -0,0 +1,77 @@
+package io.csviri.operator.glue.reconciler;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.function.Consumer;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import io.csviri.operator.glue.GlueException;
+import io.csviri.operator.glue.customresource.AbstractStatus;
+import io.csviri.operator.glue.customresource.glue.DependentResourceSpec;
+import io.csviri.operator.glue.customresource.glue.GlueSpec;
+import io.csviri.operator.glue.customresource.glue.RelatedResourceSpec;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl;
+
+import jakarta.inject.Singleton;
+
+@Singleton
+public class ValidationAndErrorHandler {
+
+  private static final Logger log = LoggerFactory.getLogger(ValidationAndErrorHandler.class);
+
+  public static final String NON_UNIQUE_NAMES_FOUND_PREFIX = "Non unique names found: ";
+
+  public <T extends CustomResource<?, ? extends AbstractStatus>> ErrorStatusUpdateControl<T> updateStatusErrorMessage(
+      Exception e,
+      T resource) {
+    log.error("Error during reconciliation of resource. Name: {} namespace: {}, Kind: {}",
+        resource.getMetadata().getName(), resource.getMetadata().getNamespace(), resource.getKind(),
+        e);
+    if (e instanceof ValidationAndErrorHandler.NonUniqueNameException ex) {
+      resource.getStatus()
+          .setErrorMessage(NON_UNIQUE_NAMES_FOUND_PREFIX + String.join(",", ex.getDuplicates()));
+      return ErrorStatusUpdateControl.updateStatus(resource).withNoRetry();
+    } else {
+      resource.getStatus().setErrorMessage("Error during reconciliation");
+      return ErrorStatusUpdateControl.updateStatus(resource);
+    }
+  }
+
+  public void checkIfNamesAreUnique(GlueSpec glueSpec) {
+    Set<String> seen = new HashSet<>();
+    List<String> duplicates = new ArrayList<>();
+
+    Consumer<String> deduplicate = n -> {
+      if (seen.contains(n)) {
+        duplicates.add(n);
+      } else {
+        seen.add(n);
+      }
+    };
+    glueSpec.getResources().stream().map(DependentResourceSpec::getName).forEach(deduplicate);
+    glueSpec.getRelatedResources().stream().map(RelatedResourceSpec::getName).forEach(deduplicate);
+
+    if (!duplicates.isEmpty()) {
+      throw new NonUniqueNameException(duplicates);
+    }
+  }
+
+  public static class NonUniqueNameException extends GlueException {
+
+    private final List<String> duplicates;
+
+    public NonUniqueNameException(List<String> duplicates) {
+      this.duplicates = duplicates;
+    }
+
+    public List<String> getDuplicates() {
+      return duplicates;
+    }
+  }
+
+}

src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java

@@ -10,12 +10,14 @@
 import io.csviri.operator.glue.conditions.ReadyCondition;
 import io.csviri.operator.glue.customresource.glue.DependentResourceSpec;
 import io.csviri.operator.glue.customresource.glue.Glue;
+import io.csviri.operator.glue.customresource.glue.GlueStatus;
 import io.csviri.operator.glue.customresource.glue.condition.ConditionSpec;
 import io.csviri.operator.glue.customresource.glue.condition.JavaScriptConditionSpec;
 import io.csviri.operator.glue.customresource.glue.condition.ReadyConditionSpec;
 import io.csviri.operator.glue.dependent.GCGenericDependentResource;
 import io.csviri.operator.glue.dependent.GenericDependentResource;
 import io.csviri.operator.glue.dependent.GenericResourceDiscriminator;
+import io.csviri.operator.glue.reconciler.ValidationAndErrorHandler;
 import io.csviri.operator.glue.templating.GenericTemplateHandler;
 import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.fabric8.kubernetes.api.model.HasMetadata;
@@ -27,16 +29,21 @@
 import io.javaoperatorsdk.operator.processing.dependent.workflow.KubernetesResourceDeletedCondition;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowBuilder;
 
+import jakarta.inject.Inject;
+
 import static io.csviri.operator.glue.Utils.getResourceForSSAFrom;
 import static io.csviri.operator.glue.reconciler.operator.GlueOperatorReconciler.PARENT_RELATED_RESOURCE_NAME;
 
 @ControllerConfiguration
-public class GlueReconciler implements Reconciler<Glue>, Cleaner<Glue> {
+public class GlueReconciler implements Reconciler<Glue>, Cleaner<Glue>, ErrorStatusHandler<Glue> {
 
   private static final Logger log = LoggerFactory.getLogger(GlueReconciler.class);
   public static final String DEPENDENT_NAME_ANNOTATION_KEY = "io.csviri.operator.resourceflow/name";
   public static final String PARENT_GLUE_FINALIZER_PREFIX = "io.csviri.operator.resourceflow.glue/";
 
+  @Inject
+  ValidationAndErrorHandler validationAndErrorHandler;
+
   private final KubernetesResourceDeletedCondition deletePostCondition =
       new KubernetesResourceDeletedCondition();
   private final InformerRegister informerRegister = new InformerRegister();
@@ -57,6 +64,9 @@ public UpdateControl<Glue> reconcile(Glue primary,
 
     log.debug("Reconciling glue. name: {} namespace: {}",
         primary.getMetadata().getName(), primary.getMetadata().getNamespace());
+
+    validationAndErrorHandler.checkIfNamesAreUnique(primary.getSpec());
+
     registerRelatedResourceInformers(context, primary);
     if (deletedGlueIfParentMarkedForDeletion(context, primary)) {
       return UpdateControl.noUpdate();
@@ -270,5 +280,12 @@ private String parentFinalizer(String glueName) {
     return PARENT_GLUE_FINALIZER_PREFIX + glueName;
   }
 
-
+  @Override
+  public ErrorStatusUpdateControl<Glue> updateErrorStatus(Glue resource, Context<Glue> context,
+      Exception e) {
+    if (resource.getStatus() == null) {
+      resource.setStatus(new GlueStatus());
+    }
+    return validationAndErrorHandler.updateStatusErrorMessage(e, resource);
+  }
 }

src/main/java/io/csviri/operator/glue/reconciler/operator/GlueOperatorReconciler.java

@@ -13,6 +13,8 @@
 import io.csviri.operator.glue.customresource.glue.RelatedResourceSpec;
 import io.csviri.operator.glue.customresource.operator.GlueOperator;
 import io.csviri.operator.glue.customresource.operator.GlueOperatorSpec;
+import io.csviri.operator.glue.customresource.operator.ResourceFlowOperatorStatus;
+import io.csviri.operator.glue.reconciler.ValidationAndErrorHandler;
 import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil;
@@ -23,17 +25,22 @@
 import io.javaoperatorsdk.operator.processing.event.source.EventSource;
 import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource;
 
+import jakarta.inject.Inject;
+
 @ControllerConfiguration
 public class GlueOperatorReconciler
     implements Reconciler<GlueOperator>, EventSourceInitializer<GlueOperator>,
-    Cleaner<GlueOperator> {
+    Cleaner<GlueOperator>, ErrorStatusHandler<GlueOperator> {
 
   private static final Logger log = LoggerFactory.getLogger(GlueOperatorReconciler.class);
 
   public static final String GLUE_LABEL_KEY = "foroperator";
   public static final String GLUE_LABEL_VALUE = "true";
   public static final String PARENT_RELATED_RESOURCE_NAME = "parent";
 
+  @Inject
+  ValidationAndErrorHandler validationAndErrorHandler;
+
   private InformerEventSource<Glue, GlueOperator> resourceFlowEventSource;
 
   @Override
@@ -43,6 +50,8 @@ public UpdateControl<GlueOperator> reconcile(GlueOperator glueOperator,
     log.info("Reconciling GlueOperator {} in namespace: {}", glueOperator.getMetadata().getName(),
         glueOperator.getMetadata().getNamespace());
 
+    validationAndErrorHandler.checkIfNamesAreUnique(glueOperator.getSpec());
+
     var targetCREventSource = getOrRegisterCustomResourceEventSource(glueOperator, context);
     targetCREventSource.list().forEach(cr -> {
       var actualResourceFlow = resourceFlowEventSource
@@ -128,6 +137,15 @@ public Map<String, EventSource> prepareEventSources(
     return EventSourceInitializer.nameEventSources(resourceFlowEventSource);
   }
 
+  @Override
+  public ErrorStatusUpdateControl<GlueOperator> updateErrorStatus(GlueOperator resource,
+      Context<GlueOperator> context, Exception e) {
+    if (resource.getStatus() == null) {
+      resource.setStatus(new ResourceFlowOperatorStatus());
+    }
+    return validationAndErrorHandler.updateStatusErrorMessage(e, resource);
+  }
+
   @Override
   public DeleteControl cleanup(GlueOperator glueOperator,
       Context<GlueOperator> context) {
@@ -141,4 +159,5 @@ private static String glueName(GenericKubernetesResource cr) {
     return KubernetesResourceUtil.sanitizeName(cr.getMetadata().getName() + "-" + cr.getKind());
   }
 
+
 }

src/test/java/io/csviri/operator/glue/GlueOperatorTest.java

@@ -14,6 +14,7 @@
 import io.csviri.operator.glue.customresource.operator.GlueOperator;
 import io.csviri.operator.glue.customresource.operator.GlueOperatorSpec;
 import io.csviri.operator.glue.customresource.operator.Parent;
+import io.csviri.operator.glue.reconciler.ValidationAndErrorHandler;
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 import io.quarkus.test.junit.QuarkusTest;
@@ -149,6 +150,20 @@ void simpleConcurrencyForMultipleOperatorTest() {
         }));
   }
 
+  @Test
+  void nonUniqueNameTest() {
+    var go = create(TestUtils
+        .loadResourceFlowOperator("/glueoperator/NonUniqueName.yaml"));
+
+    await().untilAsserted(() -> {
+      var actual = get(GlueOperator.class, go.getMetadata().getName());
+
+      assertThat(actual.getStatus()).isNotNull();
+      assertThat(actual.getStatus().getErrorMessage())
+          .startsWith(ValidationAndErrorHandler.NON_UNIQUE_NAMES_FOUND_PREFIX);
+    });
+  }
+
   TestCustomResource testCustomResource() {
     return testCustomResource(1);
   }

src/test/java/io/csviri/operator/glue/GlueTest.java

@@ -10,6 +10,7 @@
 
 import io.csviri.operator.glue.customresource.glue.DependentResourceSpec;
 import io.csviri.operator.glue.customresource.glue.Glue;
+import io.csviri.operator.glue.reconciler.ValidationAndErrorHandler;
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.Secret;
 import io.quarkus.test.junit.QuarkusTest;
@@ -222,6 +223,21 @@ void changingWorkflow() {
       assertThat(s).isNull();
     });
   }
+
+  @Test
+  void nonUniqueNameResultsInErrorMessageOnStatus() {
+    Glue glue = create(TestUtils.loadResoureFlow("/glue/NonUniqueName.yaml"));
+
+    await().untilAsserted(() -> {
+      var actualGlue = get(Glue.class, glue.getMetadata().getName());
+
+      assertThat(actualGlue.getStatus()).isNotNull();
+      assertThat(actualGlue.getStatus().getErrorMessage())
+          .startsWith(ValidationAndErrorHandler.NON_UNIQUE_NAMES_FOUND_PREFIX);
+    });
+  }
+
+
   //
   // @Disabled("Not supported in current version")
   // @Test

src/test/resources/glue/NonUniqueName.yaml

@@ -0,0 +1,23 @@
+# Invalid GLUE, presents resources with non-unique name
+apiVersion: io.csviri.operator.glue/v1beta1
+kind: Glue
+metadata:
+  name: templating-sample
+spec:
+  resources:
+    - name: configMap1
+      resource:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: cm1
+        data:
+          key: "value1"
+    - name: configMap1
+      resource:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: cm2
+        data:
+          key: "value1"

src/test/resources/glueoperator/NonUniqueName.yaml

@@ -0,0 +1,25 @@
+apiVersion: io.csviri.operator.glue/v1beta1
+kind: GlueOperator
+metadata:
+  name: non-unique-name
+spec:
+  parent:
+    apiVersion: io.csviri.operator.glue/v1
+    kind: TestCustomResource
+  resources:
+    - name: configMap1
+      resource:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: "{parent.metadata.name}"
+        data:
+          key: "{parent.spec.value}"
+    - name: configMap1
+      resource:
+        apiVersion: v1
+        kind: ConfigMap
+        metadata:
+          name: "{parent.metadata.name}"
+        data:
+          key: "{parent.spec.value}"