Skip to content

Commit

Permalink
refactor: code improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@jsenko
jsenko committed Oct 16, 2024
1 parent f963b78 commit 95555e5
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 71 deletions.
36 changes: 24 additions & 12 deletions ...oller/src/main/java/io/apicurio/registry/operator/resource/app/AppDeploymentResource.java
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
package io.apicurio.registry.operator.resource.app;

import io.apicurio.registry.operator.OperatorException;
import io.apicurio.registry.operator.api.v1.ApicurioRegistry3;
import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.EnvVar;
import io.fabric8.kubernetes.api.model.EnvVarBuilder;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import io.javaoperatorsdk.operator.api.reconciler.Context;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource;
import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
import io.javaoperatorsdk.operator.processing.event.ResourceID;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -18,6 +21,7 @@
import static io.apicurio.registry.operator.resource.ResourceFactory.COMPONENT_APP;
import static io.apicurio.registry.operator.resource.ResourceKey.APP_DEPLOYMENT_KEY;
import static io.apicurio.registry.operator.utils.Mapper.toYAML;
import static java.util.Objects.requireNonNull;

// spotless:off
@KubernetesDependent(
Expand All @@ -39,6 +43,9 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
var d = APP_DEPLOYMENT_KEY.getFactory().apply(primary);

var envVars = new LinkedHashMap<String, EnvVar>();
primary.getSpec().getApp().getEnv().forEach(e -> {
envVars.put(e.getName(), e);
});

// spotless:off
addEnvVar(envVars, new EnvVarBuilder().withName("QUARKUS_PROFILE").withValue("prod").build());
Expand All @@ -51,23 +58,28 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
addEnvVar(envVars, new EnvVarBuilder().withName("APICURIO_APIS_V2_DATE_FORMAT").withValue("yyyy-MM-dd''T''HH:mm:ssZ").build());
// spotless:on

// This must be done after any modification of the map by the operator.
primary.getSpec().getApp().getEnv().forEach(e -> {
envVars.remove(e.getName());
envVars.put(e.getName(), e);
});

for (var c : d.getSpec().getTemplate().getSpec().getContainers()) {
if (APP_CONTAINER_NAME.equals(c.getName())) {
c.setEnv(envVars.values().stream().toList());
}
}
var container = getContainer(d, APP_CONTAINER_NAME);
container.setEnv(envVars.values().stream().toList());

log.debug("Desired {} is {}", APP_DEPLOYMENT_KEY.getId(), toYAML(d));
return d;
}

public static void addEnvVar(Map<String, EnvVar> map, EnvVar envVar) {
map.put(envVar.getName(), envVar);
if (!map.containsKey(envVar.getName())) {
map.put(envVar.getName(), envVar);
}
}

public static Container getContainer(Deployment d, String name) {
requireNonNull(d);
requireNonNull(name);
for (var c : d.getSpec().getTemplate().getSpec().getContainers()) {
if (name.equals(c.getName())) {
return c;
}
}
throw new OperatorException(
"Container %s not found in Deployment %s".formatted(name, ResourceID.fromResource(d)));
}
}
17 changes: 6 additions & 11 deletions ...troller/src/main/java/io/apicurio/registry/operator/resource/ui/UIDeploymentResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static io.apicurio.registry.operator.resource.ResourceFactory.UI_CONTAINER_NAME;
import static io.apicurio.registry.operator.resource.ResourceKey.*;
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.addEnvVar;
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.getContainer;
import static io.apicurio.registry.operator.utils.IngressUtils.withIngressRule;
import static io.apicurio.registry.operator.utils.Mapper.toYAML;

Expand All @@ -40,6 +41,9 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
var d = UI_DEPLOYMENT_KEY.getFactory().apply(primary);

var envVars = new LinkedHashMap<String, EnvVar>();
primary.getSpec().getUi().getEnv().forEach(e -> {
envVars.put(e.getName(), e);
});

var sOpt = context.getSecondaryResource(APP_SERVICE_KEY.getKlass(),
APP_SERVICE_KEY.getDiscriminator());
Expand All @@ -53,17 +57,8 @@ protected Deployment desired(ApicurioRegistry3 primary, Context<ApicurioRegistry
}));
});

// This must be done after any modification of the map by the operator.
primary.getSpec().getUi().getEnv().forEach(e -> {
envVars.remove(e.getName());
envVars.put(e.getName(), e);
});

