Skip to content

Commit 50f844b

Browse files
csviricsviri
committed
feat: sanitization of resources for matching (#2042)
Signed-off-by: Attila Mészáros <[email protected]>
1 parent e7c6082 commit 50f844b

File tree

18 files changed

+346
-37
lines changed

18 files changed

+346
-37
lines changed

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

+20-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99
import org.slf4j.Logger;
1010
import org.slf4j.LoggerFactory;
1111

12+
import io.fabric8.kubernetes.api.model.ConfigMap;
1213
import io.fabric8.kubernetes.api.model.HasMetadata;
14+
import io.fabric8.kubernetes.api.model.Secret;
1315
import io.fabric8.kubernetes.client.Config;
1416
import io.fabric8.kubernetes.client.ConfigBuilder;
1517
import io.fabric8.kubernetes.client.CustomResource;
@@ -19,6 +21,7 @@
1921
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
2022
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
2123
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory;
24+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
2225
import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowFactory;
2326

2427
import static io.javaoperatorsdk.operator.api.config.ExecutorServiceManager.newThreadPoolExecutor;
@@ -335,7 +338,7 @@ default ExecutorServiceManager getExecutorServiceManager() {
335338
* resources are created/updated and match was change to use
336339
* <a href="https://kubernetes.io/docs/reference/using-api/server-side-apply/">Server-Side
337340
* Apply</a> (SSA) by default.
338-
*
341+
* <p>
339342
* SSA based create/update can be still used with the legacy matching, just overriding the match
340343
* method of Kubernetes Dependent Resource.
341344
*
@@ -345,4 +348,20 @@ default boolean ssaBasedCreateUpdateMatchForDependentResources() {
345348
return true;
346349
}
347350

351+
/**
352+
* Returns the set of default resources for which Server-Side Apply (SSA) will not be used, even
353+
* if it is the default behavior for dependent resources as specified by
354+
* {@link #ssaBasedCreateUpdateMatchForDependentResources()}. The exception to this is in the case
355+
* where the use of SSA is explicitly enabled on the dependent resource directly using
356+
* {@link KubernetesDependent#useSSA()}.
357+
* <p>
358+
* By default, SSA is disabled for {@link ConfigMap} and {@link Secret} resources.
359+
*
360+
* @return The set of resource types for which SSA will not be used
361+
* @since 4.4.0
362+
*/
363+
default Set<Class<? extends HasMetadata>> defaultNonSSAResource() {
364+
return Set.of(ConfigMap.class, Secret.class);
365+
}
366+
348367
}

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

+14
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.slf4j.Logger;
1010
import org.slf4j.LoggerFactory;
1111

12+
import io.fabric8.kubernetes.api.model.HasMetadata;
1213
import io.fabric8.kubernetes.client.KubernetesClient;
1314
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
1415

@@ -35,6 +36,7 @@ public class ConfigurationServiceOverrider {
3536
private Duration cacheSyncTimeout;
3637
private ResourceClassResolver resourceClassResolver;
3738
private Boolean ssaBasedCreateUpdateMatchForDependentResources;
39+
private Set<Class<? extends HasMetadata>> defaultNonSSAResource;
3840

3941
ConfigurationServiceOverrider(ConfigurationService original) {
4042
this.original = original;
@@ -150,6 +152,12 @@ public ConfigurationServiceOverrider withSSABasedCreateUpdateMatchForDependentRe
150152
return this;
151153
}
152154

155+
public ConfigurationServiceOverrider withDefaultNonSSAResource(
156+
Set<Class<? extends HasMetadata>> defaultNonSSAResource) {
157+
this.defaultNonSSAResource = defaultNonSSAResource;
158+
return this;
159+
}
160+
153161
public ConfigurationService build() {
154162
return new BaseConfigurationService(original.getVersion(), cloner, client) {
155163
@Override
@@ -256,6 +264,12 @@ public boolean ssaBasedCreateUpdateMatchForDependentResources() {
256264
? ssaBasedCreateUpdateMatchForDependentResources
257265
: super.ssaBasedCreateUpdateMatchForDependentResources();
258266
}
267+
268+
@Override
269+
public Set<Class<? extends HasMetadata>> defaultNonSSAResource() {
270+
return defaultNonSSAResource != null ? defaultNonSSAResource
271+
: super.defaultNonSSAResource();
272+
}
259273
};
260274
}
261275

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

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
2+
3+
import io.fabric8.kubernetes.api.model.HasMetadata;
4+
import io.fabric8.kubernetes.api.model.PersistentVolumeClaimStatus;
5+
import io.fabric8.kubernetes.api.model.Secret;
6+
import io.fabric8.kubernetes.api.model.apps.StatefulSet;
7+
import io.javaoperatorsdk.operator.api.reconciler.Context;
8+
9+
public class DesiredResourceSanitizer {
10+
11+
private DesiredResourceSanitizer() {}
12+
13+
public static <R, P extends HasMetadata> void sanitizeDesired(R desired, R actual, P primary,
14+
Context<P> context, boolean useSSA) {
15+
if (useSSA) {
16+
if (desired instanceof StatefulSet) {
17+
fillDefaultsOnVolumeClaimTemplate((StatefulSet) desired);
18+
}
19+
if (desired instanceof Secret) {
20+
checkIfStringDataUsed((Secret) desired);
21+
}
22+
}
23+
}
24+
25+
private static void checkIfStringDataUsed(Secret secret) {
26+
if (secret.getStringData() != null && !secret.getStringData().isEmpty()) {
27+
throw new IllegalStateException(
28+
"There is a known issue using StringData with SSA. Use data instead.");
29+
}
30+
}
31+
32+
private static void fillDefaultsOnVolumeClaimTemplate(StatefulSet statefulSet) {
33+
if (!statefulSet.getSpec().getVolumeClaimTemplates().isEmpty()) {
34+
statefulSet.getSpec().getVolumeClaimTemplates().forEach(t -> {
35+
if (t.getSpec().getVolumeMode() == null) {
36+
t.getSpec().setVolumeMode("Filesystem");
37+
}
38+
if (t.getStatus() == null) {
39+
t.setStatus(new PersistentVolumeClaimStatus());
40+
}
41+
if (t.getStatus().getPhase() == null) {
42+
t.getStatus().setPhase("pending");
43+
}
44+
});
45+
}
46+
}
47+
}

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

+21-9
Original file line numberDiff line numberDiff line change
@@ -103,18 +103,19 @@ public void configureWith(InformerEventSource<R, P> informerEventSource) {
103103
}
104104

105105
@SuppressWarnings("unused")
106-
public R create(R target, P primary, Context<P> context) {
106+
public R create(R desired, P primary, Context<P> context) {
107107
if (useSSA(context)) {
108108
// setting resource version for SSA so only created if it doesn't exist already
109109
var createIfNotExisting = kubernetesDependentResourceConfig == null
110110
? KubernetesDependentResourceConfig.DEFAULT_CREATE_RESOURCE_ONLY_IF_NOT_EXISTING_WITH_SSA
111111
: kubernetesDependentResourceConfig.createResourceOnlyIfNotExistingWithSSA();
112112
if (createIfNotExisting) {
113-
target.getMetadata().setResourceVersion("1");
113+
desired.getMetadata().setResourceVersion("1");
114114
}
115115
}
116-
addMetadata(false, null, target, primary);
117-
final var resource = prepare(target, primary, "Creating");
116+
addMetadata(false, null, desired, primary);
117+
sanitizeDesired(desired, null, primary, context);
118+
final var resource = prepare(desired, primary, "Creating");
118119
return useSSA(context)
119120
? resource
120121
.fieldManager(context.getControllerConfiguration().fieldManager())
@@ -123,19 +124,20 @@ public R create(R target, P primary, Context<P> context) {
123124
: resource.create();
124125
}
125126

126-
public R update(R actual, R target, P primary, Context<P> context) {
127+
public R update(R actual, R desired, P primary, Context<P> context) {
127128
if (log.isDebugEnabled()) {
128129
log.debug("Updating actual resource: {} version: {}", ResourceID.fromResource(actual),
129130
actual.getMetadata().getResourceVersion());
130131
}
131132
R updatedResource;
132-
addMetadata(false, actual, target, primary);
133+
addMetadata(false, actual, desired, primary);
134+
sanitizeDesired(desired, actual, primary, context);
133135
if (useSSA(context)) {
134-
updatedResource = prepare(target, primary, "Updating")
136+
updatedResource = prepare(desired, primary, "Updating")
135137
.fieldManager(context.getControllerConfiguration().fieldManager())
136138
.forceConflicts().serverSideApply();
137139
} else {
138-
var updatedActual = updaterMatcher.updateResource(actual, target, context);
140+
var updatedActual = updaterMatcher.updateResource(actual, desired, context);
139141
updatedResource = prepare(updatedActual, primary, "Updating").update();
140142
}
141143
log.debug("Resource version after update: {}",
@@ -146,6 +148,7 @@ public R update(R actual, R target, P primary, Context<P> context) {
146148
@Override
147149
public Result<R> match(R actualResource, P primary, Context<P> context) {
148150
final var desired = desired(primary, context);
151+
sanitizeDesired(desired, actualResource, primary, context);
149152
return match(actualResource, desired, primary, updaterMatcher, context);
150153
}
151154

@@ -189,9 +192,18 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P
189192
addReferenceHandlingMetadata(target, primary);
190193
}
191194

192-
private boolean useSSA(Context<P> context) {
195+
protected void sanitizeDesired(R desired, R actual, P primary, Context<P> context) {
196+
DesiredResourceSanitizer.sanitizeDesired(desired, actual, primary, context, useSSA(context));
197+
}
198+
199+
protected boolean useSSA(Context<P> context) {
193200
Optional<Boolean> useSSAConfig =
194201
configuration().flatMap(KubernetesDependentResourceConfig::useSSA);
202+
var configService = context.getControllerConfiguration().getConfigurationService();
203+
// don't use SSA for certain resources by default, only if explicitly overriden
204+
if (useSSAConfig.isEmpty() && configService.defaultNonSSAResource().contains(resourceType())) {
205+
return false;
206+
}
195207
return useSSAConfig.orElse(context.getControllerConfiguration().getConfigurationService()
196208
.ssaBasedCreateUpdateMatchForDependentResources());
197209
}

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ public KubernetesDependentResourceConfigBuilder<R> withGenericFilter(
7878
}
7979

8080
public KubernetesDependentResourceConfig<R> build() {
81-
return new KubernetesDependentResourceConfig<>(namespaces, labelSelector, false,
81+
return new KubernetesDependentResourceConfig<>(namespaces, labelSelector,
82+
namespaces != Constants.SAME_AS_CONTROLLER_NAMESPACES_SET,
8283
createResourceOnlyIfNotExistingWithSSA, resourceDiscriminator, useSSA, onAddFilter,
8384
onUpdateFilter, onDeleteFilter, genericFilter);
8485
}

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

+2-9
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
22

3-
import java.util.ArrayList;
4-
import java.util.HashMap;
5-
import java.util.List;
6-
import java.util.Map;
3+
import java.util.*;
74
import java.util.Map.Entry;
8-
import java.util.Optional;
9-
import java.util.Set;
10-
import java.util.SortedMap;
11-
import java.util.TreeMap;
125
import java.util.stream.Collectors;
136

147
import org.slf4j.Logger;
@@ -159,7 +152,7 @@ private static void fillResultsAndTraverseFurther(Map<String, Object> result,
159152
Object managedFieldValue) {
160153
var emptyMapValue = new HashMap<String, Object>();
161154
result.put(keyInActual, emptyMapValue);
162-
var actualMapValue = actualMap.get(keyInActual);
155+
var actualMapValue = actualMap.getOrDefault(keyInActual, Collections.emptyMap());
163156
log.debug("key: {} actual map value: {} managedFieldValue: {}", keyInActual,
164157
actualMapValue, managedFieldValue);
165158

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,10 @@ private DependnetSSACustomResource reconcileWithLegacyOperator(Operator legacyOp
144144
private Operator createOperator(KubernetesClient client, boolean legacyDependentHandling,
145145
String fieldManager) {
146146
Operator operator = new Operator(client,
147-
o -> o.withSSABasedCreateUpdateMatchForDependentResources(!legacyDependentHandling)
148-
.withCloseClientOnStop(false));
149-
operator.register(new DependentSSAReconciler(), o -> {
147+
o -> o.withCloseClientOnStop(false));
148+
var reconciler = new DependentSSAReconciler(!legacyDependentHandling);
149+
reconciler.setKubernetesClient(client);
150+
operator.register(reconciler, o -> {
150151
o.settingNamespace(namespace);
151152
if (fieldManager != null) {
152153
o.withFieldManager(fieldManager);
@@ -155,7 +156,6 @@ private Operator createOperator(KubernetesClient client, boolean legacyDependent
155156
return operator;
156157
}
157158

158-
159159
public DependnetSSACustomResource testResource() {
160160
DependnetSSACustomResource resource = new DependnetSSACustomResource();
161161
resource.setMetadata(new ObjectMetaBuilder()

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/StatefulSetDesiredSanitizerIT.java

+57
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
package io.javaoperatorsdk.operator;
2+
3+
import java.time.Duration;
4+
5+
import org.junit.jupiter.api.Test;
6+
import org.junit.jupiter.api.extension.RegisterExtension;
7+
8+
import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
9+
import io.fabric8.kubernetes.api.model.apps.StatefulSet;
10+
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
11+
import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerCustomResource;
12+
import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerDependentResource;
13+
import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerReconciler;
14+
import io.javaoperatorsdk.operator.sample.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerSpec;
15+
16+
import static org.assertj.core.api.Assertions.assertThat;
17+
import static org.awaitility.Awaitility.await;
18+
19+
public class StatefulSetDesiredSanitizerIT {
20+
21+
public static final String TEST_1 = "test1";
22+
23+
@RegisterExtension
24+
LocallyRunOperatorExtension extension =
25+
LocallyRunOperatorExtension.builder()
26+
.withReconciler(new StatefulSetDesiredSanitizerReconciler())
27+
.build();
28+
29+
@Test
30+
void testSSAMatcher() {
31+
var resource = extension.create(testResource());
32+
33+
await().pollDelay(Duration.ofMillis(200)).untilAsserted(() -> {
34+
var statefulSet = extension.get(StatefulSet.class, TEST_1);
35+
assertThat(statefulSet).isNotNull();
36+
});
37+
// make sure reconciliation happens at least once more
38+
resource.getSpec().setValue("changed value");
39+
extension.replace(resource);
40+
41+
await().untilAsserted(
42+
() -> assertThat(StatefulSetDesiredSanitizerDependentResource.nonMatchedAtLeastOnce)
43+
.isFalse());
44+
}
45+
46+
StatefulSetDesiredSanitizerCustomResource testResource() {
47+
var res = new StatefulSetDesiredSanitizerCustomResource();
48+
res.setMetadata(new ObjectMetaBuilder()
49+
.withName(TEST_1)
50+
.build());
51+
res.setSpec(new StatefulSetDesiredSanitizerSpec());
52+
res.getSpec().setValue("initial value");
53+
54+
return res;
55+
}
56+
57+
}

‎operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java

+42-3
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,43 @@
11
package io.javaoperatorsdk.operator.sample.dependentssa;
22

3+
import java.util.Map;
34
import java.util.concurrent.atomic.AtomicInteger;
45

6+
import io.fabric8.kubernetes.api.model.ConfigMap;
7+
import io.fabric8.kubernetes.client.KubernetesClient;
58
import io.javaoperatorsdk.operator.api.reconciler.*;
6-
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
9+
import io.javaoperatorsdk.operator.junit.KubernetesClientAware;
10+
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfigBuilder;
11+
import io.javaoperatorsdk.operator.processing.event.source.EventSource;
712
import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
813

9-
@ControllerConfiguration(dependents = {@Dependent(type = SSAConfigMapDependent.class)})
14+
@ControllerConfiguration
1015
public class DependentSSAReconciler
11-
implements Reconciler<DependnetSSACustomResource>, TestExecutionInfoProvider {
16+
implements Reconciler<DependnetSSACustomResource>, TestExecutionInfoProvider,
17+
KubernetesClientAware,
18+
EventSourceInitializer<DependnetSSACustomResource> {
1219

1320
private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
1421

22+
private SSAConfigMapDependent ssaConfigMapDependent = new SSAConfigMapDependent();
23+
private KubernetesClient kubernetesClient;
24+
25+
public DependentSSAReconciler() {
26+
this(true);
27+
}
28+
29+
public DependentSSAReconciler(boolean useSSA) {
30+
ssaConfigMapDependent.configureWith(new KubernetesDependentResourceConfigBuilder<ConfigMap>()
31+
.withUseSSA(useSSA)
32+
.build());
33+
}
34+
1535
@Override
1636
public UpdateControl<DependnetSSACustomResource> reconcile(
1737
DependnetSSACustomResource resource,
1838
Context<DependnetSSACustomResource> context) {
39+
40+
ssaConfigMapDependent.reconcile(resource, context);
1941
numberOfExecutions.addAndGet(1);
2042
return UpdateControl.noUpdate();
2143
}
@@ -24,4 +46,21 @@ public int getNumberOfExecutions() {
2446
return numberOfExecutions.get();
2547
}
2648

49+
@Override
50+
public KubernetesClient getKubernetesClient() {
51+
return kubernetesClient;
52+
}
53+
54+
@Override
55+
public void setKubernetesClient(KubernetesClient kubernetesClient) {
56+
this.kubernetesClient = kubernetesClient;
57+
ssaConfigMapDependent.setKubernetesClient(kubernetesClient);
58+
}
59+
60+
@Override
61+
public Map<String, EventSource> prepareEventSources(
62+
EventSourceContext<DependnetSSACustomResource> context) {
63+
return EventSourceInitializer.nameEventSourcesFromDependentResource(context,
64+
ssaConfigMapDependent);
65+
}
2766
}

0 commit comments

Comments
 (0)