chore: verify should preserve job manifest envs (#9087)

* chore: merge env poc

* chore: add tests

* chore: update patch strategy

* chore: add tests

* nit: remove unnecessary confi

integration/testdata/verify-succeed-k8s/job.yaml

@@ -0,0 +1,14 @@
+apiVersion: batch/v1
+kind: Job
+metadata:
+  name: foo
+spec:
+  template:
+    spec:
+      containers:
+        - name: foo
+          image: alpine:latest
+          env:
+            - name: FOO
+              value: ZZZ
+      restartPolicy: Never

integration/testdata/verify-succeed-k8s/skaffold.yaml

@@ -105,4 +105,15 @@ profiles:
           kubernetesCluster: {}
         container:
           name: localtask
-          image: not-built-localtask
+          image: not-built-localtask
+  - name: with-job-manifest
+    verify:
+      - name: with-job-manifest
+        executionMode:
+          kubernetesCluster:
+            jobManifestPath: job.yaml
+        container:
+          name: foo
+          image: alpine:3.15.4
+          command: ["/bin/sh"]
+          args: ["-c", "echo $FOO with-job-manifest; sleep 2; echo bye"]

integration/verify_test.go

@@ -132,6 +132,21 @@ func TestKubernetesJobVerifyPassingTestsWithEnvVar(t *testing.T) {
 	// TODO(aaron-prindle) verify that SUCCEEDED event is found where expected
 }
 
+func TestKubernetesJobVerifyEnvVarFromJobManifest(t *testing.T) {
+	MarkIntegrationTest(t, CanRunWithoutGcp)
+	tmp := t.TempDir()
+	logFile := filepath.Join(tmp, uuid.New().String()+"logs.json")
+
+	rpcPort := randomPort()
+	// `--default-repo=` is used to cancel the default repo that is set by default.
+	out, err := skaffold.Verify("--default-repo=", "--rpc-port", rpcPort,
+		"--event-log-file", logFile, "-p", "with-job-manifest").InDir("testdata/verify-succeed-k8s").RunWithCombinedOutput(t)
+	logs := string(out)
+
+	testutil.CheckError(t, false, err)
+	testutil.CheckContains(t, "ZZZ with-job-manifest", logs)
+}
+
 func TestKubernetesJobVerifyOneTestFailsWithEnvVar(t *testing.T) {
 	MarkIntegrationTest(t, CanRunWithoutGcp)
 	tmp := t.TempDir()

pkg/skaffold/verify/k8sjob/verify.go

@@ -375,7 +375,16 @@ func (v *Verifier) createJobFromManifestPath(jobName string, container latest.Ve
 
 	job.Name = jobName
 	job.Labels["skaffold.dev/run-id"] = v.labeller.GetRunID()
-	job.Spec.Template.Spec.Containers = []corev1.Container{verifyContainerToK8sContainer(container)}
+	var original corev1.Container
+	for _, c := range job.Spec.Template.Spec.Containers {
+		if c.Name == container.Name {
+			original = c
+			break
+		}
+	}
+	patchToK8sContainer(container, &original)
+
+	job.Spec.Template.Spec.Containers = []corev1.Container{original}
 	job.Spec.Template.Spec.RestartPolicy = corev1.RestartPolicyNever
 	if job.Spec.Template.Labels == nil {
 		job.Spec.Template.Labels = map[string]string{}
@@ -387,6 +396,20 @@ func (v *Verifier) createJobFromManifestPath(jobName string, container latest.Ve
 	return job, nil
 }
 
+func patchToK8sContainer(container latest.VerifyContainer, dst *corev1.Container) {
+	dst.Image = container.Image
+	dst.Command = container.Command
+	dst.Args = container.Args
+	dst.Name = container.Name
+
+	for _, e := range container.Env {
+		dst.Env = append(dst.Env, corev1.EnvVar{
+			Name:  e.Name,
+			Value: e.Value,
+		})
+	}
+}
+
 func (v *Verifier) appendEnvIntoJob(envMap map[string]string, job *batchv1.Job) {
 	var envs []corev1.EnvVar
 	for k, v := range envMap {

pkg/skaffold/verify/k8sjob/verify_test.go

@@ -0,0 +1,134 @@
+/*
+Copyright 2023 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package k8sjob
+
+import (
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+
+	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/v2/testutil"
+)
+
+func TestPatchToK8sContainer(t *testing.T) {
+	tests := []struct {
+		description     string
+		verifyContainer latest.VerifyContainer
+		k8sContainer    corev1.Container
+		expected        corev1.Container
+	}{
+		{
+			description: "update all fields",
+			verifyContainer: latest.VerifyContainer{
+				Image:   "my-image:latest",
+				Command: []string{"/bin/bash"},
+				Args:    []string{"-c", "echo hello world"},
+				Name:    "my-container",
+				Env: []latest.VerifyEnvVar{
+					{Name: "FOO", Value: "BAR"},
+				},
+			},
+			k8sContainer: corev1.Container{},
+			expected: corev1.Container{
+				Image:   "my-image:latest",
+				Command: []string{"/bin/bash"},
+				Args:    []string{"-c", "echo hello world"},
+				Name:    "my-container",
+				Env: []corev1.EnvVar{
+					{Name: "FOO", Value: "BAR"},
+				},
+			},
+		},
+		{
+			description: "update image",
+			verifyContainer: latest.VerifyContainer{
+				Image: "my-new-image:latest",
+			},
+			k8sContainer: corev1.Container{
+				Image: "my-image:latest",
+			},
+			expected: corev1.Container{
+				Image: "my-new-image:latest",
+			},
+		},
+		{
+			description: "update command",
+			verifyContainer: latest.VerifyContainer{
+				Command: []string{"/bin/ls"},
+			},
+			k8sContainer: corev1.Container{
+				Command: []string{"/bin/bash"},
+			},
+			expected: corev1.Container{
+				Command: []string{"/bin/ls"},
+			},
+		},
+		{
+			description: "update args",
+			verifyContainer: latest.VerifyContainer{
+				Args: []string{"-l"},
+			},
+			k8sContainer: corev1.Container{
+				Args: []string{"-c", "echo hello world"},
+			},
+			expected: corev1.Container{
+				Args: []string{"-l"},
+			},
+		},
+		{
+			description: "update name",
+			verifyContainer: latest.VerifyContainer{
+				Name: "my-new-container",
+			},
+			k8sContainer: corev1.Container{
+				Name: "my-container",
+			},
+			expected: corev1.Container{
+				Name: "my-new-container",
+			},
+		},
+		{
+			description: "update env",
+			verifyContainer: latest.VerifyContainer{
+				Env: []latest.VerifyEnvVar{
+					{Name: "FOO", Value: "BARR"},
+					{Name: "BAZ", Value: "QUX"},
+				},
+			},
+			k8sContainer: corev1.Container{
+				Env: []corev1.EnvVar{
+					{Name: "FOO", Value: "BAR"},
+				},
+			},
+			// Duplicate name should be ok, as the last writer should win.
+			expected: corev1.Container{
+				Env: []corev1.EnvVar{
+					{Name: "FOO", Value: "BAR"},
+					{Name: "FOO", Value: "BARR"},
+					{Name: "BAZ", Value: "QUX"},
+				},
+			},
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			patchToK8sContainer(test.verifyContainer, &test.k8sContainer)
+			t.CheckDeepEqual(test.expected, test.k8sContainer)
+		})
+	}
+}