Skip to content

Commit 7f67d56

Browse files
csviricsviri
committed
improve: secondaryToPrimary check type of owner reference (#2371)
Signed-off-by: Attila Mészáros <[email protected]>
1 parent 32d9e78 commit 7f67d56

File tree

16 files changed

+134
-62
lines changed

16 files changed

+134
-62
lines changed

‎caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/AbstractTestReconciler.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public List<EventSource> prepareEventSources(
8080
var es = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context)
8181
.withItemStore(boundedItemStore)
8282
.withSecondaryToPrimaryMapper(
83-
Mappers.fromOwnerReference(this instanceof BoundedCacheClusterScopeTestReconciler))
83+
Mappers.fromOwnerReferences(context.getPrimaryResourceClass(),
84+
this instanceof BoundedCacheClusterScopeTestReconciler))
8485
.build(), context);
8586

8687
return List.of(es);

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

+29-23
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,7 @@ protected DefaultInformerConfiguration(String labelSelector,
5050
this.followControllerNamespaceChanges = followControllerNamespaceChanges;
5151
this.groupVersionKind = groupVersionKind;
5252
this.primaryToSecondaryMapper = primaryToSecondaryMapper;
53-
this.secondaryToPrimaryMapper =
54-
Objects.requireNonNullElse(secondaryToPrimaryMapper,
55-
Mappers.fromOwnerReferences(false));
53+
this.secondaryToPrimaryMapper = secondaryToPrimaryMapper;
5654
this.onDeleteFilter = onDeleteFilter;
5755
}
5856

