Merge pull request #28188 from mtrmac/sprintf-split

Don’t use `strings.Split(fmt.Sprintf("--a b …", …), " ")`

pkg/domain/entities/scp.go

@@ -88,10 +88,3 @@ type ScpSaveToRemoteOptions struct {
 }
 
 type ScpSaveToRemoteReport struct{}
-
-type ScpCreateCommandsOptions struct {
-	// ParentFlags are the arguments to apply to the parent podman command when called via ssh
-	ParentFlags []string
-	// Podman is the path to the local podman executable
-	Podman string
-}

pkg/domain/utils/scp.go

@@ -88,11 +88,6 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) (
 		return nil, err
 	}
 
-	createCommandOpts := entities.ScpCreateCommandsOptions{}
-	createCommandOpts.ParentFlags = opts.ParentFlags
-	createCommandOpts.Podman = podman
-	saveCmd, loadCmd := CreateCommands(source, dest, createCommandOpts)
-
 	switch {
 	case source.Remote: // if we want to load FROM the remote, dest can either be local or remote in this case
 		saveToRemoteOpts := entities.ScpSaveToRemoteOptions{}
@@ -126,6 +121,13 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) (
 			}
 			break
 		}
+		loadCmd := []string{podman}
+		loadCmd = append(loadCmd, opts.ParentFlags...)
+		loadCmd = append(loadCmd, "load")
+		if source.Quiet {
+			loadCmd = append(loadCmd, "-q")
+		}
+		loadCmd = append(loadCmd, "--input", dest.File)
 		id, err := ExecPodman(dest, podman, loadCmd)
 		if err != nil {
 			return nil, err
@@ -134,6 +136,13 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) (
 			loadReport.Names = append(loadReport.Names, id)
 		}
 	case dest.Remote: // remote host load, implies source is local
+		saveCmd := []string{podman}
+		saveCmd = append(saveCmd, opts.ParentFlags...)
+		saveCmd = append(saveCmd, "save")
+		if source.Quiet {
+			saveCmd = append(saveCmd, "-q")
+		}
+		saveCmd = append(saveCmd, "--output", source.File, source.Image)
 		_, err = ExecPodman(dest, podman, saveCmd)
 		if err != nil {
 			return nil, err
@@ -331,23 +340,6 @@ func ExecPodman(dest entities.ScpTransferImageOptions, podman string, command []
 	return "", cmd.Run()
 }
 
-// CreateCommands forms the podman save and load commands used by SCP
-func CreateCommands(source entities.ScpTransferImageOptions, dest entities.ScpTransferImageOptions, opts entities.ScpCreateCommandsOptions) ([]string, []string) {
-	var parentString string
-	quiet := ""
-	if source.Quiet {
-		quiet = "-q "
-	}
-	if len(opts.ParentFlags) > 0 {
-		parentString = strings.Join(opts.ParentFlags, " ") + " " // if there are parent args, an extra space needs to be added
-	} else {
-		parentString = strings.Join(opts.ParentFlags, " ")
-	}
-	loadCmd := strings.Split(fmt.Sprintf("%s %sload %s--input %s", opts.Podman, parentString, quiet, dest.File), " ")
-	saveCmd := strings.Split(fmt.Sprintf("%s %vsave %s--output %s %s", opts.Podman, parentString, quiet, source.File, source.Image), " ")
-	return saveCmd, loadCmd
-}
-
 // parseImageSCPArg returns the valid connection, and source/destination data based off of the information provided by the user
 // arg is a string containing one of the cli arguments returned is a filled out source/destination options structs as well as a connections array and an error if applicable
 func ParseImageSCPArg(arg string) (*entities.ScpTransferImageOptions, []string, error) {

test/e2e/common_test.go

@@ -1314,9 +1314,9 @@ func (p *PodmanTestIntegration) makeOptions(args []string, options PodmanExecOpt
 		return args
 	}
 
-	var debug string
+	podmanOptions := []string{}
 	if _, ok := os.LookupEnv("E2E_DEBUG"); ok {
-		debug = "--log-level=debug --syslog=true "
+		podmanOptions = append(podmanOptions, "--log-level=debug", "--syslog=true")
 	}
 
 	eventsType := "file"
@@ -1324,8 +1324,16 @@ func (p *PodmanTestIntegration) makeOptions(args []string, options PodmanExecOpt
 		eventsType = "none"
 	}
 
-	podmanOptions := strings.Split(fmt.Sprintf("%s--root %s --runroot %s --runtime %s --conmon %s --network-config-dir %s --cgroup-manager %s --tmpdir %s --events-backend %s",
-		debug, p.Root, p.RunRoot, p.OCIRuntime, p.ConmonBinary, p.NetworkConfigDir, p.CgroupManager, p.TmpDir, eventsType), " ")
+	podmanOptions = append(podmanOptions,
+		"--root", p.Root,
+		"--runroot", p.RunRoot,
+		"--runtime", p.OCIRuntime,
+		"--conmon", p.ConmonBinary,
+		"--network-config-dir", p.NetworkConfigDir,
+		"--cgroup-manager", p.CgroupManager,
+		"--tmpdir", p.TmpDir,
+		"--events-backend", eventsType,
+	)
 
 	podmanOptions = append(podmanOptions, strings.Split(p.StorageOptions, " ")...)
 	if !options.NoCache {

test/e2e/libpod_suite_remote_test.go

@@ -122,8 +122,16 @@ func (p *PodmanTestIntegration) StopRemoteService() {
 // getRemoteOptions assembles all the podman main options
 func getRemoteOptions(p *PodmanTestIntegration, args []string) []string {
 	networkDir := p.NetworkConfigDir
-	podmanOptions := strings.Split(fmt.Sprintf("--root %s --runroot %s --runtime %s --conmon %s --network-config-dir %s --cgroup-manager %s --tmpdir %s --events-backend %s",
-		p.Root, p.RunRoot, p.OCIRuntime, p.ConmonBinary, networkDir, p.CgroupManager, p.TmpDir, "file"), " ")
+	podmanOptions := []string{
+		"--root", p.Root,
+		"--runroot", p.RunRoot,
+		"--runtime", p.OCIRuntime,
+		"--conmon", p.ConmonBinary,
+		"--network-config-dir", networkDir,
+		"--cgroup-manager", p.CgroupManager,
+		"--tmpdir", p.TmpDir,
+		"--events-backend", "file",
+	}
 
 	podmanOptions = append(podmanOptions, strings.Split(p.StorageOptions, " ")...)
 	podmanOptions = append(podmanOptions, args...)