Merge pull request #9118 from CharityKathure/windows-default-path-overwrite-fix

Fix windows default path overwrite

oci/spec_opts.go

@@ -187,13 +187,6 @@ func WithEnv(environmentVariables []string) SpecOpts {
 	}
 }
 
-// WithDefaultPathEnv sets the $PATH environment variable to the
-// default PATH defined in this package.
-func WithDefaultPathEnv(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
-	s.Process.Env = replaceOrAppendEnvValues(s.Process.Env, defaultUnixEnv)
-	return nil
-}
-
 // replaceOrAppendEnvValues returns the defaults with the overrides either
 // replaced by env key or appended to the list
 func replaceOrAppendEnvValues(defaults, overrides []string) []string {

oci/spec_opts_nonwindows.go

@@ -0,0 +1,32 @@
+//go:build !windows
+
+/*
+   Copyright The containerd 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 oci
+
+import (
+	"context"
+
+	"github.com/containerd/containerd/containers"
+)
+
+// WithDefaultPathEnv sets the $PATH environment variable to the
+// default PATH defined in this package.
+func WithDefaultPathEnv(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
+	s.Process.Env = replaceOrAppendEnvValues(s.Process.Env, defaultUnixEnv)
+	return nil
+}

oci/spec_opts_nonwindows_test.go

@@ -0,0 +1,43 @@
+//go:build !windows
+
+/*
+   Copyright The containerd 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 oci
+
+import (
+	"context"
+	"testing"
+
+	"github.com/containerd/containerd/namespaces"
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+)
+
+func TestWithDefaultPathEnv(t *testing.T) {
+	t.Parallel()
+	s := Spec{}
+	s.Process = &specs.Process{
+		Env: []string{},
+	}
+	var (
+		defaultUnixEnv = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+		ctx            = namespaces.WithNamespace(context.Background(), "test")
+	)
+	WithDefaultPathEnv(ctx, nil, nil, &s)
+	if !Contains(s.Process.Env, defaultUnixEnv) {
+		t.Fatal("default Unix Env not found")
+	}
+}

oci/spec_opts_test.go

@@ -188,22 +188,6 @@ func Contains(a []string, x string) bool {
 	return false
 }
 
-func TestWithDefaultPathEnv(t *testing.T) {
-	t.Parallel()
-	s := Spec{}
-	s.Process = &specs.Process{
-		Env: []string{},
-	}
-	var (
-		defaultUnixEnv = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
-		ctx            = namespaces.WithNamespace(context.Background(), "test")
-	)
-	WithDefaultPathEnv(ctx, nil, nil, &s)
-	if !Contains(s.Process.Env, defaultUnixEnv) {
-		t.Fatal("default Unix Env not found")
-	}
-}
-
 func TestWithProcessCwd(t *testing.T) {
 	t.Parallel()
 	s := Spec{}

oci/spec_opts_windows.go

@@ -62,3 +62,8 @@ func WithDevices(devicePath, containerPath, permissions string) SpecOpts {
 		return nil
 	}
 }
+
+// Windows containers have default path configured at bootup
+func WithDefaultPathEnv(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
+	return nil
+}

oci/spec_opts_windows_test.go

@@ -18,6 +18,7 @@ package oci
 
 import (
 	"context"
+	"os"
 	"testing"
 
 	"github.com/containerd/containerd/containers"
@@ -524,3 +525,26 @@ func TestWithImageConfigArgsEscapedWindows(t *testing.T) {
 		})
 	}
 }
+
+func TestWindowsDefaultPathEnv(t *testing.T) {
+	t.Parallel()
+	s := Spec{}
+	s.Process = &specs.Process{
+		Env: []string{},
+	}
+
+	var (
+		defaultUnixEnv = "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+		ctx            = namespaces.WithNamespace(context.Background(), "test")
+	)
+
+	//check that the default PATH environment is not null
+	if os.Getenv("PATH") == "" {
+		t.Fatal("PATH environment variable is not set")
+	}
+	WithDefaultPathEnv(ctx, nil, nil, &s)
+	//check that the path is not overwritten by the unix default path
+	if Contains(s.Process.Env, defaultUnixEnv) {
+		t.Fatal("default Windows Env overwritten by the default Unix Env")
+	}
+}