From 88a3e25b3dac43f3c1593cf3b11427593d5058b5 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 16 May 2023 10:55:10 +0300 Subject: [PATCH] Add targetOS to WithVolumes() Windows systems are capable of running both Windows Containers and Linux containers. For windows containers we need to sanitize the volume path and skip non-C volumes from the copy existing contents code path. Linux containers running on Windows and Linux must not have the path sanitized in any way. Supplying the targetOS of the container allows us to proprely decide when to activate that code path. Signed-off-by: Gabriel Adrian Samfira --- pkg/cri/opts/container.go | 32 +++++++++++++++------------- pkg/cri/sbserver/container_create.go | 2 +- pkg/cri/server/container_create.go | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/pkg/cri/opts/container.go b/pkg/cri/opts/container.go index cb89412da..754088ef2 100644 --- a/pkg/cri/opts/container.go +++ b/pkg/cri/opts/container.go @@ -55,7 +55,7 @@ func WithNewSnapshot(id string, i containerd.Image, opts ...snapshots.Opt) conta // WithVolumes copies ownership of volume in rootfs to its corresponding host path. // It doesn't update runtime spec. // The passed in map is a host path to container path map for all volumes. -func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { +func WithVolumes(volumeMounts map[string]string, targetOS string) containerd.NewContainerOpts { return func(ctx context.Context, client *containerd.Client, c *containers.Container) (err error) { if c.Snapshotter == "" { return errors.New("no snapshotter set for container") @@ -97,21 +97,23 @@ func WithVolumes(volumeMounts map[string]string) containerd.NewContainerOpts { }() for host, volume := range volumeMounts { - // Windows allows volume mounts in subfolders under C: and as any other drive letter like D:, E:, etc. - // An image may contain files inside a folder defined as a VOLUME in a Dockerfile. On Windows, images - // can only contain pre-existing files for volumes situated on the root filesystem, which is C:. - // For any other volumes, we need to skip attempting to copy existing contents. - // - // C:\some\volume --> \some\volume - // D:\some\volume --> skip - if len(volume) >= 2 && string(volume[1]) == ":" { - // Perform a case insensitive comparison to "C", and skip non-C mounted volumes. - if !strings.EqualFold(string(volume[0]), "c") { - continue + if targetOS == "windows" { + // Windows allows volume mounts in subfolders under C: and as any other drive letter like D:, E:, etc. + // An image may contain files inside a folder defined as a VOLUME in a Dockerfile. On Windows, images + // can only contain pre-existing files for volumes situated on the root filesystem, which is C:. + // For any other volumes, we need to skip attempting to copy existing contents. + // + // C:\some\volume --> \some\volume + // D:\some\volume --> skip + if len(volume) >= 2 && string(volume[1]) == ":" { + // Perform a case insensitive comparison to "C", and skip non-C mounted volumes. + if !strings.EqualFold(string(volume[0]), "c") { + continue + } + // This is a volume mounted somewhere under C:\. We strip the drive letter and allow fs.RootPath() + // to append the remaining path to the rootfs path as seen by the host OS. + volume = volume[2:] } - // This is a volume mounted somewhere under C:\. We strip the drive letter and allow fs.RootPath() - // to append the remaining path to the rootfs path as seen by the host OS. - volume = volume[2:] } src, err := fs.RootPath(root, volume) if err != nil { diff --git a/pkg/cri/sbserver/container_create.go b/pkg/cri/sbserver/container_create.go index f274e671f..435792ca4 100644 --- a/pkg/cri/sbserver/container_create.go +++ b/pkg/cri/sbserver/container_create.go @@ -223,7 +223,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta for _, v := range volumeMounts { mountMap[filepath.Clean(v.HostPath)] = v.ContainerPath } - opts = append(opts, customopts.WithVolumes(mountMap)) + opts = append(opts, customopts.WithVolumes(mountMap, image.ImageSpec.OS)) } meta.ImageRef = image.ID meta.StopSignal = image.ImageSpec.Config.StopSignal diff --git a/pkg/cri/server/container_create.go b/pkg/cri/server/container_create.go index 9483778d1..06b7fa948 100644 --- a/pkg/cri/server/container_create.go +++ b/pkg/cri/server/container_create.go @@ -203,7 +203,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta for _, v := range volumeMounts { mountMap[filepath.Clean(v.HostPath)] = v.ContainerPath } - opts = append(opts, customopts.WithVolumes(mountMap)) + opts = append(opts, customopts.WithVolumes(mountMap, image.ImageSpec.OS)) } meta.ImageRef = image.ID meta.StopSignal = image.ImageSpec.Config.StopSignal