Skip to content

Commit 4f9c19d

Browse files
committed
refactor: make matcher configurable instead of settable
Signed-off-by: Chris Laprun <[email protected]>
1 parent a9868e0 commit 4f9c19d

File tree

6 files changed

+59
-24
lines changed

6 files changed

+59
-24
lines changed

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

+19
Original file line numberDiff line numberDiff line change
@@ -27,5 +27,24 @@ boolean createResourceOnlyIfNotExistingWithSSA() default
2727
*/
2828
BooleanWithUndefined useSSA() default BooleanWithUndefined.UNDEFINED;
2929

30+
/**
31+
* The underlying Informer event source configuration
32+
*
33+
* @return the {@link Informer} configuration
34+
*/
3035
Informer informer() default @Informer;
36+
37+
/**
38+
* The specific matcher implementation to use when Server-Side Apply (SSA) is used, when case the
39+
* default one isn't working appropriately. Typically, this could be needed to cover border cases
40+
* with some Kubernetes resources that are modified by their controllers to normalize or add
41+
* default values, which could result in infinite loops with the default matcher. Using a specific
42+
* matcher could also be an optimization decision if determination of whether two resources match
43+
* can be done faster than what can be done with the default exhaustive algorithm.
44+
*
45+
* @return the class of the specific matcher to use for the associated dependent resources
46+
* @since 5.1
47+
*/
48+
Class<? extends SSABasedGenericKubernetesResourceMatcher> matcher() default
49+
SSABasedGenericKubernetesResourceMatcher.class;
3150
}

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

+13-1
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,22 @@ public KubernetesDependentResourceConfig<R> configFrom(
2222
DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA;
2323

2424
Boolean useSSA = null;
25+
SSABasedGenericKubernetesResourceMatcher<R> matcher =
26+
SSABasedGenericKubernetesResourceMatcher.getInstance();
2527
if (configAnnotation != null) {
2628
createResourceOnlyIfNotExistingWithSSA =
2729
configAnnotation.createResourceOnlyIfNotExistingWithSSA();
2830
useSSA = configAnnotation.useSSA().asBoolean();
31+
32+
// check if we have a specific matcher
33+
Class<? extends KubernetesDependentResource<?, ?>> dependentResourceClass =
34+
(Class<? extends KubernetesDependentResource<?, ?>>) spec.getDependentResourceClass();
35+
final var context =
36+
Utils.contextFor(
37+
controllerConfig, dependentResourceClass, configAnnotation.annotationType());
38+
matcher =
39+
Utils.instantiate(
40+
configAnnotation.matcher(), SSABasedGenericKubernetesResourceMatcher.class, context);
2941
}
3042

3143
var informerConfiguration =
@@ -35,7 +47,7 @@ public KubernetesDependentResourceConfig<R> configFrom(
3547
controllerConfig);
3648

3749
return new KubernetesDependentResourceConfig<>(
38-
useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration);
50+
useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration, matcher);
3951
}
4052

4153
@SuppressWarnings({"unchecked"})

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

+5-20
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,6 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
4242
private KubernetesDependentResourceConfig<R> kubernetesDependentResourceConfig;
4343
private volatile Boolean useSSA;
4444

45-
private SSABasedGenericKubernetesResourceMatcher<R> matcher =
46-
SSABasedGenericKubernetesResourceMatcher.getInstance();
47-
private volatile boolean matcherSet = false;
48-
4945
public KubernetesDependentResource(Class<R> resourceType) {
5046
this(resourceType, null);
5147
}
@@ -54,26 +50,11 @@ public KubernetesDependentResource(Class<R> resourceType, String name) {
5450
super(resourceType, name);
5551
}
5652

57-
public KubernetesDependentResource(
58-
Class<R> resourceType, String name, SSABasedGenericKubernetesResourceMatcher<R> matcher) {
59-
this(resourceType, name);
60-
this.matcher = matcher;
61-
matcherSet = true;
62-
}
63-
6453
@Override
6554
public void configureWith(KubernetesDependentResourceConfig<R> config) {
6655
this.kubernetesDependentResourceConfig = config;
6756
}
6857

