Skip to content

Commit

Permalink
refactor: replace os calls with afero.Fs ones in nerdctlCommand (#213)
Browse files Browse the repository at this point in the history
## Summary

PR fixes
#158 (comment).
Besides that, it also adds a new test case, namely `"with --env-file
flag, but the specified file does not exist"`, which becomes easier to
add after the refactoring.

## Notes

`gosec` is removed from `//nolint:errcheck,gosec` because `gosec` checks
things like `os.Open` but not `fs.Open`, where `fs` is of type
`afero.Fs`. Updated the `noling` comment accordingly.

## 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: Hsing-Yu (David) Chen <[email protected]>
  • Loading branch information
davidhsingyuchen authored Feb 10, 2023
1 parent e138f10 commit 79dc6de
Show file tree
Hide file tree
Showing 3 changed files with 184 additions and 92 deletions.
6 changes: 3 additions & 3 deletions cmd/finch/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ var newApp = func(logger flog.Logger, fp path.Finch, fs afero.Fs, fc *config.Fin
)

// append nerdctl commands
allCommands := initializeNerdctlCommands(lcc, logger)
allCommands := initializeNerdctlCommands(lcc, logger, fs)
// append finch specific commands
allCommands = append(allCommands,
newVersionCommand(lcc, logger, stdOut),
Expand Down Expand Up @@ -124,8 +124,8 @@ func virtualMachineCommands(
)
}

func initializeNerdctlCommands(lcc command.LimaCmdCreator, logger flog.Logger) []*cobra.Command {
nerdctlCommandCreator := newNerdctlCommandCreator(lcc, system.NewStdLib(), logger)
func initializeNerdctlCommands(lcc command.LimaCmdCreator, logger flog.Logger, fs afero.Fs) []*cobra.Command {
nerdctlCommandCreator := newNerdctlCommandCreator(lcc, system.NewStdLib(), logger, fs)
var allNerdctlCommands []*cobra.Command
for cmdName, cmdDescription := range nerdctlCmds {
allNerdctlCommands = append(allNerdctlCommands, nerdctlCommandCreator.create(cmdName, cmdDescription))
Expand Down
25 changes: 15 additions & 10 deletions cmd/finch/nerdctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ package main
import (
"bufio"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/spf13/afero"
"github.com/spf13/cobra"

"github.com/runfinch/finch/pkg/command"
Expand All @@ -33,12 +33,16 @@ type nerdctlCommandCreator struct {
creator command.LimaCmdCreator
systemDeps NerdctlCommandSystemDeps
logger flog.Logger
fs afero.Fs
}

func newNerdctlCommandCreator(creator command.LimaCmdCreator, systemDeps NerdctlCommandSystemDeps,
func newNerdctlCommandCreator(
creator command.LimaCmdCreator,
systemDeps NerdctlCommandSystemDeps,
logger flog.Logger,
fs afero.Fs,
) *nerdctlCommandCreator {
return &nerdctlCommandCreator{creator: creator, systemDeps: systemDeps, logger: logger}
return &nerdctlCommandCreator{creator: creator, systemDeps: systemDeps, logger: logger, fs: fs}
}

func (ncc *nerdctlCommandCreator) create(cmdName string, cmdDesc string) *cobra.Command {
Expand All @@ -49,7 +53,7 @@ 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.systemDeps, ncc.logger).runAdapter,
RunE: newNerdctlCommand(ncc.creator, ncc.systemDeps, ncc.logger, ncc.fs).runAdapter,
}

return command
Expand All @@ -59,10 +63,11 @@ type nerdctlCommand struct {
creator command.LimaCmdCreator
systemDeps NerdctlCommandSystemDeps
logger flog.Logger
fs afero.Fs
}

func newNerdctlCommand(creator command.LimaCmdCreator, systemDeps NerdctlCommandSystemDeps, logger flog.Logger) *nerdctlCommand {
return &nerdctlCommand{creator: creator, systemDeps: systemDeps, logger: logger}
func newNerdctlCommand(creator command.LimaCmdCreator, systemDeps NerdctlCommandSystemDeps, logger flog.Logger, fs afero.Fs) *nerdctlCommand {
return &nerdctlCommand{creator: creator, systemDeps: systemDeps, logger: logger, fs: fs}
}

func (nc *nerdctlCommand) runAdapter(cmd *cobra.Command, args []string) error {
Expand Down Expand Up @@ -99,7 +104,7 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error {
}

case strings.HasPrefix(arg, "--env-file"):
shouldSkip, addEnvs, err := handleEnvFile(nc.systemDeps, arg, args[i+1])
shouldSkip, addEnvs, err := handleEnvFile(nc.fs, nc.systemDeps, arg, args[i+1])
if err != nil {
return err
}
Expand Down Expand Up @@ -220,7 +225,7 @@ func handleEnv(systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool, str
return skip, ""
}

func handleEnvFile(systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool, []string, error) {
func handleEnvFile(fs afero.Fs, systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool, []string, error) {
var (
filename string
skip bool
Expand All @@ -234,11 +239,11 @@ func handleEnvFile(systemDeps NerdctlCommandSystemDeps, arg, arg2 string) (bool,
filename = arg[11:]
}

file, err := os.Open(filepath.Clean(filename))
file, err := fs.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
defer file.Close() //nolint:errcheck // We did not write to the file, and the file will be closed when the CLI process exits anyway.

scanner := bufio.NewScanner(file)

Expand Down
Loading

0 comments on commit 79dc6de

Please sign in to comment.