Skip to content

refactor: improve configuration utilities #1519

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

Merged
merged 2 commits into from
Oct 5, 2022
Merged

refactor: improve configuration utilities #1519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d10ca90
refactor: improve configuration utilities
metacosm Sep 1, 2022
06e3226
fix: account for parameterized argument
metacosm Oct 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
99 changes: 32 additions & 67 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,8 +1,6 @@
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;
Expand Down Expand Up @@ -41,10 +39,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 +146,54 @@ 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 <T> void configureFromAnnotatedReconciler(T 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);
}

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));
}
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 Down Expand Up @@ -244,12 +221,12 @@ public List<DependentResourceSpec> getDependentResources() {
throw new IllegalArgumentException(
"A DependentResource named '" + name + "' already exists: " + spec);
}
final var context = "DependentResource of type '" + dependentType.getName() + "'";
final var context = Utils.contextFor(this, dependentType, null);
spec = new DependentResourceSpec(dependentType, config, 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 +235,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 Down Expand Up @@ -299,18 +268,14 @@ private Object createKubernetesResourceConfig(Class<? extends DependentResource>


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

config =
Expand Down
113 changes: 102 additions & 11 deletions operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
package io.javaoperatorsdk.operator.api.config;

import java.io.IOException;
import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.text.SimpleDateFormat;
import java.time.Instant;
import java.util.Arrays;
import java.util.Date;
import java.util.Optional;
import java.util.Properties;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;

public class Utils {

Expand Down Expand Up @@ -106,17 +111,49 @@ public static Class<?> getFirstTypeArgumentFromExtendedClass(Class<?> clazz) {

public static Class<?> getFirstTypeArgumentFromInterface(Class<?> clazz,
Class<?> expectedImplementedInterface) {
return Arrays.stream(clazz.getGenericInterfaces())
.filter(type -> type.getTypeName().startsWith(expectedImplementedInterface.getName())
&& type instanceof ParameterizedType)
.map(ParameterizedType.class::cast)
.findFirst()
.map(t -> (Class<?>) t.getActualTypeArguments()[0])
.orElseThrow(() -> new RuntimeException(
"Couldn't retrieve generic parameter type from " + clazz.getSimpleName()
+ " because it doesn't implement "
+ expectedImplementedInterface.getSimpleName()
+ " directly"));
if (expectedImplementedInterface.isAssignableFrom(clazz)) {
final var genericInterfaces = clazz.getGenericInterfaces();
Optional<? extends Class<?>> target = Optional.empty();
if (genericInterfaces.length > 0) {
// try to find the target interface among them
target = Arrays.stream(genericInterfaces)
.filter(type -> type.getTypeName().startsWith(expectedImplementedInterface.getName())
&& type instanceof ParameterizedType)
.map(ParameterizedType.class::cast)
.findFirst()
.map(t -> {
final Type argument = t.getActualTypeArguments()[0];
if (argument instanceof Class) {
return (Class<?>) argument;
}
// account for the case where the argument itself has parameters, which we will ignore
// and just return the raw type
if (argument instanceof ParameterizedType) {
final var rawType = ((ParameterizedType) argument).getRawType();
if (rawType instanceof Class) {
return (Class<?>) rawType;
}
}
throw new IllegalArgumentException(clazz.getSimpleName() + " implements "
+ expectedImplementedInterface.getSimpleName()
+ " but indirectly. Java type erasure doesn't allow to retrieve the generic type from it. Retrieved type was: "
+ argument);
});
}

if (target.isPresent()) {
return target.get();
}

// try the parent
var parent = clazz.getSuperclass();
if (!Object.class.equals(parent)) {
return getFirstTypeArgumentFromInterface(parent, expectedImplementedInterface);
}
}
throw new IllegalArgumentException("Couldn't retrieve generic parameter type from "
+ clazz.getSimpleName() + " because it or its superclasses don't implement "
+ expectedImplementedInterface.getSimpleName());
}

public static Class<?> getFirstTypeArgumentFromSuperClassOrInterface(Class<?> clazz,
Expand Down Expand Up @@ -144,4 +181,58 @@ public static Class<?> getFirstTypeArgumentFromSuperClassOrInterface(Class<?> cl
"Couldn't retrieve generic parameter type from " + clazz.getSimpleName(), e);
}
}

public static <T> T instantiateAndConfigureIfNeeded(Class<? extends T> targetClass,
Class<T> expectedType, String context, Configurator<T> configurator) {
// if class to instantiate equals the expected interface, we cannot instantiate it so just
// return null as it means we passed on void-type default value
if (expectedType.equals(targetClass)) {
return null;
}

try {
final Constructor<? extends T> constructor = targetClass.getDeclaredConstructor();
constructor.setAccessible(true);
final var instance = constructor.newInstance();

if (configurator != null) {
configurator.configure(instance);
}

return instance;
} catch (InstantiationException | IllegalAccessException | InvocationTargetException
| NoSuchMethodException e) {
throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '"
+ targetClass.getName() + "': you need to provide an accessible no-arg constructor."
+ (context != null ? " Context: " + context : ""), e);
}
}

public static <T> T instantiate(Class<? extends T> toInstantiate, Class<T> expectedType,
String context) {
return instantiateAndConfigureIfNeeded(toInstantiate, expectedType, context, null);
}

@FunctionalInterface
public interface Configurator<T> {
void configure(T instance);
}

@SuppressWarnings("rawtypes")
public static String contextFor(ControllerConfiguration<?> controllerConfiguration,
Class<? extends DependentResource> dependentType,
Class<? extends Annotation> configurationAnnotation) {
final var annotationName =
configurationAnnotation != null ? configurationAnnotation.getSimpleName()
: io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration.class
.getSimpleName();
var context = "annotation: " + annotationName + ", ";
if (dependentType != null) {
context += "DependentResource: " + dependentType.getName() + ", ";
}
context += "reconciler: " + controllerConfiguration.getName();


return context;
}
}
11 changes: 11 additions & 0 deletions operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,14 @@
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
import io.javaoperatorsdk.operator.processing.dependent.EmptyTestDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
Expand Down Expand Up @@ -89,6 +92,14 @@ void getsFirstTypeArgumentFromInterface() {
assertThat(Utils.getFirstTypeArgumentFromInterface(EmptyTestDependentResource.class,
DependentResource.class))
.isEqualTo(Deployment.class);

assertThatIllegalArgumentException().isThrownBy(
() -> Utils.getFirstTypeArgumentFromInterface(TestKubernetesDependentResource.class,
DependentResource.class));

assertThat(Utils.getFirstTypeArgumentFromInterface(TestKubernetesDependentResource.class,
DependentResourceConfigurator.class))
.isEqualTo(KubernetesDependentResourceConfig.class);
}

@Test
Expand Down