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 <[email protected]>

cmd/finch/main.go

@@ -125,7 +125,7 @@ func virtualMachineCommands(
 }
 
 func initializeNerdctlCommands(lcc command.LimaCmdCreator, logger flog.Logger) []*cobra.Command {
-	nerdctlCommandCreator := newNerdctlCommandCreator(lcc, logger)
+	nerdctlCommandCreator := newNerdctlCommandCreator(lcc, system.NewStdLib(), logger)
 	var allNerdctlCommands []*cobra.Command
 	for cmdName, cmdDescription := range nerdctlCmds {
 		allNerdctlCommands = append(allNerdctlCommands, nerdctlCommandCreator.create(cmdName, cmdDescription))

cmd/finch/nerdctl.go

@@ -4,27 +4,41 @@
 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"
 
+// NerdctlCommandSystemDeps contains the system dependencies for newNerdctlCommand.
+//
+//go:generate mockgen -copyright_file=../../copyright_header -destination=../../pkg/mocks/nerdctl_cmd_system_deps.go -package=mocks -mock_names NerdctlCommandSystemDeps=NerdctlCommandSystemDeps -source=nerdctl.go NerdctlCommandSystemDeps
+type NerdctlCommandSystemDeps interface {
+	system.EnvChecker
+}
+
 type nerdctlCommandCreator struct {
-	creator command.LimaCmdCreator
-	logger  flog.Logger
+	creator    command.LimaCmdCreator
+	systemDeps NerdctlCommandSystemDeps
+	logger     flog.Logger
 }
 
-func newNerdctlCommandCreator(creator command.LimaCmdCreator, logger flog.Logger) *nerdctlCommandCreator {
-	return &nerdctlCommandCreator{creator: creator, logger: logger}
+func newNerdctlCommandCreator(creator command.LimaCmdCreator, systemDeps NerdctlCommandSystemDeps,
+	logger flog.Logger,
+) *nerdctlCommandCreator {
+	return &nerdctlCommandCreator{creator: creator, systemDeps: systemDeps, logger: logger}
 }
 
 func (ncc *nerdctlCommandCreator) create(cmdName string, cmdDesc string) *cobra.Command {
@@ -35,19 +49,20 @@ func (ncc *nerdctlCommandCreator) create(cmdName string, cmdDesc string) *cobra.
 		// the args passed to nerdctlCommand.run will be empty because
 		// cobra will try to parse `-d alpine` as if alpine is the value of the `-d` flag.
 		DisableFlagParsing: true,
-		RunE:               newNerdctlCommand(ncc.creator, ncc.logger).runAdapter,
+		RunE:               newNerdctlCommand(ncc.creator, ncc.systemDeps, ncc.logger).runAdapter,
 	}
 
 	return command
 }
 
 type nerdctlCommand struct {
-	creator command.LimaCmdCreator
-	logger  flog.Logger
+	creator    command.LimaCmdCreator
+	systemDeps NerdctlCommandSystemDeps
+	logger     flog.Logger
 }
 
-func newNerdctlCommand(creator command.LimaCmdCreator, logger flog.Logger) *nerdctlCommand {
-	return &nerdctlCommand{creator: creator, logger: logger}
+func newNerdctlCommand(creator command.LimaCmdCreator, systemDeps NerdctlCommandSystemDeps, logger flog.Logger) *nerdctlCommand {
+	return &nerdctlCommand{creator: creator, systemDeps: systemDeps, logger: logger}
 }
 
 func (nc *nerdctlCommand) runAdapter(cmd *cobra.Command, args []string) error {
@@ -59,18 +74,63 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
 	if err != nil {
 		return err
 	}
+	var (
+		nerdctlArgs, envs, fileEnvs []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, addEnv := handleEnv(nc.systemDeps, arg, args[i+1])
+			skip = shouldSkip
+			if addEnv != "" {
+				envs = append(envs, addEnv)
+			}
+
+		case strings.HasPrefix(arg, "--env-file"):
+			shouldSkip, addEnvs, err := handleEnvFile(nc.systemDeps, 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
+	envVars := make(map[string]string)
 
-	limaArgs := append([]string{"shell", limaInstanceName, nerdctlCmdName, cmdName}, nerdctlArgs...)
+	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}, finalArgs...)
 
 	if nc.shouldReplaceForHelp(cmdName, args) {
 		return nc.creator.RunWithReplacingStdout([]command.Replacement{{Source: "nerdctl", Target: "finch"}}, limaArgs...)
@@ -122,6 +182,92 @@ 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(systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool, string) {
+	var (
+		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, "=") {
+		return skip, envVar
+	}
+	// if no value was provided we need to check the OS environment
+	// for a value and only set if it exists in the current env
+	if val, ok := systemDeps.LookupEnv(envVar); ok {
+		return skip, fmt.Sprintf("%s=%s", envVar, val)
+	}
+	// no value found; do not set the variable in the env
+	return skip, ""
+}
+
+func handleEnvFile(systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool, []string, error) {
+	var (
+		filename 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
+
+	scanner := bufio.NewScanner(file)
+
+	var envs []string
+	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 := systemDeps.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",