diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
index fa502e043b..93985bd556 100644
--- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
+++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
@@ -20,6 +20,7 @@
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
import io.fabric8.kubernetes.client.utils.KubernetesSerialization;
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
@@ -396,7 +397,8 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() {
* either use simple patches or SSA. Setting this to {@code true}, controllers will use SSA for
* adding finalizers, managing observed generation, patching resources and status.
*
- * @return {@code true} by default
+ * @return {@code true} if Server-Side Apply (SSA) should be used when patching the primary
+ * resources, {@code false} otherwise
* @since 5.0.0
* @see ConfigurationServiceOverrider#withUseSSAToPatchPrimaryResource(boolean)
*/
@@ -404,4 +406,27 @@ default boolean useSSAToPatchPrimaryResource() {
return true;
}
+ /**
+ *
+ * Determines whether resources retrieved from caches such as via calls to
+ * {@link Context#getSecondaryResource(Class)} should be defensively cloned first.
+ *
+ *
+ *
+ * Defensive cloning to prevent problematic cache modifications (modifying the resource would
+ * otherwise modify the stored copy in the cache) was transparently done in previous JOSDK
+ * versions. This might have performance consequences and, with the more prevalent use of
+ * Server-Side Apply, where you should create a new copy of your resource with only modified
+ * fields, such modifications of these resources are less likely to occur.
+ *
+ *
+ * @return {@code true} if resources should be defensively cloned before returning them from
+ * caches, {@code false} otherwise
+ *
+ * @since 5.0.0
+ */
+ default boolean cloneSecondaryResourcesWhenGettingFromCache() {
+ return false;
+ }
+
}
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java
index d4aa430800..add050c6f0 100644
--- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java
+++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java
@@ -5,6 +5,7 @@
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.function.Consumer;
+import java.util.function.Function;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -40,6 +41,7 @@ public class ConfigurationServiceOverrider {
private Boolean previousAnnotationForDependentResources;
private Boolean parseResourceVersions;
private Boolean useSSAToPatchPrimaryResource;
+ private Boolean cloneSecondaryResourcesWhenGettingFromCache;
private DependentResourceFactory dependentResourceFactory;
ConfigurationServiceOverrider(ConfigurationService original) {
@@ -180,6 +182,12 @@ public ConfigurationServiceOverrider withUseSSAToPatchPrimaryResource(boolean va
return this;
}
+ public ConfigurationServiceOverrider withCloneSecondaryResourcesWhenGettingFromCache(
+ boolean value) {
+ this.cloneSecondaryResourcesWhenGettingFromCache = value;
+ return this;
+ }
+
public ConfigurationService build() {
return new BaseConfigurationService(original.getVersion(), cloner, client) {
@Override
@@ -187,22 +195,29 @@ public Set getKnownReconcilerNames() {
return original.getKnownReconcilerNames();
}
+ private T overriddenValueOrDefault(T value,
+ Function defaultValue) {
+ return value != null ? value : defaultValue.apply(original);
+ }
+
@Override
public boolean checkCRDAndValidateLocalModel() {
- return checkCR != null ? checkCR : original.checkCRDAndValidateLocalModel();
+ return overriddenValueOrDefault(checkCR,
+ ConfigurationService::checkCRDAndValidateLocalModel);
}
@Override
+ @SuppressWarnings("rawtypes")
public DependentResourceFactory dependentResourceFactory() {
- return dependentResourceFactory != null ? dependentResourceFactory
- : DependentResourceFactory.DEFAULT;
+ return overriddenValueOrDefault(dependentResourceFactory,
+ ConfigurationService::dependentResourceFactory);
}
@Override
public int concurrentReconciliationThreads() {
return Utils.ensureValid(
- concurrentReconciliationThreads != null ? concurrentReconciliationThreads
- : original.concurrentReconciliationThreads(),
+ overriddenValueOrDefault(concurrentReconciliationThreads,
+ ConfigurationService::concurrentReconciliationThreads),
"maximum reconciliation threads",
minimumMaxValueFor(minConcurrentReconciliationThreads),
original.concurrentReconciliationThreads());
@@ -211,8 +226,8 @@ public int concurrentReconciliationThreads() {
@Override
public int concurrentWorkflowExecutorThreads() {
return Utils.ensureValid(
- concurrentWorkflowExecutorThreads != null ? concurrentWorkflowExecutorThreads
- : original.concurrentWorkflowExecutorThreads(),
+ overriddenValueOrDefault(concurrentWorkflowExecutorThreads,
+ ConfigurationService::concurrentWorkflowExecutorThreads),
"maximum workflow execution threads",
minimumMaxValueFor(minConcurrentWorkflowExecutorThreads),
original.concurrentWorkflowExecutorThreads());
@@ -224,8 +239,8 @@ public int concurrentWorkflowExecutorThreads() {
@Deprecated(forRemoval = true)
@Override
public int minConcurrentReconciliationThreads() {
- return minConcurrentReconciliationThreads != null ? minConcurrentReconciliationThreads
- : original.minConcurrentReconciliationThreads();
+ return overriddenValueOrDefault(minConcurrentReconciliationThreads,
+ ConfigurationService::minConcurrentReconciliationThreads);
}
/**
@@ -234,30 +249,29 @@ public int minConcurrentReconciliationThreads() {
@Override
@Deprecated(forRemoval = true)
public int minConcurrentWorkflowExecutorThreads() {
- return minConcurrentWorkflowExecutorThreads != null ? minConcurrentWorkflowExecutorThreads
- : original.minConcurrentWorkflowExecutorThreads();
+ return overriddenValueOrDefault(minConcurrentWorkflowExecutorThreads,
+ ConfigurationService::minConcurrentWorkflowExecutorThreads);
}
@Override
public Metrics getMetrics() {
- return metrics != null ? metrics : original.getMetrics();
+ return overriddenValueOrDefault(metrics, ConfigurationService::getMetrics);
}
@Override
public boolean closeClientOnStop() {
- return closeClientOnStop != null ? closeClientOnStop : original.closeClientOnStop();
+ return overriddenValueOrDefault(closeClientOnStop, ConfigurationService::closeClientOnStop);
}
@Override
public ExecutorService getExecutorService() {
- return executorService != null ? executorService
- : super.getExecutorService();
+ return overriddenValueOrDefault(executorService, ConfigurationService::getExecutorService);
}
@Override
public ExecutorService getWorkflowExecutorService() {
- return workflowExecutorService != null ? workflowExecutorService
- : super.getWorkflowExecutorService();
+ return overriddenValueOrDefault(workflowExecutorService,
+ ConfigurationService::getWorkflowExecutorService);
}
@Override
@@ -274,54 +288,55 @@ public Optional getInformerStoppedHandler() {
@Override
public boolean stopOnInformerErrorDuringStartup() {
- return stopOnInformerErrorDuringStartup != null ? stopOnInformerErrorDuringStartup
- : super.stopOnInformerErrorDuringStartup();
+ return overriddenValueOrDefault(stopOnInformerErrorDuringStartup,
+ ConfigurationService::stopOnInformerErrorDuringStartup);
}
@Override
public Duration cacheSyncTimeout() {
- return cacheSyncTimeout != null ? cacheSyncTimeout : super.cacheSyncTimeout();
+ return overriddenValueOrDefault(cacheSyncTimeout, ConfigurationService::cacheSyncTimeout);
}
@Override
public ResourceClassResolver getResourceClassResolver() {
- return resourceClassResolver != null ? resourceClassResolver
- : super.getResourceClassResolver();
+ return overriddenValueOrDefault(resourceClassResolver,
+ ConfigurationService::getResourceClassResolver);
}
@Override
public boolean ssaBasedCreateUpdateMatchForDependentResources() {
- return ssaBasedCreateUpdateMatchForDependentResources != null
- ? ssaBasedCreateUpdateMatchForDependentResources
- : super.ssaBasedCreateUpdateMatchForDependentResources();
+ return overriddenValueOrDefault(ssaBasedCreateUpdateMatchForDependentResources,
+ ConfigurationService::ssaBasedCreateUpdateMatchForDependentResources);
}
@Override
public Set> defaultNonSSAResource() {
- return defaultNonSSAResource != null ? defaultNonSSAResource
- : super.defaultNonSSAResource();
+ return overriddenValueOrDefault(defaultNonSSAResource,
+ ConfigurationService::defaultNonSSAResource);
}
@Override
public boolean previousAnnotationForDependentResourcesEventFiltering() {
- return previousAnnotationForDependentResources != null
- ? previousAnnotationForDependentResources
- : super.previousAnnotationForDependentResourcesEventFiltering();
+ return overriddenValueOrDefault(previousAnnotationForDependentResources,
+ ConfigurationService::previousAnnotationForDependentResourcesEventFiltering);
}
@Override
public boolean parseResourceVersionsForEventFilteringAndCaching() {
- return parseResourceVersions != null
- ? parseResourceVersions
- : super.parseResourceVersionsForEventFilteringAndCaching();
+ return overriddenValueOrDefault(parseResourceVersions,
+ ConfigurationService::parseResourceVersionsForEventFilteringAndCaching);
}
@Override
public boolean useSSAToPatchPrimaryResource() {
- return useSSAToPatchPrimaryResource != null
- ? useSSAToPatchPrimaryResource
- : super.useSSAToPatchPrimaryResource();
+ return overriddenValueOrDefault(useSSAToPatchPrimaryResource,
+ ConfigurationService::useSSAToPatchPrimaryResource);
+ }
+ @Override
+ public boolean cloneSecondaryResourcesWhenGettingFromCache() {
+ return overriddenValueOrDefault(cloneSecondaryResourcesWhenGettingFromCache,
+ ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache);
}
};
}
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java
index 752e3ed0c2..531917adac 100644
--- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java
+++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java
@@ -361,7 +361,6 @@ private P patchResource(P resource, P originalResource) {
getVersion(resource));
log.trace("Resource before update: {}", resource);
- // todo unit test
final var finalizerName = configuration().getFinalizerName();
if (useSSA && controller.useFinalizer()) {
// addFinalizer already prevents adding an already present finalizer so no need to check
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java
index e68cd3ab25..a97897b1fa 100644
--- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java
+++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java
@@ -170,7 +170,9 @@ public Stream list(String namespace, Predicate predicate) {
public Optional get(ResourceID resourceID) {
return getSource(resourceID.getNamespace().orElse(WATCH_ALL_NAMESPACES))
.flatMap(source -> source.get(resourceID))
- .map(r -> configurationService.getResourceCloner().clone(r));
+ .map(r -> configurationService.cloneSecondaryResourcesWhenGettingFromCache()
+ ? configurationService.getResourceCloner().clone(r)
+ : r);
}
@Override
diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java
index 46bc3dd392..8b4e38ce6d 100644
--- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java
+++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java
@@ -12,7 +12,6 @@
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.fabric8.kubernetes.api.model.networking.v1.Ingress;
import io.fabric8.kubernetes.client.KubernetesClient;
-import io.fabric8.kubernetes.client.dsl.Replaceable;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.*;
@@ -90,7 +89,7 @@ public UpdateControl reconcile(WebPage webPage, Context contex
desiredHtmlConfigMap.getMetadata().getName(),
ns);
kubernetesClient.configMaps().inNamespace(ns).resource(desiredHtmlConfigMap)
- .createOr(Replaceable::update);
+ .serverSideApply();
}
var existingDeployment = context.getSecondaryResource(Deployment.class).orElse(null);
@@ -100,7 +99,7 @@ public UpdateControl reconcile(WebPage webPage, Context contex
desiredDeployment.getMetadata().getName(),
ns);
kubernetesClient.apps().deployments().inNamespace(ns).resource(desiredDeployment)
- .createOr(Replaceable::update);
+ .serverSideApply();
}
var existingService = context.getSecondaryResource(Service.class).orElse(null);
@@ -110,14 +109,14 @@ public UpdateControl reconcile(WebPage webPage, Context contex
desiredDeployment.getMetadata().getName(),
ns);
kubernetesClient.services().inNamespace(ns).resource(desiredService)
- .createOr(Replaceable::update);
+ .serverSideApply();
}
var existingIngress = context.getSecondaryResource(Ingress.class);
if (Boolean.TRUE.equals(webPage.getSpec().getExposed())) {
var desiredIngress = makeDesiredIngress(webPage);
if (existingIngress.isEmpty() || !match(desiredIngress, existingIngress.get())) {
- kubernetesClient.resource(desiredIngress).inNamespace(ns).createOr(Replaceable::update);
+ kubernetesClient.resource(desiredIngress).inNamespace(ns).serverSideApply();
}
} else
existingIngress.ifPresent(