Fix #887: Incorrect warning about overriding environment variable

CHANGELOG.md

@@ -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

jkube-kit/enricher/generic/src/main/java/org/eclipse/jkube/enricher/generic/ImageEnricher.java

@@ -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())) {

jkube-kit/enricher/generic/src/test/java/org/eclipse/jkube/enricher/generic/ImageEnricherTest.java

@@ -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";
         }};
@@ -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
@@ -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
     }
 }