Merge pull request #1292 from jsturtevant/expand-job-envs

Expand env variables for job containers to job mount path
microsoft · Feb 4, 2022 · c019e22 · c019e22
2 parents e382e6d + 9d05b5b
commit c019e22
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 11 deletions.
diff --git a/internal/jobcontainers/jobcontainer.go b/internal/jobcontainers/jobcontainer.go
@@ -39,15 +39,6 @@ func splitArgs(cmdLine string) []string {
 	return r.FindAllString(cmdLine, -1)
 }
 
-// Convert environment map to a slice of environment variables in the form [Key1=val1, key2=val2]
-func envMapToSlice(m map[string]string) []string {
-	var s []string
-	for k, v := range m {
-		s = append(s, k+"="+v)
-	}
-	return s
-}
-
 const (
 	jobContainerNameFmt = "JobContainer_%s"
 	// Environment variable set in every process in the job detailing where the containers volume
@@ -232,7 +223,15 @@ func (c *JobContainer) CreateProcess(ctx context.Context, config interface{}) (_
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get default environment block")
 	}
-	env = append(env, envMapToSlice(conf.Environment)...)
+
+	// Convert environment map to a slice of environment variables in the form [Key1=val1, key2=val2]
+	var envs []string
+	for k, v := range conf.Environment {
+		expanded, _ := c.replaceWithMountPoint(v)
+		envs = append(envs, k+"="+expanded)
+	}
+	env = append(env, envs...)
+
 	env = append(env, sandboxMountPointEnvVar+"="+c.sandboxMount)
 
 	// exec.Cmd internally does its own path resolution and as part of this checks some well known file extensions on the file given (e.g. if
@@ -603,7 +602,7 @@ func systemProcessInformation() ([]*winapi.SYSTEM_PROCESS_INFORMATION, error) {
 	return procInfos, nil
 }
 
-// Takes a string and replaces any occurences of CONTAINER_SANDBOX_MOUNT_POINT with where the containers volume is mounted, as well as returning
+// Takes a string and replaces any occurrences of CONTAINER_SANDBOX_MOUNT_POINT with where the containers' volume is mounted, as well as returning
 // if the string actually contained the environment variable.
 func (c *JobContainer) replaceWithMountPoint(str string) (string, bool) {
 	newStr := strings.ReplaceAll(str, "%"+sandboxMountPointEnvVar+"%", c.sandboxMount[:len(c.sandboxMount)-1])

diff --git a/test/cri-containerd/jobcontainer_test.go b/test/cri-containerd/jobcontainer_test.go
@@ -434,6 +434,83 @@ func Test_RunContainer_JobContainer_VolumeMount(t *testing.T) {
 	}
 }
 
+func Test_RunContainer_JobContainer_Environment(t *testing.T) {
+	client := newTestRuntimeClient(t)
+
+	type config struct {
+		name             string
+		containerName    string
+		requiredFeatures []string
+		sandboxImage     string
+		containerImage   string
+		env              []*runtime.KeyValue
+		exec             []string
+	}
+
+	tests := []config{
+		{
+			name:             "JobContainer_Env_NoMountPoint",
+			containerName:    t.Name() + "-Container-WithNoMountPoint",
+			requiredFeatures: []string{featureWCOWProcess, featureHostProcess},
+			sandboxImage:     imageWindowsNanoserver,
+			containerImage:   imageWindowsNanoserver,
+			env: []*runtime.KeyValue{
+				{
+					Key: "PATH", Value: "C:\\Windows\\system32;C:\\Windows",
+				},
+			},
+			exec: []string{"cmd", "/c", "IF", "%PATH%", "==", "C:\\Windows\\system32;C:\\Windows", "( exit 0 )", "ELSE", "(exit -1)"},
+		},
+		{
+			name:             "JobContainer_VolumeMount_WithMountPoint",
+			containerName:    t.Name() + "-Container-WithMountPoint",
+			requiredFeatures: []string{featureWCOWProcess, featureHostProcess},
+			sandboxImage:     imageWindowsNanoserver,
+			containerImage:   imageWindowsNanoserver,
+			env: []*runtime.KeyValue{
+				{
+					Key: "PATH", Value: "%CONTAINER_SANDBOX_MOUNT_POINT%\\apps\\vim\\;C:\\Windows\\system32;C:\\Windows",
+				},
+			},
+			exec: []string{"cmd", "/c", "IF", "%PATH%", "==", "%CONTAINER_SANDBOX_MOUNT_POINT%\\apps\\vim\\;C:\\Windows\\system32;C:\\Windows", "( exit -1 )", "ELSE", "(exit 0)"},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			requireFeatures(t, test.requiredFeatures...)
+
+			requiredImages := []string{test.sandboxImage, test.containerImage}
+			pullRequiredImages(t, requiredImages)
+
+			podctx := context.Background()
+			sandboxRequest := getJobContainerPodRequestWCOW(t)
+
+			podID := runPodSandbox(t, client, podctx, sandboxRequest)
+			defer removePodSandbox(t, client, podctx, podID)
+			defer stopPodSandbox(t, client, podctx, podID)
+
+			containerRequest := getJobContainerRequestWCOW(t, podID, sandboxRequest.Config, imageWindowsNanoserver, nil)
+			containerRequest.Config.Envs = test.env
+			ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
+			defer cancel()
+
+			containerID := createContainer(t, client, ctx, containerRequest)
+			defer removeContainer(t, client, ctx, containerID)
+			startContainer(t, client, ctx, containerID)
+			defer stopContainer(t, client, ctx, containerID)
+
+			r := execSync(t, client, ctx, &runtime.ExecSyncRequest{
+				ContainerId: containerID,
+				Cmd:         test.exec,
+			})
+			if r.ExitCode != 0 {
+				t.Fatalf("failed with exit code %d checking for job container mount: %s", r.ExitCode, string(r.Stderr))
+			}
+		})
+	}
+}
+
 func Test_RunContainer_WorkingDirectory_JobContainer_WCOW(t *testing.T) {
 	client := newTestRuntimeClient(t)