Skip to content

Commit

Permalink
Fix windows default path overwrite issue
Browse files Browse the repository at this point in the history
Windows Containers have a default path already configured at bootup. WithDefaultPathEnv overwrites this with a unix path

Signed-off-by: charitykathure <kathurecharity505@gmail.com>
  • Loading branch information
Charity Kathure committed Oct 5, 2023
1 parent 0066676 commit 7d63690
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 23 deletions.
7 changes: 0 additions & 7 deletions oci/spec_opts.go
Expand Up @@ -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 {
Expand Down
32 changes: 32 additions & 0 deletions 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
}
43 changes: 43 additions & 0 deletions 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")
}
}
16 changes: 0 additions & 16 deletions oci/spec_opts_test.go
Expand Up @@ -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{}
Expand Down
5 changes: 5 additions & 0 deletions oci/spec_opts_windows.go
Expand Up @@ -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
}
24 changes: 24 additions & 0 deletions oci/spec_opts_windows_test.go
Expand Up @@ -18,6 +18,7 @@ package oci

import (
"context"
"os"
"testing"

"github.com/containerd/containerd/containers"
Expand Down Expand Up @@ -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")
}
}

0 comments on commit 7d63690

Please sign in to comment.