Skip to content

Cloner interface for Custom Resource instead of ObjectMapper #611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Oct 20, 2021
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.javaoperatorsdk.operator.api.config;

import io.fabric8.kubernetes.client.CustomResource;

public interface Cloner {

CustomResource<?, ?> clone(CustomResource<?, ?> object);

}
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,24 @@
import io.javaoperatorsdk.operator.Metrics;
import io.javaoperatorsdk.operator.api.ResourceController;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

/** An interface from which to retrieve configuration information. */
public interface ConfigurationService {

ObjectMapper OBJECT_MAPPER = new ObjectMapper();
Cloner DEFAULT_CLONER = new Cloner() {
private final ObjectMapper OBJECT_MAPPER = new ObjectMapper();

@Override
public CustomResource<?, ?> clone(CustomResource<?, ?> object) {
try {
return OBJECT_MAPPER.readValue(OBJECT_MAPPER.writeValueAsString(object), object.getClass());
} catch (JsonProcessingException e) {
throw new IllegalStateException(e);
}
}
};

/**
* Retrieves the configuration associated with the specified controller
Expand Down Expand Up @@ -79,14 +91,12 @@ default int concurrentReconciliationThreads() {
}

/**
* The {@link ObjectMapper} that the operator should use to de-/serialize resources. This is
* particularly useful when frameworks can configure a specific mapper that should also be used by
* the SDK.
*
* Used to clone custom resources.
*
* @return the ObjectMapper to use
*/
default ObjectMapper getObjectMapper() {
return OBJECT_MAPPER;
default Cloner getResourceCloner() {
return DEFAULT_CLONER;
}

int DEFAULT_TERMINATION_TIMEOUT_SECONDS = 10;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@
import io.javaoperatorsdk.operator.Metrics;
import io.javaoperatorsdk.operator.api.ResourceController;

import com.fasterxml.jackson.databind.ObjectMapper;

public class ConfigurationServiceOverrider {
private final ConfigurationService original;
private Metrics metrics;
private Config clientConfig;
private boolean checkCR;
private int threadNumber;
private ObjectMapper mapper;
private Cloner cloner;
private int timeoutSeconds;

public ConfigurationServiceOverrider(
Expand All @@ -24,7 +22,7 @@ public ConfigurationServiceOverrider(
this.clientConfig = original.getClientConfiguration();
this.checkCR = original.checkCRDAndValidateLocalModel();
this.threadNumber = original.concurrentReconciliationThreads();
this.mapper = original.getObjectMapper();
this.cloner = original.getResourceCloner();
this.timeoutSeconds = original.getTerminationTimeoutSeconds();
this.metrics = original.getMetrics();
}
Expand All @@ -45,8 +43,8 @@ public ConfigurationServiceOverrider withConcurrentReconciliationThreads(int thr
return this;
}

public ConfigurationServiceOverrider withObjectMapper(ObjectMapper mapper) {
this.mapper = mapper;
public ConfigurationServiceOverrider withResourceCloner(Cloner cloner) {
this.cloner = cloner;
return this;
}

Expand Down Expand Up @@ -94,8 +92,8 @@ public int concurrentReconciliationThreads() {
}

@Override
public ObjectMapper getObjectMapper() {
return mapper;
public Cloner getResourceCloner() {
return cloner;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@
import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
import io.fabric8.kubernetes.client.informers.cache.Cache;
import io.javaoperatorsdk.operator.MissingCRDException;
import io.javaoperatorsdk.operator.api.config.Cloner;
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.processing.ConfiguredController;
import io.javaoperatorsdk.operator.processing.ResourceCache;
import io.javaoperatorsdk.operator.processing.event.AbstractEventSource;
import io.javaoperatorsdk.operator.processing.event.CustomResourceID;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName;
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;
Expand All @@ -40,15 +38,14 @@ public class CustomResourceEventSource<T extends CustomResource<?, ?>> extends A
private final ConfiguredController<T> controller;
private final Map<String, SharedIndexInformer<T>> sharedIndexInformers =
new ConcurrentHashMap<>();
private final ObjectMapper cloningObjectMapper;

private final CustomResourceEventFilter<T> filter;
private final OnceWhitelistEventFilterEventFilter<T> onceWhitelistEventFilterEventFilter;

private final Cloner cloner;

public CustomResourceEventSource(ConfiguredController<T> controller) {
this.controller = controller;
this.cloningObjectMapper =
controller.getConfiguration().getConfigurationService().getObjectMapper();
this.cloner = controller.getConfiguration().getConfigurationService().getResourceCloner();

var filters = new CustomResourceEventFilter[] {
CustomResourceEventFilters.finalizerNeededAndApplied(),
Expand Down Expand Up @@ -160,7 +157,7 @@ public Optional<T> getCustomResource(CustomResourceID resourceID) {
if (resource == null) {
return Optional.empty();
} else {
return Optional.of(clone(resource));
return Optional.of((T) (cloner.clone(resource)));
}
}

Expand All @@ -176,15 +173,6 @@ public SharedIndexInformer<T> getInformer(String namespace) {
return getInformers().get(Objects.requireNonNullElse(namespace, ANY_NAMESPACE_MAP_KEY));
}

private T clone(T customResource) {
try {
return (T) cloningObjectMapper.readValue(
cloningObjectMapper.writeValueAsString(customResource), customResource.getClass());
} catch (JsonProcessingException e) {
throw new IllegalStateException(e);
}
}

/**
* This will ensure that the next event received after this method is called will not be filtered
* out.
Expand All @@ -196,4 +184,5 @@ public void whitelistNextEvent(CustomResourceID customResourceID) {
onceWhitelistEventFilterEventFilter.whitelistNextEvent(customResourceID);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() {
oldResource.getStatus().getConfigMapStatus(),
newResource.getStatus().getConfigMapStatus()));

when(config.getConfigurationService().getObjectMapper())
.thenReturn(ConfigurationService.OBJECT_MAPPER);
when(config.getConfigurationService().getResourceCloner())
.thenReturn(ConfigurationService.DEFAULT_CLONER);

var controller = new TestConfiguredController(config);
var eventSource = new CustomResourceEventSource<>(controller);
Expand Down Expand Up @@ -142,8 +142,8 @@ public TestControllerConfig(String finalizer, boolean generationAware,
TestCustomResource.class,
mock(ConfigurationService.class));

when(getConfigurationService().getObjectMapper())
.thenReturn(ConfigurationService.OBJECT_MAPPER);
when(getConfigurationService().getResourceCloner())
.thenReturn(ConfigurationService.DEFAULT_CLONER);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ public TestConfiguration(boolean generationAware) {
null,
TestCustomResource.class,
mock(ConfigurationService.class));
when(getConfigurationService().getObjectMapper())
.thenReturn(ConfigurationService.OBJECT_MAPPER);
when(getConfigurationService().getResourceCloner())
.thenReturn(ConfigurationService.DEFAULT_CLONER);
when(getConfigurationService().getMetrics())
.thenReturn(Metrics.NOOP);
}
Expand Down