@@ -135,16 +133,24 @@ class InformerConfigurationBuilder<R extends HasMetadata> {
135133
private boolean inheritControllerNamespacesOnChange = false;
136134
private ItemStore<R> itemStore;
137135
private Long informerListLimit;
136+
private final Class<? extends HasMetadata> primaryResourceClass;
138137

139-
private InformerConfigurationBuilder(Class<R> resourceClass) {
140-
this.resourceClass = resourceClass;
141-
this.groupVersionKind = null;
138+
private InformerConfigurationBuilder(Class<R> resourceClass,
139+
Class<? extends HasMetadata> primaryResourceClass) {
140+
this(resourceClass, primaryResourceClass, null);
142141
}
143142

144143
@SuppressWarnings("unchecked")
145-
private InformerConfigurationBuilder(GroupVersionKind groupVersionKind) {
146-
this.resourceClass = (Class<R>) GenericKubernetesResource.class;
144+
private InformerConfigurationBuilder(GroupVersionKind groupVersionKind,
145+
Class<? extends HasMetadata> primaryResourceClass) {
146+
this((Class<R>) GenericKubernetesResource.class, primaryResourceClass, groupVersionKind);
147+
}
148+
149+
private InformerConfigurationBuilder(Class<R> resourceClass,
150+
Class<? extends HasMetadata> primaryResourceClass, GroupVersionKind groupVersionKind) {
151+
this.resourceClass = resourceClass;
147152
this.groupVersionKind = groupVersionKind;
153+
this.primaryResourceClass = primaryResourceClass;
148154
}
149155

150156
public <P extends HasMetadata> InformerConfigurationBuilder<R> withPrimaryToSecondaryMapper(
@@ -264,23 +270,17 @@ public InformerConfigurationBuilder<R> withInformerListLimit(Long informerListLi
264270
public InformerConfiguration<R> build() {
265271
return new DefaultInformerConfiguration<>(labelSelector, resourceClass, groupVersionKind,
266272
primaryToSecondaryMapper,
267-
secondaryToPrimaryMapper,
273+
Objects.requireNonNullElse(secondaryToPrimaryMapper,
274+
Mappers.fromOwnerReferences(HasMetadata.getApiVersion(primaryResourceClass),
275+
HasMetadata.getKind(primaryResourceClass), false)),
268276
namespaces, inheritControllerNamespacesOnChange, onAddFilter, onUpdateFilter,
269277
onDeleteFilter, genericFilter, itemStore, informerListLimit);
270278
}
271279
}
272280

273281
static <R extends HasMetadata> InformerConfigurationBuilder<R> from(
274-
Class<R> resourceClass) {
275-
return new InformerConfigurationBuilder<>(resourceClass);
276-
}
277-
278-
/**
279-
* * For the case when want to use {@link GenericKubernetesResource}
280-
*/
281-
static <R extends HasMetadata> InformerConfigurationBuilder<R> from(
282-
GroupVersionKind groupVersionKind) {
283-
return new InformerConfigurationBuilder<>(groupVersionKind);
282+
Class<R> resourceClass, Class<? extends HasMetadata> primaryResourceClass) {
283+
return new InformerConfigurationBuilder<>(resourceClass, primaryResourceClass);
284284
}
285285

286286
/**
@@ -294,20 +294,26 @@ static <R extends HasMetadata> InformerConfigurationBuilder<R> from(
294294
*/
295295
static <R extends HasMetadata> InformerConfigurationBuilder<R> from(
296296
Class<R> resourceClass, EventSourceContext<?> eventSourceContext) {
297-
return new InformerConfigurationBuilder<>(resourceClass)
297+
return new InformerConfigurationBuilder<>(resourceClass,
298+
eventSourceContext.getPrimaryResourceClass())
298299
.withNamespacesInheritedFromController(eventSourceContext);
299300
}
300301

301302
/**
302-
* * For the case when want to use {@link GenericKubernetesResource}
303+
* For the case when want to use {@link GenericKubernetesResource}
303304
*/
304-
@SuppressWarnings("unchecked")
305305
static InformerConfigurationBuilder<GenericKubernetesResource> from(
306306
GroupVersionKind groupVersionKind, EventSourceContext<?> eventSourceContext) {
307-
return new InformerConfigurationBuilder<GenericKubernetesResource>(groupVersionKind)
307+
return new InformerConfigurationBuilder<GenericKubernetesResource>(groupVersionKind,
308+
eventSourceContext.getPrimaryResourceClass())
308309
.withNamespacesInheritedFromController(eventSourceContext);
309310
}
310311

312+
static InformerConfigurationBuilder<GenericKubernetesResource> from(
313+
GroupVersionKind groupVersionKind, Class<? extends HasMetadata> primaryResourceClass) {
314+
return new InformerConfigurationBuilder<>(groupVersionKind, primaryResourceClass);
315+
}
316+
311317
@SuppressWarnings("unchecked")
312318
@Override
313319
default Class<R> getResourceClass() {

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceContext.java

+8-1
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,16 @@ public class EventSourceContext<P extends HasMetadata> {
1616
private final IndexerResourceCache<P> primaryCache;
1717
private final ControllerConfiguration<P> controllerConfiguration;
1818
private final KubernetesClient client;
19+
private final Class<P> primaryResourceClass;
1920

2021
public EventSourceContext(IndexerResourceCache<P> primaryCache,
2122
ControllerConfiguration<P> controllerConfiguration,
22-
KubernetesClient client) {
23+
KubernetesClient client,
24+
Class<P> primaryResourceClass) {
2325
this.primaryCache = primaryCache;
2426
this.controllerConfiguration = controllerConfiguration;
2527
this.client = client;
28+
this.primaryResourceClass = primaryResourceClass;
2629
}
2730

2831
/**
@@ -54,4 +57,8 @@ public ControllerConfiguration<P> getControllerConfiguration() {
5457
public KubernetesClient getClient() {
5558
return client;
5659
}
60+
61+
public Class<P> getPrimaryResourceClass() {
62+
return primaryResourceClass;
63+
}
5764
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,8 @@ public Controller(Reconciler<P> reconciler,
103103
eventSourceManager.postProcessDefaultEventSourcesAfterProcessorInitializer();
104104
controllerHealthInfo = new ControllerHealthInfo(eventSourceManager);
105105
eventSourceContext = new EventSourceContext<>(
106-
eventSourceManager.getControllerEventSource(), configuration, kubernetesClient);
106+
eventSourceManager.getControllerEventSource(), configuration, kubernetesClient,
107+
configuration.getResourceClass());
107108
initAndRegisterEventSources(eventSourceContext);
108109
configurationService.getMetrics().controllerRegistered(this);
109110
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public GenericKubernetesDependentResource(GroupVersionKind groupVersionKind) {
1717
}
1818

1919
protected InformerConfiguration.InformerConfigurationBuilder<GenericKubernetesResource> informerConfigurationBuilder() {
20-
return InformerConfiguration.from(groupVersionKind);
20+
return InformerConfiguration.from(groupVersionKind, getPrimaryResourceType());
2121
}
2222

2323
@SuppressWarnings("unchecked")

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -91,15 +91,15 @@ private void configureWith(String labelSelector, Set<String> namespaces,
9191

9292
// just to seamlessly handle GenericKubernetesDependentResource
9393
protected InformerConfiguration.InformerConfigurationBuilder<R> informerConfigurationBuilder() {
94-
return InformerConfiguration.from(resourceType());
94+
return InformerConfiguration.from(resourceType(), getPrimaryResourceType());
9595
}
9696

9797
@SuppressWarnings("unchecked")
9898
private SecondaryToPrimaryMapper<R> getSecondaryToPrimaryMapper() {
9999
if (this instanceof SecondaryToPrimaryMapper) {
100100
return (SecondaryToPrimaryMapper<R>) this;
101101
} else if (garbageCollected) {
102-
return Mappers.fromOwnerReferences(clustered);
102+
return Mappers.fromOwnerReferences(getPrimaryResourceType(), clustered);
103103
} else if (useNonOwnerRefBasedSecondaryToPrimaryMapping()) {
104104
return Mappers.fromDefaultAnnotations();
105105
} else {

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ResourceID.java

-14
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,6 @@ public static ResourceID fromResource(HasMetadata resource) {
1414
resource.getMetadata().getNamespace());
1515
}
1616

17-
public static Optional<ResourceID> fromFirstOwnerReference(HasMetadata resource) {
18-
return fromFirstOwnerReference(resource, false);
19-
}
20-
21-
public static Optional<ResourceID> fromFirstOwnerReference(HasMetadata resource,
22-
boolean clusterScoped) {
23-
var ownerReferences = resource.getMetadata().getOwnerReferences();
24-
if (!ownerReferences.isEmpty()) {
25-
return Optional.of(fromOwnerReference(resource, ownerReferences.get(0), clusterScoped));
26-
} else {
27-
return Optional.empty();
28-
}
29-
}
30-
3117
public static ResourceID fromOwnerReference(HasMetadata resource, OwnerReference ownerReference,
3218
boolean clusterScoped) {
3319
return new ResourceID(ownerReference.getName(),

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java

+25-12
Original file line numberDiff line numberDiff line change
@@ -42,24 +42,37 @@ public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromLabel(
4242
return fromMetadata(nameKey, namespaceKey, true);
4343
}
4444

45-
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReference() {
46-
return fromOwnerReference(false);
45+
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
46+
Class<? extends HasMetadata> primaryResourceType) {
47+
return fromOwnerReferences(primaryResourceType, false);
4748
}
4849

49-
/**
50-
* @param clusterScope if the owner is a cluster scoped resource
51-
* @return mapper
52-
* @param <T> type of the secondary resource, where the owner reference is
53-
*/
54-
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReference(
55-
boolean clusterScope) {
56-
return resource -> ResourceID.fromFirstOwnerReference(resource, clusterScope).map(Set::of)
57-
.orElseGet(Collections::emptySet);
50+
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
51+
Class<? extends HasMetadata> primaryResourceType, boolean clusterScoped) {
52+
return fromOwnerReferences(HasMetadata.getApiVersion(primaryResourceType),
53+
HasMetadata.getKind(primaryResourceType),
54+
clusterScoped);
55+
}
56+
57+
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
58+
HasMetadata primaryResource) {
59+
return fromOwnerReferences(primaryResource, false);
60+
}
61+
62+
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
63+
HasMetadata primaryResource,
64+
boolean clusterScoped) {
65+
return fromOwnerReferences(primaryResource.getApiVersion(), primaryResource.getKind(),
66+
clusterScoped);
5867
}
5968

6069
public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
70+
String apiVersion, String kind,
6171
boolean clusterScope) {
62-
return resource -> resource.getMetadata().getOwnerReferences().stream()
72+
return resource -> resource.getMetadata().getOwnerReferences()
73+
.stream()
74+
.filter(r -> r.getKind().equals(kind)
75+
&& r.getApiVersion().equals(apiVersion))
6376
.map(or -> ResourceID.fromOwnerReference(resource, or, clusterScope))
6477
.collect(Collectors.toSet());
6578
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/MappersTest.java

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package io.javaoperatorsdk.operator.processing.event.source.informer;
2+
3+
import java.util.UUID;
4+
5+
import org.junit.jupiter.api.Test;
6+
7+
import io.fabric8.kubernetes.api.model.ConfigMap;
8+
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
9+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
10+
import io.javaoperatorsdk.operator.TestUtils;
11+
import io.javaoperatorsdk.operator.processing.event.ResourceID;
12+
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
13+
import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceOtherV1;
14+
15+
import static org.assertj.core.api.Assertions.assertThat;
16+
import static org.junit.jupiter.api.Assertions.*;
17+
18+
class MappersTest {
19+
20+
21+
@Test
22+
void secondaryToPrimaryMapperFromOwnerReference() {
23+
var primary = TestUtils.testCustomResource();
24+
primary.getMetadata().setUid(UUID.randomUUID().toString());
25+
var secondary = getConfigMap(primary);
26+
secondary.addOwnerReference(primary);
27+
28+
var res = Mappers.fromOwnerReferences(TestCustomResource.class)
29+
.toPrimaryResourceIDs(secondary);
30+
31+
assertThat(res).contains(ResourceID.fromResource(primary));
32+
}
33+
34+
@Test
35+
void secondaryToPrimaryMapperFromOwnerReferenceFiltersByType() {
36+
var primary = TestUtils.testCustomResource();
37+
primary.getMetadata().setUid(UUID.randomUUID().toString());
38+
var secondary = getConfigMap(primary);
39+
secondary.addOwnerReference(primary);
40+
41+
var res = Mappers.fromOwnerReferences(TestCustomResourceOtherV1.class)
42+
.toPrimaryResourceIDs(secondary);
43+
44+
assertThat(res).isEmpty();
45+
}
46+
47+
48+
private static ConfigMap getConfigMap(TestCustomResource primary) {
49+
return new ConfigMapBuilder()
50+
.withMetadata(new ObjectMetaBuilder()
51+
.withName("test1")
52+
.withNamespace(primary.getMetadata().getNamespace())
53+
.build())
54+
.build();
55+
}
56+
}

‎operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceOtherV1.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
@Group("sample.javaoperatorsdk.io")
99
@Version("v1")
10-
@Kind("TestCustomResource") // this is needed to override the automatically generated kind
10+
@Kind("TestCustomResourceOtherV1") // this is needed to override the automatically generated kind
1111
public class TestCustomResourceOtherV1
1212
extends CustomResource<TestCustomResourceSpec, TestCustomResourceStatus> {
1313

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/readonly/ReadOnlyBulkDependentResource.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ private static String getName(ConfigMap cm) {
4848

4949
@Override
5050
public Set<ResourceID> toPrimaryResourceIDs(ConfigMap resource) {
51-
return Mappers.fromOwnerReferences(false).toPrimaryResourceIDs(resource);
51+
return Mappers.fromOwnerReferences(BulkDependentTestCustomResource.class, false)
52+
.toPrimaryResourceIDs(resource);
5253
}
5354
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/clusterscopedresource/ClusterScopedCustomResourceReconciler.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ private ConfigMap desired(ClusterScopedCustomResource resource) {
5656
public List<EventSource> prepareEventSources(
5757
EventSourceContext<ClusterScopedCustomResource> context) {
5858
var ies = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context)
59-
.withSecondaryToPrimaryMapper(Mappers.fromOwnerReference(true))
59+
.withSecondaryToPrimaryMapper(
60+
Mappers.fromOwnerReferences(context.getPrimaryResourceClass(), true))
6061
.withLabelSelector(TEST_LABEL_KEY + "=" + TEST_LABEL_VALUE)
6162
.build(), context);
6263
return List.of(ies);

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createupdateeventfilter/CreateUpdateEventFilterTestReconciler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ private ConfigMap createConfigMap(CreateUpdateEventFilterTestCustomResource reso
9393
public List<EventSource> prepareEventSources(
9494
EventSourceContext<CreateUpdateEventFilterTestCustomResource> context) {
9595
InformerConfiguration<ConfigMap> informerConfiguration =
96-
InformerConfiguration.from(ConfigMap.class)
96+
InformerConfiguration.from(ConfigMap.class, context)
9797
.withLabelSelector("integrationtest = " + this.getClass().getSimpleName())
9898
.build();
9999
final var informerEventSource =

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/informereventsource/InformerEventSourceTestCustomReconciler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public List<EventSource> prepareEventSources(
3636
EventSourceContext<InformerEventSourceTestCustomResource> context) {
3737

3838
InformerConfiguration<ConfigMap> config =
39-
InformerConfiguration.from(ConfigMap.class)
39+
InformerConfiguration.from(ConfigMap.class, context)
4040
.withSecondaryToPrimaryMapper(Mappers.fromAnnotation(RELATED_RESOURCE_NAME))
4141
.build();
4242

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplesecondaryeventsource/MultipleSecondaryEventSourceReconciler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public int getNumberOfExecutions() {
6464
public List<EventSource> prepareEventSources(
6565
EventSourceContext<MultipleSecondaryEventSourceCustomResource> context) {
6666

67-
var config = InformerConfiguration.from(ConfigMap.class)
67+
var config = InformerConfiguration.from(ConfigMap.class, context)
6868
.withNamespaces(context.getControllerConfiguration().getNamespaces())
6969
.withLabelSelector("multisecondary")
7070
.withSecondaryToPrimaryMapper(s -> {

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/primaryindexer/PrimaryIndexerTestReconciler.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public List<EventSource> prepareEventSources(
2222
context.getPrimaryCache().addIndexer(CONFIG_MAP_RELATION_INDEXER, indexer);
2323

2424
var informerConfiguration =
25-
InformerConfiguration.from(ConfigMap.class)
25+
InformerConfiguration.from(ConfigMap.class, context)
2626
.withSecondaryToPrimaryMapper(
2727
(ConfigMap secondaryResource) -> context
2828
.getPrimaryCache()

0 commit comments

Comments
 (0)