Skip to content

WIP: unify configuration #1440

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
Closed

WIP: unify configuration #1440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4fc4452
refactor: cleaner separation between instantiation and configuration
metacosm Sep 1, 2022
555a738
refactor: move instantiation functionality to Utils
metacosm Sep 1, 2022
33a7d3d
refactor: unify context information creation, move it to Utils
metacosm Sep 1, 2022
75822b7
refactor: improve getFirstTypeArgumentFromInterface
metacosm Sep 2, 2022
8ff0e82
feat: introduce AnnotationDependentResourceConfigurator concept
metacosm Sep 2, 2022
80fb167
fix: implement configuration method
metacosm Sep 2, 2022
81749c7
refactor: use new configuration option on SchemaDependentResource
metacosm Sep 2, 2022
46dea02
fix: deactivate Sonar password inclusion
metacosm Sep 5, 2022
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
165 changes: 51 additions & 114 deletions ...c/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package io.javaoperatorsdk.operator.api.config;

import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.time.Duration;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
Expand All @@ -17,21 +14,17 @@
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
import io.javaoperatorsdk.operator.api.reconciler.Constants;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.AnnotationDependentResourceConfigurator;
import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter;
import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
import io.javaoperatorsdk.operator.processing.retry.Retry;

Expand All @@ -41,10 +34,6 @@
public class AnnotationControllerConfiguration<P extends HasMetadata>
implements io.javaoperatorsdk.operator.api.config.ControllerConfiguration<P> {

private static final String CONTROLLER_CONFIG_ANNOTATION =
ControllerConfiguration.class.getSimpleName();
private static final String KUBE_DEPENDENT_NAME = KubernetesDependent.class.getSimpleName();

protected final Reconciler<P> reconciler;
private final ControllerConfiguration annotation;
private List<DependentResourceSpec> specs;
Expand Down Expand Up @@ -152,71 +141,70 @@ public Optional<Duration> maxReconciliationInterval() {
@Override
public RateLimiter getRateLimiter() {
final Class<? extends RateLimiter> rateLimiterClass = annotation.rateLimiter();
return instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class,
CONTROLLER_CONFIG_ANNOTATION);
return Utils.instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class,
Utils.contextFor(this, null, null), this::configureFromAnnotatedReconciler);
}

@Override
public Retry getRetry() {
final Class<? extends Retry> retryClass = annotation.retry();
return instantiateAndConfigureIfNeeded(retryClass, Retry.class, CONTROLLER_CONFIG_ANNOTATION);
return Utils.instantiateAndConfigureIfNeeded(retryClass, Retry.class,
Utils.contextFor(this, null, null), this::configureFromAnnotatedReconciler);
}


