Skip to content

Commit

Permalink
fix: correctly handle local environment value pass-through (runfinch#158
Browse files Browse the repository at this point in the history 
)

The `-e`,`--env` flag as well as values within an `--env-file` may
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]>

Issue #, if available: Fixes #35

- [x] I've reviewed the guidance in CONTRIBUTING.md

#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Signed-off-by: Phil Estes <[email protected]>
  • Loading branch information
@estesp
estesp authored Feb 9, 2023
1 parent ff8a35f commit e138f10
Show file tree
Hide file tree
Showing 8 changed files with 346 additions and 36 deletions.
2 changes: 1 addition & 1 deletion cmd/finch/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
182 changes: 164 additions & 18 deletions cmd/finch/nerdctl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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...)
Expand Down Expand Up @@ -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",
Expand Down
Loading

0 comments on commit e138f10

Please sign in to comment.