From a6025aeaec880ef99023cb664aec282452745bd4 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 9 Aug 2022 12:24:31 +0200 Subject: [PATCH] fix: make sure a DependentResourceSpec is always created complete --- .../AnnotationControllerConfiguration.java | 9 +------- .../ControllerConfigurationOverrider.java | 4 +++- .../dependent/DependentResourceSpec.java | 7 ------ .../ControllerConfigurationOverriderTest.java | 23 ++++++++++++++++--- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index d6a323d5a9..7588a9996d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -277,14 +277,7 @@ public List getDependentResources() { private Condition instantiateConditionIfNotVoid(Class condition) { if (condition != VoidCondition.class) { - try { - return condition.getDeclaredConstructor().newInstance(); - } catch (InstantiationException - | IllegalAccessException - | InvocationTargetException - | NoSuchMethodException e) { - throw new OperatorException(e); - } + return instantiateAndConfigureIfNeeded(condition, Condition.class); } return null; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 4b0766d3dc..e9cae2dccf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -172,7 +172,9 @@ public ControllerConfigurationOverrider replacingNamedDependentResourceConfig private void replaceConfig(String name, Object newConfig, DependentResourceSpec current) { namedDependentResourceSpecs.put(name, - new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name)); + new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name, + current.getDependsOn(), current.getReadyCondition(), current.getReconcileCondition(), + current.getDeletePostCondition())); } @SuppressWarnings("unchecked") diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java index 2f22dd5a96..f146d127d0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator.api.config.dependent; -import java.util.Collections; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -24,12 +23,6 @@ public class DependentResourceSpec, C> { private final Condition deletePostCondition; - public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig, - String name) { - this(dependentResourceClass, dependentResourceConfig, name, Collections.emptySet(), null, null, - null); - } - public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig, String name, Set dependsOn, Condition readyCondition, Condition reconcileCondition, Condition deletePostCondition) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java index 3eaef5e6e8..80156aaccc 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java @@ -17,6 +17,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -91,6 +92,7 @@ public Optional getSecondaryResource(ConfigMap primary) { } } + @SuppressWarnings("rawtypes") private KubernetesDependentResourceConfig extractFirstDependentKubernetesResourceConfig( io.javaoperatorsdk.operator.api.config.ControllerConfiguration configuration) { return (KubernetesDependentResourceConfig) extractDependentKubernetesResourceConfig( @@ -191,6 +193,7 @@ void configuredDependentShouldNotChangeOnParentOverrideEvenWhenInitialConfigIsSa assertEquals(Set.of(OverriddenNSDependent.DEP_NS), config.namespaces()); } + @SuppressWarnings("unchecked") @Test void dependentShouldWatchAllNamespacesIfParentDoesAsWell() { var configuration = createConfiguration(new WatchAllNamespacesReconciler()); @@ -211,6 +214,7 @@ void dependentShouldWatchAllNamespacesIfParentDoesAsWell() { assertEquals(Set.of(newNS), config.namespaces()); } + @SuppressWarnings("unchecked") @Test void shouldBePossibleToForceDependentToWatchAllNamespaces() { var configuration = createConfiguration(new DependentWatchesAllNSReconciler()); @@ -272,6 +276,7 @@ void alreadyOverriddenDependentNamespacesShouldNotBePropagated() { assertEquals(Set.of(OverriddenNSDependent.DEP_NS), config.namespaces()); } + @SuppressWarnings({"rawtypes", "unchecked"}) @Test void replaceNamedDependentResourceConfigShouldWork() { var configuration = createConfiguration(new OneDepReconciler()); @@ -284,7 +289,7 @@ void replaceNamedDependentResourceConfigShouldWork() { var dependentSpec = dependents.stream() .filter(dr -> dr.getName().equals(dependentResourceName)) - .findFirst().get(); + .findFirst().orElseThrow(); assertEquals(ReadOnlyDependent.class, dependentSpec.getDependentResourceClass()); var maybeConfig = dependentSpec.getDependentResourceConfiguration(); assertTrue(maybeConfig.isPresent()); @@ -306,12 +311,14 @@ void replaceNamedDependentResourceConfigShouldWork() { .build(); dependents = overridden.getDependentResources(); dependentSpec = dependents.stream().filter(dr -> dr.getName().equals(dependentResourceName)) - .findFirst().get(); + .findFirst().orElseThrow(); config = (KubernetesDependentResourceConfig) dependentSpec.getDependentResourceConfiguration() .orElseThrow(); assertEquals(1, config.namespaces().size()); assertEquals(labelSelector, config.labelSelector()); assertEquals(Set.of(overriddenNS), config.namespaces()); + // check that we still have the proper workflow configuration + assertTrue(dependentSpec.getReadyCondition() instanceof TestCondition); } @ControllerConfiguration(dependents = @Dependent(type = ReadOnlyDependent.class)) @@ -332,8 +339,18 @@ public UpdateControl reconcile(ConfigMap resource, Context } } + private static class TestCondition implements Condition { + + @Override + public boolean isMet(DependentResource dependentResource, + ConfigMap primary, Context context) { + return true; + } + } + @ControllerConfiguration(namespaces = OneDepReconciler.CONFIGURED_NS, - dependents = @Dependent(type = ReadOnlyDependent.class)) + dependents = @Dependent(type = ReadOnlyDependent.class, + readyPostcondition = TestCondition.class)) private static class OneDepReconciler implements Reconciler { private static final String CONFIGURED_NS = "foo";