From 4e04db6f5d7a3b03d06909969f09985ed566cc39 Mon Sep 17 00:00:00 2001
From: shuangkun tian <72060326+shuangkun@users.noreply.github.com>
Date: Sat, 13 Jan 2024 00:35:46 +0800
Subject: [PATCH] fix: merge env bug in workflow-controller-configmap and
 container. Fixes #12424 (#12426)

Signed-off-by: shuangkun <tsk2013uestc@163.com>
Co-authored-by: sherwinkoo29 <sherwinkoo@163.com>
---
 workflow/controller/workflowpod.go      |  7 +-
 workflow/controller/workflowpod_test.go | 85 +++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go
index a5fd427b06e0..43b39195b0d7 100644
--- a/workflow/controller/workflowpod.go
+++ b/workflow/controller/workflowpod.go
@@ -118,10 +118,13 @@ func (woc *wfOperationCtx) createWorkflowPod(ctx context.Context, nodeName strin
 			if err != nil {
 				return nil, err
 			}
-			if err := json.Unmarshal(a, &c); err != nil {
+
+			mergedContainerByte, err := strategicpatch.StrategicMergePatch(a, b, apiv1.Container{})
+			if err != nil {
 				return nil, err
 			}
-			if err = json.Unmarshal(b, &c); err != nil {
+			c = apiv1.Container{}
+			if err := json.Unmarshal(mergedContainerByte, &c); err != nil {
 				return nil, err
 			}
 		}
diff --git a/workflow/controller/workflowpod_test.go b/workflow/controller/workflowpod_test.go
index 73a88896b807..2fa427444637 100644
--- a/workflow/controller/workflowpod_test.go
+++ b/workflow/controller/workflowpod_test.go
@@ -1878,3 +1878,88 @@ func TestProgressEnvVars(t *testing.T) {
 		})
 	})
 }
+
+var helloWorldWfWithEnvReferSecret = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-world
+spec:
+  entrypoint: whalesay
+  templates:
+  - name: whalesay
+    metadata:
+      annotations:
+        annotationKey1: "annotationValue1"
+        annotationKey2: "annotationValue2"
+      labels:
+        labelKey1: "labelValue1"
+        labelKey2: "labelValue2"
+    container:
+      image: docker/whalesay:latest
+      command: [cowsay]
+      args: ["hello world"]
+      env:
+      - name: ENV3
+        valueFrom:
+          secretKeyRef:
+            name: mysecret
+            key: sec
+`
+
+func TestMergeEnvVars(t *testing.T) {
+	setup := func(t *testing.T, options ...interface{}) (context.CancelFunc, *apiv1.Pod) {
+		cancel, controller := newController(options...)
+
+		wf := wfv1.MustUnmarshalWorkflow(helloWorldWfWithEnvReferSecret)
+		ctx := context.Background()
+		woc := newWorkflowOperationCtx(wf, controller)
+		err := woc.setExecWorkflow(ctx)
+		require.NoError(t, err)
+		mainCtrSpec := &apiv1.Container{
+			Name:            common.MainContainerName,
+			SecurityContext: &apiv1.SecurityContext{},
+			Env: []apiv1.EnvVar{
+				{
+					Name:  "ENV1",
+					Value: "env1",
+				},
+				{
+					Name:  "ENV2",
+					Value: "env2",
+				},
+			},
+		}
+		woc.controller.Config.MainContainer = mainCtrSpec
+		mainCtr := woc.execWf.Spec.Templates[0].Container
+
+		pod, err := woc.createWorkflowPod(ctx, wf.Name, []apiv1.Container{*mainCtr}, &wf.Spec.Templates[0], &createWorkflowPodOpts{})
+		require.NoError(t, err)
+		assert.NotNil(t, pod)
+		return cancel, pod
+	}
+
+	t.Run("test merge envs", func(t *testing.T) {
+		cancel, pod := setup(t)
+		defer cancel()
+		assert.Contains(t, pod.Spec.Containers[1].Env, apiv1.EnvVar{
+			Name:  "ENV1",
+			Value: "env1",
+		})
+		assert.Contains(t, pod.Spec.Containers[1].Env, apiv1.EnvVar{
+			Name:  "ENV2",
+			Value: "env2",
+		})
+		assert.Contains(t, pod.Spec.Containers[1].Env, apiv1.EnvVar{
+			Name: "ENV3",
+			ValueFrom: &apiv1.EnvVarSource{
+				SecretKeyRef: &apiv1.SecretKeySelector{
+					LocalObjectReference: apiv1.LocalObjectReference{
+						Name: "mysecret",
+					},
+					Key: "sec",
+				},
+			},
+		})
+	})
+}