Skip to content
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

feat: allow manually specifying CRDs in test extension #2569

Merged
merged 2 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
@@ -1,6 +1,8 @@
package io.javaoperatorsdk.operator.junit;

import java.io.ByteArrayInputStream;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
Expand Down Expand Up @@ -43,6 +45,7 @@ public class LocallyRunOperatorExtension extends AbstractOperatorExtension {
private final List<LocalPortForward> localPortForwards;
private final List<Class<? extends CustomResource>> additionalCustomResourceDefinitions;
private final Map<Reconciler, RegisteredController> registeredControllers;
private final Map<Class<? extends CustomResource>, String> crdMappings;

private LocallyRunOperatorExtension(
List<ReconcilerSpec> reconcilers,
Expand All @@ -56,7 +59,8 @@ private LocallyRunOperatorExtension(
KubernetesClient kubernetesClient,
Consumer<ConfigurationServiceOverrider> configurationServiceOverrider,
Function<ExtensionContext, String> namespaceNameSupplier,
Function<ExtensionContext, String> perClassNamespaceNameSupplier) {
Function<ExtensionContext, String> perClassNamespaceNameSupplier,
Map<Class<? extends CustomResource>, String> crdMappings) {
super(
infrastructure,
infrastructureTimeout,
Expand All @@ -70,8 +74,13 @@ private LocallyRunOperatorExtension(
this.portForwards = portForwards;
this.localPortForwards = new ArrayList<>(portForwards.size());
this.additionalCustomResourceDefinitions = additionalCustomResourceDefinitions;
this.operator = new Operator(getKubernetesClient(), configurationServiceOverrider);
configurationServiceOverrider = configurationServiceOverrider != null
? configurationServiceOverrider
.andThen(overrider -> overrider.withKubernetesClient(kubernetesClient))
: overrider -> overrider.withKubernetesClient(kubernetesClient);
this.operator = new Operator(configurationServiceOverrider);
this.registeredControllers = new HashMap<>();
this.crdMappings = crdMappings;
}

/**
Expand All @@ -83,6 +92,52 @@ public static Builder builder() {
return new Builder();
}

public static void applyCrd(Class<? extends HasMetadata> resourceClass, KubernetesClient client) {
applyCrd(ReconcilerUtils.getResourceTypeName(resourceClass), client);
}

/**
* Applies the CRD associated with the specified resource name to the cluster. Note that the CRD
* is assumed to have been generated in this case from the Java classes and is therefore expected
* to be found in the standard location with the default name for such CRDs and assumes a v1
* version of the CRD spec is used. This means that, provided a given {@code resourceTypeName},
* the associated CRD is expected to be found at {@code META-INF/fabric8/resourceTypeName-v1.yml}
* in the project's classpath.
*
* @param resourceTypeName the standard resource name for CRDs i.e. {@code plural.group}
* @param client the kubernetes client to use to connect to the cluster
*/
public static void applyCrd(String resourceTypeName, KubernetesClient client) {
String path = "/META-INF/fabric8/" + resourceTypeName + "-v1.yml";
try (InputStream is = LocallyRunOperatorExtension.class.getResourceAsStream(path)) {
applyCrd(is, path, client);
} catch (IllegalStateException e) {
// rethrow directly
throw e;
} catch (IOException e) {
throw new IllegalStateException("Cannot apply CRD yaml: " + path, e);
}
}

private static void applyCrd(InputStream is, String path, KubernetesClient client) {
try {
if (is == null) {
throw new IllegalStateException("Cannot find CRD at " + path);
}
var crdString = new String(is.readAllBytes(), StandardCharsets.UTF_8);
LOGGER.debug("Applying CRD: {}", crdString);
final var crd = client.load(new ByteArrayInputStream(crdString.getBytes()));
crd.serverSideApply();
Thread.sleep(CRD_READY_WAIT); // readiness is not applicable for CRD, just wait a little
LOGGER.debug("Applied CRD with path: {}", path);
} catch (InterruptedException ex) {
LOGGER.error("Interrupted.", ex);
Thread.currentThread().interrupt();
} catch (Exception ex) {
throw new IllegalStateException("Cannot apply CRD yaml: " + path, ex);
}
}

private Stream<Reconciler> reconcilers() {
return reconcilers.stream().map(reconcilerSpec -> reconcilerSpec.reconciler);
}
Expand Down Expand Up @@ -134,14 +189,14 @@ protected void before(ExtensionContext context) {
.withName(podName).portForward(ref.getPort(), ref.getLocalPort()));
}

additionalCustomResourceDefinitions
.forEach(cr -> applyCrd(ReconcilerUtils.getResourceTypeName(cr)));
additionalCustomResourceDefinitions.forEach(this::applyCrd);

for (var ref : reconcilers) {
final var config = operator.getConfigurationService().getConfigurationFor(ref.reconciler);
final var oconfig = override(config);

if (Namespaced.class.isAssignableFrom(config.getResourceClass())) {
final var resourceClass = config.getResourceClass();
if (Namespaced.class.isAssignableFrom(resourceClass)) {
oconfig.settingNamespace(namespace);
}

Expand All @@ -153,8 +208,8 @@ protected void before(ExtensionContext context) {
}

// only try to apply a CRD for the reconciler if it is associated to a CR
if (CustomResource.class.isAssignableFrom(config.getResourceClass())) {
applyCrd(config.getResourceTypeName());
if (CustomResource.class.isAssignableFrom(resourceClass)) {
applyCrd(resourceClass);
}

var registeredController = this.operator.register(ref.reconciler, oconfig.build());
Expand All @@ -165,31 +220,24 @@ protected void before(ExtensionContext context) {
this.operator.start();
}

private void applyCrd(String resourceTypeName) {
applyCrd(resourceTypeName, getKubernetesClient());
}

public static void applyCrd(Class<? extends HasMetadata> resourceClass, KubernetesClient client) {
applyCrd(ReconcilerUtils.getResourceTypeName(resourceClass), client);
}

public static void applyCrd(String resourceTypeName, KubernetesClient client) {
String path = "/META-INF/fabric8/" + resourceTypeName + "-v1.yml";
try (InputStream is = LocallyRunOperatorExtension.class.getResourceAsStream(path)) {
if (is == null) {
throw new IllegalStateException("Cannot find CRD at " + path);
/**
* Applies the CRD associated with the specified custom resource, first checking if a CRD has been
* manually specified using {@link Builder#withCRDMapping(Class, String)}, otherwise assuming that
* its CRD should be found in the standard location as explained in
* {@link LocallyRunOperatorExtension#applyCrd(String, KubernetesClient)}
*
* @param crClass the custom resource class for which we want to apply the CRD
*/
public void applyCrd(Class<? extends CustomResource> crClass) {
final var path = crdMappings.get(crClass);
if (path != null) {
try (InputStream inputStream = new FileInputStream(path)) {
applyCrd(inputStream, path, getKubernetesClient());
} catch (IOException e) {
throw new IllegalStateException("Cannot apply CRD yaml: " + path, e);
}
var crdString = new String(is.readAllBytes(), StandardCharsets.UTF_8);
LOGGER.debug("Applying CRD: {}", crdString);
final var crd = client.load(new ByteArrayInputStream(crdString.getBytes()));
crd.serverSideApply();
Thread.sleep(CRD_READY_WAIT); // readiness is not applicable for CRD, just wait a little
LOGGER.debug("Applied CRD with path: {}", path);
} catch (InterruptedException ex) {
LOGGER.error("Interrupted.", ex);
Thread.currentThread().interrupt();
} catch (Exception ex) {
throw new IllegalStateException("Cannot apply CRD yaml: " + path, ex);
} else {
applyCrd(crClass, getKubernetesClient());
}
}

Expand Down Expand Up @@ -218,13 +266,15 @@ public static class Builder extends AbstractBuilder<Builder> {
private final List<ReconcilerSpec> reconcilers;
private final List<PortForwardSpec> portForwards;
private final List<Class<? extends CustomResource>> additionalCustomResourceDefinitions;
private final Map<Class<? extends CustomResource>, String> crdMappings;
private KubernetesClient kubernetesClient;

protected Builder() {
super();
this.reconcilers = new ArrayList<>();
this.portForwards = new ArrayList<>();
this.additionalCustomResourceDefinitions = new ArrayList<>();
this.crdMappings = new HashMap<>();
}

public Builder withReconciler(
Expand Down Expand Up @@ -279,6 +329,12 @@ public Builder withAdditionalCustomResourceDefinition(
return this;
}

public Builder withCRDMapping(Class<? extends CustomResource> customResourceClass,
String path) {
crdMappings.put(customResourceClass, path);
return this;
}

public LocallyRunOperatorExtension build() {
return new LocallyRunOperatorExtension(
reconcilers,
Expand All @@ -290,7 +346,8 @@ public LocallyRunOperatorExtension build() {
waitForNamespaceDeletion,
oneNamespacePerClass,
kubernetesClient,
configurationServiceOverrider, namespaceNameSupplier, perClassNamespaceNameSupplier);
configurationServiceOverrider, namespaceNameSupplier, perClassNamespaceNameSupplier,
crdMappings);
}
}

Expand Down
19 changes: 19 additions & 0 deletions operator-framework/src/test/crd/test.crd
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
name: tests.crd.example
spec:
group: crd.example
names:
kind: Test
singular: test
plural: tests
scope: Namespaced
versions:
- name: v1
schema:
openAPIV3Schema:
properties:
type: "object"
served: true
storage: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package io.javaoperatorsdk.operator;

import java.time.Duration;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
import io.fabric8.kubernetes.model.annotation.Group;
import io.fabric8.kubernetes.model.annotation.Kind;
import io.fabric8.kubernetes.model.annotation.Version;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;

import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;

public class CRDMappingInTestExtensionIT {
private final KubernetesClient client = new KubernetesClientBuilder().build();

@RegisterExtension
LocallyRunOperatorExtension operator =
LocallyRunOperatorExtension.builder()
.withReconciler(new TestReconciler())
.withCRDMapping(TestCR.class, "src/test/crd/test.crd")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding this API, not sure if we need to specify the class here, that is rather limiting, if someone for example uses dynamic API does not have the class. It would be enough to have just the path to cover even that possibility.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This feature is meant for people who use the contract-first approach so they would have the class handy and most likely wouldn't use generic resources but I guess we could use the CRD name instead of the CR class.

Copy link
Collaborator

@csviri csviri Nov 7, 2024

Choose a reason for hiding this comment

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

I mean we can make it even cover even that case (So dynamic resources handling with GenericKubernetesResource)

Why we need a name or any id ? cannot we have an API just with a path?

Copy link
Collaborator Author

@metacosm metacosm Nov 7, 2024

Choose a reason for hiding this comment

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

Because that would imply parsing the file to check the name, which I would rather avoid. It also allows to potentially override a generated CRD with a manual one if needed (though, this probably is more of a corner case).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, but should we rather parse the CRD and index it based on API version. And check if that is the same as resources API version mentioned in reconciler. (But always apply it even if there is no reconciler corresponding to it). This also delegates responsibility to the user to name it correctly, what we could definitely simplify in the background seamlessly.

Copy link
Collaborator

@csviri csviri Nov 8, 2024

Choose a reason for hiding this comment

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

So basically I think we should parse the crd to have nicer API here. If you want I can do it in a separate PR. (But we should not release before)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, please open a PR with that change.

.build();

@Test
void correctlyAppliesManuallySpecifiedCRD() {
operator.applyCrd(TestCR.class);

final var crdClient = client.apiextensions().v1().customResourceDefinitions();
await().pollDelay(Duration.ofMillis(150))
.untilAsserted(() -> assertThat(crdClient.withName("tests.crd.example").get()).isNotNull());
}

@Group("crd.example")
@Version("v1")
@Kind("Test")
private static class TestCR extends CustomResource<Void, Void> implements Namespaced {
}

@ControllerConfiguration
private static class TestReconciler implements Reconciler<TestCR> {
@Override
public UpdateControl<TestCR> reconcile(TestCR resource, Context<TestCR> context)
throws Exception {
return null;
}
}
}
Loading