Skip to content

Commit

Permalink
Fix eclipse-jkube#887: Incorrect warning about overriding environment…
Browse files Browse the repository at this point in the history 
… variable

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.