From 3e21e2ef5668d6199424d5bc0f7af7e869dc3853 Mon Sep 17 00:00:00 2001 From: Phil Estes Date: Fri, 13 Jan 2023 15:45:20 -0500 Subject: [PATCH] fix: properly handle local environment value pass-through The -e,--env flag as well as values within an --env-file can contain only a variable name with no "={VALUE}" portion. These entries provide a shorthand to say "pass the existing environment value of this variable into the container". Because of the VM boundary we need to extrapolate these values on the Finch side and pass them as discrete values into the Lima VM on the nerdctl command line. Also the file referenced by --env-file may not be accessible inside the VM, so we translate each entry in the file into -e entries on the command line rather than fail on the VM side, unable to locate the file being referenced. Signed-off-by: Phil Estes --- cmd/finch/nerdctl.go | 150 ++++++++++++++++++++++++++++++++++++++++--- pkg/system/stdlib.go | 4 ++ 2 files changed, 145 insertions(+), 9 deletions(-) diff --git a/cmd/finch/nerdctl.go b/cmd/finch/nerdctl.go index 86f437bbf..4cb15c283 100644 --- a/cmd/finch/nerdctl.go +++ b/cmd/finch/nerdctl.go @@ -4,16 +4,20 @@ package main import ( + "bufio" "fmt" + "os" + "path/filepath" + "strings" "github.com/spf13/cobra" + "github.com/runfinch/finch/pkg/command" "github.com/runfinch/finch/pkg/flog" + "github.com/runfinch/finch/pkg/lima" + "github.com/runfinch/finch/pkg/system" "k8s.io/apimachinery/pkg/util/sets" - - "github.com/runfinch/finch/pkg/command" - "github.com/runfinch/finch/pkg/lima" ) const nerdctlCmdName = "nerdctl" @@ -59,18 +63,59 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { if err != nil { return err } + var ( + nerdctlArgs, envs, fileEnvs []string + envVars = make(map[string]string) + skip bool + ) - var nerdctlArgs []string - for _, arg := range args { - if arg == "--debug" { + for i, arg := range args { + // parsing environment values from the command line may pre-fetch and + // consume the next argument; this loop variable will skip these pre-consumed + // entries from the command line + if skip { + skip = false + continue + } + switch { + case arg == "--debug": // explicitly setting log level to avoid `--debug` flag being interpreted as nerdctl command nc.logger.SetLevel(flog.Debug) - continue + case argIsEnv(arg): + shouldSkip, addEnvs := handleEnv(arg, args[i+1]) + skip = shouldSkip + envs = append(envs, addEnvs...) + case strings.HasPrefix(arg, "--env-file"): + shouldSkip, addEnvs, err := handleEnvFile(arg, args[i+1]) + if err != nil { + return err + } + skip = shouldSkip + fileEnvs = append(fileEnvs, addEnvs...) + default: + nerdctlArgs = append(nerdctlArgs, arg) } - nerdctlArgs = append(nerdctlArgs, arg) } + // to handle environment variables properly, we add all entries found via + // env-file includes to the map first and then all command line environment + // flags, making sure that command line overrides environment file options, + // and that later command line flags override earlier ones + for _, e := range fileEnvs { + evar, eval, _ := strings.Cut(e, "=") + envVars[evar] = eval + } + for _, e := range envs { + evar, eval, _ := strings.Cut(e, "=") + envVars[evar] = eval + } + + var finalArgs []string + for key, val := range envVars { + finalArgs = append(finalArgs, "-e", fmt.Sprintf("%s=%s", key, val)) + } + finalArgs = append(finalArgs, nerdctlArgs...) - limaArgs := append([]string{"shell", limaInstanceName, nerdctlCmdName, cmdName}, nerdctlArgs...) + limaArgs := append([]string{"shell", limaInstanceName, nerdctlCmdName, cmdName}, finalArgs...) if nc.shouldReplaceForHelp(cmdName, args) { return nc.creator.RunWithReplacingStdout([]command.Replacement{{Source: "nerdctl", Target: "finch"}}, limaArgs...) @@ -122,6 +167,93 @@ func (nc *nerdctlCommand) shouldReplaceForHelp(cmdName string, args []string) bo return false } +func argIsEnv(arg string) bool { + if strings.HasPrefix(arg, "-e") || (strings.HasPrefix(arg, "--env") && !strings.HasPrefix(arg, "--env-file")) { + return true + } + return false +} + +func handleEnv(arg, arg2 string) (bool, []string) { + stdlib := system.NewStdLib() + var ( + envs []string + envVar string + skip bool + ) + switch arg { + case "-e", "--env": + skip = true + envVar = arg2 + default: + // flag and value are in the same string + if strings.HasPrefix(arg, "-e") { + envVar = arg[2:] + } else { + // only other case is "--env="; skip that prefix + envVar = arg[6:] + } + } + + if !strings.Contains(envVar, "=") { + // need to lookup value from OS environment, and only set if it exists in the env + if val, ok := stdlib.LookupEnv(envVar); ok { + envs = append(envs, fmt.Sprintf("%s=%s", envVar, val)) + } + } else { + envs = append(envs, envVar) + } + return skip, envs +} + +func handleEnvFile(arg, arg2 string) (bool, []string, error) { + var ( + filename string + envs []string + skip bool + ) + + switch arg { + case "--env-file": + skip = true + filename = arg2 + default: + filename = arg[11:] + } + + file, err := os.Open(filepath.Clean(filename)) + if err != nil { + return false, []string{}, err + } + defer file.Close() //nolint:errcheck,gosec // close of a file in O_RDONLY mode has no gosec issue + + stdlib := system.NewStdLib() + scanner := bufio.NewScanner(file) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if len(line) == 0 { + continue + } + switch { + case strings.HasPrefix(line, "#"): + // ignore comments + continue + case !strings.Contains(line, "="): + // only has the variable name; need to lookup value + if val, ok := stdlib.LookupEnv(line); ok { + envs = append(envs, fmt.Sprintf("%s=%s", line, val)) + } + default: + // contains a name and value + envs = append(envs, line) + } + } + if err := scanner.Err(); err != nil { + return skip, []string{}, err + } + return skip, envs, nil +} + var nerdctlCmds = map[string]string{ "build": "Build an image from Dockerfile", "builder": "Manage builds", diff --git a/pkg/system/stdlib.go b/pkg/system/stdlib.go index 8dde8d2f4..0f9f1d1c7 100644 --- a/pkg/system/stdlib.go +++ b/pkg/system/stdlib.go @@ -38,6 +38,10 @@ func (s *StdLib) Env(key string) string { return os.Getenv(key) } +func (s *StdLib) LookupEnv(key string) (string, bool) { + return os.LookupEnv(key) +} + func (s *StdLib) Stdin() *os.File { return os.Stdin }