Skip to content

Commit 457fb69

Browse files
metacosmcsviri
metacosm
authored and
csviri
committed
feat: introduce AnnotationDependentResourceConfigurator concept (#1554)
The idea is to be able to configure dependents using annotations directly so that we can remove the special handling of KubernetesDependentResource from AnnotationControllerConfiguration. This results in dependent resources being instantiated and configured directly when processed from the annotations in the managed case, thus rendering the DependentResourceFactory concept obsolete. This should also lead to further simplification later.
1 parent e83772f commit 457fb69

File tree

19 files changed

+257
-179
lines changed

19 files changed

+257
-179
lines changed

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

Lines changed: 27 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import java.lang.annotation.Annotation;
44
import java.time.Duration;
5-
import java.util.Arrays;
65
import java.util.Collections;
76
import java.util.LinkedHashMap;
87
import java.util.List;
@@ -15,20 +14,18 @@
1514
import io.javaoperatorsdk.operator.OperatorException;
1615
import io.javaoperatorsdk.operator.ReconcilerUtils;
1716
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
18-
import io.javaoperatorsdk.operator.api.reconciler.*;
17+
import io.javaoperatorsdk.operator.api.reconciler.Constants;
1918
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
19+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
2020
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
2121
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
22-
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
23-
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
24-
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
22+
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.AnnotationDependentResourceConfigurator;
2523
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
2624
import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
2725
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
2826
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
2927
import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
3028
import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
31-
import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter;
3229
import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
3330
import io.javaoperatorsdk.operator.processing.retry.Retry;
3431

@@ -158,7 +155,7 @@ public Retry getRetry() {
158155

159156

160157
@SuppressWarnings("unchecked")
161-
private <T> void configureFromAnnotatedReconciler(T instance) {
158+
private void configureFromAnnotatedReconciler(Object instance) {
162159
if (instance instanceof AnnotationConfigurable) {
163160
AnnotationConfigurable configurable = (AnnotationConfigurable) instance;
164161
final Class<? extends Annotation> configurationClass =
@@ -171,6 +168,22 @@ private <T> void configureFromAnnotatedReconciler(T instance) {
171168
}
172169
}
173170

171+
@SuppressWarnings("unchecked")
172+
private void configureFromCustomAnnotation(Object instance) {
173+
if (instance instanceof AnnotationDependentResourceConfigurator) {
174+
AnnotationDependentResourceConfigurator configurator =
175+
(AnnotationDependentResourceConfigurator) instance;
176+
final Class<? extends Annotation> configurationClass =
177+
(Class<? extends Annotation>) Utils.getFirstTypeArgumentFromInterface(
178+
instance.getClass(), AnnotationDependentResourceConfigurator.class);
179+
final var configAnnotation = instance.getClass().getAnnotation(configurationClass);
180+
// always called even if the annotation is null so that implementations can provide default
181+
// values
182+
final var config = configurator.configFrom(configAnnotation, this);
183+
configurator.configureWith(config);
184+
}
185+
}
186+
174187
@Override
175188
@SuppressWarnings("unchecked")
176189
public Optional<OnAddFilter<P>> onAddFilter() {
@@ -208,22 +221,24 @@ public List<DependentResourceSpec> getDependentResources() {
208221

209222
final var specsMap = new LinkedHashMap<String, DependentResourceSpec>(dependents.length);
210223
for (Dependent dependent : dependents) {
211-
Object config = null;
212224
final Class<? extends DependentResource> dependentType = dependent.type();
213-
if (KubernetesDependentResource.class.isAssignableFrom(dependentType)) {
214-
config = createKubernetesResourceConfig(dependentType);
215-
}
216225

217226
final var name = getName(dependent, dependentType);
218227
var spec = specsMap.get(name);
219228
if (spec != null) {
220229
throw new IllegalArgumentException(
221230
"A DependentResource named '" + name + "' already exists: " + spec);
222231
}
232+
233+
final var dependentResource = Utils.instantiateAndConfigureIfNeeded(dependentType,
234+
DependentResource.class,
235+
Utils.contextFor(this, dependentType, Dependent.class),
236+
this::configureFromCustomAnnotation);
237+
223238
var eventSourceName = dependent.useEventSourceWithName();
224239
eventSourceName = Constants.NO_VALUE_SET.equals(eventSourceName) ? null : eventSourceName;
225240
final var context = Utils.contextFor(this, dependentType, null);
226-
spec = new DependentResourceSpec(dependentType, config, name,
241+
spec = new DependentResourceSpec(dependentResource, name,
227242
Set.of(dependent.dependsOn()),
228243
Utils.instantiate(dependent.readyPostcondition(), Condition.class, context),
229244
Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context),
@@ -245,52 +260,6 @@ private String getName(Dependent dependent, Class<? extends DependentResource> d
245260
return name;
246261
}
247262

248-
@SuppressWarnings({"rawtypes", "unchecked"})
249-
private Object createKubernetesResourceConfig(Class<? extends DependentResource> dependentType) {
250-
251-
Object config;
252-
final var kubeDependent = dependentType.getAnnotation(KubernetesDependent.class);
253-
254-
var namespaces = getNamespaces();
255-
var configuredNS = false;
256-
String labelSelector = null;
257-
OnAddFilter<? extends HasMetadata> onAddFilter = null;
258-
OnUpdateFilter<? extends HasMetadata> onUpdateFilter = null;
259-
OnDeleteFilter<? extends HasMetadata> onDeleteFilter = null;
260-
GenericFilter<? extends HasMetadata> genericFilter = null;
261-
ResourceDiscriminator<?, ? extends HasMetadata> resourceDiscriminator = null;
262-
if (kubeDependent != null) {
263-
if (!Arrays.equals(KubernetesDependent.DEFAULT_NAMESPACES,
264-
kubeDependent.namespaces())) {
265-
namespaces = Set.of(kubeDependent.namespaces());
266-
configuredNS = true;
267-
}
268-
final var fromAnnotation = kubeDependent.labelSelector();
269-
labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation;
270-
271-
final var context =
272-
Utils.contextFor(this, dependentType, null);
273-
onAddFilter = Utils.instantiate(kubeDependent.onAddFilter(), OnAddFilter.class, context);
274-
onUpdateFilter =
275-
Utils.instantiate(kubeDependent.onUpdateFilter(), OnUpdateFilter.class, context);
276-
onDeleteFilter =
277-
Utils.instantiate(kubeDependent.onDeleteFilter(), OnDeleteFilter.class, context);
278-
genericFilter =
279-
Utils.instantiate(kubeDependent.genericFilter(), GenericFilter.class, context);
280-
281-
resourceDiscriminator =
282-
Utils.instantiate(kubeDependent.resourceDiscriminator(),
283-
ResourceDiscriminator.class, context);
284-
}
285-
286-
config =
287-
new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS,
288-
resourceDiscriminator, onAddFilter,
289-
onUpdateFilter, onDeleteFilter, genericFilter);
290-
291-
return config;
292-
}
293-
294263
public static <T> T valueOrDefault(
295264
ControllerConfiguration controllerConfiguration,
296265
Function<ControllerConfiguration, T> mapper,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,9 @@ default ObjectMapper getObjectMapper() {
140140
return Serialization.jsonMapper();
141141
}
142142

143+
@Deprecated(forRemoval = true)
143144
default DependentResourceFactory dependentResourceFactory() {
144-
return new DependentResourceFactory() {};
145+
return null;
145146
}
146147

147148
default Optional<LeaderElectionConfiguration> getLeaderElectionConfiguration() {

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

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,11 @@
66
import java.util.List;
77
import java.util.Optional;
88
import java.util.Set;
9-
import java.util.function.Predicate;
109
import java.util.stream.Collectors;
1110

1211
import io.fabric8.kubernetes.api.model.HasMetadata;
1312
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
14-
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
13+
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
1514
import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
1615
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
1716
import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
@@ -159,42 +158,39 @@ public ControllerConfigurationOverrider<R> withGenericFilter(GenericFilter<R> ge
159158
return this;
160159
}
161160

161+
@SuppressWarnings("unchecked")
162162
public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig(String name,
163163
Object dependentResourceConfig) {
164164

165165
var current = namedDependentResourceSpecs.get(name);
166166
if (current == null) {
167167
throw new IllegalArgumentException("Cannot find a DependentResource named: " + name);
168168
}
169-
replaceConfig(name, dependentResourceConfig, current);
170-
return this;
171-
}
172169

173-
private void replaceConfig(String name, Object newConfig, DependentResourceSpec<?, ?> current) {
174-
namedDependentResourceSpecs.put(name,
175-
new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name,
176-
current.getDependsOn(), current.getReadyCondition(), current.getReconcileCondition(),
177-
current.getDeletePostCondition(), current.getUseEventSourceWithName().orElse(null)));
170+
var dependentResource = current.getDependentResource();
171+
if (dependentResource instanceof DependentResourceConfigurator) {
172+
var configurator = (DependentResourceConfigurator) dependentResource;
173+
configurator.configureWith(dependentResourceConfig);
174+
}
175+
176+
return this;
178177
}
179178

180-
@SuppressWarnings("unchecked")
181179
public ControllerConfiguration<R> build() {
182-
// propagate namespaces if needed
183-
final List<DependentResourceSpec> newDependentSpecs;
184180
final var hasModifiedNamespaces = !original.getNamespaces().equals(namespaces);
185-
newDependentSpecs = namedDependentResourceSpecs.entrySet().stream()
186-
.map(drsEntry -> {
187-
final var spec = drsEntry.getValue();
188-
189-
// if the spec has a config and it's a KubernetesDependentResourceConfig, update the
190-
// namespaces if needed, otherwise, just return the existing spec
191-
final Optional<?> maybeConfig = spec.getDependentResourceConfiguration();
192-
return maybeConfig.filter(KubernetesDependentResourceConfig.class::isInstance)
193-
.map(KubernetesDependentResourceConfig.class::cast)
194-
.filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured))
195-
.map(c -> updateSpec(drsEntry.getKey(), spec, c))
196-
.orElse(drsEntry.getValue());
197-
}).collect(Collectors.toUnmodifiableList());
181+
final var newDependentSpecs = namedDependentResourceSpecs.values().stream()
182+
.peek(spec -> {
183+
// if the dependent resource has a NamespaceChangeable config
184+
// update the namespaces if needed, otherwise, do nothing
185+
if (hasModifiedNamespaces) {
186+
final Optional<?> maybeConfig = spec.getDependentResourceConfiguration();
187+
maybeConfig
188+
.filter(NamespaceChangeable.class::isInstance)
189+
.map(NamespaceChangeable.class::cast)
190+
.filter(NamespaceChangeable::allowsNamespaceChanges)
191+
.ifPresent(nc -> nc.changeNamespaces(namespaces));
192+
}
193+
}).collect(Collectors.toList());
198194

199195
return new DefaultControllerConfiguration<>(
200196
original.getAssociatedReconcilerClassName(),
@@ -215,15 +211,6 @@ public ControllerConfiguration<R> build() {
215211
newDependentSpecs);
216212
}
217213

218-
@SuppressWarnings({"rawtypes", "unchecked"})
219-
private DependentResourceSpec<?, ?> updateSpec(String name, DependentResourceSpec spec,
220-
KubernetesDependentResourceConfig c) {
221-
return new DependentResourceSpec(spec.getDependentResourceClass(),
222-
c.setNamespaces(namespaces), name, spec.getDependsOn(), spec.getReadyCondition(),
223-
spec.getReconcileCondition(), spec.getDeletePostCondition(),
224-
(String) spec.getUseEventSourceWithName().orElse(null));
225-
}
226-
227214
public static <R extends HasMetadata> ControllerConfigurationOverrider<R> override(
228215
ControllerConfiguration<R> original) {
229216
return new ControllerConfigurationOverrider<>(original);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ public interface NamespaceChangeable {
1616
*/
1717
void changeNamespaces(Set<String> namespaces);
1818

19+
@SuppressWarnings("unused")
1920
default void changeNamespaces(String... namespaces) {
20-
changeNamespaces(
21-
namespaces != null ? Set.of(namespaces) : DEFAULT_NAMESPACES_SET);
21+
changeNamespaces(namespaces != null ? Set.of(namespaces) : DEFAULT_NAMESPACES_SET);
2222
}
2323

2424
default boolean allowsNamespaceChanges() {

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@ public static Class<?> getFirstTypeArgumentFromExtendedClass(Class<?> clazz) {
111111

112112
public static Class<?> getFirstTypeArgumentFromInterface(Class<?> clazz,
113113
Class<?> expectedImplementedInterface) {
114+
return getTypeArgumentFromInterfaceByIndex(clazz, expectedImplementedInterface, 0);
115+
}
116+
117+
public static Class<?> getTypeArgumentFromInterfaceByIndex(Class<?> clazz,
118+
Class<?> expectedImplementedInterface, int index) {
114119
if (expectedImplementedInterface.isAssignableFrom(clazz)) {
115120
final var genericInterfaces = clazz.getGenericInterfaces();
116121
Optional<? extends Class<?>> target = Optional.empty();
@@ -122,7 +127,7 @@ public static Class<?> getFirstTypeArgumentFromInterface(Class<?> clazz,
122127
.map(ParameterizedType.class::cast)
123128
.findFirst()
124129
.map(t -> {
125-
final Type argument = t.getActualTypeArguments()[0];
130+
final Type argument = t.getActualTypeArguments()[index];
126131
if (argument instanceof Class) {
127132
return (Class<?>) argument;
128133
}
@@ -148,7 +153,7 @@ public static Class<?> getFirstTypeArgumentFromInterface(Class<?> clazz,
148153
// try the parent
149154
var parent = clazz.getSuperclass();
150155
if (!Object.class.equals(parent)) {
151-
return getFirstTypeArgumentFromInterface(parent, expectedImplementedInterface);
156+
return getTypeArgumentFromInterfaceByIndex(parent, expectedImplementedInterface, index);
152157
}
153158
}
154159
throw new IllegalArgumentException("Couldn't retrieve generic parameter type from "

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

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
import java.util.Optional;
55
import java.util.Set;
66

7+
import io.fabric8.kubernetes.api.model.HasMetadata;
78
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
9+
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
810
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
911

10-
public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {
12+
public class DependentResourceSpec<R, P extends HasMetadata, C> {
1113

12-
private final Class<T> dependentResourceClass;
13-
14-
private final C dependentResourceConfig;
14+
private final DependentResource<R, P> dependentResource;
1515

1616
private final String name;
1717

@@ -25,12 +25,11 @@ public class DependentResourceSpec<T extends DependentResource<?, ?>, C> {
2525

2626
private final String useEventSourceWithName;
2727

28-
public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourceConfig,
28+
public DependentResourceSpec(DependentResource<R, P> dependentResource,
2929
String name, Set<String> dependsOn, Condition<?, ?> readyCondition,
3030
Condition<?, ?> reconcileCondition, Condition<?, ?> deletePostCondition,
3131
String useEventSourceWithName) {
32-
this.dependentResourceClass = dependentResourceClass;
33-
this.dependentResourceConfig = dependentResourceConfig;
32+
this.dependentResource = dependentResource;
3433
this.name = name;
3534
this.dependsOn = dependsOn;
3635
this.readyCondition = readyCondition;
@@ -39,12 +38,28 @@ public DependentResourceSpec(Class<T> dependentResourceClass, C dependentResourc
3938
this.useEventSourceWithName = useEventSourceWithName;
4039
}
4140

42-
public Class<T> getDependentResourceClass() {
43-
return dependentResourceClass;
41+
public DependentResourceSpec(DependentResourceSpec<R, P, C> other) {
42+
this.dependentResource = other.dependentResource;
43+
this.name = other.name;
44+
this.dependsOn = other.dependsOn;
45+
this.readyCondition = other.readyCondition;
46+
this.reconcileCondition = other.reconcileCondition;
47+
this.deletePostCondition = other.deletePostCondition;
48+
this.useEventSourceWithName = other.useEventSourceWithName;
49+
}
50+
51+
@SuppressWarnings("unchecked")
52+
public Class<DependentResource<R, P>> getDependentResourceClass() {
53+
return (Class<DependentResource<R, P>>) dependentResource.getClass();
4454
}
4555

56+
@SuppressWarnings({"unchecked", "rawtypes"})
4657
public Optional<C> getDependentResourceConfiguration() {
47-
return Optional.ofNullable(dependentResourceConfig);
58+
if (dependentResource instanceof DependentResourceConfigurator) {
59+
var configurator = (DependentResourceConfigurator) dependentResource;
60+
return configurator.configuration();
61+
}
62+
return Optional.empty();
4863
}
4964

5065
public String getName() {
@@ -54,8 +69,7 @@ public String getName() {
5469
@Override
5570
public String toString() {
5671
return "DependentResourceSpec{ name='" + name +
57-
"', type=" + dependentResourceClass.getCanonicalName() +
58-
", config=" + dependentResourceConfig + '}';
72+
"', type=" + getDependentResourceClass().getCanonicalName() + '}';
5973
}
6074

6175
@Override
@@ -66,7 +80,7 @@ public boolean equals(Object o) {
6680
if (o == null || getClass() != o.getClass()) {
6781
return false;
6882
}
69-
DependentResourceSpec<?, ?> that = (DependentResourceSpec<?, ?>) o;
83+
DependentResourceSpec<?, ?, ?> that = (DependentResourceSpec<?, ?, ?>) o;
7084
return name.equals(that.name);
7185
}
7286

@@ -94,6 +108,10 @@ public Condition getDeletePostCondition() {
94108
return deletePostCondition;
95109
}
96110

111+
public DependentResource<R, P> getDependentResource() {
112+
return dependentResource;
113+
}
114+
97115
public Optional<String> getUseEventSourceWithName() {
98116
return Optional.ofNullable(useEventSourceWithName);
99117
}

0 commit comments

Comments
 (0)