for (var c : d.getSpec().getTemplate().getSpec().getContainers()) {
if (UI_CONTAINER_NAME.equals(c.getName())) {
c.setEnv(envVars.values().stream().toList());
}
}
var container = getContainer(d, UI_CONTAINER_NAME);
container.setEnv(envVars.values().stream().toList());

log.debug("Desired {} is {}", UI_DEPLOYMENT_KEY.getId(), toYAML(d));
return d;
Expand Down
61 changes: 13 additions & 48 deletions operator/controller/src/test/java/io/apicurio/registry/operator/it/EnvITTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

import static io.apicurio.registry.operator.resource.ResourceFactory.APP_CONTAINER_NAME;
import static io.apicurio.registry.operator.resource.ResourceFactory.UI_CONTAINER_NAME;
import static io.apicurio.registry.operator.resource.app.AppDeploymentResource.getContainer;
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;

Expand Down Expand Up @@ -49,26 +50,14 @@ void testEnvVars() {

await().ignoreExceptions().until(() -> {

List<EnvVar> appEnv = null;
var appContainers = client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-app-deployment").get().getSpec().getTemplate().getSpec().getContainers();
for (var c : appContainers) {
if (APP_CONTAINER_NAME.equals(c.getName())) {
appEnv = c.getEnv();
}
}
var appEnv = getContainer(client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-app-deployment").get(), APP_CONTAINER_NAME).getEnv();
assertThat(appEnv).map(EnvVar::getName).containsOnlyOnce(defaultAppEnv);
var QUARKUS_HTTP_ACCESS_LOG_ENABLED = appEnv.stream().filter(e -> "QUARKUS_HTTP_ACCESS_LOG_ENABLED".equals(e.getName())).map(EnvVar::getValue).findAny().orElse(null);
var QUARKUS_HTTP_ACCESS_LOG_ENABLED = appEnv.stream().filter(e -> "QUARKUS_HTTP_ACCESS_LOG_ENABLED".equals(e.getName())).map(EnvVar::getValue).findAny().get();
assertThat(QUARKUS_HTTP_ACCESS_LOG_ENABLED).isEqualTo("true");

List<EnvVar> uiEnv = null;
var uiContainers = client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-ui-deployment").get().getSpec().getTemplate().getSpec().getContainers();
for (var c : uiContainers) {
if (UI_CONTAINER_NAME.equals(c.getName())) {
uiEnv = c.getEnv();
}
}
var uiEnv = getContainer(client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-ui-deployment").get(), UI_CONTAINER_NAME).getEnv();
assertThat(uiEnv).map(EnvVar::getName).containsOnlyOnce(defaultUIEnv);
var REGISTRY_API_URL = uiEnv.stream().filter(e -> "REGISTRY_API_URL".equals(e.getName())).map(EnvVar::getValue).findAny().orElse(null);
var REGISTRY_API_URL = uiEnv.stream().filter(e -> "REGISTRY_API_URL".equals(e.getName())).map(EnvVar::getValue).findAny().get();
assertThat(REGISTRY_API_URL).startsWith("http://");

return true;
Expand All @@ -89,30 +78,18 @@ void testEnvVars() {

await().ignoreExceptions().until(() -> {

List<EnvVar> appEnv = null;
var appContainers = client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-app-deployment").get().getSpec().getTemplate().getSpec().getContainers();
for (var c : appContainers) {
if (APP_CONTAINER_NAME.equals(c.getName())) {
appEnv = c.getEnv();
}
}
var appEnv = getContainer(client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-app-deployment").get(), APP_CONTAINER_NAME).getEnv();
assertThat(appEnv).map(EnvVar::getName).containsOnlyOnce(defaultAppEnv);
var QUARKUS_HTTP_ACCESS_LOG_ENABLED = appEnv.stream().filter(e -> "QUARKUS_HTTP_ACCESS_LOG_ENABLED".equals(e.getName())).map(EnvVar::getValue).findAny().orElse(null);
var QUARKUS_HTTP_ACCESS_LOG_ENABLED = appEnv.stream().filter(e -> "QUARKUS_HTTP_ACCESS_LOG_ENABLED".equals(e.getName())).map(EnvVar::getValue).findAny().get();
assertThat(QUARKUS_HTTP_ACCESS_LOG_ENABLED).isEqualTo("false");
assertThat(appEnv).containsSubsequence(
new EnvVarBuilder().withName("APP_VAR_1_NAME").withValue("APP_VAR_1_VALUE").build(),
new EnvVarBuilder().withName("APP_VAR_2_NAME").withValue("APP_VAR_2_VALUE").build()
);

List<EnvVar> uiEnv = null;
var uiContainers = client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-ui-deployment").get().getSpec().getTemplate().getSpec().getContainers();
for (var c : uiContainers) {
if (UI_CONTAINER_NAME.equals(c.getName())) {
uiEnv = c.getEnv();
}
}
var uiEnv = getContainer(client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-ui-deployment").get(), UI_CONTAINER_NAME).getEnv();
assertThat(uiEnv).map(EnvVar::getName).containsOnlyOnce(defaultUIEnv);
var REGISTRY_API_URL = uiEnv.stream().filter(e -> "REGISTRY_API_URL".equals(e.getName())).map(EnvVar::getValue).findAny().orElse(null);
var REGISTRY_API_URL = uiEnv.stream().filter(e -> "REGISTRY_API_URL".equals(e.getName())).map(EnvVar::getValue).findAny().get();
assertThat(REGISTRY_API_URL).isEqualTo("FOO");
assertThat(uiEnv).containsSubsequence(
new EnvVarBuilder().withName("UI_VAR_1_NAME").withValue("UI_VAR_1_VALUE").build(),
Expand All @@ -139,31 +116,19 @@ void testEnvVars() {

await().ignoreExceptions().until(() -> {

List<EnvVar> appEnv = null;
var appContainers = client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-app-deployment").get().getSpec().getTemplate().getSpec().getContainers();
for (var c : appContainers) {
if (APP_CONTAINER_NAME.equals(c.getName())) {
appEnv = c.getEnv();
}
}
var appEnv = getContainer(client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-app-deployment").get(), APP_CONTAINER_NAME).getEnv();
assertThat(appEnv).map(EnvVar::getName).containsOnlyOnce(defaultAppEnv);
var QUARKUS_HTTP_ACCESS_LOG_ENABLED = appEnv.stream().filter(e -> "QUARKUS_HTTP_ACCESS_LOG_ENABLED".equals(e.getName())).map(EnvVar::getValue).findAny().orElse(null);
var QUARKUS_HTTP_ACCESS_LOG_ENABLED = appEnv.stream().filter(e -> "QUARKUS_HTTP_ACCESS_LOG_ENABLED".equals(e.getName())).map(EnvVar::getValue).findAny().get();
assertThat(QUARKUS_HTTP_ACCESS_LOG_ENABLED).isEqualTo("false");
assertThat(appEnv).containsSubsequence(
new EnvVarBuilder().withName("APP_VAR_2_NAME").withValue("APP_VAR_2_VALUE").build(),
new EnvVarBuilder().withName("APP_VAR_1_NAME").withValue("APP_VAR_1_VALUE").build()
);


List<EnvVar> uiEnv = null;
var uiContainers = client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-ui-deployment").get().getSpec().getTemplate().getSpec().getContainers();
for (var c : uiContainers) {
if (UI_CONTAINER_NAME.equals(c.getName())) {
uiEnv = c.getEnv();
}
}
var uiEnv = getContainer(client.apps().deployments().inNamespace(namespace).withName(registry.getMetadata().getName() + "-ui-deployment").get(), UI_CONTAINER_NAME).getEnv();
assertThat(uiEnv).map(EnvVar::getName).containsOnlyOnce(defaultUIEnv);
var REGISTRY_API_URL = uiEnv.stream().filter(e -> "REGISTRY_API_URL".equals(e.getName())).map(EnvVar::getValue).findAny().orElse(null);
var REGISTRY_API_URL = uiEnv.stream().filter(e -> "REGISTRY_API_URL".equals(e.getName())).map(EnvVar::getValue).findAny().get();
assertThat(REGISTRY_API_URL).isEqualTo("FOO");
assertThat(uiEnv).containsSubsequence(
new EnvVarBuilder().withName("UI_VAR_2_NAME").withValue("UI_VAR_2_VALUE").build(),
Expand Down

0 comments on commit 95555e5

Please sign in to comment.