diff --git a/src/main/java/io/csviri/operator/glue/customresource/AbstractStatus.java b/src/main/java/io/csviri/operator/glue/customresource/AbstractStatus.java new file mode 100644 index 0000000..4c604ed --- /dev/null +++ b/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; + } + +} diff --git a/src/main/java/io/csviri/operator/glue/customresource/glue/GlueStatus.java b/src/main/java/io/csviri/operator/glue/customresource/glue/GlueStatus.java index 29bd7d8..3bd28fa 100644 --- a/src/main/java/io/csviri/operator/glue/customresource/glue/GlueStatus.java +++ b/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 { } diff --git a/src/main/java/io/csviri/operator/glue/customresource/operator/ResourceFlowOperatorStatus.java b/src/main/java/io/csviri/operator/glue/customresource/operator/ResourceFlowOperatorStatus.java index 7250e58..6a32439 100644 --- a/src/main/java/io/csviri/operator/glue/customresource/operator/ResourceFlowOperatorStatus.java +++ b/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 { } diff --git a/src/main/java/io/csviri/operator/glue/reconciler/ValidationAndErrorHandler.java b/src/main/java/io/csviri/operator/glue/reconciler/ValidationAndErrorHandler.java new file mode 100644 index 0000000..c8a1339 --- /dev/null +++ b/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 > ErrorStatusUpdateControl 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 seen = new HashSet<>(); + List duplicates = new ArrayList<>(); + + Consumer 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 duplicates; + + public NonUniqueNameException(List duplicates) { + this.duplicates = duplicates; + } + + public List getDuplicates() { + return duplicates; + } + } + +} diff --git a/src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java b/src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java index 704d1ba..b1cd064 100644 --- a/src/main/java/io/csviri/operator/glue/reconciler/glue/GlueReconciler.java +++ b/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, Cleaner { +public class GlueReconciler implements Reconciler, Cleaner, ErrorStatusHandler { 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 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 updateErrorStatus(Glue resource, Context context, + Exception e) { + if (resource.getStatus() == null) { + resource.setStatus(new GlueStatus()); + } + return validationAndErrorHandler.updateStatusErrorMessage(e, resource); + } } diff --git a/src/main/java/io/csviri/operator/glue/reconciler/operator/GlueOperatorReconciler.java b/src/main/java/io/csviri/operator/glue/reconciler/operator/GlueOperatorReconciler.java index 3f3c234..2b6378d 100644 --- a/src/main/java/io/csviri/operator/glue/reconciler/operator/GlueOperatorReconciler.java +++ b/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,10 +25,12 @@ 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, EventSourceInitializer, - Cleaner { + Cleaner, ErrorStatusHandler { private static final Logger log = LoggerFactory.getLogger(GlueOperatorReconciler.class); @@ -34,6 +38,9 @@ public class GlueOperatorReconciler public static final String GLUE_LABEL_VALUE = "true"; public static final String PARENT_RELATED_RESOURCE_NAME = "parent"; + @Inject + ValidationAndErrorHandler validationAndErrorHandler; + private InformerEventSource resourceFlowEventSource; @Override @@ -43,6 +50,8 @@ public UpdateControl 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 prepareEventSources( return EventSourceInitializer.nameEventSources(resourceFlowEventSource); } + @Override + public ErrorStatusUpdateControl updateErrorStatus(GlueOperator resource, + Context context, Exception e) { + if (resource.getStatus() == null) { + resource.setStatus(new ResourceFlowOperatorStatus()); + } + return validationAndErrorHandler.updateStatusErrorMessage(e, resource); + } + @Override public DeleteControl cleanup(GlueOperator glueOperator, Context context) { @@ -141,4 +159,5 @@ private static String glueName(GenericKubernetesResource cr) { return KubernetesResourceUtil.sanitizeName(cr.getMetadata().getName() + "-" + cr.getKind()); } + } diff --git a/src/test/java/io/csviri/operator/glue/GlueOperatorTest.java b/src/test/java/io/csviri/operator/glue/GlueOperatorTest.java index f30176b..aed156f 100644 --- a/src/test/java/io/csviri/operator/glue/GlueOperatorTest.java +++ b/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); } diff --git a/src/test/java/io/csviri/operator/glue/GlueTest.java b/src/test/java/io/csviri/operator/glue/GlueTest.java index c154556..649c132 100644 --- a/src/test/java/io/csviri/operator/glue/GlueTest.java +++ b/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 diff --git a/src/test/resources/glue/NonUniqueName.yaml b/src/test/resources/glue/NonUniqueName.yaml new file mode 100644 index 0000000..5063044 --- /dev/null +++ b/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" diff --git a/src/test/resources/glueoperator/NonUniqueName.yaml b/src/test/resources/glueoperator/NonUniqueName.yaml new file mode 100644 index 0000000..346fac3 --- /dev/null +++ b/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}" \ No newline at end of file