Skip to content

feat: use fabric8 client json mapper by default #1382

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 2 commits into from
Aug 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,16 @@
public class AbstractConfigurationService implements ConfigurationService {
private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
private final Version version;
private final Cloner cloner;

public AbstractConfigurationService(Version version) {
this.version = version;
this.cloner = ConfigurationService.super.getResourceCloner();
}

public AbstractConfigurationService(Version version, Cloner cloner) {
this.version = version;
this.cloner = cloner;
}

protected <R extends HasMetadata> void register(ControllerConfiguration<R> config) {
Expand Down Expand Up @@ -77,10 +84,12 @@ protected <R extends HasMetadata> String keyFor(Reconciler<R> reconciler) {
return ReconcilerUtils.getNameFor(reconciler);
}

@SuppressWarnings("unused")
protected ControllerConfiguration getFor(String reconcilerName) {
return configurations.get(reconcilerName);
}

@SuppressWarnings("unused")
protected Stream<ControllerConfiguration> controllerConfigurations() {
return configurations.values().stream();
}
Expand All @@ -94,4 +103,9 @@ public Set<String> getKnownReconcilerNames() {
public Version getVersion() {
return version;
}

@Override
public Cloner getResourceCloner() {
return cloner;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ public BaseConfigurationService(Version version) {
super(version);
}

public BaseConfigurationService(Version version, Cloner cloner) {
super(version, cloner);
}

public BaseConfigurationService() {
this(Utils.loadFromProperties());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.Config;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.utils.Serialization;
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory;
Expand All @@ -17,20 +18,6 @@
/** An interface from which to retrieve configuration information. */
public interface ConfigurationService {

ObjectMapper OBJECT_MAPPER = new ObjectMapper();

Cloner DEFAULT_CLONER = new Cloner() {
@SuppressWarnings("unchecked")
@Override
public HasMetadata clone(HasMetadata 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 reconciler
*
Expand Down Expand Up @@ -93,12 +80,25 @@ default int concurrentReconciliationThreads() {
}

/**
* Used to clone custom resources.
* Used to clone custom resources. It is strongly suggested that implementors override this method
* since the default implementation creates a new {@link Cloner} instance each time this method is
* called.
*
* @return the ObjectMapper to use
* @return the configured {@link Cloner}
*/
default Cloner getResourceCloner() {
return DEFAULT_CLONER;
return new Cloner() {
@SuppressWarnings("unchecked")
@Override
public HasMetadata clone(HasMetadata object) {
try {
final var mapper = getObjectMapper();
return mapper.readValue(mapper.writeValueAsString(object), object.getClass());
} catch (JsonProcessingException e) {
throw new IllegalStateException(e);
}
}
};
}

int DEFAULT_TERMINATION_TIMEOUT_SECONDS = 10;
Expand Down Expand Up @@ -126,7 +126,7 @@ default boolean closeClientOnStop() {
}

default ObjectMapper getObjectMapper() {
return OBJECT_MAPPER;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the constant be removed altogether, as well, then?

return Serialization.jsonMapper();
}

default DependentResourceFactory dependentResourceFactory() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public ConfigurationServiceOverrider withExecutorService(ExecutorService executo
}

public ConfigurationService build() {
return new BaseConfigurationService(original.getVersion()) {
return new BaseConfigurationService(original.getVersion(), cloner) {
@Override
public Set<String> getKnownReconcilerNames() {
return original.getKnownReconcilerNames();
Expand All @@ -93,11 +93,6 @@ public int concurrentReconciliationThreads() {
return threadNumber;
}

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

@Override
public int getTerminationTimeoutSeconds() {
return timeoutSeconds;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResource;
import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResourceSpec;
Expand Down Expand Up @@ -52,7 +52,7 @@ void executeUpdateForTestingCacheUpdateForGetResource() {

awaitForDeploymentReadyReplicas(1);

var clonedCr = ConfigurationService.DEFAULT_CLONER.clone(createdCR);
var clonedCr = ConfigurationServiceProvider.instance().getResourceCloner().clone(createdCR);
clonedCr.getSpec().setReplicaCount(2);
operator.replace(clonedCr);

Expand Down