From a49df292d00af9e4b8a247c0b24c0faa5d465515 Mon Sep 17 00:00:00 2001 From: Sam Chew Date: Thu, 3 Oct 2024 00:40:47 -0700 Subject: [PATCH] feat: Add BindMount handler for Linux (#1119) Add BindMount handler for Linux Add handleBindMount to remove consistency entity\nRevise comments for linting and clarity Signed-off-by: Sam Chew --- cmd/finch/nerdctl.go | 67 ++++++++++++++++++++ cmd/finch/nerdctl_darwin.go | 67 +------------------- cmd/finch/nerdctl_darwin_test.go | 86 ++++++++++++++++++++++++++ cmd/finch/nerdctl_native.go | 27 ++++++--- cmd/finch/nerdctl_windows.go | 101 +++++++------------------------ 5 files changed, 198 insertions(+), 150 deletions(-) diff --git a/cmd/finch/nerdctl.go b/cmd/finch/nerdctl.go index cde5cf652..996535a18 100644 --- a/cmd/finch/nerdctl.go +++ b/cmd/finch/nerdctl.go @@ -218,6 +218,9 @@ var argHandlerMap = map[string]map[string]argHandler{ "image build": { "--load": handleDockerBuildLoad, }, + "container run": { + "--mount": handleBindMounts, + }, } var cmdFlagSetMap = map[string]map[string]sets.Set[string]{ @@ -339,3 +342,67 @@ func handleDockerCompatInspect(_ NerdctlCommandSystemDeps, fc *config.Finch, cmd return nil } + +// handles the argument & value of --mount option +// +// invokes OS specific path handler for source path of the bind mount +// and removes the consistency key-value entity from value +func handleBindMounts(systemDeps NerdctlCommandSystemDeps, _ *config.Finch, nerdctlCmdArgs []string, index int) error { + prefix := nerdctlCmdArgs[index] + var ( + v string + found bool + before string + ) + if strings.Contains(nerdctlCmdArgs[index], "=") { + before, v, found = strings.Cut(prefix, "=") + } else { + if (index + 1) < len(nerdctlCmdArgs) { + v = nerdctlCmdArgs[index+1] + } else { + return fmt.Errorf("invalid positional parameter for %s", prefix) + } + } + + // eg --mount type=bind,source="$(pwd)"/target,target=/app,readonly + // eg --mount type=bind, source=${pwd}/source_dir, target=/target_dir, consistency=cached + // https://docs.docker.com/storage/bind-mounts/#choose-the--v-or---mount-flag order does not matter, so convert to a map + entries := strings.Split(v, ",") + m := make(map[string]string) + ro := []string{} + for _, e := range entries { + parts := strings.Split(e, "=") + if len(parts) < 2 { + ro = append(ro, parts...) + } else { + m[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) + } + } + // Check if type is bind mount, else return + if m["type"] != "bind" { + return nil + } + + // Remove 'consistency' key-value pair, if present + delete(m, "consistency") + + // Invoke the OS specific path handler + err := handleBindMountPath(systemDeps, m) + if err != nil { + return err + } + + // Convert to string representation + s := mapToString(m) + // append read-only key if present + if len(ro) > 0 { + s = s + "," + strings.Join(ro, ",") + } + if found { + nerdctlCmdArgs[index] = fmt.Sprintf("%s=%s", before, s) + } else { + nerdctlCmdArgs[index+1] = s + } + + return nil +} diff --git a/cmd/finch/nerdctl_darwin.go b/cmd/finch/nerdctl_darwin.go index 4ed4164ed..437ee043e 100644 --- a/cmd/finch/nerdctl_darwin.go +++ b/cmd/finch/nerdctl_darwin.go @@ -13,7 +13,6 @@ import ( "github.com/lima-vm/lima/pkg/networks" "github.com/runfinch/finch/pkg/command" - "github.com/runfinch/finch/pkg/config" "github.com/runfinch/finch/pkg/flog" ) @@ -23,11 +22,7 @@ func convertToWSLPath(_ NerdctlCommandSystemDeps, _ string) (string, error) { var osAliasMap = map[string]string{} -var osArgHandlerMap = map[string]map[string]argHandler{ - "container run": { - "--mount": handleBindMounts, - }, -} +var osArgHandlerMap = map[string]map[string]argHandler{} var osCommandHandlerMap = map[string]commandHandler{} @@ -50,64 +45,8 @@ func resolveIP(host string, logger flog.Logger, _ command.Creator) (string, erro return host, nil } -// removes the consistency key-value entity from --mount. -func handleBindMounts(_ NerdctlCommandSystemDeps, _ *config.Finch, nerdctlCmdArgs []string, index int) error { - prefix := nerdctlCmdArgs[index] - var ( - v string - found bool - before string - ) - if strings.Contains(nerdctlCmdArgs[index], "=") { - before, v, found = strings.Cut(prefix, "=") - } else { - if (index + 1) < len(nerdctlCmdArgs) { - v = nerdctlCmdArgs[index+1] - } else { - return fmt.Errorf("invalid positional parameter for %s", prefix) - } - } - - // This is where the 'consistency=cached' strings should be removed.... - // "consistency will be one of the keys in the following map" - - // eg --mount type=bind,source="$(pwd)"/target,target=/app,readonly - // eg --mount type=bind, - // source=/Users/stchew/projs/arbtest_devcontainers_extensions, - // target=/workspaces/arbtest_devcontainers_extensions, - // consistency=cached - // https://docs.docker.com/storage/bind-mounts/#choose-the--v-or---mount-flag order does not matter, so convert to a map - entries := strings.Split(v, ",") - m := make(map[string]string) - ro := []string{} - for _, e := range entries { - parts := strings.Split(e, "=") - if len(parts) < 2 { - ro = append(ro, parts...) - } else { - m[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) - } - } - // Check if type is bind mount, else return - if m["type"] != "bind" { - return nil - } - - // Remove 'consistency' key-value pair - delete(m, "consistency") - - // Convert to string representation - s := mapToString(m) - // append read-only key if present - if len(ro) > 0 { - s = s + "," + strings.Join(ro, ",") - } - if found { - nerdctlCmdArgs[index] = fmt.Sprintf("%s=%s", before, s) - } else { - nerdctlCmdArgs[index+1] = s - } - +func handleBindMountPath(_ NerdctlCommandSystemDeps, _ map[string]string) error { + // Do nothing by default return nil } diff --git a/cmd/finch/nerdctl_darwin_test.go b/cmd/finch/nerdctl_darwin_test.go index fc99f001b..fac4e4b46 100644 --- a/cmd/finch/nerdctl_darwin_test.go +++ b/cmd/finch/nerdctl_darwin_test.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "path/filepath" + "strings" "testing" "github.com/golang/mock/gomock" @@ -1208,6 +1209,36 @@ func TestNerdctlCommand_run(t *testing.T) { c.EXPECT().Run() }, }, + { + name: "bindmount with src and consistency", + cmdName: "run", + fc: &config.Finch{}, + args: []string{"--mount", "type=bind,src=./src,consistency=cached", "alpine:latest"}, + wantErr: nil, + mockSvc: func( + _ *testing.T, + lcc *mocks.NerdctlCmdCreator, + _ *mocks.CommandCreator, + ncsd *mocks.NerdctlCommandSystemDeps, + logger *mocks.Logger, + ctrl *gomock.Controller, + _ 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("AWS_ACCESS_KEY_ID").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SECRET_ACCESS_KEY").Return("", false) + ncsd.EXPECT().LookupEnv("AWS_SESSION_TOKEN").Return("", false) + ncsd.EXPECT().LookupEnv("COSIGN_PASSWORD").Return("", false) + ncsd.EXPECT().LookupEnv("COMPOSE_FILE").Return("", false) + c := mocks.NewCommand(ctrl) + lcc.EXPECT().Create("shell", limaInstanceName, "sudo", "-E", nerdctlCmdName, "container", "run", + "--mount", ContainsMultipleStrs([]string{"bind", "type", "!consistency"}), "alpine:latest").Return(c) + c.EXPECT().Run() + }, + }, } for _, tc := range testCases { @@ -1664,3 +1695,58 @@ func TestNerdctlCommand_run_miscCommand(t *testing.T) { }) } } + +type ContainsSubstring struct { + substr string +} + +func (m *ContainsSubstring) Matches(x interface{}) bool { + s, ok := x.(string) + if !ok { + return false + } + return strings.Contains(s, m.substr) +} + +func (m *ContainsSubstring) String() string { + return fmt.Sprintf("contains substring %q", m.substr) +} + +func ContainsStr(substr string) gomock.Matcher { + return &ContainsSubstring{substr: substr} +} + +type ContainsMultipleSubstrings struct { + substrs []string +} + +func (m *ContainsMultipleSubstrings) Matches(x interface{}) bool { + s, ok := x.(string) + if !ok { + return false + } + // Check if each substrings is present in the input string + // except strings that start with "!" + passTest := true + for _, substr := range m.substrs { + if substr[0] == '!' { + if strings.Contains(s, substr[1:]) { + passTest = false + } + continue + } + + if !strings.Contains(s, substr) { + passTest = false + } + } + return passTest +} + +func (m *ContainsMultipleSubstrings) String() string { + return fmt.Sprintf("contains substrings %q", m.substrs) +} + +func ContainsMultipleStrs(substrs []string) gomock.Matcher { + return &ContainsMultipleSubstrings{substrs: substrs} +} diff --git a/cmd/finch/nerdctl_native.go b/cmd/finch/nerdctl_native.go index 784043741..d1b910c38 100644 --- a/cmd/finch/nerdctl_native.go +++ b/cmd/finch/nerdctl_native.go @@ -16,7 +16,6 @@ import ( ) func (nc *nerdctlCommand) run(cmdName string, args []string) error { - var ( hasCmdHandler, hasArgHandler bool cmdHandler commandHandler @@ -70,14 +69,12 @@ func (nc *nerdctlCommand) run(cmdName string, args []string) error { } } - // TODO: Extra manipulation if overwriting cmdName with alias - //splitName := strings.Split(cmdName, " ") - //cmdArgs := append([]string{splitName[0]}, splitName[1:]...) - //cmdArgs = append(cmdArgs, args...) - - cmdArgs := append([]string{cmdName}, args...) + // Extra manipulation for cases that overwrite cmdName with alias + splitName := strings.Split(cmdName, " ") + cmdArgs := append([]string{splitName[0]}, splitName[1:]...) + cmdArgs = append(cmdArgs, args...) - if nc.shouldReplaceForHelp(cmdName, args) { + if nc.shouldReplaceForHelp(splitName[0], args) { return nc.ncc.RunWithReplacingStdout( []command.Replacement{{Source: "nerdctl", Target: "finch"}}, cmdArgs..., @@ -92,3 +89,17 @@ var osAliasMap = map[string]string{} var osArgHandlerMap = map[string]map[string]argHandler{} var osCommandHandlerMap = map[string]commandHandler{} + +func mapToString(m map[string]string) string { + var parts []string + for k, v := range m { + part := fmt.Sprintf("%s=%s", k, v) + parts = append(parts, part) + } + return strings.Join(parts, ",") +} + +func handleBindMountPath(_ NerdctlCommandSystemDeps, _ map[string]string) error { + // Do nothing by default + return nil +} diff --git a/cmd/finch/nerdctl_windows.go b/cmd/finch/nerdctl_windows.go index 1d55085b1..8e5bbf842 100644 --- a/cmd/finch/nerdctl_windows.go +++ b/cmd/finch/nerdctl_windows.go @@ -49,7 +49,6 @@ var osArgHandlerMap = map[string]map[string]argHandler{ "--cidfile": handleFilePath, "-v": handleVolume, "--volume": handleVolume, - "--mount": handleBindMounts, }, "compose": { "--file": handleFilePath, @@ -169,83 +168,6 @@ func handleVolume(systemDeps NerdctlCommandSystemDeps, _ *config.Finch, nerdctlC return nil } -// translates source path of the bind mount to wslpath for --mount option. -// -// and removes the consistency key-value entity from --mount -func handleBindMounts(systemDeps NerdctlCommandSystemDeps, _ *config.Finch, nerdctlCmdArgs []string, index int) error { - prefix := nerdctlCmdArgs[index] - var ( - v string - found bool - before string - ) - if strings.Contains(nerdctlCmdArgs[index], "=") { - before, v, found = strings.Cut(prefix, "=") - } else { - if (index + 1) < len(nerdctlCmdArgs) { - v = nerdctlCmdArgs[index+1] - } else { - return fmt.Errorf("invalid positional parameter for %s", prefix) - } - } - - // e.g. --mount type=bind,source="$(pwd)"/target,target=/app,readonly - // e.g. --mount type=bind,source=/Users/stchew/projs/arbtest_devcontainers_extensions, - // target=/workspaces/arbtest_devcontainers_extensions,consistency=cached - // https://docs.docker.com/storage/bind-mounts/#choose-the--v-or---mount-flag order does not matter, so convert to a map - entries := strings.Split(v, ",") - m := make(map[string]string) - ro := []string{} - for _, e := range entries { - parts := strings.Split(e, "=") - if len(parts) < 2 { - ro = append(ro, parts...) - } else { - m[strings.TrimSpace(parts[0])] = strings.TrimSpace(parts[1]) - } - } - // Check if type is bind mount, else return - if m["type"] != "bind" { - return nil - } - - // Remove 'consistency' key-value pair - delete(m, "consistency") - - // Handle src/source path - var k string - path, ok := m["src"] - if !ok { - path, ok = m["source"] - k = "source" - } else { - k = "src" - } - // If there is no src or source or not a windows path, do nothing, let nerdctl handle error - if !ok || !strings.Contains(path, `\`) { - return nil - } - wslPath, err := convertToWSLPath(systemDeps, path) - if err != nil { - return err - } - m[k] = wslPath - - // Convert to string representation - s := mapToString(m) - // append read-only key if present - if len(ro) > 0 { - s = s + "," + strings.Join(ro, ",") - } - if found { - nerdctlCmdArgs[index] = fmt.Sprintf("%s=%s", before, s) - } else { - nerdctlCmdArgs[index+1] = s - } - - return nil -} - // handles --output/-o for build command. func handleOutputOption(systemDeps NerdctlCommandSystemDeps, _ *config.Finch, nerdctlCmdArgs []string, index int) error { prefix := nerdctlCmdArgs[index] @@ -386,6 +308,29 @@ func imageBuildHandler(systemDeps NerdctlCommandSystemDeps, _ *config.Finch, _ * return nil } +func handleBindMountPath(systemDeps NerdctlCommandSystemDeps, m map[string]string) error { + // Handle src/source path + var k string + path, ok := m["src"] + if !ok { + path, ok = m["source"] + k = "source" + } else { + k = "src" + } + // If there is no src or source or not a windows path, do nothing, let nerdctl handle error + if !ok || !strings.Contains(path, `\`) { + return nil + } + wslPath, err := convertToWSLPath(systemDeps, path) + if err != nil { + return err + } + + m[k] = wslPath + return nil +} + func mapToString(m map[string]string) string { var parts []string for k, v := range m {