From 66f0e6d53688b5f957295add102cd737df3e86a6 Mon Sep 17 00:00:00 2001 From: Ziwen Ning Date: Thu, 25 May 2023 22:31:19 -0700 Subject: [PATCH] fix: sort volume args in DOCKER_COMPAT mode Signed-off-by: Ziwen Ning --- cmd/finch/nerdctl.go | 71 +++++++++++++++++++++++++++++++++++++++ cmd/finch/nerdctl_test.go | 47 ++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) diff --git a/cmd/finch/nerdctl.go b/cmd/finch/nerdctl.go index 6fd76edc4..0056cae29 100644 --- a/cmd/finch/nerdctl.go +++ b/cmd/finch/nerdctl.go @@ -6,7 +6,9 @@ package main import ( "bufio" "fmt" + "os" "path/filepath" + "sort" "strings" dockerops "github.com/docker/docker/opts" @@ -85,8 +87,11 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { var ( nerdctlArgs, envs, fileEnvs []string skip bool + volumes []string ) + dockerCompat := isDockerCompatEnvSet(nc.systemDeps) + 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 @@ -121,10 +126,27 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { arg = fmt.Sprintf("%s%s", arg[0:11], resolvedIP) } nerdctlArgs = append(nerdctlArgs, arg) + case strings.HasPrefix(arg, "-v") || strings.HasPrefix(arg, "--volume"): + if dockerCompat { + volume, shouldSkip := getVolume(arg, args[i+1]) + volumes = append(volumes, volume) + skip = shouldSkip + } else { + nerdctlArgs = append(nerdctlArgs, arg) + } default: nerdctlArgs = append(nerdctlArgs, arg) } } + + if dockerCompat { + // Reverse sort to append to beginning. + volumes = reverseSortVolumes(volumes) + for _, vol := range volumes { + nerdctlArgs = append([]string{"-v", vol}, nerdctlArgs...) + } + } + // 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, @@ -300,6 +322,17 @@ func handleEnvFile(fs afero.Fs, systemDeps NerdctlCommandSystemDeps, arg, arg2 s return skip, envs, nil } +func getVolume(arg, nextArg string) (string, bool) { + if strings.Contains(arg, "=") { + if strings.HasPrefix(arg, "--volume=") { + return arg[9:], false + } else if strings.HasPrefix(arg, "-v=") { + return arg[3:], false + } + } + return nextArg, true +} + func resolveIP(host string, logger flog.Logger) string { parts := strings.SplitN(host, ":", 2) // If the IP Address is a string called "host-gateway", replace this value with the IP address that can be used to @@ -313,6 +346,44 @@ func resolveIP(host string, logger flog.Logger) string { return host } +type volume struct { + original string + destination string +} + +func reverseSortVolumes(volumes []string) []string { + volumeSlices := make([]volume, 0) + for _, vol := range volumes { + splitVol := strings.Split(vol, ":") + if len(splitVol) >= 2 { + volumeSlices = append(volumeSlices, volume{ + original: vol, + destination: splitVol[1], + }) + } + } + sort.Slice(volumeSlices, func(i, j int) bool { + // Consistent with the less function in Docker. + // https://github.com/moby/moby/blob/0db417451313474133c5ed62bbf95e2d3c92444d/daemon/volumes.go#L45 + c1 := strings.Count(filepath.Clean(volumeSlices[i].destination), string(os.PathSeparator)) + c2 := strings.Count(filepath.Clean(volumeSlices[j].destination), string(os.PathSeparator)) + return c1 > c2 + }) + sortedVolumes := make([]string, 0) + for _, vol := range volumeSlices { + sortedVolumes = append(sortedVolumes, vol.original) + } + return sortedVolumes +} + +// isDockerCompatEnvSet checks if the FINCH_DOCKER_COMPAT environment variable is set, so that callers can +// modify behavior to match docker instead of nerdctl. +// Intended to be used for temporary workarounds until changes are merged upstream. +func isDockerCompatEnvSet(systemDeps NerdctlCommandSystemDeps) bool { + _, s := systemDeps.LookupEnv("FINCH_DOCKER_COMPAT") + return s +} + var nerdctlCmds = map[string]string{ "build": "Build an image from Dockerfile", "builder": "Manage builds", diff --git a/cmd/finch/nerdctl_test.go b/cmd/finch/nerdctl_test.go index f218ee05c..33837cb6b 100644 --- a/cmd/finch/nerdctl_test.go +++ b/cmd/finch/nerdctl_test.go @@ -53,6 +53,7 @@ func TestNerdctlCommand_runAdaptor(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "info").Return(c) @@ -104,6 +105,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "build", "-t", "demo", ".").Return(c) @@ -205,6 +207,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) logger.EXPECT().SetLevel(flog.Debug) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) @@ -229,6 +232,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) c := mocks.NewCommand(ctrl) ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("ARG3") @@ -255,6 +259,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) c := mocks.NewCommand(ctrl) ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("ARG3").Return("val3", true) @@ -284,6 +289,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) c := mocks.NewCommand(ctrl) ncsd.EXPECT().LookupEnv("ARG2") ncsd.EXPECT().LookupEnv("NOTSETARG") @@ -314,6 +320,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) c := mocks.NewCommand(ctrl) ncsd.EXPECT().LookupEnv("ARG2").Return("val2", true) ncsd.EXPECT().LookupEnv("NOTSETARG") @@ -341,6 +348,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) }, }, { @@ -360,6 +368,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name") ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) @@ -385,6 +394,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", @@ -409,6 +419,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", @@ -433,6 +444,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) logger.EXPECT().Debugf(`Resolving special IP "host-gateway" to %q for host %q`, "192.168.5.2", "name") ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) @@ -458,6 +470,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", @@ -465,6 +478,35 @@ func TestNerdctlCommand_run(t *testing.T) { c.EXPECT().Run() }, }, + { + name: "with multiple nested volumes", + cmdName: "run", + args: []string{ + "--rm", "-v", "/tmp:/tmp1/tmp2:rro", "--volume", "/tmp:/tmp1:rprivate,rro", "-v=/tmp:/:rro", + "--volume=/tmp:/tmp1/tmp3/tmp4:rshared", "alpine:latest", + }, + wantErr: nil, + mockSvc: func( + t *testing.T, + lcc *mocks.LimaCmdCreator, + ncsd *mocks.NerdctlCommandSystemDeps, + logger *mocks.Logger, + ctrl *gomock.Controller, + fs afero.Fs, + ) { + getVMStatusC := mocks.NewCommand(ctrl) + lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) + getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) + logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("1", true) + ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) + c := mocks.NewCommand(ctrl) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "run", + "-v", "/tmp:/:rro", "-v", "/tmp:/tmp1:rprivate,rro", "-v", "/tmp:/tmp1/tmp2:rro", + "-v", "/tmp:/tmp1/tmp3/tmp4:rshared", "--rm", "alpine:latest").Return(c) + c.EXPECT().Run() + }, + }, { name: "with --help flag", cmdName: "pull", @@ -482,6 +524,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) lcc.EXPECT().RunWithReplacingStdout( testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help").Return(nil) @@ -504,6 +547,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) lcc.EXPECT().RunWithReplacingStdout( testStdoutRs, "shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "pull", "test:tag", "--help"). @@ -527,6 +571,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("test", true) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName, @@ -551,6 +596,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("test", true) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test", nerdctlCmdName, @@ -575,6 +621,7 @@ func TestNerdctlCommand_run(t *testing.T) { lcc.EXPECT().CreateWithoutStdio("ls", "-f", "{{.Status}}", limaInstanceName).Return(getVMStatusC) getVMStatusC.EXPECT().Output().Return([]byte("Running"), nil) logger.EXPECT().Debugf("Status of virtual machine: %s", "Running") + ncsd.EXPECT().LookupEnv("FINCH_DOCKER_COMPAT").Return("", false) ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("test", true) c := mocks.NewCommand(ctrl) lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", "COSIGN_PASSWORD=test",