Skip to content

Commit 5c59681

Browse files
metacosmmetacosm
committed
refactor: use available DependentResource instance to configure
Signed-off-by: Chris Laprun <claprun@redhat.com>
1 parent 0c4984a commit 5c59681

File tree

7 files changed

+134
-132
lines changed

7 files changed

+134
-132
lines changed

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/ConfigurationConverter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
import java.lang.annotation.Annotation;
44

55
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
6+
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
67

78
public interface ConfigurationConverter<A extends Annotation, C> {
89

910
C configFrom(A configAnnotation, ControllerConfiguration<?> parentConfiguration,
10-
DependentResourceSpec<?, ?> spec);
11+
DependentResourceSpec<?, ?> spec, DependentResource<?, ?> dependentResource);
1112
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceConfigurationResolver.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,13 @@ private DependentResourceConfigurationResolver() {}
2323
public static <C extends ControllerConfiguration<? extends HasMetadata>> void configure(
2424
DependentResource dependentResource, DependentResourceSpec spec, C parentConfiguration) {
2525
if (dependentResource instanceof DependentResourceConfigurator configurator) {
26-
final var config = configurationFor(spec, parentConfiguration);
26+
final var config = configurationFor(spec, parentConfiguration, dependentResource);
2727
configurator.configureWith(config);
2828
}
2929
}
3030

3131
public static <C extends ControllerConfiguration<? extends HasMetadata>> Object configurationFor(
32-
DependentResourceSpec spec, C parentConfiguration) {
32+
DependentResourceSpec spec, C parentConfiguration, DependentResource dependentResource) {
3333

3434
// first check if the parent configuration has potentially already resolved the configuration
3535
if (parentConfiguration instanceof DependentResourceConfigurationProvider provider) {
@@ -40,13 +40,13 @@ public static <C extends ControllerConfiguration<? extends HasMetadata>> Object
4040
}
4141

4242
// find Configured-annotated class if it exists
43-
return extractConfigurationFromConfigured(spec,
44-
parentConfiguration);
43+
return extractConfigurationFromConfigured(spec, parentConfiguration, dependentResource);
4544
}
4645

