Skip to content

fix: generate proper resource name & finalizer for group-less resources #814

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 3 commits into from
Jan 12, 2022
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.
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
Expand Up @@ -2,6 +2,8 @@

import java.util.Locale;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
Expand All @@ -10,9 +12,50 @@
public class ReconcilerUtils {

private static final String FINALIZER_NAME_SUFFIX = "/finalizer";
protected static final String MISSING_GROUP_SUFFIX = ".javaoperatorsdk.io";

public static String getDefaultFinalizerName(String crdName) {
return crdName + FINALIZER_NAME_SUFFIX;
// prevent instantiation of util class
private ReconcilerUtils() {}

public static boolean isFinalizerValid(String finalizer) {
// todo: use fabric8 method when 5.12 is released
// return HasMetadata.validateFinalizer(finalizer);
final var validator = new HasMetadata() {

@Override
public ObjectMeta getMetadata() {
throw new UnsupportedOperationException();
}

@Override
public void setMetadata(ObjectMeta objectMeta) {
throw new UnsupportedOperationException();
}

@Override
public void setApiVersion(String s) {
throw new UnsupportedOperationException();
}
};
return validator.isFinalizerValid(finalizer);
}

public static String getResourceTypeName(Class<? extends HasMetadata> resourceClass) {
// todo: use fabric8 method when 5.12 is released
// return HasMetadata.getFullResourceName(resourceClass);
final var group = HasMetadata.getGroup(resourceClass);
final var plural = HasMetadata.getPlural(resourceClass);
return (group == null || group.isEmpty()) ? plural : plural + "." + group;
}

public static String getDefaultFinalizerName(Class<? extends HasMetadata> resourceClass) {
var resourceName = getResourceTypeName(resourceClass);
// resource names for historic resources such as Pods are missing periods and therefore do not
// constitute valid domain names as mandated by Kubernetes so generate one that does
if (resourceName.indexOf('.') < 0) {
resourceName = resourceName + MISSING_GROUP_SUFFIX;
}
return resourceName + FINALIZER_NAME_SUFFIX;
}

public static String getNameFor(Class<? extends Reconciler> reconcilerClass) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ default String getName() {
}

default String getResourceTypeName() {
return HasMetadata.getFullResourceName(getResourceClass());
return ReconcilerUtils.getResourceTypeName(getResourceClass());
}

default String getFinalizer() {
return ReconcilerUtils.getDefaultFinalizerName(getResourceTypeName());
return ReconcilerUtils.getDefaultFinalizerName(getResourceClass());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,35 @@

import org.junit.jupiter.api.Test;

import io.fabric8.kubernetes.api.model.Pod;
import io.javaoperatorsdk.operator.sample.simple.TestCustomReconciler;
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;

import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultFinalizerName;
import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultNameFor;
import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultReconcilerName;
import static io.javaoperatorsdk.operator.ReconcilerUtils.isFinalizerValid;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

class ReconcilerUtilsTest {

@Test
void getDefaultResourceControllerName() {
void defaultReconcilerNameShouldWork() {
assertEquals(
"testcustomreconciler",
ReconcilerUtils.getDefaultReconcilerName(
TestCustomReconciler.class.getCanonicalName()));
getDefaultReconcilerName(TestCustomReconciler.class.getCanonicalName()));
assertEquals(
getDefaultNameFor(TestCustomReconciler.class),
getDefaultReconcilerName(TestCustomReconciler.class.getCanonicalName()));
assertEquals(
getDefaultNameFor(TestCustomReconciler.class),
getDefaultReconcilerName(TestCustomReconciler.class.getSimpleName()));
}

@Test
void defaultFinalizerShouldWork() {
assertTrue(isFinalizerValid(getDefaultFinalizerName(Pod.class)));
assertTrue(isFinalizerValid(getDefaultFinalizerName(TestCustomResource.class)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
Expand All @@ -31,9 +32,15 @@ public String getName() {
@Override
public String getFinalizer() {
if (annotation == null || annotation.finalizerName().isBlank()) {
return ReconcilerUtils.getDefaultFinalizerName(getResourceTypeName());
return ReconcilerUtils.getDefaultFinalizerName(getResourceClass());
} else {
return annotation.finalizerName();
final var finalizer = annotation.finalizerName();
if (Constants.NO_FINALIZER.equals(finalizer) || ReconcilerUtils.isFinalizerValid(finalizer)) {
return finalizer;
} else {
throw new IllegalArgumentException(finalizer
+ " is not a valid finalizer. See https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#finalizers for details");
}
}
}

Expand Down Expand Up @@ -93,7 +100,7 @@ public ResourceEventFilter<R> getEventFilter() {
answer = answer.and(filter);
}
} catch (Exception e) {
throw new RuntimeException(e);
throw new IllegalArgumentException(e);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;

public class DefaultConfigurationServiceTest {
class DefaultConfigurationServiceTest {

public static final String CUSTOM_FINALIZER_NAME = "a.custom/finalizer";

Expand Down Expand Up @@ -73,29 +73,29 @@ void attemptingToRetrieveAnUnknownControllerShouldLogWarning() {
}

@Test
public void returnsValuesFromControllerAnnotationFinalizer() {
void returnsValuesFromControllerAnnotationFinalizer() {
final var reconciler = new TestCustomReconciler();
final var configuration =
DefaultConfigurationService.instance().getConfigurationFor(reconciler);
assertEquals(CustomResource.getCRDName(TestCustomResource.class),
configuration.getResourceTypeName());
assertEquals(
ReconcilerUtils.getDefaultFinalizerName(configuration.getResourceTypeName()),
ReconcilerUtils.getDefaultFinalizerName(TestCustomResource.class),
configuration.getFinalizer());
assertEquals(TestCustomResource.class, configuration.getResourceClass());
assertFalse(configuration.isGenerationAware());
}

@Test
public void returnCustomerFinalizerNameIfSet() {
void returnCustomerFinalizerNameIfSet() {
final var reconciler = new TestCustomFinalizerReconciler();
final var configuration =
DefaultConfigurationService.instance().getConfigurationFor(reconciler);
assertEquals(CUSTOM_FINALIZER_NAME, configuration.getFinalizer());
}

@Test
public void supportsInnerClassCustomResources() {
void supportsInnerClassCustomResources() {
final var reconciler = new TestCustomFinalizerReconciler();
assertDoesNotThrow(
() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.concurrent.atomic.AtomicInteger;

import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.reconciler.*;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
Expand All @@ -14,8 +13,7 @@ public class EventSourceTestCustomReconciler
TestExecutionInfoProvider {

public static final String FINALIZER_NAME =
ReconcilerUtils.getDefaultFinalizerName(
CustomResource.getCRDName(EventSourceTestCustomResource.class));
ReconcilerUtils.getDefaultFinalizerName(EventSourceTestCustomResource.class);
public static final int TIMER_PERIOD = 500;
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
Expand All @@ -18,8 +17,7 @@ public class RetryTestCustomReconciler
implements Reconciler<RetryTestCustomResource>, TestExecutionInfoProvider {

public static final String FINALIZER_NAME =
ReconcilerUtils.getDefaultFinalizerName(
CustomResource.getCRDName(RetryTestCustomResource.class));
ReconcilerUtils.getDefaultFinalizerName(RetryTestCustomResource.class);
private static final Logger log =
LoggerFactory.getLogger(RetryTestCustomReconciler.class);
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.fabric8.kubernetes.api.model.ConfigMap;
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.reconciler.*;
Expand All @@ -26,7 +25,7 @@ public class TestReconciler
private static final Logger log = LoggerFactory.getLogger(TestReconciler.class);

public static final String FINALIZER_NAME =
ReconcilerUtils.getDefaultFinalizerName(CustomResource.getCRDName(TestCustomResource.class));
ReconcilerUtils.getDefaultFinalizerName(TestCustomResource.class);

private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
private KubernetesClient kubernetesClient;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.client.CustomResource;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
Expand All @@ -18,8 +17,7 @@ public class SubResourceTestCustomReconciler
implements Reconciler<SubResourceTestCustomResource>, TestExecutionInfoProvider {

public static final String FINALIZER_NAME =
ReconcilerUtils.getDefaultFinalizerName(
CustomResource.getCRDName(SubResourceTestCustomResource.class));
ReconcilerUtils.getDefaultFinalizerName(SubResourceTestCustomResource.class);
private static final Logger log =
LoggerFactory.getLogger(SubResourceTestCustomReconciler.class);
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
Expand Down