Skip to content

feat: name uniqueness validation #79

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

Merged
merged 7 commits into from
Apr 17, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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;
}

}
Original file line number Diff line number Diff line change
@@ -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 {

}
Original file line number Diff line number Diff line change
@@ -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 {

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

}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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());
}


}
15 changes: 15 additions & 0 deletions src/test/java/io/csviri/operator/glue/GlueOperatorTest.java
Original file line number Diff line number Diff line change
@@ -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);
}
16 changes: 16 additions & 0 deletions src/test/java/io/csviri/operator/glue/GlueTest.java
Original file line number Diff line number Diff line change
@@ -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
23 changes: 23 additions & 0 deletions src/test/resources/glue/NonUniqueName.yaml
Original file line number Diff line number Diff line change
@@ -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"
25 changes: 25 additions & 0 deletions src/test/resources/glueoperator/NonUniqueName.yaml
Original file line number Diff line number Diff line change
@@ -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}"