Adds support for Windows ArgsEscaped images

jterry75 · jterry75 · commit de3d9993f5d2 · 2022-03-01T13:40:44.000-08:00
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 &lt;jlterry@amazon.com&gt;
diff --git a/oci/spec_opts.go b/oci/spec_opts.go
@@ -218,6 +218,7 @@ func WithProcessArgs(args ...string) SpecOpts {
 	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
 		setProcess(s)
 		s.Process.Args = args
+		s.Process.CommandLine = ""
 		return nil
 	}
 }
@@ -347,17 +348,19 @@ func WithImageConfigArgs(image Image, args []string) SpecOpts {
 			return err
 		}
 		var (
-			ociimage v1.Image
-			config   v1.ImageConfig
+			imageConfigBytes []byte
+			ociimage         v1.Image
+			config           v1.ImageConfig
 		)
 		switch ic.MediaType {
 		case v1.MediaTypeImageConfig, images.MediaTypeDockerSchema2Config:
-			p, err := content.ReadBlob(ctx, image.ContentStore(), ic)
+			var err error
+			imageConfigBytes, err = content.ReadBlob(ctx, image.ContentStore(), ic)
 			if err != nil {
 				return err
 			}
 
-			if err := json.Unmarshal(p, &ociimage); err != nil {
+			if err := json.Unmarshal(imageConfigBytes, &ociimage); err != nil {
 				return err
 			}
 			config = ociimage.Config
@@ -393,12 +396,72 @@ func WithImageConfigArgs(image Image, args []string) SpecOpts {
 			// even if there is no specified user in the image config
 			return WithAdditionalGIDs("root")(ctx, client, c, s)
 		} else if s.Windows != nil {
+			// imageExtended is a superset of the oci Image struct that changes
+			// the Config type to be imageConfigExtended in order to add the
+			// ability to deserialize `ArgsEscaped` which is not an OCI field,
+			// but is supported by Docker built images.
+			type imageExtended struct {
+				Config struct {
+					ArgsEscaped bool `json:"ArgsEscaped,omitempty"`
+				}
+			}
+			// Deserialize the extended image format for Windows.
+			var ociImageExtended imageExtended
+			if err := json.Unmarshal(imageConfigBytes, &ociImageExtended); err != nil {
+				return err
+			}
+			argsEscaped := ociImageExtended.Config.ArgsEscaped
+
 			s.Process.Env = replaceOrAppendEnvValues(config.Env, s.Process.Env)
+
+			// To support Docker ArgsEscaped on Windows we need to combine the
+			// image Entrypoint & (Cmd Or User Args) while taking into account
+			// if Docker has already escaped them in the image config. When
+			// Docker sets `ArgsEscaped==true` in the config it has pre-escaped
+			// either Entrypoint or Cmd or both. Cmd should always be treated as
+			// arguments appended to Entrypoint unless:
+			//
+			// 1. Entrypoint does not exist, in which case Cmd[0] is the
+			// executable.
+			//
+			// 2. The user overrides the Cmd with User Args when activating the
+			// container in which case those args should be appended to the
+			// Entrypoint if it exists.
+			//
+			// To effectively do this we need to know if the arguments came from
+			// the user or if the arguments came from the image config when
+			// ArgsEscaped==true. In this case we only want to escape the
+			// additional user args when forming the complete CommandLine. This
+			// is safe in both cases of Entrypoint or Cmd being set because
+			// Docker will always escape them to an array of length one. Thus in
+			// both cases it is the "executable" portion of the command.
+			//
+			// In the case ArgsEscaped==false, Entrypoint or Cmd will contain
+			// any number of entries that are all unescaped and can simply be
+			// combined (potentially overwriting Cmd with User Args if present)
+			// and forwarded the container start as an Args array.
 			cmd := config.Cmd
+			cmdFromImage := true
 			if len(args) > 0 {
 				cmd = args
+				cmdFromImage = false
+			}
+
+			cmd = append(config.Entrypoint, cmd...)
+			if len(cmd) == 0 {
+				return errors.New("no arguments specified")
+			}
+
+			if argsEscaped && (len(config.Entrypoint) > 0 || cmdFromImage) {
+				s.Process.Args = nil
+				s.Process.CommandLine = cmd[0]
+				if len(cmd) > 1 {
+					s.Process.CommandLine += " " + escapeAndCombineArgs(cmd[1:])
+				}
+			} else {
+				s.Process.Args = cmd
+				s.Process.CommandLine = ""
 			}
-			s.Process.Args = append(config.Entrypoint, cmd...)
 
 			s.Process.Cwd = config.WorkingDir
 			s.Process.User = specs.User{
diff --git a/oci/spec_opts_linux.go b/oci/spec_opts_linux.go
@@ -153,3 +153,7 @@ func WithRdt(closID, l3CacheSchema, memBwSchema string) SpecOpts {
 		return nil
 	}
 }
+
+func escapeAndCombineArgs(args []string) string {
+	panic("not supported")
+}
diff --git a/oci/spec_opts_unix.go b/oci/spec_opts_unix.go
@@ -57,3 +57,7 @@ func WithCPUCFS(quota int64, period uint64) SpecOpts {
 		return nil
 	}
 }
+
+func escapeAndCombineArgs(args []string) string {
+	panic("not supported")
+}
diff --git a/oci/spec_opts_windows.go b/oci/spec_opts_windows.go
@@ -19,9 +19,12 @@ package oci
 import (
 	"context"
 	"errors"
+	"strings"
 
 	"github.com/containerd/containerd/containers"
+
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"golang.org/x/sys/windows"
 )
 
 // WithWindowsCPUCount sets the `Windows.Resources.CPU.Count` section to the
@@ -89,3 +92,11 @@ func WithWindowsNetworkNamespace(ns string) SpecOpts {
 		return nil
 	}
 }
+
+func escapeAndCombineArgs(args []string) string {
+	escaped := make([]string, len(args))
+	for i, a := range args {
+		escaped[i] = windows.EscapeArg(a)
+	}
+	return strings.Join(escaped, " ")
+}
diff --git a/oci/spec_opts_windows_test.go b/oci/spec_opts_windows_test.go

Original file line number	Diff line number	Diff line change
`@@ -153,3 +153,7 @@ func WithRdt(closID, l3CacheSchema, memBwSchema string) SpecOpts {`
`153`	`153`	`return nil`
`154`	`154`	`}`
`155`	`155`	`}`
	`156`	`+`
	`157`	`+func escapeAndCombineArgs(args []string) string {`
	`158`	`+ panic("not supported")`
	`159`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -57,3 +57,7 @@ func WithCPUCFS(quota int64, period uint64) SpecOpts {`
`57`	`57`	`return nil`
`58`	`58`	`}`
`59`	`59`	`}`
	`60`	`+`
	`61`	`+func escapeAndCombineArgs(args []string) string {`
	`62`	`+ panic("not supported")`
	`63`	`+}`