From 540ec4c7fc8c5e80ade715c1a0466c3c2a8a7135 Mon Sep 17 00:00:00 2001 From: David Trudgian Date: Tue, 6 Dec 2022 12:12:17 +0000 Subject: [PATCH] feat: oci: enable --env-file in --oci mode Allow --env-file to be used to provide environment variables in a file, when running a container in --oci mode. We use the same approach as the native runtime for compatibility. The env file is evaluated in the embedded shell interpreter, but starting with an empty environment. This handles quoting, comments etc. for us, and keeps maximum compatibility with the existing handling. Fixes sylabs/singularity#1030 Signed-off-by: Edita Kizinevic --- .../runtime/launcher/native/launcher_linux.go | 6 +- .../runtime/launcher/oci/launcher_linux.go | 16 ++-- .../pkg/runtime/launcher/oci/process_linux.go | 38 +++++++++ .../launcher/oci/process_linux_test.go | 84 +++++++++++++++++++ 4 files changed, 135 insertions(+), 9 deletions(-) diff --git a/internal/pkg/runtime/launcher/native/launcher_linux.go b/internal/pkg/runtime/launcher/native/launcher_linux.go index 5242122afd..b3ee366312 100644 --- a/internal/pkg/runtime/launcher/native/launcher_linux.go +++ b/internal/pkg/runtime/launcher/native/launcher_linux.go @@ -975,7 +975,7 @@ func (l *Launcher) setEnvVars(ctx context.Context, args []string) error { content, err := os.ReadFile(l.cfg.EnvFile) if err != nil { - return fmt.Errorf("could not read %q environment file: %w", l.cfg.EnvFile, err) + return fmt.Errorf("could not read environment file %q: %w", l.cfg.EnvFile, err) } envvars, err := interpreter.EvaluateEnv(ctx, content, args, currentEnv) @@ -990,7 +990,7 @@ func (l *Launcher) setEnvVars(ctx context.Context, args []string) error { for _, envar := range envvars { e := strings.SplitN(envar, "=", 2) if len(e) != 2 { - sylog.Warningf("Ignore environment variable %q: '=' is missing", envar) + sylog.Warningf("Ignored environment variable %q: '=' is missing", envar) continue } // Don't attempt to overwrite bash builtin readonly vars @@ -1000,7 +1000,7 @@ func (l *Launcher) setEnvVars(ctx context.Context, args []string) error { } // Ensure we don't overwrite --env variables with environment file if _, ok := l.cfg.Env[e[0]]; ok { - sylog.Warningf("Ignore environment variable %s from %s: override from --env", e[0], l.cfg.EnvFile) + sylog.Warningf("Ignored environment variable %s from %s: override from --env", e[0], l.cfg.EnvFile) } else { l.cfg.Env[e[0]] = e[1] } diff --git a/internal/pkg/runtime/launcher/oci/launcher_linux.go b/internal/pkg/runtime/launcher/oci/launcher_linux.go index 2a55551758..1cd2300cd0 100644 --- a/internal/pkg/runtime/launcher/oci/launcher_linux.go +++ b/internal/pkg/runtime/launcher/oci/launcher_linux.go @@ -128,9 +128,6 @@ func checkOpts(lo launcher.Options) error { badOpt = append(badOpt, "ContainLibs") } - if lo.EnvFile != "" { - badOpt = append(badOpt, "EnvFile") - } if lo.CleanEnv { badOpt = append(badOpt, "CleanEnv") } @@ -320,11 +317,18 @@ func (l *Launcher) Exec(ctx context.Context, image string, process string, args // Assemble the runtime & user-requested environment, which will be merged // with the image ENV and set in the container at runtime. rtEnv := defaultEnv(image, bundleDir) - // APPTAINERENV_ + // APPTAINERENV_ has lowest priority rtEnv = mergeMap(rtEnv, apptainerEnvMap()) - // --env flag + // --env-file can override APPTAINERENV_ + if l.cfg.EnvFile != "" { + e, err := envFileMap(ctx, l.cfg.EnvFile) + if err != nil { + return err + } + rtEnv = mergeMap(rtEnv, e) + } + // --env flag can override --env-file and APPTAINERENV_ rtEnv = mergeMap(rtEnv, l.cfg.Env) - // TODO - --env-file b, err := native.New( native.OptBundlePath(bundleDir), diff --git a/internal/pkg/runtime/launcher/oci/process_linux.go b/internal/pkg/runtime/launcher/oci/process_linux.go index ad15049c08..9390935f70 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux.go +++ b/internal/pkg/runtime/launcher/oci/process_linux.go @@ -10,12 +10,14 @@ package oci import ( + "context" "fmt" "os" "strings" "github.com/apptainer/apptainer/internal/pkg/fakeroot" "github.com/apptainer/apptainer/internal/pkg/util/env" + "github.com/apptainer/apptainer/internal/pkg/util/shell/interpreter" "github.com/apptainer/apptainer/internal/pkg/util/user" "github.com/opencontainers/runtime-spec/specs-go" ) @@ -170,3 +172,39 @@ func apptainerEnvMap() map[string]string { return apptainerEnv } + +// envFileMap returns a map of KEY=VAL env vars from an environment file +func envFileMap(ctx context.Context, f string) (map[string]string, error) { + envMap := map[string]string{} + + content, err := os.ReadFile(f) + if err != nil { + return envMap, fmt.Errorf("could not read environment file %q: %w", f, err) + } + + // Use the embedded shell interpreter to evaluate the env file, with an empty starting environment. + // Shell takes care of comments, quoting etc. for us and keeps compatibility with native runtime. + env, err := interpreter.EvaluateEnv(ctx, content, []string{}, []string{}) + if err != nil { + return envMap, fmt.Errorf("while processing %s: %w", f, err) + } + + for _, envVar := range env { + parts := strings.SplitN(envVar, "=", 2) + if len(parts) < 2 { + continue + } + // Strip out the runtime env vars set by the shell interpreter + if parts[0] == "GID" || + parts[0] == "HOME" || + parts[0] == "IFS" || + parts[0] == "OPTIND" || + parts[0] == "PWD" || + parts[0] == "UID" { + continue + } + envMap[parts[0]] = parts[1] + } + + return envMap, nil +} diff --git a/internal/pkg/runtime/launcher/oci/process_linux_test.go b/internal/pkg/runtime/launcher/oci/process_linux_test.go index c144325b0d..9fffa596d3 100644 --- a/internal/pkg/runtime/launcher/oci/process_linux_test.go +++ b/internal/pkg/runtime/launcher/oci/process_linux_test.go @@ -10,7 +10,9 @@ package oci import ( + "context" "os" + "path/filepath" "reflect" "testing" ) @@ -62,3 +64,85 @@ func TestApptainerEnvMap(t *testing.T) { }) } } + +func TestEnvFileMap(t *testing.T) { + tests := []struct { + name string + envFile string + want map[string]string + wantErr bool + }{ + { + name: "EmptyFile", + envFile: "", + want: map[string]string{ + "EUID": "0", + }, + wantErr: false, + }, + { + name: "Simple", + envFile: `FOO=BAR + ABC=123`, + want: map[string]string{ + "EUID": "0", + "FOO": "BAR", + "ABC": "123", + }, + wantErr: false, + }, + { + name: "DoubleQuote", + envFile: `FOO="FOO BAR"`, + want: map[string]string{ + "EUID": "0", + "FOO": "FOO BAR", + }, + wantErr: false, + }, + { + name: "SingleQuote", + envFile: `FOO='FOO BAR'`, + want: map[string]string{ + "EUID": "0", + "FOO": "FOO BAR", + }, + wantErr: false, + }, + { + name: "MultiLine", + envFile: "FOO=\"FOO\nBAR\"", + want: map[string]string{ + "EUID": "0", + "FOO": "FOO\nBAR", + }, + wantErr: false, + }, + { + name: "Invalid", + envFile: "!!!@@NOTAVAR", + want: map[string]string{}, + wantErr: true, + }, + } + + tmpDir := t.TempDir() + envFile := filepath.Join(tmpDir, "env-file") + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if err := os.WriteFile(envFile, []byte(tt.envFile), 0o755); err != nil { + t.Fatalf("Could not write test env-file: %v", err) + } + + got, err := envFileMap(context.Background(), envFile) + if (err != nil) != tt.wantErr { + t.Errorf("envFileMap() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("envFileMap() = %v, want %v", got, tt.want) + } + }) + } +}