Skip to content

Commit 59f1569

Browse files
metacosmmetacosm
authored
fix: namespaces overriding should propagate to dependents if needed (#1124)
* fix: record if namespaces were configured on dependent * fix: revert more specific generics in getDependentResources The issue here is that implies that all dependent resources are supposed to be parameterized the same way, which isn't the case. * fix: make sure all dependents are added when overriding namespaces Fixes #1122
1 parent 62daa4a commit 59f1569

File tree

9 files changed

+390
-58
lines changed

9 files changed

+390
-58
lines changed

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

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
package io.javaoperatorsdk.operator.api.config;
22

33
import java.time.Duration;
4-
import java.util.*;
4+
import java.util.Arrays;
5+
import java.util.Collections;
6+
import java.util.LinkedHashMap;
7+
import java.util.List;
8+
import java.util.Optional;
9+
import java.util.Set;
510
import java.util.function.Function;
11+
import java.util.stream.Collectors;
612

713
import io.fabric8.kubernetes.api.model.HasMetadata;
814
import io.javaoperatorsdk.operator.ReconcilerUtils;
@@ -17,12 +23,13 @@
1723
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
1824
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
1925

26+
@SuppressWarnings("rawtypes")
2027
public class AnnotationControllerConfiguration<R extends HasMetadata>
2128
implements io.javaoperatorsdk.operator.api.config.ControllerConfiguration<R> {
2229

2330
protected final Reconciler<R> reconciler;
2431
private final ControllerConfiguration annotation;
25-
private List<DependentResourceSpec<?, ?>> specs;
32+
private List<DependentResourceSpec> specs;
2633

2734
public AnnotationControllerConfiguration(Reconciler<R> reconciler) {
2835
this.reconciler = reconciler;
@@ -131,43 +138,61 @@ public static <T> T valueOrDefault(
131138

132139
@SuppressWarnings({"rawtypes", "unchecked"})
133140
@Override
134-
public List<DependentResourceSpec<?, ?>> getDependentResources() {
141+
public List<DependentResourceSpec> getDependentResources() {
135142
if (specs == null) {
136143
final var dependents =
137144
valueOrDefault(annotation, ControllerConfiguration::dependents, new Dependent[] {});
138145
if (dependents.length == 0) {
139-
return Collections.emptyList();
146+
specs = Collections.emptyList();
147+
return specs;
140148
}
141149

142-
specs = new ArrayList<>(dependents.length);
150+
final var specsMap = new LinkedHashMap<String, DependentResourceSpec>(dependents.length);
143151
for (Dependent dependent : dependents) {
144152
Object config = null;
145153
final Class<? extends DependentResource> dependentType = dependent.type();
146154
if (KubernetesDependentResource.class.isAssignableFrom(dependentType)) {
147-
final var kubeDependent = dependentType.getAnnotation(KubernetesDependent.class);
148-
final var namespaces =
149-
Utils.valueOrDefault(
150-
kubeDependent,
151-
KubernetesDependent::namespaces,
152-
this.getNamespaces().toArray(new String[0]));
153-
final var labelSelector =
154-
Utils.valueOrDefault(kubeDependent, KubernetesDependent::labelSelector, null);
155-
config =
156-
new KubernetesDependentResourceConfig(namespaces, labelSelector);
155+
config = createKubernetesResourceConfig(dependentType);
157156
}
158-
var name = dependent.name();
159-
if (name.isBlank()) {
160-
name = DependentResource.defaultNameFor(dependentType);
161-
}
162-
for (var spec : specs) {
163-
if (spec.getName().equals(name)) {
164-
throw new IllegalArgumentException(
165-
"A DependentResource named: " + name + " already exists: " + spec);
166-
}
157+
158+
final var name = getName(dependent, dependentType);
159+
final var spec = specsMap.get(name);
160+
if (spec != null) {
161+
throw new IllegalArgumentException(
162+
"A DependentResource named: " + name + " already exists: " + spec);
167163
}
168-
specs.add(new DependentResourceSpec(dependentType, config, name));
164+
specsMap.put(name, new DependentResourceSpec(dependentType, config, name));
169165
}
166+
167+
specs = specsMap.values().stream().collect(Collectors.toUnmodifiableList());
168+
}
169+
return specs;
170+
}
171+
172+
private String getName(Dependent dependent, Class<? extends DependentResource> dependentType) {
173+
var name = dependent.name();
174+
if (name.isBlank()) {
175+
name = DependentResource.defaultNameFor(dependentType);
176+
}
177+
return name;
178+
}
179+
180+
private Object createKubernetesResourceConfig(Class<? extends DependentResource> dependentType) {
181+
Object config;
182+
final var kubeDependent = dependentType.getAnnotation(KubernetesDependent.class);
183+
184+
var namespaces = getNamespaces();
185+
var configuredNS = false;
186+
if (kubeDependent != null && !Arrays.equals(KubernetesDependent.DEFAULT_NAMESPACES,
187+
kubeDependent.namespaces())) {
188+
namespaces = Set.of(kubeDependent.namespaces());
189+
configuredNS = true;
170190
}
171-
return Collections.unmodifiableList(specs);
191+
192+
final var labelSelector =
193+
Utils.valueOrDefault(kubeDependent, KubernetesDependent::labelSelector, null);
194+
config =
195+
new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS);
196+
return config;
172197
}
173198
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,8 @@ default ResourceEventFilter<R> getEventFilter() {
4646
return ResourceEventFilters.passthrough();
4747
}
4848

49-
default List<DependentResourceSpec<?, ?>> getDependentResources() {
49+
@SuppressWarnings("rawtypes")
50+
default List<DependentResourceSpec> getDependentResources() {
5051
return Collections.emptyList();
5152
}
5253

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

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,17 @@
44
import java.util.HashSet;
55
import java.util.LinkedHashMap;
66
import java.util.List;
7+
import java.util.Optional;
78
import java.util.Set;
9+
import java.util.function.Predicate;
810
import java.util.stream.Collectors;
911

1012
import io.fabric8.kubernetes.api.model.HasMetadata;
1113
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
14+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
1215
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
1316

14-
@SuppressWarnings({"unused"})
17+
@SuppressWarnings({"rawtypes", "unused"})
1518
public class ControllerConfigurationOverrider<R extends HasMetadata> {
1619

1720
private String finalizer;
@@ -22,7 +25,7 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
2225
private ResourceEventFilter<R> customResourcePredicate;
2326
private final ControllerConfiguration<R> original;
2427
private Duration reconciliationMaxInterval;
25-
private final LinkedHashMap<String, DependentResourceSpec<?, ?>> namedDependentResourceSpecs;
28+
private final LinkedHashMap<String, DependentResourceSpec> namedDependentResourceSpecs;
2629

2730
private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
2831
finalizer = original.getFinalizerName();
@@ -95,16 +98,39 @@ public ControllerConfigurationOverrider<R> withReconciliationMaxInterval(
9598
public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig(String name,
9699
Object dependentResourceConfig) {
97100

98-
var namedRDS = namedDependentResourceSpecs.get(name);
99-
if (namedRDS == null) {
101+
var current = namedDependentResourceSpecs.get(name);
102+
if (current == null) {
100103
throw new IllegalArgumentException("Cannot find a DependentResource named: " + name);
101104
}
102-
namedDependentResourceSpecs.put(name, new DependentResourceSpec<>(
103-
namedRDS.getDependentResourceClass(), dependentResourceConfig, name));
105+
replaceConfig(name, dependentResourceConfig, current);
104106
return this;
105107
}
106108

109+
private void replaceConfig(String name, Object newConfig, DependentResourceSpec<?, ?> current) {
110+
namedDependentResourceSpecs.put(name,
111+
new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name));
112+
}
113+
114+
@SuppressWarnings("unchecked")
107115
public ControllerConfiguration<R> build() {
116+
// propagate namespaces if needed
117+
final List<DependentResourceSpec> newDependentSpecs;
118+
final var hasModifiedNamespaces = !original.getNamespaces().equals(namespaces);
119+
newDependentSpecs = namedDependentResourceSpecs.entrySet().stream()
120+
.map(drsEntry -> {
121+
final var spec = drsEntry.getValue();
122+
123+
// if the spec has a config and it's a KubernetesDependentResourceConfig, update the
124+
// namespaces if needed, otherwise, just return the existing spec
125+
final Optional<?> maybeConfig = spec.getDependentResourceConfiguration();
126+
final Class<?> drClass = drsEntry.getValue().getDependentResourceClass();
127+
return maybeConfig.filter(KubernetesDependentResourceConfig.class::isInstance)
128+
.map(KubernetesDependentResourceConfig.class::cast)
129+
.filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured))
130+
.map(c -> updateSpec(drsEntry.getKey(), drClass, c))
131+
.orElse(drsEntry.getValue());
132+
}).collect(Collectors.toUnmodifiableList());
133+
108134
return new DefaultControllerConfiguration<>(
109135
original.getAssociatedReconcilerClassName(),
110136
original.getName(),
@@ -117,7 +143,13 @@ public ControllerConfiguration<R> build() {
117143
customResourcePredicate,
118144
original.getResourceClass(),
119145
reconciliationMaxInterval,
120-
namedDependentResourceSpecs.values().stream().collect(Collectors.toUnmodifiableList()));
146+
newDependentSpecs);
147+
}
148+
149+
@SuppressWarnings({"rawtypes", "unchecked"})
150+
private DependentResourceSpec<?, ?> updateSpec(String name, Class<?> drClass,
151+
KubernetesDependentResourceConfig c) {
152+
return new DependentResourceSpec(drClass, c.setNamespaces(namespaces), name);
121153
}
122154

123155
public static <R extends HasMetadata> ControllerConfigurationOverrider<R> override(

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
package io.javaoperatorsdk.operator.api.config;
22

33
import java.time.Duration;
4-
import java.util.*;
4+
import java.util.Collections;
5+
import java.util.List;
6+
import java.util.Optional;
7+
import java.util.Set;
58

69
import io.fabric8.kubernetes.api.model.HasMetadata;
710
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
811
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
912

13+
@SuppressWarnings("rawtypes")
1014
public class DefaultControllerConfiguration<R extends HasMetadata>
1115
extends DefaultResourceConfiguration<R>
1216
implements ControllerConfiguration<R> {
@@ -18,7 +22,7 @@ public class DefaultControllerConfiguration<R extends HasMetadata>
1822
private final boolean generationAware;
1923
private final RetryConfiguration retryConfiguration;
2024
private final ResourceEventFilter<R> resourceEventFilter;
21-
private final List<DependentResourceSpec<?, ?>> dependents;
25+
private final List<DependentResourceSpec> dependents;
2226
private final Duration reconciliationMaxInterval;
2327

2428
// NOSONAR constructor is meant to provide all information
@@ -34,7 +38,7 @@ public DefaultControllerConfiguration(
3438
ResourceEventFilter<R> resourceEventFilter,
3539
Class<R> resourceClass,
3640
Duration reconciliationMaxInterval,
37-
List<DependentResourceSpec<?, ?>> dependents) {
41+
List<DependentResourceSpec> dependents) {
3842
super(labelSelector, resourceClass, namespaces);
3943
this.associatedControllerClassName = associatedControllerClassName;
4044
this.name = name;
@@ -87,7 +91,7 @@ public ResourceEventFilter<R> getEventFilter() {
8791
}
8892

8993
@Override
90-
public List<DependentResourceSpec<?, ?>> getDependentResources() {
94+
public List<DependentResourceSpec> getDependentResources() {
9195
return dependents;
9296
}
9397

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
@Retention(RetentionPolicy.RUNTIME)
1111
@Target({ElementType.TYPE})
1212
public @interface KubernetesDependent {
13+
String SAME_AS_PARENT = "JOSDK_SAME_AS_PARENT";
14+
String WATCH_ALL_NAMESPACES = "JOSDK_ALL_NAMESPACES";
15+
16+
String[] DEFAULT_NAMESPACES = {SAME_AS_PARENT};
1317

1418
/**
1519
* Specified which namespaces this Controller monitors for custom resources events. If no
@@ -18,7 +22,7 @@
1822
*
1923
* @return the list of namespaces this controller monitors
2024
*/
21-
String[] namespaces() default {};
25+
String[] namespaces() default {SAME_AS_PARENT};
2226

2327
/**
2428
* Optional label selector used to identify the set of custom resources the controller will acc

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public KubernetesDependentResource(Class<R> resourceType) {
5050

5151
@Override
5252
public void configureWith(KubernetesDependentResourceConfig config) {
53-
configureWith(config.labelSelector(), Set.of(config.namespaces()));
53+
configureWith(config.labelSelector(), config.namespaces());
5454
}
5555

5656
@SuppressWarnings("unchecked")

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

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

3+
import java.util.Collections;
4+
import java.util.Set;
5+
36
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
47

58
public class KubernetesDependentResourceConfig {
69

7-
private String[] namespaces = new String[0];
10+
private Set<String> namespaces = Collections.emptySet();
811
private String labelSelector = NO_VALUE_SET;
912

13+
private boolean namespacesWereConfigured = false;
14+
1015
public KubernetesDependentResourceConfig() {}
1116

12-
public KubernetesDependentResourceConfig(String[] namespaces,
13-
String labelSelector) {
17+
public KubernetesDependentResourceConfig(Set<String> namespaces, String labelSelector,
18+
boolean configuredNS) {
1419
this.namespaces = namespaces;
1520
this.labelSelector = labelSelector;
21+
namespacesWereConfigured = configuredNS;
22+
}
23+
24+
public KubernetesDependentResourceConfig(Set<String> namespaces, String labelSelector) {
25+
this(namespaces, labelSelector, true);
1626
}
1727

18-
public KubernetesDependentResourceConfig setNamespaces(String[] namespaces) {
28+
public KubernetesDependentResourceConfig setNamespaces(Set<String> namespaces) {
29+
this.namespacesWereConfigured = true;
1930
this.namespaces = namespaces;
2031
return this;
2132
}
@@ -25,11 +36,19 @@ public KubernetesDependentResourceConfig setLabelSelector(String labelSelector)
2536
return this;
2637
}
2738

28-
public String[] namespaces() {
29-
return namespaces;
39+
public Set<String> namespaces() {
40+
if (!namespaces.contains(KubernetesDependent.WATCH_ALL_NAMESPACES)) {
41+
return namespaces;
42+
} else {
43+
return Collections.emptySet();
44+
}
3045
}
3146

3247
public String labelSelector() {
3348
return labelSelector;
3449
}
50+
51+
public boolean wereNamespacesConfigured() {
52+
return namespacesWereConfigured;
53+
}
3554
}

0 commit comments

Comments
 (0)