Skip to content

Commit eb4e720

Browse files
committed
fix: retrieve DependentResource based on name instead of class
This causes issues if the class is proxied for any reason (e.g. because it's a CDI bean).
1 parent 8078fec commit eb4e720

File tree

6 files changed

+66
-41
lines changed

6 files changed

+66
-41
lines changed

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,10 @@ public interface DependentResource<R, P extends HasMetadata> {
99
ReconcileResult<R> reconcile(P primary, Context<P> context);
1010

1111
Optional<R> getResource(P primaryResource);
12+
13+
Class<R> resourceType();
14+
15+
default String name() {
16+
return getClass().getCanonicalName();
17+
}
1218
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
package io.javaoperatorsdk.operator.api.reconciler.dependent.managed;
22

3-
import java.util.Collections;
4-
import java.util.List;
3+
import java.util.Map;
54
import java.util.Optional;
65
import java.util.concurrent.ConcurrentHashMap;
7-
import java.util.stream.Collectors;
86

97
import io.javaoperatorsdk.operator.OperatorException;
108
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
@@ -16,7 +14,7 @@
1614
@SuppressWarnings("rawtypes")
1715
public class ManagedDependentResourceContext {
1816

19-
private final List<DependentResource> dependentResources;
17+
private final Map<String, DependentResource> dependentResources;
2018
private final ConcurrentHashMap attributes = new ConcurrentHashMap();
2119

2220
/**
@@ -70,43 +68,42 @@ public <T> T getMandatory(Object key, Class<T> expectedType) {
7068
+ ") is missing or not of the expected type"));
7169
}
7270

73-
public ManagedDependentResourceContext(List<DependentResource> dependentResources) {
74-
this.dependentResources = Collections.unmodifiableList(dependentResources);
71+
public ManagedDependentResourceContext(Map<String, DependentResource> dependentResources) {
72+
this.dependentResources = dependentResources;
7573
}
7674

7775
/**
7876
* Retrieve all the known {@link DependentResource} implementations
7977
*
80-
* @return a list of known {@link DependentResource} implementations
78+
* @return known {@link DependentResource} implementations keyed by their name
8179
*/
82-
public List<DependentResource> getDependentResources() {
80+
public Map<String, DependentResource> getDependentResources() {
8381
return dependentResources;
8482
}
8583

8684
/**
8785
* Retrieve the dependent resource implementation associated with the specified resource type.
8886
*
89-
* @param resourceClass the dependent resource class for which we want to retrieve the associated
90-
* dependent resource implementation
87+
* @param name the name of the {@link DependentResource} implementation we want to retrieve
88+
* @param resourceClass the resource class for which we want to retrieve the associated dependent
89+
* resource implementation
9190
* @param <T> the type of the resources for which we want to retrieve the associated dependent
9291
* resource implementation
9392
* @return the associated dependent resource implementation if it exists or an exception if it
9493
* doesn't or several implementations are associated with the specified resource type
9594
*/
96-
@SuppressWarnings("unchecked")
97-
public <T extends DependentResource> T getDependentResource(Class<T> resourceClass) {
98-
var resourceList =
99-
dependentResources.stream()
100-
.filter(dr -> dr.getClass().equals(resourceClass))
101-
.collect(Collectors.toList());
102-
if (resourceList.isEmpty()) {
103-
throw new OperatorException(
104-
"No dependent resource found for class: " + resourceClass.getName());
95+
@SuppressWarnings({"unchecked"})
96+
public <T> DependentResource<T, ?> getDependentResource(String name, Class<T> resourceClass) {
97+
var dependentResource = dependentResources.get(name);
98+
if (dependentResource == null) {
99+
throw new OperatorException("No dependent resource found with name: " + name);
105100
}
106-
if (resourceList.size() > 1) {
101+
final var actual = dependentResource.resourceType();
102+
if (!actual.equals(resourceClass)) {
107103
throw new OperatorException(
108-
"More than one dependent resource found for class: " + resourceClass.getName());
104+
"Dependent resource implementation doesn't match expected resource type, was: "
105+
+ actual.getName() + " instead of: " + resourceClass.getName());
109106
}
110-
return (T) resourceList.get(0);
107+
return dependentResource;
111108
}
112109
}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java

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

3+
import java.util.Collections;
34
import java.util.LinkedList;
45
import java.util.List;
6+
import java.util.Map;
57
import java.util.Optional;
8+
import java.util.function.Function;
69
import java.util.stream.Collectors;
710

811
import org.slf4j.Logger;
@@ -51,7 +54,7 @@ public class Controller<P extends HasMetadata> implements Reconciler<P>, Cleaner
5154
private final ControllerConfiguration<P> configuration;
5255
private final KubernetesClient kubernetesClient;
5356
private final EventSourceManager<P> eventSourceManager;
54-
private final List<DependentResource> dependents;
57+
private final Map<String, DependentResource> dependents;
5558
private final boolean contextInitializer;
5659
private final boolean hasDeleterDependents;
5760
private final boolean isCleaner;
@@ -71,15 +74,16 @@ public Controller(Reconciler<P> reconciler,
7174
eventSourceManager = new EventSourceManager<>(this);
7275

7376
final var hasDeleterHolder = new boolean[] {false};
74-
dependents = configuration.getDependentResources().stream()
75-
.map(drs -> createAndConfigureFrom(drs, kubernetesClient))
76-
.peek(d -> {
77-
// check if any dependent implements Deleter to record that fact
78-
if (!hasDeleterHolder[0] && d instanceof Deleter) {
79-
hasDeleterHolder[0] = true;
80-
}
81-
})
82-
.collect(Collectors.toList());
77+
dependents = Collections.unmodifiableMap(
78+
configuration.getDependentResources().stream()
79+
.map(drs -> createAndConfigureFrom(drs, kubernetesClient))
80+
.peek(d -> {
81+
// check if any dependent implements Deleter to record that fact
82+
if (!hasDeleterHolder[0] && d instanceof Deleter) {
83+
hasDeleterHolder[0] = true;
84+
}
85+
})
86+
.collect(Collectors.toMap(DependentResource::name, Function.identity())));
8387

8488
hasDeleterDependents = hasDeleterHolder[0];
8589
isCleaner = reconciler instanceof Cleaner;
@@ -133,7 +137,7 @@ public String successTypeName(DeleteControl deleteControl) {
133137
public DeleteControl execute() {
134138
initContextIfNeeded(resource, context);
135139
if (hasDeleterDependents) {
136-
dependents.stream()
140+
dependents.values().stream()
137141
.filter(d -> d instanceof Deleter)
138142
.map(Deleter.class::cast)
139143
.forEach(deleter -> deleter.delete(resource, context));
@@ -179,7 +183,7 @@ public String successTypeName(UpdateControl<P> result) {
179183
@Override
180184
public UpdateControl<P> execute() throws Exception {
181185
initContextIfNeeded(resource, context);
182-
dependents.forEach(dependent -> dependent.reconcile(resource, context));
186+
dependents.values().forEach(dependent -> dependent.reconcile(resource, context));
183187
return reconciler.reconcile(resource, context);
184188
}
185189
});
@@ -188,7 +192,7 @@ public UpdateControl<P> execute() throws Exception {
188192
@Override
189193
public List<EventSource> prepareEventSources(EventSourceContext<P> context) {
190194
List<EventSource> sources = new LinkedList<>();
191-
dependents.stream()
195+
dependents.values().stream()
192196
.filter(dependentResource -> dependentResource instanceof EventSourceProvider)
193197
.map(EventSourceProvider.class::cast)
194198
.map(provider -> provider.initEventSource(context))
@@ -330,7 +334,7 @@ public boolean useFinalizer() {
330334
}
331335

332336
@SuppressWarnings("rawtypes")
333-
public List<DependentResource> getDependents() {
337+
public Map<String, DependentResource> getDependents() {
334338
return dependents;
335339
}
336340
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ public ReconcileResult<Deployment> reconcile(TestCustomResource primary,
101101
public Optional<Deployment> getResource(TestCustomResource primaryResource) {
102102
return Optional.empty();
103103
}
104+
105+
@Override
106+
public Class<Deployment> resourceType() {
107+
return Deployment.class;
108+
}
109+
104110
}
105111

106112
public static class TestKubernetesDependentResource

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractSimpleDependentResourceTest.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,11 @@
1818
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
1919

2020
import static org.assertj.core.api.Assertions.assertThat;
21-
import static org.mockito.Mockito.*;
21+
import static org.mockito.Mockito.any;
22+
import static org.mockito.Mockito.mock;
23+
import static org.mockito.Mockito.times;
24+
import static org.mockito.Mockito.verify;
25+
import static org.mockito.Mockito.when;
2226

2327
@SuppressWarnings("unchecked")
2428
class AbstractSimpleDependentResourceTest {
@@ -136,5 +140,10 @@ protected SampleExternalResource desired(TestCustomResource primary,
136140
Context<TestCustomResource> context) {
137141
return SampleExternalResource.testResource1();
138142
}
143+
144+
@Override
145+
public Class<SampleExternalResource> resourceType() {
146+
return SampleExternalResource.class;
147+
}
139148
}
140149
}

sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@
44
import org.slf4j.LoggerFactory;
55

66
import io.fabric8.kubernetes.api.model.Secret;
7-
import io.javaoperatorsdk.operator.api.reconciler.*;
7+
import io.javaoperatorsdk.operator.api.reconciler.Context;
8+
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
9+
import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusHandler;
10+
import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl;
11+
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
12+
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
813
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
914
import io.javaoperatorsdk.operator.sample.dependent.SchemaDependentResource;
1015
import io.javaoperatorsdk.operator.sample.dependent.SecretDependentResource;
@@ -30,9 +35,7 @@ public UpdateControl<MySQLSchema> reconcile(MySQLSchema schema, Context<MySQLSch
3035
// we only need to update the status if we just built the schema, i.e. when it's present in the
3136
// context
3237
Secret secret = context.getSecondaryResource(Secret.class).orElseThrow();
33-
SchemaDependentResource schemaDependentResource = context.managedDependentResourceContext()
34-
.getDependentResource(SchemaDependentResource.class);
35-
return schemaDependentResource.fetchResource(schema).map(s -> {
38+
return context.getSecondaryResource(MySQLSchema.class).map(s -> {
3639
updateStatusPojo(schema, secret.getMetadata().getName(),
3740
secret.getData().get(MYSQL_SECRET_USERNAME));
3841
log.info("Schema {} created - updating CR status", schema.getMetadata().getName());

0 commit comments

Comments
 (0)