From 014849a4b0dcf88831921f34ba751b0400716203 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 3 Feb 2022 14:20:51 -0800 Subject: [PATCH 1/2] Expand env variables to job mount path When installing tools into container it is sometimes desirable to have those tools on the path for the current process. Since the Windows doesn't support variable expansion on the PATH variable and the job mount path as something that isn't know at runtime we need to expand it for the passer. This could go away if the mount path is no longer used in the future. Signed-off-by: James Sturtevant --- internal/jobcontainers/jobcontainer.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/internal/jobcontainers/jobcontainer.go b/internal/jobcontainers/jobcontainer.go index 2b9343fb4a..d1be8635fa 100644 --- 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]) From 9d05b5b66d4cc3e678b4cd87b50a54977820101c Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Thu, 3 Feb 2022 16:17:35 -0800 Subject: [PATCH 2/2] Add test case for job env expansion Signed-off-by: James Sturtevant --- test/cri-containerd/jobcontainer_test.go | 77 ++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/test/cri-containerd/jobcontainer_test.go b/test/cri-containerd/jobcontainer_test.go index 45ba95af60..05397c8036 100644 --- 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)