Adds support for Windows ArgsEscaped images

Adds support for Windows container images built by Docker that contain the ArgsEscaped boolean in the ImageConfig. This is a non-OCI entry that tells the runtime that the Entrypoint and/or Cmd are a single element array with the args pre-escaped into a single CommandLine that should be passed directly to Windows rather than passed as an args array which will be additionally escaped. Signed-off-by: Justin Terry <jlterry@amazon.com>
2022-01-26 14:19:28 -08:00
parent 52471721fd
commit de3d9993f5
5 changed files with 525 additions and 5 deletions
--- a/oci/spec_opts_windows_test.go
+++ b/oci/spec_opts_windows_test.go
@@ -18,10 +18,15 @@ package oci

 import (
 	"context"
+	"encoding/json"
 	"testing"

 	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/namespaces"
+
+	"github.com/opencontainers/go-digest"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/opencontainers/runtime-spec/specs-go"
 )

 func TestWithCPUCount(t *testing.T) {
@@ -108,3 +113,436 @@ func TestWithWindowNetworksAllowUnqualifiedDNSQuery(t *testing.T) {
 		}
 	}
 }
+
+func newFakeArgsEscapedImage(config ocispec.ImageConfig) (Image, error) {
+	type imageExtended struct {
+		Config struct {
+			ocispec.ImageConfig
+			ArgsEscaped bool `json:"ArgsEscaped,omitempty"`
+		}
+	}
+
+	// Copy to extended format.
+	configExtended := imageExtended{}
+	configExtended.Config.ImageConfig = config
+	configExtended.Config.ArgsEscaped = true
+
+	configBlob, err := json.Marshal(configExtended)
+	if err != nil {
+		return nil, err
+	}
+	configDescriptor := ocispec.Descriptor{
+		MediaType: ocispec.MediaTypeImageConfig,
+		Digest:    digest.NewDigestFromBytes(digest.SHA256, configBlob),
+	}
+
+	return fakeImage{
+		config: configDescriptor,
+		blobs: map[string]blob{
+			configDescriptor.Digest.String(): configBlob,
+		},
+	}, nil
+}
+
+// TestWithProcessArgsOverwritesWithImage verifies that when calling
+// WithImageConfig followed by WithProcessArgs when `ArgsEscaped==false` that
+// the process args overwrite the image args.
+func TestWithProcessArgsOverwritesWithImage(t *testing.T) {
+	t.Parallel()
+
+	img, err := newFakeImage(ocispec.Image{
+		Config: ocispec.ImageConfig{
+			Entrypoint: []string{"powershell.exe", "-Command", "Write-Host Hello"},
+			Cmd:        []string{"cmd.exe", "/S", "/C", "echo Hello"},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	s := Spec{
+		Version: specs.Version,
+		Root:    &specs.Root{},
+		Windows: &specs.Windows{},
+	}
+
+	args := []string{"cmd.exe", "echo", "should be set"}
+	opts := []SpecOpts{
+		WithImageConfig(img),
+		WithProcessArgs(args...),
+	}
+
+	for _, opt := range opts {
+		if err := opt(nil, nil, nil, &s); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if err := assertEqualsStringArrays(args, s.Process.Args); err != nil {
+		t.Fatal(err)
+	}
+	if s.Process.CommandLine != "" {
+		t.Fatalf("Expected empty CommandLine, got: '%s'", s.Process.CommandLine)
+	}
+}
+
+// TestWithProcessArgsOverwritesWithImageArgsEscaped verifies that when calling
+// WithImageConfig followed by WithProcessArgs when `ArgsEscaped==true` that the
+// process args overwrite the image args.
+func TestWithProcessArgsOverwritesWithImageArgsEscaped(t *testing.T) {
+	t.Parallel()
+
+	img, err := newFakeArgsEscapedImage(ocispec.ImageConfig{
+		Entrypoint: []string{`powershell.exe -Command "C:\My Data\MyExe.exe" -arg1 "-arg2 value2"`},
+		Cmd:        []string{`cmd.exe /S /C "C:\test path\test.exe"`},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	s := Spec{
+		Version: specs.Version,
+		Root:    &specs.Root{},
+		Windows: &specs.Windows{},
+	}
+
+	args := []string{"cmd.exe", "echo", "should be set"}
+	opts := []SpecOpts{
+		WithImageConfig(img),
+		WithProcessArgs(args...),
+	}
+
+	for _, opt := range opts {
+		if err := opt(nil, nil, nil, &s); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if err := assertEqualsStringArrays(args, s.Process.Args); err != nil {
+		t.Fatal(err)
+	}
+	if s.Process.CommandLine != "" {
+		t.Fatalf("Expected empty CommandLine, got: '%s'", s.Process.CommandLine)
+	}
+}
+
+// TestWithImageOverwritesWithProcessArgs verifies that when calling
+// WithProcessArgs followed by WithImageConfig `ArgsEscaped==false` that the
+// image args overwrites process args.
+func TestWithImageOverwritesWithProcessArgs(t *testing.T) {
+	t.Parallel()
+
+	img, err := newFakeImage(ocispec.Image{
+		Config: ocispec.ImageConfig{
+			Entrypoint: []string{"powershell.exe", "-Command"},
+			Cmd:        []string{"Write-Host", "echo Hello"},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	s := Spec{
+		Version: specs.Version,
+		Root:    &specs.Root{},
+		Windows: &specs.Windows{},
+	}
+
+	opts := []SpecOpts{
+		WithProcessArgs("cmd.exe", "echo", "should not be set"),
+		WithImageConfig(img),
+	}
+
+	for _, opt := range opts {
+		if err := opt(nil, nil, nil, &s); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	expectedArgs := []string{"powershell.exe", "-Command", "Write-Host", "echo Hello"}
+	if err := assertEqualsStringArrays(expectedArgs, s.Process.Args); err != nil {
+		t.Fatal(err)
+	}
+	if s.Process.CommandLine != "" {
+		t.Fatalf("Expected empty CommandLine, got: '%s'", s.Process.CommandLine)
+	}
+}
+
+// TestWithImageOverwritesWithProcessArgs verifies that when calling
+// WithProcessArgs followed by WithImageConfig `ArgsEscaped==true` that the
+// image args overwrites process args.
+func TestWithImageArgsEscapedOverwritesWithProcessArgs(t *testing.T) {
+	t.Parallel()
+
+	img, err := newFakeArgsEscapedImage(ocispec.ImageConfig{
+		Entrypoint: []string{`powershell.exe -Command "C:\My Data\MyExe.exe" -arg1 "-arg2 value2"`},
+		Cmd:        []string{`cmd.exe /S /C "C:\test path\test.exe"`},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	s := Spec{
+		Version: specs.Version,
+		Root:    &specs.Root{},
+		Windows: &specs.Windows{},
+	}
+
+	opts := []SpecOpts{
+		WithProcessArgs("cmd.exe", "echo", "should not be set"),
+		WithImageConfig(img),
+	}
+
+	expectedCommandLine := `powershell.exe -Command "C:\My Data\MyExe.exe" -arg1 "-arg2 value2" "cmd.exe /S /C \"C:\test path\test.exe\""`
+
+	for _, opt := range opts {
+		if err := opt(nil, nil, nil, &s); err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	if s.Process.Args != nil {
+		t.Fatalf("Expected empty Process.Args, got: '%v'", s.Process.Args)
+	}
+	if expectedCommandLine != s.Process.CommandLine {
+		t.Fatalf("Expected CommandLine '%s', got: '%s'", expectedCommandLine, s.Process.CommandLine)
+	}
+}
+
+func TestWithImageConfigArgsWindows(t *testing.T) {
+	testcases := []struct {
+		name       string
+		entrypoint []string
+		cmd        []string
+		args       []string
+
+		expectError bool
+		// When ArgsEscaped==false we always expect args and CommandLine==""
+		expectedArgs []string
+	}{
+		{
+			// This is not really a valid test case since Docker would have made
+			// the default cmd to be the shell. So just verify it hits the error
+			// case we expect.
+			name:        "EmptyEntrypoint_EmptyCmd_EmptyArgs",
+			entrypoint:  nil,
+			cmd:         nil,
+			args:        nil,
+			expectError: true,
+		},
+		{
+			name:         "EmptyEntrypoint_EmptyCmd_Args",
+			entrypoint:   nil,
+			cmd:          nil,
+			args:         []string{"additional", "args"},
+			expectedArgs: []string{"additional", "args"},
+		},
+		{
+			name:         "EmptyEntrypoint_Cmd_EmptyArgs",
+			entrypoint:   nil,
+			cmd:          []string{"cmd", "args"},
+			args:         nil,
+			expectedArgs: []string{"cmd", "args"},
+		},
+		{
+			name:         "EmptyEntrypoint_Cmd_Args",
+			entrypoint:   nil,
+			cmd:          []string{"cmd", "args"},
+			args:         []string{"additional", "args"},
+			expectedArgs: []string{"additional", "args"}, // Args overwrite Cmd
+		},
+		{
+			name:         "Entrypoint_EmptyCmd_EmptyArgs",
+			entrypoint:   []string{"entrypoint", "args"},
+			cmd:          nil,
+			args:         nil,
+			expectedArgs: []string{"entrypoint", "args"},
+		},
+		{
+			name:         "Entrypoint_EmptyCmd_Args",
+			entrypoint:   []string{"entrypoint", "args"},
+			cmd:          nil,
+			args:         []string{"additional", "args"},
+			expectedArgs: []string{"entrypoint", "args", "additional", "args"},
+		},
+		{
+			name:         "Entrypoint_Cmd_EmptyArgs",
+			entrypoint:   []string{"entrypoint", "args"},
+			cmd:          []string{"cmd", "args"},
+			args:         nil,
+			expectedArgs: []string{"entrypoint", "args", "cmd", "args"},
+		},
+		{
+			name:         "Entrypoint_Cmd_Args",
+			entrypoint:   []string{"entrypoint", "args"},
+			cmd:          []string{"cmd", "args"},
+			args:         []string{"additional", "args"}, // Args overwrites Cmd
+			expectedArgs: []string{"entrypoint", "args", "additional", "args"},
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			img, err := newFakeImage(ocispec.Image{
+				Config: ocispec.ImageConfig{
+					Entrypoint: tc.entrypoint,
+					Cmd:        tc.cmd,
+				},
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			s := Spec{
+				Version: specs.Version,
+				Root:    &specs.Root{},
+				Windows: &specs.Windows{},
+			}
+
+			opts := []SpecOpts{
+				WithImageConfigArgs(img, tc.args),
+			}
+
+			for _, opt := range opts {
+				if err := opt(nil, nil, nil, &s); err != nil {
+					if tc.expectError {
+						continue
+					}
+					t.Fatal(err)
+				}
+			}
+
+			if err := assertEqualsStringArrays(tc.expectedArgs, s.Process.Args); err != nil {
+				t.Fatal(err)
+			}
+			if s.Process.CommandLine != "" {
+				t.Fatalf("Expected empty CommandLine, got: '%s'", s.Process.CommandLine)
+			}
+		})
+	}
+}
+
+func TestWithImageConfigArgsEscapedWindows(t *testing.T) {
+	testcases := []struct {
+		name       string
+		entrypoint []string
+		cmd        []string
+		args       []string
+
+		expectError         bool
+		expectedArgs        []string
+		expectedCommandLine string
+	}{
+		{
+			// This is not really a valid test case since Docker would have made
+			// the default cmd to be the shell. So just verify it hits the error
+			// case we expect.
+			name:                "EmptyEntrypoint_EmptyCmd_EmptyArgs",
+			entrypoint:          nil,
+			cmd:                 nil,
+			args:                nil,
+			expectError:         true,
+			expectedArgs:        nil,
+			expectedCommandLine: "",
+		},
+		{
+			// This case is special for ArgsEscaped, since there is no Image
+			// Default Args should be passed as ProcessArgs not as Cmdline
+			name:                "EmptyEntrypoint_EmptyCmd_Args",
+			entrypoint:          nil,
+			cmd:                 nil,
+			args:                []string{"additional", "-args", "hello world"},
+			expectedArgs:        []string{"additional", "-args", "hello world"},
+			expectedCommandLine: "",
+		},
+		{
+			name:                "EmptyEntrypoint_Cmd_EmptyArgs",
+			entrypoint:          nil,
+			cmd:                 []string{`cmd -args "hello world"`},
+			args:                nil,
+			expectedCommandLine: `cmd -args "hello world"`,
+		},
+		{
+			// This case is a second special case for ArgsEscaped, since Args
+			// overwrite Cmd the args are not from the image, so ArgsEscaped
+			// should be ignored, and passed as Args not CommandLine.
+			name:                "EmptyEntrypoint_Cmd_Args",
+			entrypoint:          nil,
+			cmd:                 []string{`cmd -args "hello world"`},
+			args:                []string{"additional", "args"},
+			expectedArgs:        []string{"additional", "args"}, // Args overwrite Cmd
+			expectedCommandLine: "",
+		},
+		{
+			name:                "Entrypoint_EmptyCmd_EmptyArgs",
+			entrypoint:          []string{`"C:\My Folder\MyProcess.exe" -arg1 "test value"`},
+			cmd:                 nil,
+			args:                nil,
+			expectedCommandLine: `"C:\My Folder\MyProcess.exe" -arg1 "test value"`,
+		},
+		{
+			name:                "Entrypoint_EmptyCmd_Args",
+			entrypoint:          []string{`"C:\My Folder\MyProcess.exe" -arg1 "test value"`},
+			cmd:                 nil,
+			args:                []string{"additional", "args with spaces"},
+			expectedCommandLine: `"C:\My Folder\MyProcess.exe" -arg1 "test value" additional "args with spaces"`,
+		},
+		{
+			// This case will not work in Docker today so adding the test to
+			// confirm we fail in the same way. Although the appending of
+			// Entrypoint + " " + Cmd here works, Cmd is double escaped and the
+			// container would not launch. This is because when Docker built
+			// such an image it escaped both Entrypoint and Cmd. However the
+			// docs say that CMD should always be appened to entrypoint if not
+			// overwritten so this results in an incorrect cmdline.
+			name:                "Entrypoint_Cmd_EmptyArgs",
+			entrypoint:          []string{`"C:\My Folder\MyProcess.exe" -arg1 "test value"`},
+			cmd:                 []string{`cmd -args "hello world"`},
+			args:                nil,
+			expectedCommandLine: `"C:\My Folder\MyProcess.exe" -arg1 "test value" "cmd -args \"hello world\""`,
+		},
+		{
+			name:                "Entrypoint_Cmd_Args",
+			entrypoint:          []string{`"C:\My Folder\MyProcess.exe" -arg1 "test value"`},
+			cmd:                 []string{`cmd -args "hello world"`},
+			args:                []string{"additional", "args with spaces"}, // Args overwrites Cmd
+			expectedCommandLine: `"C:\My Folder\MyProcess.exe" -arg1 "test value" additional "args with spaces"`,
+		},
+	}
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			img, err := newFakeArgsEscapedImage(ocispec.ImageConfig{
+				Entrypoint: tc.entrypoint,
+				Cmd:        tc.cmd,
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			s := Spec{
+				Version: specs.Version,
+				Root:    &specs.Root{},
+				Windows: &specs.Windows{},
+			}
+
+			opts := []SpecOpts{
+				WithImageConfigArgs(img, tc.args),
+			}
+
+			for _, opt := range opts {
+				if err := opt(nil, nil, nil, &s); err != nil {
+					if tc.expectError {
+						continue
+					}
+					t.Fatal(err)
+				}
+			}
+
+			if err := assertEqualsStringArrays(tc.expectedArgs, s.Process.Args); err != nil {
+				t.Fatal(err)
+			}
+			if tc.expectedCommandLine != s.Process.CommandLine {
+				t.Fatalf("Expected CommandLine: '%s', got: '%s'", tc.expectedCommandLine, s.Process.CommandLine)
+			}
+		})
+	}
+}