@SuppressWarnings("unchecked")
protected <T> T instantiateAndConfigureIfNeeded(Class<? extends T> targetClass,
Class<T> expectedType, String context) {
try {
final Constructor<? extends T> constructor = targetClass.getDeclaredConstructor();
constructor.setAccessible(true);
final var instance = constructor.newInstance();
if (instance instanceof AnnotationConfigurable) {
AnnotationConfigurable configurable = (AnnotationConfigurable) instance;
final Class<? extends Annotation> configurationClass =
(Class<? extends Annotation>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(
targetClass, AnnotationConfigurable.class);
final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass);
if (configAnnotation != null) {
configurable.initFrom(configAnnotation);
}
private void configureFromAnnotatedReconciler(Object instance) {
if (instance instanceof AnnotationConfigurable) {
AnnotationConfigurable configurable = (AnnotationConfigurable) instance;
final Class<? extends Annotation> configurationClass =
(Class<? extends Annotation>) Utils.getFirstTypeArgumentFromSuperClassOrInterface(
instance.getClass(), AnnotationConfigurable.class);
final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass);
if (configAnnotation != null) {
configurable.initFrom(configAnnotation);
}
return instance;
} catch (InstantiationException | IllegalAccessException | InvocationTargetException
| NoSuchMethodException e) {
throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '"
+ targetClass.getName() + "' for '" + getName()
+ "' reconciler in " + context
+ ". You need to provide an accessible no-arg constructor.", e);
}
}

@Override
@SuppressWarnings("unchecked")
public Optional<OnAddFilter<P>> onAddFilter() {
return (Optional<OnAddFilter<P>>) createFilter(annotation.onAddFilter(), OnAddFilter.class,
CONTROLLER_CONFIG_ANNOTATION);
private void configureFromCustomAnnotation(Object instance) {
if (instance instanceof AnnotationDependentResourceConfigurator) {
AnnotationDependentResourceConfigurator configurator =
(AnnotationDependentResourceConfigurator) instance;
final Class<? extends Annotation> configurationClass =
(Class<? extends Annotation>) Utils.getFirstTypeArgumentFromInterface(
instance.getClass(), AnnotationDependentResourceConfigurator.class);
final var configAnnotation = instance.getClass().getAnnotation(configurationClass);
// always called even if the annotation is null so that implementations can provide default
// values
final var config = configurator.configFrom(configAnnotation, this);
configurator.configureWith(config);
}
}

protected <T> Optional<? extends T> createFilter(Class<? extends T> filter, Class<T> defaultValue,
String origin) {
if (defaultValue.equals(filter)) {
return Optional.empty();
} else {
return Optional.of(instantiateAndConfigureIfNeeded(filter, defaultValue, origin));
}
@Override
@SuppressWarnings("unchecked")
public Optional<OnAddFilter<P>> onAddFilter() {
return Optional.ofNullable(
Utils.instantiate(annotation.onAddFilter(), OnAddFilter.class,
Utils.contextFor(this, null, null)));
}

@SuppressWarnings("unchecked")
@Override
public Optional<OnUpdateFilter<P>> onUpdateFilter() {
return (Optional<OnUpdateFilter<P>>) createFilter(annotation.onUpdateFilter(),
OnUpdateFilter.class, CONTROLLER_CONFIG_ANNOTATION);
return Optional.ofNullable(
Utils.instantiate(annotation.onUpdateFilter(), OnUpdateFilter.class,
Utils.contextFor(this, null, null)));
}

@SuppressWarnings("unchecked")
@Override
public Optional<GenericFilter<P>> genericFilter() {
return (Optional<GenericFilter<P>>) createFilter(annotation.genericFilter(),
GenericFilter.class, CONTROLLER_CONFIG_ANNOTATION);
return Optional.ofNullable(
Utils.instantiate(annotation.genericFilter(), GenericFilter.class,
Utils.contextFor(this, null, null)));
}

@SuppressWarnings({"rawtypes", "unchecked"})
Expand All @@ -232,24 +220,26 @@ public List<DependentResourceSpec> getDependentResources() {

final var specsMap = new LinkedHashMap<String, DependentResourceSpec>(dependents.length);
for (Dependent dependent : dependents) {
Object config = null;
final Class<? extends DependentResource> dependentType = dependent.type();
if (KubernetesDependentResource.class.isAssignableFrom(dependentType)) {
config = createKubernetesResourceConfig(dependentType);
}

final var name = getName(dependent, dependentType);
var spec = specsMap.get(name);
if (spec != null) {
throw new IllegalArgumentException(
"A DependentResource named '" + name + "' already exists: " + spec);
}
final var context = "DependentResource of type '" + dependentType.getName() + "'";
spec = new DependentResourceSpec(dependentType, config, name,

final var dependentResource = Utils.instantiateAndConfigureIfNeeded(dependentType,
DependentResource.class,
Utils.contextFor(this, dependentType, Dependent.class),
this::configureFromCustomAnnotation);

final var context = Utils.contextFor(this, dependentType, null);
spec = new DependentResourceSpec(dependentResource, name,
Set.of(dependent.dependsOn()),
instantiateConditionIfNotDefault(dependent.readyPostcondition(), context),
instantiateConditionIfNotDefault(dependent.reconcilePrecondition(), context),
instantiateConditionIfNotDefault(dependent.deletePostcondition(), context));
Utils.instantiate(dependent.readyPostcondition(), Condition.class, context),
Utils.instantiate(dependent.reconcilePrecondition(), Condition.class, context),
Utils.instantiate(dependent.deletePostcondition(), Condition.class, context));
specsMap.put(name, spec);
}

Expand All @@ -258,14 +248,6 @@ public List<DependentResourceSpec> getDependentResources() {
return specs;
}

protected Condition<?, ?> instantiateConditionIfNotDefault(Class<? extends Condition> condition,
String context) {
if (condition != Condition.class) {
return instantiateAndConfigureIfNeeded(condition, Condition.class, context);
}
return null;
}

private String getName(Dependent dependent, Class<? extends DependentResource> dependentType) {
var name = dependent.name();
if (name.isBlank()) {
Expand All @@ -274,51 +256,6 @@ private String getName(Dependent dependent, Class<? extends DependentResource> d
return name;
}

@SuppressWarnings({"rawtypes", "unchecked"})
private Object createKubernetesResourceConfig(Class<? extends DependentResource> dependentType) {

Object config;
final var kubeDependent = dependentType.getAnnotation(KubernetesDependent.class);

var namespaces = getNamespaces();
var configuredNS = false;
String labelSelector = null;
OnAddFilter<? extends HasMetadata> onAddFilter = null;
OnUpdateFilter<? extends HasMetadata> onUpdateFilter = null;
OnDeleteFilter<? extends HasMetadata> onDeleteFilter = null;
GenericFilter<? extends HasMetadata> genericFilter = null;
if (kubeDependent != null) {
if (!Arrays.equals(KubernetesDependent.DEFAULT_NAMESPACES,
kubeDependent.namespaces())) {
namespaces = Set.of(kubeDependent.namespaces());
configuredNS = true;
}

final var fromAnnotation = kubeDependent.labelSelector();
labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation;


final var context =
KUBE_DEPENDENT_NAME + " annotation on " + dependentType.getName() + " DependentResource";
onAddFilter = createFilter(kubeDependent.onAddFilter(), OnAddFilter.class, context)
.orElse(null);
onUpdateFilter =
createFilter(kubeDependent.onUpdateFilter(), OnUpdateFilter.class, context)
.orElse(null);
onDeleteFilter =
createFilter(kubeDependent.onDeleteFilter(), OnDeleteFilter.class, context)
.orElse(null);
genericFilter =
createFilter(kubeDependent.genericFilter(), GenericFilter.class, context)
.orElse(null);
}

config =
new KubernetesDependentResourceConfig(namespaces, labelSelector, configuredNS, onAddFilter,
onUpdateFilter, onDeleteFilter, genericFilter);

return config;
}

public static <T> T valueOrDefault(
ControllerConfiguration controllerConfiguration,
Expand Down
3 changes: 2 additions & 1 deletion ...ework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,9 @@ default ObjectMapper getObjectMapper() {
return Serialization.jsonMapper();
}

@Deprecated(forRemoval = true)
default DependentResourceFactory dependentResourceFactory() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if understand this. It's null since not used anymore I guess.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Backwards compatibility :)

return new DependentResourceFactory() {};
return null;
}

default Optional<LeaderElectionConfiguration> getLeaderElectionConfiguration() {
Expand Down
70 changes: 40 additions & 30 deletions ...rc/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
Expand Down Expand Up @@ -159,42 +161,58 @@ public ControllerConfigurationOverrider<R> withGenericFilter(GenericFilter<R> ge
return this;
}

@SuppressWarnings("unchecked")
public ControllerConfigurationOverrider<R> replacingNamedDependentResourceConfig(String name,
Object dependentResourceConfig) {

var current = namedDependentResourceSpecs.get(name);
if (current == null) {
throw new IllegalArgumentException("Cannot find a DependentResource named: " + name);
}
replaceConfig(name, dependentResourceConfig, current);
return this;
}

private void replaceConfig(String name, Object newConfig, DependentResourceSpec<?, ?> current) {
namedDependentResourceSpecs.put(name,
new DependentResourceSpec<>(current.getDependentResourceClass(), newConfig, name,
current.getDependsOn(), current.getReadyCondition(), current.getReconcileCondition(),
current.getDeletePostCondition()));
var dependentResource = current.getDependentResource();
if (dependentResource instanceof DependentResourceConfigurator) {
var configurator = (DependentResourceConfigurator) dependentResource;
configurator.configureWith(dependentResourceConfig);
}

return this;
}

@SuppressWarnings("unchecked")
public ControllerConfiguration<R> build() {
// todo: this should be abstracted by introducing an interface to deal with listening to
// namespace changes as possibly other things than the informers might be interested in reacting
// to such changes
// propagate namespaces if needed
final List<DependentResourceSpec> newDependentSpecs;

final var hasModifiedNamespaces = !original.getNamespaces().equals(namespaces);
newDependentSpecs = namedDependentResourceSpecs.entrySet().stream()
.map(drsEntry -> {
final var spec = drsEntry.getValue();

// if the spec has a config and it's a KubernetesDependentResourceConfig, update the
// namespaces if needed, otherwise, just return the existing spec
final Optional<?> maybeConfig = spec.getDependentResourceConfiguration();
return maybeConfig.filter(KubernetesDependentResourceConfig.class::isInstance)
.map(KubernetesDependentResourceConfig.class::cast)
.filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured))
.map(c -> updateSpec(drsEntry.getKey(), spec, c))
.orElse(drsEntry.getValue());
}).collect(Collectors.toUnmodifiableList());
final var newDependentSpecs = namedDependentResourceSpecs.values().stream()
.map(spec -> {
// if the dependent resource has a config and it's a KubernetesDependentResourceConfig,
// update
// the namespaces if needed, otherwise, do nothing
DependentResource dependent = spec.getDependentResource();
Optional<DependentResourceSpec> updated = Optional.empty();
if (hasModifiedNamespaces && dependent instanceof DependentResourceConfigurator) {
DependentResourceConfigurator configurator = (DependentResourceConfigurator) dependent;
final Optional<?> config = configurator.configuration();
updated = config.filter(KubernetesDependentResourceConfig.class::isInstance)
.map(KubernetesDependentResourceConfig.class::cast)
.filter(Predicate.not(KubernetesDependentResourceConfig::wereNamespacesConfigured))
.map(c -> {
// update the namespaces of the config, configure the dependent with it and update
// the spec
c.setNamespaces(namespaces);
configurator.configureWith(c);
return new DependentResourceSpec(dependent, spec.getName(), spec.getDependsOn(),
spec.getReadyCondition(), spec.getReconcileCondition(),
spec.getDeletePostCondition());
});
}

return updated.orElse(spec);
}).collect(Collectors.toList());

return new DefaultControllerConfiguration<>(
original.getAssociatedReconcilerClassName(),
Expand All @@ -215,14 +233,6 @@ public ControllerConfiguration<R> build() {
newDependentSpecs);
}

@SuppressWarnings({"rawtypes", "unchecked"})
private DependentResourceSpec<?, ?> updateSpec(String name, DependentResourceSpec spec,
KubernetesDependentResourceConfig c) {
return new DependentResourceSpec(spec.getDependentResourceClass(),
c.setNamespaces(namespaces), name, spec.getDependsOn(), spec.getReadyCondition(),
spec.getReconcileCondition(), spec.getDeletePostCondition());
}

public static <R extends HasMetadata> ControllerConfigurationOverrider<R> override(
ControllerConfiguration<R> original) {
return new ControllerConfigurationOverrider<>(original);
Expand Down
Loading