69-
public void setMatcher(SSABasedGenericKubernetesResourceMatcher<R> matcher) {
70-
if (matcherSet) {
71-
throw new IllegalStateException("Can only set a matcher once.");
72-
}
73-
this.matcher = matcher;
74-
matcherSet = true;
75-
}
76-
7758
@SuppressWarnings("unused")
7859
public R create(R desired, P primary, Context<P> context) {
7960
if (useSSA(context)) {
@@ -131,7 +112,11 @@ public Result<R> match(R actualResource, R desired, P primary, Context<P> contex
131112
final boolean matches;
132113
addMetadata(true, actualResource, desired, primary, context);
133114
if (useSSA(context)) {
134-
matches = matcher.matches(actualResource, desired, context);
115+
matches =
116+
configuration()
117+
.map(KubernetesDependentResourceConfig::matcher)
118+
.orElse(SSABasedGenericKubernetesResourceMatcher.getInstance())
119+
.matches(actualResource, desired, context);
135120
} else {
136121
matches =
137122
GenericKubernetesResourceMatcher.match(desired, actualResource, false, false, context)

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

+9-1
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,18 @@ public class KubernetesDependentResourceConfig<R extends HasMetadata> {
1010
private final Boolean useSSA;
1111
private final boolean createResourceOnlyIfNotExistingWithSSA;
1212
private final InformerConfiguration<R> informerConfig;
13+
private final SSABasedGenericKubernetesResourceMatcher<R> matcher;
1314

1415
public KubernetesDependentResourceConfig(
1516
Boolean useSSA,
1617
boolean createResourceOnlyIfNotExistingWithSSA,
17-
InformerConfiguration<R> informerConfig) {
18+
InformerConfiguration<R> informerConfig,
19+
SSABasedGenericKubernetesResourceMatcher<R> matcher) {
1820
this.useSSA = useSSA;
1921
this.createResourceOnlyIfNotExistingWithSSA = createResourceOnlyIfNotExistingWithSSA;
2022
this.informerConfig = informerConfig;
23+
this.matcher =
24+
matcher != null ? matcher : SSABasedGenericKubernetesResourceMatcher.getInstance();
2125
}
2226

2327
public boolean createResourceOnlyIfNotExistingWithSSA() {
@@ -31,4 +35,8 @@ public Boolean useSSA() {
3135
public InformerConfiguration<R> informerConfig() {
3236
return informerConfig;
3337
}
38+
39+
public SSABasedGenericKubernetesResourceMatcher<R> matcher() {
40+
return matcher;
41+
}
3442
}

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

+8-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public final class KubernetesDependentResourceConfigBuilder<R extends HasMetadat
88
private boolean createResourceOnlyIfNotExistingWithSSA;
99
private Boolean useSSA = null;
1010
private InformerConfiguration<R> informerConfiguration;
11+
private SSABasedGenericKubernetesResourceMatcher<R> matcher;
1112

1213
public KubernetesDependentResourceConfigBuilder() {}
1314

@@ -29,8 +30,14 @@ public KubernetesDependentResourceConfigBuilder<R> withKubernetesDependentInform
2930
return this;
3031
}
3132

33+
public KubernetesDependentResourceConfigBuilder<R> withSSAMatcher(
34+
SSABasedGenericKubernetesResourceMatcher<R> matcher) {
35+
this.matcher = matcher;
36+
return this;
37+
}
38+
3239
public KubernetesDependentResourceConfig<R> build() {
3340
return new KubernetesDependentResourceConfig<>(
34-
useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration);
41+
useSSA, createResourceOnlyIfNotExistingWithSSA, informerConfiguration, matcher);
3542
}
3643
}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java

+5-1
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,10 @@ void testSanitizeState_daemonSetWithResources_withMismatch() {
242242
@ValueSource(booleans = {true, false})
243243
void testCustomMatcher_returnsExpectedMatchBasedOnReadOnlyLabel(boolean readOnly) {
244244
var dr = new ConfigMapDR();
245+
dr.configureWith(
246+
new KubernetesDependentResourceConfigBuilder()
247+
.withSSAMatcher(new ReadOnlyAwareMatcher())
248+
.build());
245249
var desiredConfigMap =
246250
loadResource("configmap.empty-owner-reference-desired.yaml", ConfigMap.class);
247251
desiredConfigMap.getData().put("key1", "another value");
@@ -261,7 +265,7 @@ void testCustomMatcher_returnsExpectedMatchBasedOnReadOnlyLabel(boolean readOnly
261265

262266
private static class ConfigMapDR extends KubernetesDependentResource<ConfigMap, ConfigMap> {
263267
public ConfigMapDR() {
264-
super(ConfigMap.class, "foo", new ReadOnlyAwareMatcher<>());
268+
super(ConfigMap.class);
265269
}
266270
}
267271

0 commit comments

Comments
 (0)