Skip to content

Commit

Permalink
Fix #887: Incorrect warning about overriding environment variable
Browse files Browse the repository at this point in the history 
Don't log warning about not overriding environment variable when both
old and new values are same.

Signed-off-by: Rohan Kumar <[email protected]>
  • Loading branch information
@rohanKanojia
rohanKanojia committed Nov 11, 2021
1 parent 40b0561 commit 9c9d274
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 34 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Usage:
./scripts/extract-changelog-for-version.sh 1.3.37 5
```
### 1.6.0-SNAPSHOT
* Fix #887: Incorrect warning about overriding environment variable

### 1.5.1 (2021-10-28)
* Fix #1084: Gradle dependencies should be test or provided scope
Expand Down
25 changes: 8 additions & 17 deletions ...-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/ImageEnricher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,30 +283,21 @@ private void mergeEnvVariables(Container container) {
.withName(resourceEnvEntry.getKey())
.withValue(resourceEnvEntry.getValue())
.build();
if (!hasEnvWithName(containerEnvVars, newEnvVar.getName())) {

EnvVar oldEnvVar = containerEnvVars.stream()
.filter(e -> e.getName().equals(newEnvVar.getName()))
.findFirst().orElse(null);
if (oldEnvVar == null) {
containerEnvVars.add(newEnvVar);
} else {
} else if (!newEnvVar.getValue().equals(oldEnvVar.getValue())) {
log.warn(
"Environment variable %s will not be overridden: trying to set the value %s, but its actual value is %s",
newEnvVar.getName(), newEnvVar.getValue(), getEnvValue(containerEnvVars, newEnvVar.getName()));
"Environment variable %s will not be overridden: trying to set the value %s, but its actual value is %s",
newEnvVar.getName(), newEnvVar.getValue(), oldEnvVar.getValue());
}
}
});
}

private String getEnvValue(List<EnvVar> envVars, String name) {
for (EnvVar var : envVars) {
if (var.getName().equals(name)) {
return var.getValue();
}
}
return "(not found)";
}

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())) {
Expand Down
76 changes: 59 additions & 17 deletions .../enricher/generic/src/test/java/org/eclipse/jkube/enricher/generic/ImageEnricherTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,9 @@ public class ImageEnricherTest {
@Before
public void prepareMock() {
// Setup mock behaviour
givenResourceConfigWithEnvVar("MY_KEY", "MY_VALUE");
// @formatter:off
new Expectations() {{
Configuration configuration = Configuration.builder()
.resource(ResourceConfig.builder()
.env(Collections.singletonMap("MY_KEY", "MY_VALUE"))
.build())
.image(imageConfiguration)
.build();
context.getConfiguration(); result = configuration;

imageConfiguration.getName(); result = "busybox";
imageConfiguration.getAlias(); result = "busybox";
}};
Expand All @@ -78,15 +71,15 @@ public void checkEnrichDeployment() throws Exception {
KubernetesListBuilder builder = new KubernetesListBuilder().addToItems(new DeploymentBuilder().build());

imageEnricher.create(PlatformMode.kubernetes, builder);
assertCorrectlyGeneratedResources(builder.build(), "Deployment");
assertCorrectlyGeneratedResources(builder.build(), "Deployment", "MY_KEY", "MY_VALUE");
}

@Test
public void checkEnrichReplicaSet() throws Exception {
KubernetesListBuilder builder = new KubernetesListBuilder().addToItems(new ReplicaSetBuilder().build());

imageEnricher.create(PlatformMode.kubernetes, builder);
assertCorrectlyGeneratedResources(builder.build(), "ReplicaSet");
assertCorrectlyGeneratedResources(builder.build(), "ReplicaSet", "MY_KEY", "MY_VALUE");
}

@Test
Expand All @@ -96,41 +89,90 @@ public void checkEnrichReplicationController() throws Exception {
.endReplicationControllerItem();

imageEnricher.create(PlatformMode.kubernetes, builder);
assertCorrectlyGeneratedResources(builder.build(), "ReplicationController");
assertCorrectlyGeneratedResources(builder.build(), "ReplicationController", "MY_KEY", "MY_VALUE");
}

@Test
public void checkEnrichDaemonSet() throws Exception {
KubernetesListBuilder builder = new KubernetesListBuilder().addToItems(new DaemonSetBuilder().build());

imageEnricher.create(PlatformMode.kubernetes, builder);
assertCorrectlyGeneratedResources(builder.build(), "DaemonSet");
assertCorrectlyGeneratedResources(builder.build(), "DaemonSet", "MY_KEY", "MY_VALUE");
}

@Test
public void checkEnrichStatefulSet() throws Exception {
KubernetesListBuilder builder = new KubernetesListBuilder().addToItems(new StatefulSetBuilder().build());

imageEnricher.create(PlatformMode.kubernetes, builder);
assertCorrectlyGeneratedResources(builder.build(), "StatefulSet");
assertCorrectlyGeneratedResources(builder.build(), "StatefulSet", "MY_KEY", "MY_VALUE");
}

@Test
public void checkEnrichDeploymentConfig() throws Exception {
KubernetesListBuilder builder = new KubernetesListBuilder().addToItems(new DeploymentConfigBuilder().build());

imageEnricher.create(PlatformMode.kubernetes, builder);
assertCorrectlyGeneratedResources(builder.build(), "DeploymentConfig");
assertCorrectlyGeneratedResources(builder.build(), "DeploymentConfig", "MY_KEY", "MY_VALUE");
}

@Test
public void create_whenEnvironmentVariableAbsent_thenAddsEnvironmentVariable() throws JsonProcessingException {
// Given
KubernetesListBuilder builder = new KubernetesListBuilder().addToItems(new DeploymentBuilder().build());

// When
imageEnricher.create(PlatformMode.kubernetes, builder);

// Then
assertCorrectlyGeneratedResources(builder.build(), "Deployment", "MY_KEY", "MY_VALUE");
}

private void assertCorrectlyGeneratedResources(KubernetesList list, String kind) throws JsonProcessingException {
@Test
public void create_whenEnvironmentVariablePresentWithDifferentValue_thenOldValueIsPreserved() throws JsonProcessingException {
// Given
givenResourceConfigWithEnvVar("key", "valueNew");
KubernetesListBuilder builder = new KubernetesListBuilder().addToItems(new DeploymentBuilder()
.withNewSpec()
.withNewTemplate()
.withNewSpec()
.addNewContainer()
.addNewEnv().withName("key").withValue("valueOld").endEnv()
.endContainer()
.endSpec()
.endTemplate()
.endSpec()
.build());

// When
imageEnricher.create(PlatformMode.kubernetes, builder);

// Then
assertCorrectlyGeneratedResources(builder.build(), "Deployment", "key", "valueOld");
}

private void assertCorrectlyGeneratedResources(KubernetesList list, String kind, String expectedKey, String expectedValue) throws JsonProcessingException {
assertEquals(1, list.getItems().size());

String json = ResourceUtil.toJson(list.getItems().get(0));
assertThat(json, JsonPathMatchers.isJson());
assertThat(json, JsonPathMatchers.hasJsonPath("$.kind", Matchers.equalTo(kind)));

assertThat(json, JsonPathMatchers.hasJsonPath("$.spec.template.spec.containers[0].env[0].name", Matchers.equalTo("MY_KEY")));
assertThat(json, JsonPathMatchers.hasJsonPath("$.spec.template.spec.containers[0].env[0].value", Matchers.equalTo("MY_VALUE")));
assertThat(json, JsonPathMatchers.hasJsonPath("$.spec.template.spec.containers[0].env[0].name", Matchers.equalTo(expectedKey)));
assertThat(json, JsonPathMatchers.hasJsonPath("$.spec.template.spec.containers[0].env[0].value", Matchers.equalTo(expectedValue)));
}

private void givenResourceConfigWithEnvVar(String name, String value) {
// @formatter:off
new Expectations() {{
Configuration configuration = Configuration.builder()
.resource(ResourceConfig.builder()
.env(Collections.singletonMap(name, value))
.build())
.image(imageConfiguration)
.build();
context.getConfiguration(); result = configuration;
}};
// @formatter:on
}
}

0 comments on commit 9c9d274

Please sign in to comment.