Skip to content

Commit

Permalink
fix: Enricher defined Container env vars get merged with vars defined…
Browse files Browse the repository at this point in the history
… in Image Build Configuration

Signed-off-by: Marc Nuri <marc@marcnuri.com>
  • Loading branch information
manusa committed Sep 23, 2020
1 parent cbea193 commit e36a9b0
Show file tree
Hide file tree
Showing 22 changed files with 588 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Usage:
### 1.0.1-SNAPSHOT
* Fix #381: Remove root as default user in AssemblyConfigurationUtils#getAssemblyConfigurationOrCreateDefault
* Fix #358: Prometheus is enabled by default, opt-out via AB_PROMETHEUS_OFF required to disable (like in FMP)
* Fix #384: Enricher defined Container environment variables get merged with vars defined in Image Build Configuration

### 1.0.0 (2020-09-09)
* Fix #351: Fix AutoTLSEnricher - add annotation + volume config to resource
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* Copyright (c) 2019 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at:
*
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
*/
package org.eclipse.jkube.enricher.generic;

import java.util.ArrayList;
import java.util.List;

import lombok.AllArgsConstructor;
import lombok.Getter;
import org.eclipse.jkube.kit.common.Configs;
import org.eclipse.jkube.kit.config.image.ImageConfiguration;
import org.eclipse.jkube.kit.config.image.build.BuildConfiguration;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
import org.eclipse.jkube.kit.enricher.api.BaseEnricher;
import org.eclipse.jkube.kit.enricher.api.JKubeEnricherContext;

import io.fabric8.kubernetes.api.builder.TypedVisitor;
import io.fabric8.kubernetes.api.model.ContainerBuilder;
import io.fabric8.kubernetes.api.model.EnvVar;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;

import static org.eclipse.jkube.kit.common.Configs.asBoolean;

