From 44dd933638af8edd01101b060363c449bb2c2ca2 Mon Sep 17 00:00:00 2001 From: Jose Diaz-Gonzalez Date: Tue, 13 Feb 2024 01:09:24 -0500 Subject: [PATCH] refactor: remove all calls to common.NewShellCmd At this point, the only usage of go-sh should be by plugin trigger calls. --- plugins/builder/functions.go | 28 ++-- plugins/common/docker.go | 171 +++++++++----------- plugins/common/io.go | 19 +-- plugins/network/functions.go | 18 +-- plugins/network/subcommands.go | 46 ++---- plugins/ps/functions.go | 49 +++--- plugins/registry/subcommands.go | 27 ++-- plugins/repo/repo.go | 24 ++- plugins/repo/subcommands.go | 19 ++- plugins/scheduler-docker-local/functions.go | 40 +++-- 10 files changed, 207 insertions(+), 234 deletions(-) diff --git a/plugins/builder/functions.go b/plugins/builder/functions.go index 488e1e662..f69f8cdee 100644 --- a/plugins/builder/functions.go +++ b/plugins/builder/functions.go @@ -1,8 +1,7 @@ package builder import ( - "bytes" - "errors" + "fmt" "os" "path" "strings" @@ -11,25 +10,18 @@ import ( ) func listImagesByImageRepo(imageRepo string) ([]string, error) { - command := []string{ - common.DockerBin(), - "image", - "ls", - "--quiet", - imageRepo, - } - - var stderr bytes.Buffer - listCmd := common.NewShellCmd(strings.Join(command, " ")) - listCmd.ShowOutput = false - listCmd.Command.Stderr = &stderr - b, err := listCmd.Output() - + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: []string{"image", "ls", "--quiet", imageRepo}, + }) if err != nil { - return []string{}, errors.New(strings.TrimSpace(stderr.String())) + return []string{}, fmt.Errorf("Unable to list images: %w", err) + } + if result.ExitCode != 0 { + return []string{}, fmt.Errorf("Unable to list images: %s", result.StderrContents()) } - output := strings.Split(strings.TrimSpace(string(b[:])), "\n") + output := strings.Split(result.StdoutContents(), "\n") return output, nil } diff --git a/plugins/common/docker.go b/plugins/common/docker.go index def86cd61..3ee17915f 100644 --- a/plugins/common/docker.go +++ b/plugins/common/docker.go @@ -1,8 +1,6 @@ package common import ( - "bytes" - "errors" "fmt" "os" "strings" @@ -108,32 +106,20 @@ func CopyFromImage(appName string, image string, source string, destination stri if err != nil { return fmt.Errorf("Unable to create temporary container: %v", err) } + defer ContainerRemove(containerID) // docker cp exits with status 1 when run as non-root user when it tries to chown the file // after successfully copying the file. Thus, we suppress stderr. // ref: https://github.com/dotcloud/docker/issues/3986 - containerCopyCmd := NewShellCmd(strings.Join([]string{ - DockerBin(), - "container", - "cp", - fmt.Sprintf("%s:%s", containerID, source), - tmpFile.Name(), - }, " ")) - containerCopyCmd.ShowOutput = false - fileCopied := containerCopyCmd.Execute() - - containerRemoveCmd := NewShellCmd(strings.Join([]string{ - DockerBin(), - "container", - "rm", - "--force", - containerID, - }, " ")) - containerRemoveCmd.ShowOutput = false - containerRemoveCmd.Execute() - - if !fileCopied { - return fmt.Errorf("Unable to copy file %s from image", source) + result, err := CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: []string{"container", "cp", fmt.Sprintf("%s:%s", containerID, source), tmpFile.Name()}, + }) + if err != nil { + return fmt.Errorf("Unable to copy file %s from image: %w", source, err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to copy file %s from image: %v", source, result.StderrContents()) } fi, err := os.Stat(tmpFile.Name()) @@ -146,18 +132,13 @@ func CopyFromImage(appName string, image string, source string, destination stri } // workaround when owner is root. seems to only happen when running inside docker - dos2unixCmd := NewShellCmd(strings.Join([]string{ - "dos2unix", - "-l", - "-n", - tmpFile.Name(), - destination, - }, " ")) - dos2unixCmd.ShowOutput = false - dos2unixCmd.Execute() + CallExecCommand(ExecCommandInput{ + Command: "dos2unix", + Args: []string{"-l", "-n", tmpFile.Name(), destination}, + }) // nolint: errcheck // add trailing newline for certain places where file parsing depends on it - result, err := CallExecCommand(ExecCommandInput{ + result, err = CallExecCommand(ExecCommandInput{ Command: "tail", Args: []string{"-c1", destination}, }) @@ -244,25 +225,26 @@ func DockerCleanup(appName string, forceCleanup bool) error { // DockerContainerCreate creates a new container and returns the container ID func DockerContainerCreate(image string, containerCreateArgs []string) (string, error) { - cmd := []string{ - DockerBin(), + args := []string{ "container", "create", } - cmd = append(cmd, containerCreateArgs...) - cmd = append(cmd, image) + args = append(args, containerCreateArgs...) + args = append(args, image) - var stderr bytes.Buffer - containerCreateCmd := NewShellCmd(strings.Join(cmd, " ")) - containerCreateCmd.ShowOutput = false - containerCreateCmd.Command.Stderr = &stderr - b, err := containerCreateCmd.Output() + result, err := CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: args, + }) if err != nil { - return "", errors.New(strings.TrimSpace(stderr.String())) + return "", fmt.Errorf("Unable to create container: %w", err) + } + if result.ExitCode != 0 { + return "", fmt.Errorf("Unable to create container: %s", result.StderrContents()) } - return strings.TrimSpace(string(b[:])), nil + return result.StdoutContents(), nil } // DockerInspect runs an inspect command with a given format against a container or image ID @@ -359,21 +341,22 @@ func RemoveImages(imageIDs []string) error { return nil } - command := []string{ - DockerBin(), + args := []string{ "image", "rm", } - command = append(command, imageIDs...) + args = append(args, imageIDs...) - var stderr bytes.Buffer - rmCmd := NewShellCmd(strings.Join(command, " ")) - rmCmd.ShowOutput = false - rmCmd.Command.Stderr = &stderr - rmCmd.Execute() - if _, err := rmCmd.Output(); err != nil { - return errors.New(strings.TrimSpace(stderr.String())) + result, err := CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: args, + }) + if err != nil { + return fmt.Errorf("Unable to remove images: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to remove images: %s", result.StderrContents()) } return nil @@ -381,15 +364,16 @@ func RemoveImages(imageIDs []string) error { // VerifyImage returns true if docker image exists in local repo func VerifyImage(image string) bool { - imageCmd := NewShellCmd(strings.Join([]string{DockerBin(), "image", "inspect", image}, " ")) - imageCmd.ShowOutput = false - return imageCmd.Execute() + result, err := CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: []string{"image", "inspect", image}, + }) + return err == nil && result.ExitCode == 0 } // DockerFilterContainers returns a slice of container IDs based on the passed in filters func DockerFilterContainers(filters []string) ([]string, error) { - command := []string{ - DockerBin(), + args := []string{ "container", "ls", "--quiet", @@ -397,27 +381,27 @@ func DockerFilterContainers(filters []string) ([]string, error) { } for _, filter := range filters { - command = append(command, "--filter", filter) + args = append(args, "--filter", filter) } - var stderr bytes.Buffer - listCmd := NewShellCmd(strings.Join(command, " ")) - listCmd.ShowOutput = false - listCmd.Command.Stderr = &stderr - b, err := listCmd.Output() - + result, err := CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: args, + }) if err != nil { - return []string{}, errors.New(strings.TrimSpace(stderr.String())) + return []string{}, fmt.Errorf("Unable to filter containers: %w", err) + } + if result.ExitCode != 0 { + return []string{}, fmt.Errorf("Unable to filter containers: %s", result.StderrContents()) } - output := strings.Split(strings.TrimSpace(string(b[:])), "\n") + output := strings.Split(result.StdoutContents(), "\n") return output, nil } // DockerFilterImages returns a slice of image IDs based on the passed in filters func DockerFilterImages(filters []string) ([]string, error) { - command := []string{ - DockerBin(), + args := []string{ "image", "ls", "--quiet", @@ -425,20 +409,21 @@ func DockerFilterImages(filters []string) ([]string, error) { } for _, filter := range filters { - command = append(command, "--filter", filter) + args = append(args, "--filter", filter) } - var stderr bytes.Buffer - listCmd := NewShellCmd(strings.Join(command, " ")) - listCmd.ShowOutput = false - listCmd.Command.Stderr = &stderr - b, err := listCmd.Output() - + result, err := CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: args, + }) if err != nil { - return []string{}, errors.New(strings.TrimSpace(stderr.String())) + return []string{}, fmt.Errorf("Unable to filter images: %w", err) + } + if result.ExitCode != 0 { + return []string{}, fmt.Errorf("Unable to filter images: %s", result.StderrContents()) } - output := strings.Split(strings.TrimSpace(string(b[:])), "\n") + output := strings.Split(result.StdoutContents(), "\n") return output, nil } @@ -455,8 +440,7 @@ func listContainers(status string, appName string) ([]string, error) { } func pruneUnusedImages(appName string) { - command := []string{ - DockerBin(), + args := []string{ "image", "prune", "--all", @@ -465,26 +449,23 @@ func pruneUnusedImages(appName string) { fmt.Sprintf("label=com.dokku.app-name=%v", appName), } - var stderr bytes.Buffer - pruneCmd := NewShellCmd(strings.Join(command, " ")) - pruneCmd.ShowOutput = false - pruneCmd.Command.Stderr = &stderr - pruneCmd.Execute() + CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: args, + }) // nolint: errcheck } // DockerRemoveContainers will call `docker container rm` on the specified containers func DockerRemoveContainers(containerIDs []string) { - command := []string{ - DockerBin(), + args := []string{ "container", "rm", } - command = append(command, containerIDs...) + args = append(args, containerIDs...) - var stderr bytes.Buffer - rmCmd := NewShellCmd(strings.Join(command, " ")) - rmCmd.ShowOutput = false - rmCmd.Command.Stderr = &stderr - rmCmd.Execute() + CallExecCommand(ExecCommandInput{ + Command: DockerBin(), + Args: args, + }) // nolint: errcheck } diff --git a/plugins/common/io.go b/plugins/common/io.go index b39d24dba..5ad0cf33b 100644 --- a/plugins/common/io.go +++ b/plugins/common/io.go @@ -38,15 +38,16 @@ func Copy(src, dst string) error { } // ensure file has the correct line endings - dos2unixCmd := NewShellCmd(strings.Join([]string{ - "dos2unix", - "-l", - "-n", - src, - dst, - }, " ")) - dos2unixCmd.ShowOutput = false - dos2unixCmd.Execute() + result, err := CallExecCommand(ExecCommandInput{ + Command: "dos2unix", + Args: []string{"-l", "-n", src, dst}, + }) + if err != nil { + return fmt.Errorf("Error running dos2unix: %s", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Error running dos2unix: %s", result.StderrContents()) + } // ensure file permissions are correct b, err := os.ReadFile(dst) diff --git a/plugins/network/functions.go b/plugins/network/functions.go index a39b5fd5a..eb9e096c4 100644 --- a/plugins/network/functions.go +++ b/plugins/network/functions.go @@ -1,7 +1,6 @@ package network import ( - "bytes" "errors" "fmt" "strings" @@ -16,7 +15,6 @@ func attachAppToNetwork(containerID string, networkName string, appName string, } cmdParts := []string{ - common.DockerBin(), "network", "connect", } @@ -43,14 +41,16 @@ func attachAppToNetwork(containerID string, networkName string, appName string, cmdParts = append(cmdParts, networkName) cmdParts = append(cmdParts, containerID) - attachCmd := common.NewShellCmd(strings.Join(cmdParts, " ")) - var stderr bytes.Buffer - attachCmd.ShowOutput = false - attachCmd.Command.Stderr = &stderr - _, err := attachCmd.Output() + + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: cmdParts, + }) if err != nil { - err = errors.New(strings.TrimSpace(stderr.String())) - return fmt.Errorf("Unable to attach container to network: %v", err.Error()) + return fmt.Errorf("Unable to attach container to network: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to attach container to network: %s", result.StderrContents()) } return nil diff --git a/plugins/network/subcommands.go b/plugins/network/subcommands.go index 987caf136..cc2865dc0 100644 --- a/plugins/network/subcommands.go +++ b/plugins/network/subcommands.go @@ -1,7 +1,6 @@ package network import ( - "bytes" "errors" "fmt" "os" @@ -13,22 +12,15 @@ import ( // CommandCreate is an alias for "docker network create" func CommandCreate(networkName string) error { common.LogInfo1Quiet(fmt.Sprintf("Creating network %v", networkName)) - createCmd := common.NewShellCmd(strings.Join([]string{ - common.DockerBin(), - "network", - "create", - "--attachable", - "--label", - fmt.Sprintf("com.dokku.network-name=%v", networkName), - networkName, - }, " ")) - var stderr bytes.Buffer - createCmd.ShowOutput = false - createCmd.Command.Stderr = &stderr - _, err := createCmd.Output() - + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: []string{"network", "create", "--attachable", "--label", fmt.Sprintf("com.dokku.network-name=%v", networkName), networkName}, + }) if err != nil { - err = errors.New(strings.TrimSpace(stderr.String())) + return fmt.Errorf("Unable to create network: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to create network: %s", result.StderrContents()) } return err @@ -47,22 +39,18 @@ func CommandDestroy(networkName string, forceDestroy bool) error { } common.LogInfo1Quiet(fmt.Sprintf("Destroying network %v", networkName)) - destroyCmd := common.NewShellCmd(strings.Join([]string{ - common.DockerBin(), - "network", - "rm", - networkName, - }, " ")) - var stderr bytes.Buffer - destroyCmd.ShowOutput = false - destroyCmd.Command.Stderr = &stderr - _, err := destroyCmd.Output() - + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: []string{"network", "rm", networkName}, + }) if err != nil { - err = errors.New(strings.TrimSpace(stderr.String())) + return fmt.Errorf("Unable to destroy network: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to destroy network: %s", result.StderrContents()) } - return err + return nil } // CommandExists checks if a network exists diff --git a/plugins/ps/functions.go b/plugins/ps/functions.go index ca7b52e49..c77475c0f 100644 --- a/plugins/ps/functions.go +++ b/plugins/ps/functions.go @@ -1,7 +1,6 @@ package ps import ( - "bytes" "errors" "fmt" "os" @@ -24,26 +23,18 @@ func getProcfileCommand(procfilePath string, processType string, port int) (stri return "", errors.New("No procfile found") } - shellCmd := common.NewShellCmd(strings.Join([]string{ - "procfile-util", - "show", - "--procfile", - procfilePath, - "--process-type", - processType, - "--default-port", - strconv.Itoa(port), - }, " ")) - var stderr bytes.Buffer - shellCmd.ShowOutput = false - shellCmd.Command.Stderr = &stderr - b, err := shellCmd.Output() - + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: "procfile-util", + Args: []string{"show", "--procfile", procfilePath, "--process-type", processType, "--default-port", strconv.Itoa(port)}, + }) if err != nil { - return "", fmt.Errorf(strings.TrimSpace(stderr.String())) + return "", fmt.Errorf("Error running procfile-util: %s", err) + } + if result.ExitCode != 0 { + return "", fmt.Errorf("Error running procfile-util: %s", result.StderrContents()) } - return strings.TrimSpace(string(b[:])), nil + return result.StdoutContents(), nil } func getProcfilePath(appName string) string { @@ -154,22 +145,18 @@ func parseProcessTuples(processTuples []string) (FormationSlice, error) { func processesInProcfile(procfilePath string) (map[string]bool, error) { processes := map[string]bool{} - shellCmd := common.NewShellCmd(strings.Join([]string{ - "procfile-util", - "list", - "--procfile", - procfilePath, - }, " ")) - var stderr bytes.Buffer - shellCmd.ShowOutput = false - shellCmd.Command.Stderr = &stderr - b, err := shellCmd.Output() - + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: "procfile-util", + Args: []string{"list", "--procfile", procfilePath}, + }) if err != nil { - return processes, fmt.Errorf(strings.TrimSpace(stderr.String())) + return processes, fmt.Errorf("Error listing processes: %s", err) + } + if result.ExitCode != 0 { + return processes, fmt.Errorf("Error listing processes: %s", result.StderrContents()) } - for _, s := range strings.Split(strings.TrimSpace(string(b[:])), "\n") { + for _, s := range strings.Split(result.StdoutContents(), "\n") { processes[s] = true } diff --git a/plugins/registry/subcommands.go b/plugins/registry/subcommands.go index 34cf076f6..d25046bf1 100644 --- a/plugins/registry/subcommands.go +++ b/plugins/registry/subcommands.go @@ -3,6 +3,7 @@ package registry import ( "bytes" "errors" + "fmt" "io" "os" "strings" @@ -35,25 +36,23 @@ func CommandLogin(server string, username string, password string, passwordStdin server = "docker.io" } - command := []string{ - common.DockerBin(), - "login", - "--username", - username, - "--password-stdin", - server, - } - buffer := bytes.Buffer{} buffer.Write([]byte(password + "\n")) - loginCmd := common.NewShellCmd(strings.Join(command, " ")) - loginCmd.Command.Stdin = &buffer - if !loginCmd.Execute() { - return errors.New("Failed to log into registry") + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: []string{"login", "--username", username, "--password-stdin", server}, + Stdin: &buffer, + StreamStderr: true, + }) + if err != nil { + return fmt.Errorf("Unable to run docker login: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to run docker login: %s", result.StderrContents()) } - _, err := common.CallPlugnTrigger(common.PlugnTriggerInput{ + _, err = common.CallPlugnTrigger(common.PlugnTriggerInput{ Trigger: "post-registry-login", Args: []string{server, username}, StreamStdio: true, diff --git a/plugins/repo/repo.go b/plugins/repo/repo.go index 1f459fe1e..8b909156f 100644 --- a/plugins/repo/repo.go +++ b/plugins/repo/repo.go @@ -2,8 +2,6 @@ package repo import ( "fmt" - "os" - "strings" "github.com/dokku/dokku/plugins/common" ) @@ -33,18 +31,16 @@ func PurgeCache(appName string) error { if len(containerIDs) > 0 { common.DockerRemoveContainers(containerIDs) } - purgeCacheCmd := common.NewShellCmd(strings.Join([]string{ - common.DockerBin(), - "volume", - "rm", "-f", fmt.Sprintf("cache-%s", appName)}, " ")) - purgeCacheCmd.ShowOutput = false - purgeCacheCmd.Command.Stderr = os.Stderr - if !purgeCacheCmd.Execute() { - exitCode := 1 - if purgeCacheCmd.ExitError != nil { - exitCode = purgeCacheCmd.ExitError.ExitCode() - } - return &PurgeCacheFailed{exitCode} + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: common.DockerBin(), + Args: []string{"volume", "rm", "-f", fmt.Sprintf("cache-%s", appName)}, + StreamStderr: true, + }) + if err != nil { + return fmt.Errorf("Unable to remove cache volume: %w", err) + } + if result.ExitCode != 0 { + return &PurgeCacheFailed{result.ExitCode} } return nil diff --git a/plugins/repo/subcommands.go b/plugins/repo/subcommands.go index 3f72740ff..376e23a41 100644 --- a/plugins/repo/subcommands.go +++ b/plugins/repo/subcommands.go @@ -1,6 +1,8 @@ package repo import ( + "fmt" + "github.com/dokku/dokku/plugins/common" ) @@ -14,9 +16,20 @@ func CommandGc(appName string) error { cmdEnv := map[string]string{ "GIT_DIR": appRoot, } - gitGcCmd := common.NewShellCmd("git gc --aggressive") - gitGcCmd.Env = cmdEnv - gitGcCmd.Execute() + + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: "git", + Args: []string{"gc", "--aggressive"}, + Env: cmdEnv, + StreamStdio: true, + }) + if err != nil { + return fmt.Errorf("Unable to run git gc: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to run git gc: %s", result.StderrContents()) + } + return nil } diff --git a/plugins/scheduler-docker-local/functions.go b/plugins/scheduler-docker-local/functions.go index cc5395186..5be75642b 100644 --- a/plugins/scheduler-docker-local/functions.go +++ b/plugins/scheduler-docker-local/functions.go @@ -15,17 +15,28 @@ import ( ) func deleteCrontab() error { - command := common.NewShellCmd("sudo /usr/bin/crontab -l -u dokku") - command.ShowOutput = false - if !command.Execute() { - return nil + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: "/usr/bin/crontab", + Args: []string{"-l", "-u", "dokku"}, + Sudo: true, + }) + if err != nil { + return fmt.Errorf("Unable to remove schedule file: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to remove schedule file: %s", result.StderrContents()) } - command = common.NewShellCmd("sudo /usr/bin/crontab -r -u dokku") - command.ShowOutput = false - out, err := command.CombinedOutput() + result, err = common.CallExecCommand(common.ExecCommandInput{ + Command: "/usr/bin/crontab", + Args: []string{"-r", "-u", "dokku"}, + Sudo: true, + }) if err != nil { - return fmt.Errorf("Unable to remove schedule file: %v", string(out)) + return fmt.Errorf("Unable to remove schedule file: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to remove schedule file: %s", result.StderrContents()) } common.LogInfo1("Removed") @@ -139,11 +150,16 @@ func writeCronEntries() error { return fmt.Errorf("Unable to template out schedule file: %v", err) } - command := common.NewShellCmd(fmt.Sprintf("sudo /usr/bin/crontab -u dokku %s", tmpFile.Name())) - command.ShowOutput = false - out, err := command.CombinedOutput() + result, err := common.CallExecCommand(common.ExecCommandInput{ + Command: "/usr/bin/crontab", + Args: []string{"-u", "dokku", tmpFile.Name()}, + Sudo: true, + }) if err != nil { - return fmt.Errorf("Unable to update schedule file: %s", string(out)) + return fmt.Errorf("Unable to update schedule file: %w", err) + } + if result.ExitCode != 0 { + return fmt.Errorf("Unable to update schedule file: %s", result.StderrContents()) } common.LogInfo1("Updated schedule file")