47-
public static <C extends ControllerConfiguration<? extends HasMetadata>> Object extractConfigurationFromConfigured(
46+
protected static <C extends ControllerConfiguration<? extends HasMetadata>> Object extractConfigurationFromConfigured(
4847
DependentResourceSpec spec,
49-
C parentConfiguration) {
48+
C parentConfiguration,
49+
DependentResource dependentResource) {
5050
Class<? extends DependentResource> dependentResourceClass = spec.getDependentResourceClass();
5151
var converterAnnotationPair = converters.get(dependentResourceClass);
5252

@@ -78,7 +78,7 @@ public static <C extends ControllerConfiguration<? extends HasMetadata>> Object
7878

7979
// always called even if the annotation is null so that implementations can provide default
8080
// values
81-
return converter.configFrom(configAnnotation, parentConfiguration, spec);
81+
return converter.configFrom(configAnnotation, parentConfiguration, spec, dependentResource);
8282
}
8383

8484
private static ConfiguredClassPair getConfigured(

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java

Lines changed: 82 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
22

3-
import java.lang.reflect.InvocationTargetException;
43
import java.util.Set;
54

65
import org.slf4j.Logger;
@@ -35,7 +34,7 @@ public class KubernetesDependentConverter<R extends HasMetadata, P extends HasMe
3534
@SuppressWarnings({"unchecked", "rawtypes"})
3635
public KubernetesDependentResourceConfig<R> configFrom(KubernetesDependent configAnnotation,
3736
ControllerConfiguration<?> controllerConfig,
38-
DependentResourceSpec<?, ?> spec) {
37+
DependentResourceSpec<?, ?> spec, DependentResource<?, ?> dependentResource) {
3938

4039
var createResourceOnlyIfNotExistingWithSSA =
4140
DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA;
@@ -50,7 +49,8 @@ public KubernetesDependentResourceConfig<R> configFrom(KubernetesDependent confi
5049

5150
var informerConfiguration = createInformerConfiguration(configAnnotation,
5251
controllerConfig,
53-
spec);
52+
(DependentResourceSpec<R, P>) spec,
53+
(DependentResource<R, P>) dependentResource);
5454

5555
return new KubernetesDependentResourceConfig(useSSA, createResourceOnlyIfNotExistingWithSSA,
5656
informerConfiguration);
@@ -60,105 +60,93 @@ public KubernetesDependentResourceConfig<R> configFrom(KubernetesDependent confi
6060
@SuppressWarnings({"unchecked", "rawtypes"})
6161
private InformerConfiguration<R> createInformerConfiguration(KubernetesDependent configAnnotation,
6262
ControllerConfiguration<?> controllerConfig,
63-
DependentResourceSpec<?, ?> spec) {
64-
try {
65-
Class<? extends KubernetesDependentResource<?, ?>> dependentResourceClass =
66-
(Class<? extends KubernetesDependentResource<?, ?>>) spec.getDependentResourceClass();
67-
// this is quite ugly but getting the resource type would be quite cumbersome
68-
var resourceType = dependentResourceClass.getConstructor().newInstance().resourceType();
69-
70-
InformerConfiguration.InformerConfigurationBuilder informerConfig;
71-
if (configAnnotation != null && configAnnotation.informerConfig() != null &&
72-
!Constants.NO_VALUE_SET.equals(configAnnotation.informerConfig().groupVersionKind())) {
73-
74-
if (!GenericKubernetesResource.class.isAssignableFrom(resourceType)) {
75-
throw new IllegalStateException("If GroupVersionKind is set the resource type must be " +
76-
"GenericKubernetesDependentResource. For: " + dependentResourceClass.getName());
77-
}
78-
79-
informerConfig = InformerConfiguration.from(
80-
GroupVersionKind.fromString(configAnnotation.informerConfig().groupVersionKind()),
81-
controllerConfig.getResourceClass());
82-
} else {
83-
informerConfig =
84-
InformerConfiguration.from(resourceType, controllerConfig.getResourceClass());
63+
DependentResourceSpec<R, P> spec, DependentResource<R, P> dependentResource) {
64+
Class<? extends KubernetesDependentResource<?, ?>> dependentResourceClass =
65+
(Class<? extends KubernetesDependentResource<?, ?>>) spec.getDependentResourceClass();
66+
var resourceType = dependentResource.resourceType();
67+
68+
InformerConfiguration.InformerConfigurationBuilder informerConfig;
69+
if (configAnnotation != null && configAnnotation.informerConfig() != null &&
70+
!Constants.NO_VALUE_SET.equals(configAnnotation.informerConfig().groupVersionKind())) {
71+
72+
if (!GenericKubernetesResource.class.isAssignableFrom(resourceType)) {
73+
throw new IllegalStateException("If GroupVersionKind is set the resource type must be " +
74+
"GenericKubernetesDependentResource. For: " + dependentResourceClass.getName());
8575
}
8676

87-
if (configAnnotation != null && configAnnotation.informerConfig() != null) {
88-
89-
if (!Constants.NO_VALUE_SET.equals(configAnnotation.informerConfig().name())) {
90-
informerConfig.withName(configAnnotation.informerConfig().name());
91-
} else if (spec.getName() != null && !Constants.NO_VALUE_SET.equals(spec.getName())) {
92-
informerConfig.withName(spec.getName());
93-
} else {
94-
informerConfig.withName(DependentResource.defaultNameFor(dependentResourceClass));
95-
}
96-
97-
var namespaces = Set.of(configAnnotation.informerConfig().namespaces());
98-
informerConfig.withNamespaces(namespaces);
99-
100-
final var fromAnnotation = configAnnotation.informerConfig().labelSelector();
101-
var labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation;
102-
informerConfig.withLabelSelector(labelSelector);
103-
104-
final var context = Utils.contextFor(controllerConfig, dependentResourceClass,
105-
configAnnotation.annotationType());
106-
107-
var onAddFilter = Utils.instantiate(configAnnotation.informerConfig().onAddFilter(),
108-
OnAddFilter.class, context);
109-
informerConfig.withOnAddFilter((OnAddFilter<? super R>) onAddFilter);
110-
111-
var onUpdateFilter =
112-
Utils.instantiate(configAnnotation.informerConfig().onUpdateFilter(),
113-
OnUpdateFilter.class, context);
114-
informerConfig.withOnUpdateFilter((OnUpdateFilter<? super R>) onUpdateFilter);
115-
116-
var onDeleteFilter =
117-
Utils.instantiate(configAnnotation.informerConfig().onDeleteFilter(),
118-
OnDeleteFilter.class, context);
119-
informerConfig.withOnDeleteFilter((OnDeleteFilter<? super R>) onDeleteFilter);
120-
121-
var genericFilter =
122-
Utils.instantiate(configAnnotation.informerConfig().genericFilter(),
123-
GenericFilter.class,
124-
context);
125-
126-
informerConfig.withGenericFilter((GenericFilter<? super R>) genericFilter);
127-
128-
informerConfig.followControllerNamespacesOnChange(
129-
configAnnotation.informerConfig().followControllerNamespacesOnChange());
130-
131-
var primaryToSecondaryMapper =
132-
Utils.instantiate(configAnnotation.informerConfig().primaryToSecondaryMapper(),
133-
PrimaryToSecondaryMapper.class, context);
134-
informerConfig.withPrimaryToSecondaryMapper(primaryToSecondaryMapper);
135-
136-
var secondaryToPrimaryMapper =
137-
Utils.instantiate(configAnnotation.informerConfig().secondaryToPrimaryMapper(),
138-
SecondaryToPrimaryMapper.class, context);
139-
if (secondaryToPrimaryMapper != null) {
140-
informerConfig.withSecondaryToPrimaryMapper(secondaryToPrimaryMapper);
141-
}
77+
informerConfig = InformerConfiguration.from(
78+
GroupVersionKind.fromString(configAnnotation.informerConfig().groupVersionKind()),
79+
controllerConfig.getResourceClass());
80+
} else {
81+
informerConfig =
82+
InformerConfiguration.from(resourceType, controllerConfig.getResourceClass());
83+
}
84+
85+
if (configAnnotation != null && configAnnotation.informerConfig() != null) {
86+
87+
if (!Constants.NO_VALUE_SET.equals(configAnnotation.informerConfig().name())) {
88+
informerConfig.withName(configAnnotation.informerConfig().name());
89+
} else if (spec.getName() != null && !Constants.NO_VALUE_SET.equals(spec.getName())) {
90+
informerConfig.withName(spec.getName());
14291
} else {
143-
getSecondaryToPrimaryMapper(dependentResourceClass,
144-
controllerConfig.getResourceClass())
145-
.ifPresentOrElse(informerConfig::withSecondaryToPrimaryMapper, () -> {
146-
if (spec.getUseEventSourceWithName().isEmpty()) {
147-
log.warn("No SecondaryToPrimaryMapper going to be set for dependent resource." +
148-
" This might be an issue with the setup of the dependent resource");
149-
}
150-
});
92+
informerConfig.withName(DependentResource.defaultNameFor(dependentResourceClass));
15193
}
15294

95+
var namespaces = Set.of(configAnnotation.informerConfig().namespaces());
96+
informerConfig.withNamespaces(namespaces);
15397

98+
final var fromAnnotation = configAnnotation.informerConfig().labelSelector();
99+
var labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation;
100+
informerConfig.withLabelSelector(labelSelector);
154101

155-
return informerConfig.build();
156-
} catch (InstantiationException | IllegalAccessException | InvocationTargetException
157-
| NoSuchMethodException e) {
158-
throw new RuntimeException(e);
159-
}
160-
}
102+
final var context = Utils.contextFor(controllerConfig, dependentResourceClass,
103+
configAnnotation.annotationType());
104+
105+
var onAddFilter = Utils.instantiate(configAnnotation.informerConfig().onAddFilter(),
106+
OnAddFilter.class, context);
107+
informerConfig.withOnAddFilter((OnAddFilter<? super R>) onAddFilter);
161108

109+
var onUpdateFilter =
110+
Utils.instantiate(configAnnotation.informerConfig().onUpdateFilter(),
111+
OnUpdateFilter.class, context);
112+
informerConfig.withOnUpdateFilter((OnUpdateFilter<? super R>) onUpdateFilter);
162113

114+
var onDeleteFilter =
115+
Utils.instantiate(configAnnotation.informerConfig().onDeleteFilter(),
116+
OnDeleteFilter.class, context);
117+
informerConfig.withOnDeleteFilter((OnDeleteFilter<? super R>) onDeleteFilter);
163118

119+
var genericFilter =
120+
Utils.instantiate(configAnnotation.informerConfig().genericFilter(),
121+
GenericFilter.class,
122+
context);
123+
124+
informerConfig.withGenericFilter((GenericFilter<? super R>) genericFilter);
125+
126+
informerConfig.followControllerNamespacesOnChange(
127+
configAnnotation.informerConfig().followControllerNamespacesOnChange());
128+
129+
var primaryToSecondaryMapper =
130+
Utils.instantiate(configAnnotation.informerConfig().primaryToSecondaryMapper(),
131+
PrimaryToSecondaryMapper.class, context);
132+
informerConfig.withPrimaryToSecondaryMapper(primaryToSecondaryMapper);
133+
134+
var secondaryToPrimaryMapper =
135+
Utils.instantiate(configAnnotation.informerConfig().secondaryToPrimaryMapper(),
136+
SecondaryToPrimaryMapper.class, context);
137+
if (secondaryToPrimaryMapper != null) {
138+
informerConfig.withSecondaryToPrimaryMapper(secondaryToPrimaryMapper);
139+
}
140+
} else {
141+
getSecondaryToPrimaryMapper(dependentResourceClass,
142+
controllerConfig.getResourceClass())
143+
.ifPresentOrElse(informerConfig::withSecondaryToPrimaryMapper, () -> {
144+
if (spec.getUseEventSourceWithName().isEmpty()) {
145+
log.warn("No SecondaryToPrimaryMapper going to be set for dependent resource." +
146+
" This might be an issue with the setup of the dependent resource");
147+
}
148+
});
149+
}
150+
return informerConfig.build();
151+
}
164152
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@
2323
import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected;
2424
import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
2525
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
26-
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.*;
26+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.InformerConfig;
27+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
28+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
29+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
30+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfigBuilder;
2731
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
2832
import io.javaoperatorsdk.operator.processing.event.source.informer.Mappers;
2933

@@ -74,11 +78,12 @@ private KubernetesDependentResourceConfig extractFirstDependentKubernetesResourc
7478
configuration, 0);
7579
}
7680

77-
private Object extractDependentKubernetesResourceConfig(
81+
private static Object extractDependentKubernetesResourceConfig(
7882
io.javaoperatorsdk.operator.api.config.ControllerConfiguration<?> configuration, int index) {
7983
final var spec =
8084
configuration.getWorkflowSpec().orElseThrow().getDependentResourceSpecs().get(index);
81-
return DependentResourceConfigurationResolver.configurationFor(spec, configuration);
85+
return DependentResourceConfigurationResolver.configurationFor(spec, configuration,
86+
Utils.instantiate(spec.getDependentResourceClass(), DependentResource.class, null));
8287
}
8388

8489
private io.javaoperatorsdk.operator.api.config.ControllerConfiguration<?> createConfiguration(
@@ -292,8 +297,7 @@ void replaceNamedDependentResourceConfigShouldWork() {
292297
.filter(dr -> dr.getName().equals(dependentResourceName))
293298
.findFirst().orElseThrow();
294299
assertEquals(ReadOnlyDependent.class, dependentSpec.getDependentResourceClass());
295-
var maybeConfig =
296-
DependentResourceConfigurationResolver.configurationFor(dependentSpec, configuration);
300+
var maybeConfig = extractFirstDependentKubernetesResourceConfig(configuration);
297301
assertNotNull(maybeConfig);
298302
assertInstanceOf(KubernetesDependentResourceConfig.class, maybeConfig);
299303

@@ -323,7 +327,8 @@ void replaceNamedDependentResourceConfigShouldWork() {
323327
dependentSpec = dependents.stream().filter(dr -> dr.getName().equals(dependentResourceName))
324328
.findFirst().orElseThrow();
325329
config = (KubernetesDependentResourceConfig) DependentResourceConfigurationResolver
326-
.configurationFor(dependentSpec, overridden);
330+
.configurationFor(dependentSpec, overridden, Utils
331+
.instantiate(dependentSpec.getDependentResourceClass(), DependentResource.class, null));
327332
assertEquals(1, config.informerConfiguration().getNamespaces().size());
328333
assertEquals(labelSelector, config.informerConfiguration().getLabelSelector());
329334
assertEquals(Set.of(overriddenNS), config.informerConfiguration().getNamespaces());

0 commit comments

Comments
 (0)