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

improve: simpler api for adding additional CRD file #2574

Merged
merged 4 commits into from
Nov 19, 2024
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 @@ -2,6 +2,7 @@

import java.io.ByteArrayInputStream;
import java.io.FileInputStream;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
Expand All @@ -21,6 +22,7 @@

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.api.model.Namespaced;
import io.fabric8.kubernetes.api.model.apiextensions.v1.CustomResourceDefinition;
import io.fabric8.kubernetes.client.CustomResource;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.LocalPortForward;
Expand All @@ -45,7 +47,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<String, String> crdMappings;
private final List<String> additionalCrds;

private LocallyRunOperatorExtension(
List<ReconcilerSpec> reconcilers,
Expand All @@ -60,7 +62,7 @@ private LocallyRunOperatorExtension(
Consumer<ConfigurationServiceOverrider> configurationServiceOverrider,
Function<ExtensionContext, String> namespaceNameSupplier,
Function<ExtensionContext, String> perClassNamespaceNameSupplier,
Map<String, String> crdMappings) {
List<String> additionalCrds) {
super(
infrastructure,
infrastructureTimeout,
Expand All @@ -80,7 +82,7 @@ private LocallyRunOperatorExtension(
: overrider -> overrider.withKubernetesClient(kubernetesClient);
this.operator = new Operator(configurationServiceOverrider);
this.registeredControllers = new HashMap<>();
this.crdMappings = crdMappings;
this.additionalCrds = additionalCrds;
}

/**
Expand Down Expand Up @@ -119,6 +121,10 @@ public static void applyCrd(String resourceTypeName, KubernetesClient client) {
}
}

public static void applyCrd(CustomResourceDefinition crd, KubernetesClient client) {
client.resource(crd).serverSideApply();
}

private static void applyCrd(InputStream is, String path, KubernetesClient client) {
try {
if (is == null) {
Expand All @@ -138,6 +144,17 @@ private static void applyCrd(InputStream is, String path, KubernetesClient clien
}
}

public static List<CustomResourceDefinition> parseCrds(String path, KubernetesClient client) {
try (InputStream is = new FileInputStream(path)) {
return client.load(new ByteArrayInputStream(is.readAllBytes()))
.items().stream().map(i -> (CustomResourceDefinition) i).collect(Collectors.toList());
} catch (FileNotFoundException e) {
throw new RuntimeException(e);
} catch (IOException e) {
throw new RuntimeException(e);
}
}

private Stream<Reconciler> reconcilers() {
return reconcilers.stream().map(reconcilerSpec -> reconcilerSpec.reconciler);
}
Expand Down Expand Up @@ -190,7 +207,7 @@ protected void before(ExtensionContext context) {
}

additionalCustomResourceDefinitions.forEach(this::applyCrd);

Map<String, CustomResourceDefinition> unappliedCRDs = getAdditionalCRDsFromFiles();
for (var ref : reconcilers) {
final var config = operator.getConfigurationService().getConfigurationFor(ref.reconciler);
final var oconfig = override(config);
Expand All @@ -207,29 +224,40 @@ protected void before(ExtensionContext context) {
ref.controllerConfigurationOverrider.accept(oconfig);
}

final var unapplied = new HashMap<>(crdMappings);
final var resourceTypeName = ReconcilerUtils.getResourceTypeName(resourceClass);
// only try to apply a CRD for the reconciler if it is associated to a CR
if (CustomResource.class.isAssignableFrom(resourceClass)) {
applyCrd(resourceTypeName);
unapplied.remove(resourceTypeName);
if (unappliedCRDs.get(resourceTypeName) != null) {
applyCrd(resourceTypeName);
unappliedCRDs.remove(resourceTypeName);
} else {
applyCrd(resourceClass);
}
}

// apply yet unapplied CRDs
unapplied.keySet().forEach(this::applyCrd);

var registeredController = this.operator.register(ref.reconciler, oconfig.build());
registeredControllers.put(ref.reconciler, registeredController);
}
unappliedCRDs.keySet().forEach(this::applyCrd);

LOGGER.debug("Starting the operator locally");
this.operator.start();
}

private Map<String, CustomResourceDefinition> getAdditionalCRDsFromFiles() {
Map<String, CustomResourceDefinition> crdMappings = new HashMap<>();
additionalCrds.forEach(p -> {
var crds = parseCrds(p, getKubernetesClient());
crds.forEach(c -> crdMappings.put(c.getMetadata().getName(), c));
});
return crdMappings;
}

/**
* 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
* manually specified using {@link Builder#withAdditionalCRD(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
Expand All @@ -239,16 +267,7 @@ public void applyCrd(Class<? extends CustomResource> crClass) {
}

public void applyCrd(String resourceTypeName) {
final var path = crdMappings.get(resourceTypeName);
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);
}
} else {
applyCrd(resourceTypeName, getKubernetesClient());
}
applyCrd(resourceTypeName, getKubernetesClient());
}

@Override
Expand Down Expand Up @@ -277,6 +296,7 @@ public static class Builder extends AbstractBuilder<Builder> {
private final List<PortForwardSpec> portForwards;
private final List<Class<? extends CustomResource>> additionalCustomResourceDefinitions;
private final Map<String, String> crdMappings;
private final List<String> additionalCRDs = new ArrayList<>();
private KubernetesClient kubernetesClient;

protected Builder() {
Expand Down Expand Up @@ -339,13 +359,8 @@ public Builder withAdditionalCustomResourceDefinition(
return this;
}

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

public Builder withCRDMapping(String resourceTypeName, String path) {
crdMappings.put(resourceTypeName, path);
public Builder withAdditionalCRD(String path) {
additionalCRDs.add(path);
return this;
}

Expand All @@ -360,8 +375,9 @@ public LocallyRunOperatorExtension build() {
waitForNamespaceDeletion,
oneNamespacePerClass,
kubernetesClient,
configurationServiceOverrider, namespaceNameSupplier, perClassNamespaceNameSupplier,
crdMappings);
configurationServiceOverrider, namespaceNameSupplier,
perClassNamespaceNameSupplier,
additionalCRDs);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class CRDMappingInTestExtensionIT {
LocallyRunOperatorExtension operator =
LocallyRunOperatorExtension.builder()
.withReconciler(new TestReconciler())
.withCRDMapping("tests.crd.example", "src/test/crd/test.crd")
.withAdditionalCRD("src/test/resources/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.

This was explicitly not put in resources to check that the file could be accessed from a "random" part of the file system, not just from the classpath.

Copy link
Collaborator Author

@csviri csviri Nov 15, 2024

Choose a reason for hiding this comment

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

I see, isn't it trivial? I would rather stick with this, but no strong opinion, if you insist I can change it back.

.build();

@Test
Expand Down
Loading