From 5ad641836769b17ae7f28fd3d169d0bde21aafaa Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Wed, 6 Mar 2024 15:37:02 +0000 Subject: [PATCH 1/2] fix: oci: set default PATH if image does not OCI images almost always set `PATH` in their config. However, some don't. Most recently, `docker://busybox:latest` changed and the new version: * Doesn't set `PATH` in its config. * Has a CMD of `sh` which has to be found on a `PATH`. Singularity's OCI mode was failing to find `sh`. Other runtimes set a default `PATH` in the container, so running `busybox:latest` still works. This PR ensures we set a default `PATH` in the container where the image does not provide one. Fixes #2721 --- CHANGELOG.md | 6 +++++ .../pkg/runtime/launcher/oci/process_linux.go | 10 +++++--- .../launcher/oci/process_linux_test.go | 23 +++++++++++++++++-- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b788d18b8a..0fed8af513 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # SingularityCE Changelog +## Change Since Last Release + +### Bug Fixes + +- Set default `PATH` in container run in OCI-Mode when image does not set `PATH`. + ## 4.1.2 \[2024-03-05\] ### Bug Fixes diff --git a/internal/pkg/runtime/launcher/oci/process_linux.go b/internal/pkg/runtime/launcher/oci/process_linux.go index b89e084b92..a9bc547d1e 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux.go +++ b/internal/pkg/runtime/launcher/oci/process_linux.go @@ -376,6 +376,12 @@ func (l *Launcher) getProcessEnv(imageSpec imgspecv1.Image, hostEnv []string, us } func setOCIPath(g *generate.Generator, prependPath, path, appendPath string) { + // If we have no PATH at this point, then the image didn't define one, and + // the user didn't specify a full PATH override. We need to use a default, + // to find bare `CMD`s e.g. issue #2721. + if path == "" { + path = env.DefaultPath + } // Compute and set optionally APPEND-ed / PREPEND-ed PATH. if appendPath != "" { path = strings.TrimSuffix(path, ":") + ":" + appendPath @@ -383,9 +389,7 @@ func setOCIPath(g *generate.Generator, prependPath, path, appendPath string) { if prependPath != "" { path = prependPath + ":" + strings.TrimPrefix(path, ":") } - if path != "" { - g.AddProcessEnv("PATH", path) - } + g.AddProcessEnv("PATH", path) } func setNativePath(g *generate.Generator, prependPath, path, appendPath string) { diff --git a/internal/pkg/runtime/launcher/oci/process_linux_test.go b/internal/pkg/runtime/launcher/oci/process_linux_test.go index 11c2e99edf..0ef2fc6d36 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux_test.go +++ b/internal/pkg/runtime/launcher/oci/process_linux_test.go @@ -141,6 +141,8 @@ func TestGetProcessArgs(t *testing.T) { } func TestGetProcessEnvOCI(t *testing.T) { + defaultPathEnv := "PATH=" + env.DefaultPath + tests := []struct { name string noCompat bool @@ -155,7 +157,10 @@ func TestGetProcessEnvOCI(t *testing.T) { imageEnv: []string{}, hostEnv: []string{}, userEnv: map[string]string{}, - wantEnv: []string{"LD_LIBRARY_PATH=/.singularity.d/libs"}, + wantEnv: []string{ + defaultPathEnv, + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, }, { name: "PassTERM", @@ -165,6 +170,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{}, wantEnv: []string{ "TERM=xterm-256color", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, @@ -176,6 +182,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{}, wantEnv: []string{ "HTTP_PROXY=proxy.example.com:3128", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, @@ -185,7 +192,10 @@ func TestGetProcessEnvOCI(t *testing.T) { imageEnv: []string{}, hostEnv: []string{"NOT_FOR_CONTAINER=true"}, userEnv: map[string]string{}, - wantEnv: []string{"LD_LIBRARY_PATH=/.singularity.d/libs"}, + wantEnv: []string{ + defaultPathEnv, + "LD_LIBRARY_PATH=/.singularity.d/libs", + }, }, { name: "ImagePath", @@ -239,6 +249,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{}, wantEnv: []string{ "LD_LIBRARY_PATH=/foo:/.singularity.d/libs", + defaultPathEnv, }, }, { @@ -248,6 +259,7 @@ func TestGetProcessEnvOCI(t *testing.T) { hostEnv: []string{}, userEnv: map[string]string{"LD_LIBRARY_PATH": "/foo"}, wantEnv: []string{ + defaultPathEnv, "LD_LIBRARY_PATH=/foo:/.singularity.d/libs", }, }, @@ -259,6 +271,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{"LD_LIBRARY_PATH": "/bar"}, wantEnv: []string{ "LD_LIBRARY_PATH=/bar:/.singularity.d/libs", + defaultPathEnv, }, }, { @@ -269,6 +282,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{}, wantEnv: []string{ "FOO=bar", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, @@ -280,6 +294,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{"FOO": "baz"}, wantEnv: []string{ "FOO=baz", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, @@ -292,6 +307,7 @@ func TestGetProcessEnvOCI(t *testing.T) { wantEnv: []string{ "FOO=bar", "ABC=123", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, @@ -303,6 +319,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{}, wantEnv: []string{ "FOO=bar", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, @@ -314,6 +331,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{}, wantEnv: []string{ "FOO=baz", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, @@ -325,6 +343,7 @@ func TestGetProcessEnvOCI(t *testing.T) { userEnv: map[string]string{"FOO": "baz"}, wantEnv: []string{ "FOO=baz", + defaultPathEnv, "LD_LIBRARY_PATH=/.singularity.d/libs", }, }, From 41f91e82892819ff8b79d3048f0d2275b5f109e4 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Wed, 6 Mar 2024 16:29:56 +0000 Subject: [PATCH 2/2] ci: add squashfs-tools-ng as dependency in CI environment Ubuntu 22.04 `sqfstar` is a 4.5 version that has a bug that will cause conversion of OCI images with certain forms of symlink to fail. Our Deb packaging already recommends `squashfs-tools-ng` for `tar2sqfs` instead, so make that available in the CI environment. --- .circleci/config.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.circleci/config.yml b/.circleci/config.yml index 756340cf78..83d8caee5e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -98,6 +98,7 @@ commands: libtool \ pkg-config \ squashfs-tools \ + squashfs-tools-ng \ uidmap \ zlib1g-dev - run: