scp: add option types + *WithOpts functions

Priot to this commit, many scp functions existed without option structs, which would make extending functionality (adding new options) impossible without breaking changes, or without adding redundant wrapper functions.

This commit adds in new option types for various scp related functions. It also adds relevant *WithOpts counterparts to existing functions, which use these new option types.

This commit also adds in an experimental `ScpWithOpts` function to image engine implementations, which should eventually replace the existing `ImageEngine.Scp()` function. However, this replacement can't be done without breaking the `ImageEngine` interface, so it should happen at a later major version upgrade.

The commit also renames the existing `ImageScpOptions` entity type to `ScpTransferImageOptions`, and adds an alias mapping the old to the new name to prevent breakage. This is because the previous `ImageScpOptions` was inaccurate, as it is not the actual options for `ImageEngine.Scp()`. The new name is more accurate. Also, if the signature of `ImageEngine.Scp()` is eventually changed to match `ScpWithOpts` as discussed above, `ImageScpBaseOptions` could then be more accurately renamed to `ImageScpOptions`

This commit SHOULD NOT introduce any breaking changes. The breakages could be made in a later commit that would replace the existing functions with the *WithOpts counterparts.

Signed-off-by: Zachary Hanham <z.hanham00@gmail.com>

pkg/api/handlers/libpod/images.go

@@ -713,18 +713,22 @@ func ImageScp(w http.ResponseWriter, r *http.Request) {
 
 	sourceArg := utils.GetName(r)
 
-	rep, source, dest, _, err := domainUtils.ExecuteTransfer(sourceArg, query.Destination, []string{}, query.Quiet, ssh.GolangMode)
+	opts := entities.ScpExecuteTransferOptions{}
+	opts.ParentFlags = []string{}
+	opts.Quiet = query.Quiet
+	opts.SshMode = ssh.GolangMode
+	report, err := domainUtils.ExecuteTransferWithOpts(sourceArg, query.Destination, opts)
 	if err != nil {
 		utils.Error(w, http.StatusInternalServerError, err)
 		return
 	}
 
-	if source != nil || dest != nil {
+	if report.Source != nil || report.Dest != nil {
 		utils.Error(w, http.StatusBadRequest, fmt.Errorf("cannot use the user transfer function on the remote client: %w", define.ErrInvalidArg))
 		return
 	}
 
-	utils.WriteResponse(w, http.StatusOK, &reports.ScpReport{Id: rep.Names[0]})
+	utils.WriteResponse(w, http.StatusOK, &reports.ScpReport{Id: report.LoadReport.Names[0]})
 }
 
 // Resolve the passed (short) name to one more candidates it may resolve to.

pkg/domain/entities/images.go

@@ -305,22 +305,14 @@ type ImageSaveOptions struct {
 	SignaturePolicy string
 }
 
-// ImageScpOptions provide options for securely copying images to and from a remote host
-type ImageScpOptions struct {
-	// Remote determines if this entity is operating on a remote machine
-	Remote bool `json:"remote,omitempty"`
-	// File is the input/output file for the save and load Operation
-	File string `json:"file,omitempty"`
-	// Quiet Determines if the save and load operation will be done quietly
-	Quiet bool `json:"quiet,omitempty"`
-	// Image is the image the user is providing to save and load
-	Image string `json:"image,omitempty"`
-	// User is used in conjunction with Transfer to determine if a valid user was given to save from/load into
-	User string `json:"user,omitempty"`
-	// Tag is the name to be used for the image on the destination
-	Tag string `json:"tag,omitempty"`
+// ImageScpBaseOptions provides options for ImageEngine.ScpWithOpts()
+type ImageScpBaseOptions struct {
+	ScpExecuteTransferOptions
 }
 
+// ImageScpReport provides results from ImageEngine.ScpWithOpts()
+type ImageScpReport struct{}
+
 // ImageScpConnections provides the ssh related information used in remote image transfer
 type ImageScpConnections struct {
 	// Connections holds the raw string values for connections (ssh or unix)

pkg/domain/entities/scp.go

@@ -0,0 +1,93 @@
+package entities
+
+import (
+	"net/url"
+
+	"github.com/containers/common/pkg/ssh"
+)
+
+// ScpTransferImageOptions provide options for securely copying images to and from a remote host
+type ScpTransferImageOptions struct {
+	// Remote determines if this entity is operating on a remote machine
+	Remote bool `json:"remote,omitempty"`
+	// File is the input/output file for the save and load Operation
+	File string `json:"file,omitempty"`
+	// Quiet Determines if the save and load operation will be done quietly
+	Quiet bool `json:"quiet,omitempty"`
+	// Image is the image the user is providing to save and load
+	Image string `json:"image,omitempty"`
+	// User is used in conjunction with Transfer to determine if a valid user was given to save from/load into
+	User string `json:"user,omitempty"`
+	// Tag is the name to be used for the image on the destination
+	Tag string `json:"tag,omitempty"`
+}
+
+// Deprecated: Use ScpTransferImageOptions
+type ImageScpOptions = ScpTransferImageOptions
+
+type ScpLoadReport = ImageLoadReport
+
+type ScpExecuteTransferOptions struct {
+	// ParentFlags are the arguments to apply to the parent podman command when called via ssh
+	ParentFlags []string
+	// Quiet Determines if the save and load operation will be done quietly
+	Quiet bool
+	// SshMode is the specified ssh.EngineMode which should be used
+	SshMode ssh.EngineMode
+}
+
+type ScpExecuteTransferReport struct {
+	// LoadReport provides results from calling podman load
+	LoadReport *ScpLoadReport
+	// Source contains data relating to the source of the image to transfer
+	Source *ScpTransferImageOptions
+	// Dest contains data relating to the destination of the image to transfer
+	Dest *ScpTransferImageOptions
+	// ParentFlags are the arguments to apply to the parent podman command when called via ssh
+	ParentFlags []string
+}
+
+type ScpLoadToRemoteOptions struct {
+	// Dest contains data relating to the destination of the image to transfer
+	Dest ScpTransferImageOptions
+	// LocalFile is a path to a local file containing saved image data to transfer
+	LocalFile string
+	// Tag is the name of the tag to be given to the loaded image (unused)
+	Tag string
+	// Url points to the remote location for loading to
+	Url *url.URL
+	// Iden is a path to an optional identity file with ssh key
+	Iden string
+	// SshMode is the specified ssh.EngineMode which should be used
+	SshMode ssh.EngineMode
+}
+
+type ScpLoadToRemoteReport struct {
+	// Response contains any additional information from the executed load command
+	Response string
+	// Id is the identifier of the loaded image
+	Id string
+}
+
+type ScpSaveToRemoteOptions struct {
+	Image string
+	// LocalFile is a path to a local file to copy the saved image to
+	LocalFile string
+	// Tag is the name of the tag to be given to the saved image (unused)
+	Tag string
+	// Url points to the remote location for saving from
+	Url *url.URL
+	// Iden is a path to an optional identity file with ssh key
+	Iden string
+	// SshMode is the specified ssh.EngineMode which should be used
+	SshMode ssh.EngineMode
+}
+
+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/infra/abi/images.go

@@ -773,20 +773,29 @@ func (ir *ImageEngine) Sign(ctx context.Context, names []string, options entitie
 }
 
 func (ir *ImageEngine) Scp(ctx context.Context, src, dst string, parentFlags []string, quiet bool, sshMode ssh.EngineMode) error {
-	rep, source, dest, flags, err := domainUtils.ExecuteTransfer(src, dst, parentFlags, quiet, sshMode)
+	opts := entities.ImageScpBaseOptions{}
+	opts.ParentFlags = parentFlags
+	opts.Quiet = quiet
+	opts.SshMode = sshMode
+	_, err := ir.ScpWithOpts(ctx, src, dst, opts)
+	return err
+}
+
+func (ir *ImageEngine) ScpWithOpts(ctx context.Context, src, dst string, opts entities.ImageScpBaseOptions) (*entities.ImageScpReport, error) {
+	report, err := domainUtils.ExecuteTransferWithOpts(src, dst, opts.ScpExecuteTransferOptions)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	if (rep == nil && err == nil) && (source != nil && dest != nil) { // we need to execute the transfer
-		err := Transfer(ctx, *source, *dest, flags)
+	if (report.LoadReport == nil && err == nil) && (report.Source != nil && report.Dest != nil) { // we need to execute the transfer
+		err := Transfer(ctx, *report.Source, *report.Dest, report.ParentFlags)
 		if err != nil {
-			return err
+			return nil, err
 		}
 	}
-	return nil
+	return &entities.ImageScpReport{}, nil
 }
 
-func Transfer(ctx context.Context, source entities.ImageScpOptions, dest entities.ImageScpOptions, parentFlags []string) error {
+func Transfer(ctx context.Context, source entities.ScpTransferImageOptions, dest entities.ScpTransferImageOptions, parentFlags []string) error {
 	if source.User == "" {
 		return fmt.Errorf("you must define a user when transferring from root to rootless storage: %w", define.ErrInvalidArg)
 	}
@@ -801,7 +810,7 @@ func Transfer(ctx context.Context, source entities.ImageScpOptions, dest entitie
 }
 
 // TransferRootless creates new podman processes using exec.Command and sudo, transferring images between the given source and destination users
-func transferRootless(source entities.ImageScpOptions, dest entities.ImageScpOptions, podman string, parentFlags []string) error {
+func transferRootless(source entities.ScpTransferImageOptions, dest entities.ScpTransferImageOptions, podman string, parentFlags []string) error {
 	var cmdSave *exec.Cmd
 	saveCommand, loadCommand := parentFlags, parentFlags
 	saveCommand = append(saveCommand, []string{"save"}...)
@@ -842,7 +851,7 @@ func transferRootless(source entities.ImageScpOptions, dest entities.ImageScpOpt
 }
 
 // transferRootful creates new podman processes using exec.Command and a new uid/gid alongside a cleared environment
-func transferRootful(source entities.ImageScpOptions, dest entities.ImageScpOptions, podman string, parentFlags []string) error {
+func transferRootful(source entities.ScpTransferImageOptions, dest entities.ScpTransferImageOptions, podman string, parentFlags []string) error {
 	basicCommand := make([]string, 0, len(parentFlags)+1)
 	basicCommand = append(basicCommand, podman)
 	basicCommand = append(basicCommand, parentFlags...)

pkg/domain/infra/tunnel/images.go

@@ -416,21 +416,30 @@ func (ir *ImageEngine) Sign(ctx context.Context, names []string, options entitie
 }
 
 func (ir *ImageEngine) Scp(ctx context.Context, src, dst string, parentFlags []string, quiet bool, sshMode ssh.EngineMode) error {
+	opts := entities.ImageScpBaseOptions{}
+	opts.ParentFlags = parentFlags
+	opts.Quiet = quiet
+	opts.SshMode = sshMode
+	_, err := ir.ScpWithOpts(ctx, src, dst, opts)
+	return err
+}
+
+func (ir *ImageEngine) ScpWithOpts(ctx context.Context, src, dst string, opts entities.ImageScpBaseOptions) (*entities.ImageScpReport, error) {
 	options := new(images.ScpOptions)
 
 	var destination *string
 	if len(dst) > 1 {
 		destination = &dst
 	}
-	options.Quiet = &quiet
+	options.Quiet = &opts.Quiet
 	options.Destination = destination
 
 	rep, err := images.Scp(ir.ClientCtx, &src, destination, *options)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	fmt.Println("Loaded Image(s):", rep.Id)
 
-	return nil
+	return &entities.ImageScpReport{}, nil
 }