diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java index 53e9122fb2..a30c335fb0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java @@ -24,6 +24,10 @@ public class ReconcilerUtils { private static final String FINALIZER_NAME_SUFFIX = "/finalizer"; protected static final String MISSING_GROUP_SUFFIX = ".javaoperatorsdk.io"; + private static final String GET_SPEC = "getSpec"; + private static final String SET_SPEC = "setSpec"; + private static final Pattern API_URI_PATTERN = + Pattern.compile(".*http(s?)://[^/]*/api(s?)/(\\S*).*"); // NOSONAR: input is controlled // prevent instantiation of util class private ReconcilerUtils() {} @@ -94,25 +98,55 @@ public static boolean specsEqual(HasMetadata r1, HasMetadata r2) { // will be replaced with: https://github.com/fabric8io/kubernetes-client/issues/3816 public static Object getSpec(HasMetadata resource) { + // optimize CustomResource case + if (resource instanceof CustomResource) { + CustomResource cr = (CustomResource) resource; + return cr.getSpec(); + } + try { - Method getSpecMethod = resource.getClass().getMethod("getSpec"); + Method getSpecMethod = resource.getClass().getMethod(GET_SPEC); return getSpecMethod.invoke(resource); } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw new IllegalStateException("No spec found on resource", e); + throw noSpecException(resource, e); } } + @SuppressWarnings("unchecked") public static Object setSpec(HasMetadata resource, Object spec) { + // optimize CustomResource case + if (resource instanceof CustomResource) { + CustomResource cr = (CustomResource) resource; + cr.setSpec(spec); + return null; + } + try { Class resourceClass = resource.getClass(); - Method setSpecMethod = - resource.getClass().getMethod("setSpec", getSpecClass(resourceClass, spec)); + + // if given spec is null, find the method just using its name + Method setSpecMethod; + if (spec != null) { + setSpecMethod = resourceClass.getMethod(SET_SPEC, spec.getClass()); + } else { + setSpecMethod = Arrays.stream(resourceClass.getMethods()) + .filter(method -> SET_SPEC.equals(method.getName())) + .findFirst() + .orElseThrow(() -> noSpecException(resource, null)); + } + return setSpecMethod.invoke(resource, spec); } catch (NoSuchMethodException | InvocationTargetException | IllegalAccessException e) { - throw new IllegalStateException("No spec found on resource", e); + throw noSpecException(resource, e); } } + private static IllegalStateException noSpecException(HasMetadata resource, + ReflectiveOperationException e) { + return new IllegalStateException("No spec found on resource " + resource.getClass().getName(), + e); + } + public static T loadYaml(Class clazz, Class loader, String yaml) { try (InputStream is = loader.getResourceAsStream(yaml)) { if (Builder.class.isAssignableFrom(clazz)) { @@ -149,9 +183,7 @@ private static boolean matchesResourceType(String resourceTypeName, } else { // extract matching information from URI in the message if available final var message = exception.getMessage(); - final var regex = Pattern - .compile(".*http(s?)://[^/]*/api(s?)/(\\S*).*") // NOSONAR: input is controlled - .matcher(message); + final var regex = API_URI_PATTERN.matcher(message); if (regex.matches()) { var group = regex.group(3); if (group.endsWith(".")) { @@ -168,14 +200,4 @@ private static boolean matchesResourceType(String resourceTypeName, } return false; } - - // CustomResouce has a parameterized parameter type - private static Class getSpecClass(Class resourceClass, Object spec) { - if (CustomResource.class.isAssignableFrom(resourceClass)) { - return Object.class; - } else { - return spec.getClass(); - } - } - } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java index 25bdcf205f..2b4d89ae06 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java @@ -6,6 +6,7 @@ import io.fabric8.kubernetes.api.model.ContainerBuilder; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.Namespace; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodSpec; @@ -85,6 +86,16 @@ void getsSpecWithReflection() { assertThat(spec.getReplicas()).isEqualTo(5); } + @Test + void properlyHandlesNullSpec() { + Namespace ns = new Namespace(); + + final var spec = ReconcilerUtils.getSpec(ns); + assertThat(spec).isNull(); + + ReconcilerUtils.setSpec(ns, null); + } + @Test void setsSpecWithReflection() { Deployment deployment = new Deployment(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java index e67d579adc..9f1856edbc 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java @@ -1,10 +1,14 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; import java.util.HashMap; +import java.util.List; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import io.fabric8.kubernetes.api.model.Namespace; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.api.model.NamespaceSpec; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; @@ -25,12 +29,9 @@ static void setUp() { when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); } - ResourceUpdatePreProcessor processor = - GenericResourceUpdatePreProcessor.processorFor(Deployment.class); - - @Test void preservesValues() { + var processor = GenericResourceUpdatePreProcessor.processorFor(Deployment.class); var desired = createDeployment(); var actual = createDeployment(); actual.getMetadata().setLabels(new HashMap<>()); @@ -45,6 +46,34 @@ void preservesValues() { assertThat(result.getSpec().getRevisionHistoryLimit()).isEqualTo(10); } + @Test + void checkNamespaces() { + var processor = GenericResourceUpdatePreProcessor.processorFor(Namespace.class); + var desired = new NamespaceBuilder().withNewMetadata().withName("foo").endMetadata().build(); + var actual = new NamespaceBuilder().withNewMetadata().withName("foo").endMetadata().build(); + actual.getMetadata().setLabels(new HashMap<>()); + actual.getMetadata().getLabels().put("additionalActualKey", "value"); + actual.getMetadata().setResourceVersion("1234"); + + var result = processor.replaceSpecOnActual(actual, desired, context); + assertThat(result.getMetadata().getLabels().get("additionalActualKey")).isEqualTo("value"); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("1234"); + + desired.setSpec(new NamespaceSpec(List.of("halkyon.io/finalizer"))); + + result = processor.replaceSpecOnActual(actual, desired, context); + assertThat(result.getMetadata().getLabels().get("additionalActualKey")).isEqualTo("value"); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("1234"); + assertThat(result.getSpec().getFinalizers()).containsExactly("halkyon.io/finalizer"); + + desired = new NamespaceBuilder().withNewMetadata().withName("foo").endMetadata().build(); + + result = processor.replaceSpecOnActual(actual, desired, context); + assertThat(result.getMetadata().getLabels().get("additionalActualKey")).isEqualTo("value"); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("1234"); + assertThat(result.getSpec()).isNull(); + } + Deployment createDeployment() { return ReconcilerUtils.loadYaml( Deployment.class, GenericResourceUpdatePreProcessorTest.class, "nginx-deployment.yaml");