/**
* Enricher to merge <code>JAVA_OPTIONS</code> environment variables defined in {@link BuildConfiguration#getEnv()}
* with those added to {@link io.fabric8.kubernetes.api.model.Container} by other enrichers.
*
* <p> This prevents Container environment variables from overriding and hiding the initial JAVA_OPTIONS value
* possibly added by some <code>Generator</code>.
*/
public class ContainerEnvJavaOptionsMergeEnricher extends BaseEnricher {

@AllArgsConstructor
private enum Config implements Configs.Config {
// What pull policy to use when fetching images
DISABLE("disable", "false");

@Getter
protected String key;
@Getter
protected String defaultValue;
}

public ContainerEnvJavaOptionsMergeEnricher(JKubeEnricherContext enricherContext) {
super(enricherContext, "jkube-container-env-java-options");
}

@Override
public void enrich(PlatformMode platformMode, KubernetesListBuilder builder) {
if (!asBoolean(getConfig(Config.DISABLE)) && hasImageConfiguration()) {
builder.accept(new ContainerEnvJavaOptionsMergeVisitor(getImages()));
}
}

static final class ContainerEnvJavaOptionsMergeVisitor extends TypedVisitor<ContainerBuilder> {

private static final String ENV_KEY = "JAVA_OPTIONS";

private final List<ImageConfiguration> imageConfigurations;

public ContainerEnvJavaOptionsMergeVisitor(List<ImageConfiguration> imageConfigurations) {
this.imageConfigurations = imageConfigurations;
}

@Override
public void visit(ContainerBuilder containerBuilder) {
imageConfigurations.stream()
.filter(ic -> ImageEnricher.containerImageName(ic).equals(containerBuilder.getImage()))
.filter(ic -> !ic.getBuild().getEnv().isEmpty())
.filter(ic -> ic.getBuild().getEnv().containsKey(ENV_KEY))
.findFirst()
.ifPresent(ic -> containerBuilder.withEnv(mergeEnv(containerBuilder.buildEnv(), ic)));
}

private List<EnvVar> mergeEnv(List<EnvVar> envVars, ImageConfiguration imageConfiguration) {
final List<EnvVar> ret = new ArrayList<>();
for (EnvVar env : envVars) {
if (env.getName().equalsIgnoreCase(ENV_KEY)) {
final EnvVar merged = new EnvVar();
merged.setName(env.getName());
merged.setValueFrom(env.getValueFrom());
merged.setValue(String.format("%s %s",
imageConfiguration.getBuild().getEnv().getOrDefault(ENV_KEY, ""),
env.getValue()
));
ret.add(merged);
} else {
ret.add(env);
}
}
return ret;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@

import static org.eclipse.jkube.kit.enricher.api.util.KubernetesResourceUtil.extractContainerName;


/**
* Merge in image configuration like the image name into ReplicaSet and ReplicationController's
* Pod specification.
Expand Down Expand Up @@ -94,7 +93,7 @@ public void create(PlatformMode platformMode, KubernetesListBuilder builder) {
return;
}

// Ensure that all contoller have template specs
// Ensure that all controller have template specs
ensureTemplateSpecs(builder);

// Update containers in template specs
Expand Down Expand Up @@ -247,12 +246,10 @@ private void mergeContainerName(ImageConfiguration imageConfiguration, Container

private void mergeImage(ImageConfiguration imageConfiguration, Container container) {
if (StringUtils.isBlank(container.getImage())) {
String prefix = "";
if (StringUtils.isNotBlank(imageConfiguration.getRegistry())) {
log.verbose("Using registry %s for the image", imageConfiguration.getRegistry());
prefix = imageConfiguration.getRegistry() + "/";
}
String imageFullName = prefix + imageConfiguration.getName();
final String imageFullName = containerImageName(imageConfiguration);
log.verbose("Setting image %s", imageFullName);
container.setImage(imageFullName);
}
Expand Down Expand Up @@ -310,4 +307,11 @@ private boolean hasEnvWithName(List<EnvVar> envVars, String name) {
return envVars.stream().anyMatch(e -> e.getName().equals(name));
}

static String containerImageName(ImageConfiguration imageConfiguration) {
String prefix = "";
if (StringUtils.isNotBlank(imageConfiguration.getRegistry())) {
prefix = imageConfiguration.getRegistry() + "/";
}
return prefix + imageConfiguration.getName();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,7 @@ org.eclipse.jkube.enricher.generic.FileDataSecretEnricher
org.eclipse.jkube.enricher.generic.openshift.ImageChangeTriggerEnricher

# Add an ingress on demand when on Kubernetes
org.eclipse.jkube.enricher.generic.IngressEnricher
org.eclipse.jkube.enricher.generic.IngressEnricher

# Merge JAVA_OPTIONS environment variables from Image Build Configurations and enriched Containers
org.eclipse.jkube.enricher.generic.ContainerEnvJavaOptionsMergeEnricher
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* Copyright (c) 2019 Red Hat, Inc.
* This program and the accompanying materials are made
* available under the terms of the Eclipse Public License 2.0
* which is available at:
*
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* Red Hat, Inc. - initial API and implementation
*/
package org.eclipse.jkube.enricher.generic;

import java.util.Collections;
import java.util.List;
import java.util.Properties;
import java.util.stream.Collectors;

import io.fabric8.kubernetes.api.model.EnvVar;
import io.fabric8.kubernetes.api.model.apps.Deployment;
import org.eclipse.jkube.kit.config.image.ImageConfiguration;

import io.fabric8.kubernetes.api.model.Container;
import io.fabric8.kubernetes.api.model.ContainerBuilder;
import io.fabric8.kubernetes.api.model.KubernetesListBuilder;
import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
import mockit.Expectations;
import mockit.Mocked;
import org.eclipse.jkube.kit.config.resource.PlatformMode;
import org.eclipse.jkube.kit.enricher.api.JKubeEnricherContext;
import org.eclipse.jkube.kit.enricher.api.model.Configuration;
import org.junit.Before;
import org.junit.Test;

import static org.assertj.core.api.Assertions.assertThat;

@SuppressWarnings("ResultOfMethodCallIgnored")
public class ContainerEnvJavaOptionsMergeTest {

@SuppressWarnings("unused")
@Mocked
private ImageConfiguration imageConfiguration;

@Mocked
private JKubeEnricherContext context;

private ContainerEnvJavaOptionsMergeEnricher containerEnvJavaOptionsMergeEnricher;
private KubernetesListBuilder kubernetesListBuilder;
private Properties properties;

@Before
public void setUp() {
containerEnvJavaOptionsMergeEnricher = new ContainerEnvJavaOptionsMergeEnricher(context);
kubernetesListBuilder = new KubernetesListBuilder();
properties = new Properties();
// @formatter:off
kubernetesListBuilder.addToItems(new DeploymentBuilder().withNewSpec()
.withNewTemplate()
.withNewSpec()
.addToContainers(new ContainerBuilder()
.withImage("the-image:latest")
.addToEnv(new EnvVar("JAVA_OPTIONS", "val-from-container", null))
.build())
.endSpec()
.endTemplate()
.endSpec().build());
new Expectations() {{
context.getConfiguration(); result = Configuration.builder().image(imageConfiguration).build();
context.getProperties(); result = properties;
}};
// @formatter:on
}

@Test
public void enrichWithDefaultsShouldMergeValues() {
// Given
// @formatter:off
new Expectations() {{
imageConfiguration.getName(); result = "the-image:latest"; minTimes = 0;
imageConfiguration.getBuild().getEnv(); result = Collections.singletonMap("JAVA_OPTIONS", "val-from-ic"); minTimes = 0;
}};
// @formatter:on
// When
containerEnvJavaOptionsMergeEnricher.enrich(PlatformMode.kubernetes, kubernetesListBuilder);
// Then
assertThat(containerList(kubernetesListBuilder))
.flatExtracting("env")
.hasSize(1)
.containsOnly(new EnvVar("JAVA_OPTIONS", "val-from-ic val-from-container", null));
}

@Test
public void enrichWithDisabledShouldDoNothing() {
// Given
properties.put("jkube.enricher.jkube-container-env-java-options.disable", "true");
// When
containerEnvJavaOptionsMergeEnricher.enrich(PlatformMode.kubernetes, kubernetesListBuilder);
// Then
assertThat(containerList(kubernetesListBuilder))
.flatExtracting("env")
.hasSize(1)
.containsOnly(new EnvVar("JAVA_OPTIONS", "val-from-container", null));
}

static List<Container> containerList(KubernetesListBuilder kubernetesListBuilder) {
return kubernetesListBuilder.build().getItems().stream()
.map(Deployment.class::cast)
.flatMap(d -> d.getSpec().getTemplate().getSpec().getContainers().stream())
.collect(Collectors.toList());
}

}
Loading

0 comments on commit e36a9b0

Please sign in to comment.