From 011b65956968f831571a0e3f8717d2575653630b Mon Sep 17 00:00:00 2001 From: Scott Callaway Date: Wed, 5 Aug 2026 16:16:30 +0100 Subject: [PATCH 1/7] pkg/domain/utils: tidy SaveToRemote's remote commands Three small things in one place, all groundwork rather than behaviour: The path SaveToRemote gets back from the remote mktemp still carries the trailing newline ssh.Exec hands over with the rest of the raw output. That is harmless while the path is only ever the last thing on a command line, but it is a trap for anything that appends to it. Trim it. The host, identity, port and user were restated in full for every command. State them once and let each command copy the value and add its own argv. Removing a file on the far end had one caller and was about to have more, so give it a name. It also gains -f, since a caller cleaning up after a failure cannot know which of the paths it is removing were created. Lastly, the ssh operations a transfer performs are gathered into one value the exported entry points pass in. Nothing about the options changes: SaveToRemote and LoadToRemote keep taking their options struct, and the body moves to an unexported function taking that same struct plus the operations to run it with. That is what lets a test assert the sequence of remote commands, and what is streamed to them, without a host to run against. Signed-off-by: Scott Callaway --- pkg/domain/utils/scp.go | 62 +++++++++++++++++--- pkg/domain/utils/scp_test.go | 106 +++++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+), 9 deletions(-) diff --git a/pkg/domain/utils/scp.go b/pkg/domain/utils/scp.go index 8cbdd9a479..74e2b192c8 100644 --- a/pkg/domain/utils/scp.go +++ b/pkg/domain/utils/scp.go @@ -2,6 +2,7 @@ package utils import ( "fmt" + "io" "net/url" "os" "os/exec" @@ -249,6 +250,10 @@ func LoginUser(user string) (*exec.Cmd, error) { // and copies the saved image dir over to the remote host and then loads it onto the machine // returns a report containing ssh response string and the id of the loaded image, or an error func LoadToRemote(opts entities.ScpLoadToRemoteOptions) (*entities.ScpLoadToRemoteReport, error) { + return loadToRemote(sshRunner, opts) +} + +func loadToRemote(run remoteRunner, opts entities.ScpLoadToRemoteOptions) (*entities.ScpLoadToRemoteReport, error) { port := 0 urlPort := opts.URL.Port() if urlPort != "" { @@ -265,7 +270,7 @@ func LoadToRemote(opts entities.ScpLoadToRemoteOptions) (*entities.ScpLoadToRemo } defer input.Close() - out, err := ssh.ExecWithInput(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"podman", "image", "load"}}, opts.SSHMode, input) + out, err := run.execWithInput(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"podman", "image", "load"}}, opts.SSHMode, input) if err != nil { return nil, err } @@ -276,7 +281,7 @@ func LoadToRemote(opts entities.ScpLoadToRemoteOptions) (*entities.ScpLoadToRemo outArr := strings.Split(rep, " ") id := outArr[len(outArr)-1] if len(opts.Dest.Tag) > 0 { // tag the remote image using the output ID - _, err := ssh.Exec(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"podman", "image", "tag", id, opts.Dest.Tag}}, opts.SSHMode) + _, err := run.exec(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"podman", "image", "tag", id, opts.Dest.Tag}}, opts.SSHMode) if err != nil { return nil, err } @@ -284,10 +289,45 @@ func LoadToRemote(opts entities.ScpLoadToRemoteOptions) (*entities.ScpLoadToRemo return &entities.ScpLoadToRemoteReport{Response: rep, ID: id}, nil } +// trimRemotePath drops the trailing newline ssh.Exec hands back with the rest of +// a remote command's raw output. +func trimRemotePath(out string) string { + return strings.TrimSpace(out) +} + +// remoteExec is ssh.Exec, taken as an argument so the commands built for a remote +// host can be exercised without one. +type remoteExec func(opts *ssh.ConnectionExecOptions, mode ssh.EngineMode) (string, error) + +// remoteRunner is every ssh operation a transfer performs, in one value. The +// exported entry points pass sshRunner; a test passes its own to assert the +// command sequence, and what is streamed, without a host to run against. +type remoteRunner struct { + exec remoteExec + execWithInput func(opts *ssh.ConnectionExecOptions, mode ssh.EngineMode, input io.Reader) (string, error) + scp func(opts *ssh.ConnectionScpOptions, mode ssh.EngineMode) (string, error) +} + +var sshRunner = remoteRunner{exec: ssh.Exec, execWithInput: ssh.ExecWithInput, scp: ssh.Scp} + +// removeRemoteFiles deletes paths on the host described by execOpts. Best effort: +// a failure is logged, not returned. +func removeRemoteFiles(run remoteExec, execOpts ssh.ConnectionExecOptions, sshMode ssh.EngineMode, paths ...string) { + rm := execOpts + rm.Args = append([]string{"rm", "-f"}, paths...) + if _, err := run(&rm, sshMode); err != nil { + logrus.Errorf("Removing file on endpoint: %v", err) + } +} + // SaveToRemote takes image information and remote connection information. it connects to the specified client // and saves the specified image on the remote machine and then copies it to the specified local location // returns an error if one occurs. func SaveToRemote(opts entities.ScpSaveToRemoteOptions) (*entities.ScpSaveToRemoteReport, error) { + return saveToRemote(sshRunner, opts) +} + +func saveToRemote(run remoteRunner, opts entities.ScpSaveToRemoteOptions) (*entities.ScpSaveToRemoteReport, error) { if opts.Tag != "" { return nil, fmt.Errorf("renaming of an image is currently not supported: %w", define.ErrInvalidArg) } @@ -302,10 +342,15 @@ func SaveToRemote(opts entities.ScpSaveToRemoteOptions) (*entities.ScpSaveToRemo } } - remoteFile, err := ssh.Exec(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"mktemp"}}, opts.SSHMode) + execOpts := ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User} + + mktemp := execOpts + mktemp.Args = []string{"mktemp"} + remoteFile, err := run.exec(&mktemp, opts.SSHMode) if err != nil { return nil, err } + remoteFile = trimRemotePath(remoteFile) saveArgs := []string{"podman", "image", "save", opts.Image} if opts.Format != "" { @@ -314,20 +359,19 @@ func SaveToRemote(opts entities.ScpSaveToRemoteOptions) (*entities.ScpSaveToRemo saveArgs = append(saveArgs, "--output", remoteFile) - _, err = ssh.Exec(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: saveArgs}, opts.SSHMode) + save := execOpts + save.Args = saveArgs + _, err = run.exec(&save, opts.SSHMode) if err != nil { return nil, err } scpConnOpts := ssh.ConnectionScpOptions{User: opts.URL.User, Identity: opts.Iden, Port: port, Source: "ssh://" + opts.URL.User.String() + "@" + opts.URL.Hostname() + ":" + remoteFile, Destination: opts.LocalFile} - scpRep, err := ssh.Scp(&scpConnOpts, opts.SSHMode) + scpRep, err := run.scp(&scpConnOpts, opts.SSHMode) if err != nil { return nil, err } - _, err = ssh.Exec(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"rm", scpRep}}, opts.SSHMode) - if err != nil { - logrus.Errorf("Removing file on endpoint: %v", err) - } + removeRemoteFiles(run.exec, execOpts, opts.SSHMode, scpRep) return &entities.ScpSaveToRemoteReport{}, nil } diff --git a/pkg/domain/utils/scp_test.go b/pkg/domain/utils/scp_test.go index 3504b4840a..735f4ddfab 100644 --- a/pkg/domain/utils/scp_test.go +++ b/pkg/domain/utils/scp_test.go @@ -2,9 +2,12 @@ package utils import ( "fmt" + "io" + "strings" "testing" "github.com/stretchr/testify/assert" + "go.podman.io/common/pkg/ssh" "go.podman.io/podman/v6/pkg/domain/entities" ) @@ -73,6 +76,45 @@ func TestValidateSCPArgs(t *testing.T) { } } +// The trailing newline is harmless while the path is last on a command line, but +// anything appending to it splices the newline into the middle. +func TestTrimRemotePath(t *testing.T) { + tests := []struct { + name string + out string + want string + }{ + { + name: "path as mktemp prints it", + out: "/tmp/tmp.5CGFmzWnCu\n", + want: "/tmp/tmp.5CGFmzWnCu", + }, + { + name: "path with a carriage return", + out: "/tmp/tmp.5CGFmzWnCu\r\n", + want: "/tmp/tmp.5CGFmzWnCu", + }, + { + name: "path already trimmed", + out: "/tmp/tmp.5CGFmzWnCu", + want: "/tmp/tmp.5CGFmzWnCu", + }, + { + name: "empty output", + out: "", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := trimRemotePath(tt.out) + assert.Equal(t, tt.want, got) + // Appending has to stay on one line: this is what the trim is for. + assert.NotContains(t, got+".gz", "\n") + }) + } +} + func TestParseImageSCPArg(t *testing.T) { tests := []struct { name string @@ -103,3 +145,67 @@ func TestParseImageSCPArg(t *testing.T) { }) } } + +// fakeRemote records the commands that would have run over ssh and replays a +// canned error for each. +type fakeRemote struct { + // errs is consulted per call, by index; nil succeeds. + errs []error + // out is what a command prints, by index, for the calls whose output the + // transfer reads back. + out []string + argv [][]string + // input is everything streamed to the command that took a stream. + input []byte + // scpOpts records each copy that was asked for, and scpErr fails it. + scpOpts []ssh.ConnectionScpOptions + scpErr error +} + +func (f *fakeRemote) exec(opts *ssh.ConnectionExecOptions, _ ssh.EngineMode) (string, error) { + return f.record(opts.Args) +} + +func (f *fakeRemote) execWithInput(opts *ssh.ConnectionExecOptions, _ ssh.EngineMode, input io.Reader) (string, error) { + var err error + if f.input, err = io.ReadAll(input); err != nil { + return "", err + } + return f.record(opts.Args) +} + +// scp mirrors ssh.Scp, which reports back the remote path it copied. Returning +// it is what lets a test see which path the cleanup is given. +func (f *fakeRemote) scp(opts *ssh.ConnectionScpOptions, _ ssh.EngineMode) (string, error) { + f.scpOpts = append(f.scpOpts, *opts) + if f.scpErr != nil { + return "", f.scpErr + } + // The source is an ssh:// URL with the path after the last colon. + return opts.Source[strings.LastIndex(opts.Source, ":")+1:], nil +} + +func (f *fakeRemote) record(argv []string) (string, error) { + f.argv = append(f.argv, argv) + i := len(f.argv) - 1 + + var err error + if i < len(f.errs) { + err = f.errs[i] + } + if i < len(f.out) { + return f.out[i], err + } + return "", err +} + +func (f *fakeRemote) runner() remoteRunner { + return remoteRunner{exec: f.exec, execWithInput: f.execWithInput, scp: f.scp} +} + +// -f matters: the cleanup has to tolerate a path that was never created. +func TestRemoveRemoteFiles(t *testing.T) { + remote := &fakeRemote{} + removeRemoteFiles(remote.exec, ssh.ConnectionExecOptions{}, ssh.GolangMode, "/tmp/a", "/tmp/b") + assert.Equal(t, [][]string{{"rm", "-f", "/tmp/a", "/tmp/b"}}, remote.argv) +} From 69e300867eb2f96a80824ba172406c2b5ff8d2d1 Mon Sep 17 00:00:00 2001 From: Scott Callaway Date: Wed, 5 Aug 2026 16:16:31 +0100 Subject: [PATCH 2/7] image scp: describe and validate the compression options Groundwork for compressing the transfer archive: the options themselves, the set of algorithms that may be requested, and the check ExecuteTransfer runs before it does anything else. Nothing acts on them yet. The set of formats is deliberately narrower than what c/image knows. Every entry has to satisfy three things: podman load has to detect and decompress it from the stream alone, c/image has to be able to compress it (it only decompresses bzip2 and xz), and a command line compressor of the same name has to exist for the case where the archive is produced on a remote host. gzip and zstd qualify, and they match the vocabulary --compression-format already uses on podman push. One table drives the accepted formats, their level ranges, and the list offered on the command line, so there is nothing to keep in sync. A transfer between two users on the same machine never crosses a network, so a requested format is reported as ignored there rather than refused. The validation is worded without flag names because it also runs on the API path, where the caller never passed a flag. Signed-off-by: Scott Callaway --- pkg/domain/entities/scp.go | 13 +++ pkg/domain/utils/scp.go | 8 ++ pkg/domain/utils/scp_compression.go | 75 +++++++++++++ pkg/domain/utils/scp_compression_test.go | 133 +++++++++++++++++++++++ 4 files changed, 229 insertions(+) create mode 100644 pkg/domain/utils/scp_compression.go create mode 100644 pkg/domain/utils/scp_compression_test.go diff --git a/pkg/domain/entities/scp.go b/pkg/domain/entities/scp.go index 7b9c522784..02726e632e 100644 --- a/pkg/domain/entities/scp.go +++ b/pkg/domain/entities/scp.go @@ -6,6 +6,16 @@ import ( "go.podman.io/common/pkg/ssh" ) +// ScpCompressionOptions describes how the transfer archive should be compressed. +type ScpCompressionOptions struct { + // CompressionFormat is the algorithm used to compress the archive before it + // is sent over the network. An empty string disables compression. + CompressionFormat string `json:"compressionFormat,omitempty"` + // CompressionLevel is the level handed to the compressor. A nil value uses + // the algorithm's default. + CompressionLevel *int `json:"compressionLevel,omitempty"` +} + // 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 @@ -33,6 +43,9 @@ type ScpExecuteTransferOptions struct { SSHMode ssh.EngineMode // SaveFormat is the format for podman save (oci-archive or docker-archive). Empty means default of podman save (docker-archive). SaveFormat string + // ScpCompressionOptions describes how to compress the archive before it is + // sent over the network. + ScpCompressionOptions } type ScpExecuteTransferReport struct { diff --git a/pkg/domain/utils/scp.go b/pkg/domain/utils/scp.go index 74e2b192c8..79f4115a7e 100644 --- a/pkg/domain/utils/scp.go +++ b/pkg/domain/utils/scp.go @@ -24,6 +24,10 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) ( sshInfo := entities.ImageScpConnections{} loadReport := entities.ScpLoadReport{Names: []string{}} + if err := ValidateScpCompression(opts.ScpCompressionOptions); err != nil { + return nil, err + } + podman, err := os.Executable() if err != nil { return nil, err @@ -174,6 +178,10 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) ( return nil, err } default: // else native load, both source and dest are local and transferring between users + if opts.CompressionFormat != "" { + // Nothing crosses the network here, so compressing would only burn CPU. + logrus.Warnf("Ignoring compression format %q: it only applies to transfers over ssh", opts.CompressionFormat) + } if source.User == "" { // source user has to be set, destination does not source.User = os.Getenv("USER") if source.User == "" { diff --git a/pkg/domain/utils/scp_compression.go b/pkg/domain/utils/scp_compression.go new file mode 100644 index 0000000000..2a93ad56ab --- /dev/null +++ b/pkg/domain/utils/scp_compression.go @@ -0,0 +1,75 @@ +package utils + +import ( + "fmt" + "maps" + "slices" + "strings" + + "go.podman.io/podman/v6/libpod/define" + "go.podman.io/podman/v6/pkg/domain/entities" +) + +// scpCompressionFormat describes one compression algorithm. A local archive is +// compressed with the c/image package; one on a remote host is compressed by +// running the command line compressor described here. +type scpCompressionFormat struct { + bin string + // args overwrite an existing output file, stay quiet, and remove the input + // once done. gzip removes the input by default, zstd needs --rm. + args []string + // ext is the suffix the compressor appends to the file it compresses. + ext string + // Bounds are what the command line compressor accepts, applied to the local + // path too so a level means one thing per algorithm. + minLevel, maxLevel int +} + +// scpCompressionFormats is the single source of truth for the accepted formats. +// An entry has to be detectable by podman load from the stream alone, +// compressible by c/image (which only decompresses bzip2 and xz), and available +// as a command of the same name for the remote case. +var scpCompressionFormats = map[string]scpCompressionFormat{ + "gzip": {bin: "gzip", args: []string{"-f", "-q"}, ext: ".gz", minLevel: 1, maxLevel: 9}, + // zstd goes up to 22, but only with --ultra; 19 is the plain maximum. + "zstd": {bin: "zstd", args: []string{"-f", "-q", "--rm"}, ext: ".zst", minLevel: 1, maxLevel: 19}, +} + +// ScpCompressionFormats lists the accepted --compression-format values. +func ScpCompressionFormats() []string { + return slices.Sorted(maps.Keys(scpCompressionFormats)) +} + +// scpCompressionFormatByName gives every caller the same rejection wording. +func scpCompressionFormatByName(name string) (scpCompressionFormat, error) { + format, ok := scpCompressionFormats[name] + if !ok { + return scpCompressionFormat{}, fmt.Errorf("unsupported compression format %q, choose from: %s: %w", + name, strings.Join(ScpCompressionFormats(), ", "), define.ErrInvalidArg) + } + return format, nil +} + +// ValidateScpCompression checks the format is one podman image scp can apply and +// that the level, if any, is in range. The errors avoid flag names because this +// also runs on the API path. +func ValidateScpCompression(opts entities.ScpCompressionOptions) error { + if opts.CompressionFormat == "" { + if opts.CompressionLevel != nil { + return fmt.Errorf("a compression level requires a compression format: %w", define.ErrInvalidArg) + } + return nil + } + + format, err := scpCompressionFormatByName(opts.CompressionFormat) + if err != nil { + return err + } + + if opts.CompressionLevel != nil && (*opts.CompressionLevel < format.minLevel || *opts.CompressionLevel > format.maxLevel) { + return fmt.Errorf("compression level %d is out of range for %q, must be between %d and %d: %w", + *opts.CompressionLevel, opts.CompressionFormat, format.minLevel, format.maxLevel, define.ErrInvalidArg) + } + + return nil +} diff --git a/pkg/domain/utils/scp_compression_test.go b/pkg/domain/utils/scp_compression_test.go new file mode 100644 index 0000000000..722e59f6cf --- /dev/null +++ b/pkg/domain/utils/scp_compression_test.go @@ -0,0 +1,133 @@ +package utils + +import ( + "os" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.podman.io/podman/v6/libpod/define" + "go.podman.io/podman/v6/pkg/domain/entities" +) + +func TestValidateScpCompression(t *testing.T) { + level := func(l int) *int { return &l } + + tests := []struct { + name string + opts entities.ScpCompressionOptions + wantErr string + }{ + { + name: "no compression requested", + opts: entities.ScpCompressionOptions{}, + }, + { + name: "level without a format", + opts: entities.ScpCompressionOptions{CompressionLevel: level(9)}, + // A level on its own would be silently ignored, so reject it. + wantErr: "a compression level requires a compression format", + }, + { + name: "unknown format", + opts: entities.ScpCompressionOptions{CompressionFormat: "lz4"}, + // zstd:chunked and friends are known to c/image but are not + // whole-stream formats podman load can pick up on its own. + wantErr: `unsupported compression format "lz4"`, + }, + { + name: "zstd:chunked is not accepted", + opts: entities.ScpCompressionOptions{CompressionFormat: "zstd:chunked"}, + // c/image knows this one, podman image scp deliberately does not. + wantErr: `unsupported compression format "zstd:chunked"`, + }, + { + name: "level above the format's range", + opts: entities.ScpCompressionOptions{CompressionFormat: "gzip", CompressionLevel: level(10)}, + // zstd would accept 10, gzip stops at 9. + wantErr: `compression level 10 is out of range for "gzip", must be between 1 and 9`, + }, + { + name: "level below the format's range", + opts: entities.ScpCompressionOptions{CompressionFormat: "zstd", CompressionLevel: level(0)}, + wantErr: `compression level 0 is out of range for "zstd", must be between 1 and 19`, + }, + { + name: "bzip2 is not accepted", + opts: entities.ScpCompressionOptions{CompressionFormat: "bzip2"}, + // c/image can only decompress bzip2, so it cannot be produced here. + wantErr: `unsupported compression format "bzip2"`, + }, + { + name: "zstd accepts level 19", + opts: entities.ScpCompressionOptions{CompressionFormat: "zstd", CompressionLevel: level(19)}, + }, + { + name: "gzip accepts level 9", + opts: entities.ScpCompressionOptions{CompressionFormat: "gzip", CompressionLevel: level(9)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateScpCompression(tt.opts) + if tt.wantErr == "" { + assert.NoError(t, err) + return + } + assert.ErrorContains(t, err, tt.wantErr) + }) + } +} + +// The list is part of the command's interface: it drives the flag's choices. +func TestScpCompressionFormatsAreUsable(t *testing.T) { + assert.Equal(t, []string{"gzip", "zstd"}, ScpCompressionFormats()) + for _, format := range ScpCompressionFormats() { + assert.NoError(t, ValidateScpCompression(entities.ScpCompressionOptions{CompressionFormat: format})) + } +} + +// The API path does not go through flag parsing. Ordering is checked by its +// consequence: ExecuteTransfer creates its temporary file straight after +// validating, so a later check would leave one behind on every rejected request. +func TestExecuteTransferRejectsBadCompressionBeforeDoingAnything(t *testing.T) { + level := func(l int) *int { return &l } + + tests := []struct { + name string + opts entities.ScpCompressionOptions + wantErr string + }{ + { + name: "level without a format", + opts: entities.ScpCompressionOptions{CompressionLevel: level(9)}, + wantErr: "a compression level requires a compression format", + }, + { + name: "level out of range", + opts: entities.ScpCompressionOptions{CompressionFormat: "gzip", CompressionLevel: level(10)}, + wantErr: `compression level 10 is out of range for "gzip"`, + }, + { + name: "format podman image scp cannot produce", + opts: entities.ScpCompressionOptions{CompressionFormat: "bzip2"}, + wantErr: `unsupported compression format "bzip2"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + + _, err := ExecuteTransfer("alpine", "QA::", entities.ScpExecuteTransferOptions{ScpCompressionOptions: tt.opts}) + assert.ErrorContains(t, err, tt.wantErr) + assert.ErrorIs(t, err, define.ErrInvalidArg) + + entries, readErr := os.ReadDir(tmp) + require.NoError(t, readErr) + assert.Empty(t, entries, "the transfer's temporary file was created before the options were rejected") + }) + } +} From 4c7a1a8408632aef0d704b3add7191467dc097f5 Mon Sep 17 00:00:00 2001 From: Scott Callaway Date: Wed, 5 Aug 2026 16:16:31 +0100 Subject: [PATCH 3/7] image scp: compress a locally produced archive into the ssh stream podman save writes docker-archive layers uncompressed, so podman image scp puts the whole archive on the wire as is. Compressing it first takes a docker-archive to around half its size or less, which is why people work around this today with podman save | zstd, a manual copy and podman load on the far side. An oci-archive keeps whatever compression its layers already have, so there is little to gain there; the man page records the difference. When the archive is produced locally it can be compressed on the way out: the c/image compression package wraps the file as it is streamed into the ssh connection feeding the remote podman image load. No second temporary file, nothing buffered in full. Nothing is needed on the destination. podman load detects the compression from the stream and decompresses it itself, for docker-archive via c/image's AutoDecompress and for oci-archive via c/storage's DecompressStream. Both are covered, since which one runs depends on --format. Signed-off-by: Scott Callaway --- pkg/domain/entities/scp.go | 3 + pkg/domain/utils/scp.go | 15 ++- pkg/domain/utils/scp_compression.go | 38 ++++++ pkg/domain/utils/scp_compression_test.go | 149 +++++++++++++++++++++++ 4 files changed, 204 insertions(+), 1 deletion(-) diff --git a/pkg/domain/entities/scp.go b/pkg/domain/entities/scp.go index 02726e632e..491ec06794 100644 --- a/pkg/domain/entities/scp.go +++ b/pkg/domain/entities/scp.go @@ -83,6 +83,9 @@ type ScpLoadToRemoteOptions struct { Iden string // SSHMode is the specified ssh.EngineMode which should be used SSHMode ssh.EngineMode + // ScpCompressionOptions compresses LocalFile as it is streamed. Must be empty + // when LocalFile is already compressed. + ScpCompressionOptions } type ScpLoadToRemoteReport struct { diff --git a/pkg/domain/utils/scp.go b/pkg/domain/utils/scp.go index 79f4115a7e..490704287a 100644 --- a/pkg/domain/utils/scp.go +++ b/pkg/domain/utils/scp.go @@ -164,6 +164,8 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) ( loadToRemoteOpts.URL = sshInfo.URI[0] loadToRemoteOpts.Iden = sshInfo.Identities[0] loadToRemoteOpts.SSHMode = opts.SSHMode + // Compress on the fly: only compressed bytes cross the network. + loadToRemoteOpts.ScpCompressionOptions = opts.ScpCompressionOptions loadToRemoteRep, err := LoadToRemote(loadToRemoteOpts) if err != nil { return nil, err @@ -278,7 +280,18 @@ func loadToRemote(run remoteRunner, opts entities.ScpLoadToRemoteOptions) (*enti } defer input.Close() - out, err := run.execWithInput(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"podman", "image", "load"}}, opts.SSHMode, input) + var stream io.Reader = input + if opts.CompressionFormat != "" { + // The remote podman load detects the compression itself. + compressed, err := compressReader(input, opts.ScpCompressionOptions) + if err != nil { + return nil, err + } + defer compressed.Close() + stream = compressed + } + + out, err := run.execWithInput(&ssh.ConnectionExecOptions{Host: opts.URL.String(), Identity: opts.Iden, Port: port, User: opts.URL.User, Args: []string{"podman", "image", "load"}}, opts.SSHMode, stream) if err != nil { return nil, err } diff --git a/pkg/domain/utils/scp_compression.go b/pkg/domain/utils/scp_compression.go index 2a93ad56ab..2b896bc39e 100644 --- a/pkg/domain/utils/scp_compression.go +++ b/pkg/domain/utils/scp_compression.go @@ -2,10 +2,12 @@ package utils import ( "fmt" + "io" "maps" "slices" "strings" + "go.podman.io/image/v5/pkg/compression" "go.podman.io/podman/v6/libpod/define" "go.podman.io/podman/v6/pkg/domain/entities" ) @@ -73,3 +75,39 @@ func ValidateScpCompression(opts entities.ScpCompressionOptions) error { return nil } + +// compressReader returns input compressed with the given format. Compression +// runs in a goroutine feeding a pipe, so the archive is never held in memory in +// full. The caller must close the returned reader. +func compressReader(input io.Reader, opts entities.ScpCompressionOptions) (io.ReadCloser, error) { + // Not straight to c/image: it also compresses xz and zstd:chunked, which + // podman image scp does not offer. + if _, err := scpCompressionFormatByName(opts.CompressionFormat); err != nil { + return nil, err + } + + algorithm, err := compression.AlgorithmByName(opts.CompressionFormat) + if err != nil { + return nil, err + } + + reader, writer := io.Pipe() + compressor, err := compression.CompressStream(writer, algorithm, opts.CompressionLevel) + if err != nil { + _ = writer.Close() + _ = reader.Close() + return nil, err + } + + go func() { + _, err := io.Copy(compressor, input) + // Closing the compressor flushes the trailer, so its error matters too. + if closeErr := compressor.Close(); err == nil { + err = closeErr + } + // Always close so a reader blocked in Read() is released. + _ = writer.CloseWithError(err) + }() + + return reader, nil +} diff --git a/pkg/domain/utils/scp_compression_test.go b/pkg/domain/utils/scp_compression_test.go index 722e59f6cf..b5d058845d 100644 --- a/pkg/domain/utils/scp_compression_test.go +++ b/pkg/domain/utils/scp_compression_test.go @@ -1,13 +1,22 @@ package utils import ( + "bytes" + "errors" + "io" + "net/url" "os" + "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.podman.io/common/pkg/ssh" + "go.podman.io/image/v5/pkg/compression" "go.podman.io/podman/v6/libpod/define" "go.podman.io/podman/v6/pkg/domain/entities" + "go.podman.io/storage/pkg/archive" ) func TestValidateScpCompression(t *testing.T) { @@ -131,3 +140,143 @@ func TestExecuteTransferRejectsBadCompressionBeforeDoingAnything(t *testing.T) { }) } } + +// The feature rests on podman load recognising the compression unaided, and the +// two archive formats reach that through different detectors. +func TestCompressReaderIsDetectedByBothLoadPaths(t *testing.T) { + payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) + + for _, format := range ScpCompressionFormats() { + t.Run(format, func(t *testing.T) { + reader, err := compressReader(bytes.NewReader(payload), entities.ScpCompressionOptions{CompressionFormat: format}) + require.NoError(t, err) + defer reader.Close() + compressed, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Less(t, len(compressed), len(payload), "compressed output should be smaller than the input") + + // docker-archive: c/image tarfile.Reader uses AutoDecompress. + viaCImage, isCompressed, err := compression.AutoDecompress(bytes.NewReader(compressed)) + require.NoError(t, err) + require.True(t, isCompressed) + defer viaCImage.Close() + roundTripped, err := io.ReadAll(viaCImage) + require.NoError(t, err) + assert.Equal(t, payload, roundTripped) + + // oci-archive: c/storage archive.Untar uses DecompressStream. + viaCStorage, err := archive.DecompressStream(bytes.NewReader(compressed)) + require.NoError(t, err) + defer viaCStorage.Close() + roundTripped, err = io.ReadAll(viaCStorage) + require.NoError(t, err) + assert.Equal(t, payload, roundTripped) + }) + } +} + +func TestCompressReaderWithLevel(t *testing.T) { + payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) + + level := 1 + reader, err := compressReader(bytes.NewReader(payload), entities.ScpCompressionOptions{ + CompressionFormat: "gzip", + CompressionLevel: &level, + }) + require.NoError(t, err) + defer reader.Close() + + compressed, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Less(t, len(compressed), len(payload)) +} + +// c/image can compress xz and zstd:chunked, so these catch compressReader going +// straight to it. "lz4" would not: c/image does not know it either. +func TestCompressReaderRejectsFormatsOutsideTheTable(t *testing.T) { + for _, format := range []string{"xz", "zstd:chunked", "lz4"} { + t.Run(format, func(t *testing.T) { + _, err := compressReader(bytes.NewReader(nil), entities.ScpCompressionOptions{CompressionFormat: format}) + assert.ErrorContains(t, err, "unsupported compression format") + assert.ErrorIs(t, err, define.ErrInvalidArg) + }) + } +} + +func TestCompressReaderPropagatesReadError(t *testing.T) { + reader, err := compressReader(&failingReader{}, entities.ScpCompressionOptions{CompressionFormat: "gzip"}) + require.NoError(t, err) + defer reader.Close() + + _, err = io.ReadAll(reader) + assert.ErrorContains(t, err, "read failed") +} + +// The path itself needs two hosts, so without this the flag could stop being +// honoured and every other test would still pass. +// A level the compressor will not take has to fail before anything is streamed; +// only the API path can reach this, as the flag path validates the range first. +func TestCompressReaderRejectsUnusableLevel(t *testing.T) { + level := 100 + _, err := compressReader(bytes.NewReader(nil), + entities.ScpCompressionOptions{CompressionFormat: "gzip", CompressionLevel: &level}) + assert.ErrorContains(t, err, "invalid compression level") +} + +// The local source path never writes a compressed file: it compresses into the +// ssh stream feeding the remote podman image load. +func TestLoadToRemoteCompressesTheStream(t *testing.T) { + payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) + + url, err := url.Parse("ssh://root@example.test:22") + require.NoError(t, err) + + archiveFile := filepath.Join(t.TempDir(), "archive") + require.NoError(t, os.WriteFile(archiveFile, payload, 0o600)) + + baseOpts := entities.ScpLoadToRemoteOptions{ + LocalFile: archiveFile, + URL: url, + SSHMode: ssh.GolangMode, + } + + for _, format := range ScpCompressionFormats() { + t.Run(format, func(t *testing.T) { + remote := &fakeRemote{out: []string{"Loaded image: quay.io/libpod/alpine:latest"}} + opts := baseOpts + opts.ScpCompressionOptions = entities.ScpCompressionOptions{CompressionFormat: format} + + rep, err := loadToRemote(remote.runner(), opts) + require.NoError(t, err) + assert.Equal(t, "quay.io/libpod/alpine:latest", rep.ID) + assert.Equal(t, [][]string{{"podman", "image", "load"}}, remote.argv) + + assert.Less(t, len(remote.input), len(payload), "the stream should be compressed") + decompressed, err := archive.DecompressStream(bytes.NewReader(remote.input)) + require.NoError(t, err) + defer decompressed.Close() + roundTripped, err := io.ReadAll(decompressed) + require.NoError(t, err) + assert.Equal(t, payload, roundTripped, "the remote podman load must see the archive unchanged") + }) + } + + // The remote to remote path will rely on this: once the archive has been + // compressed on the source host, this leg has to stream it untouched rather + // than compress it a second time. + t.Run("without a format the file is streamed as it is", func(t *testing.T) { + remote := &fakeRemote{out: []string{"Loaded image: quay.io/libpod/alpine:latest"}} + + _, err := loadToRemote(remote.runner(), baseOpts) + require.NoError(t, err) + assert.Equal(t, payload, remote.input) + }) +} + +var errRead = errors.New("read failed") + +type failingReader struct{} + +func (*failingReader) Read([]byte) (int, error) { + return 0, errRead +} From c6029f9ab7055946baa906aa2ad484fa80af36bc Mon Sep 17 00:00:00 2001 From: Scott Callaway Date: Wed, 5 Aug 2026 16:16:31 +0100 Subject: [PATCH 4/7] image scp: compress a remotely produced archive on the source host When the source is a remote host the archive is produced there, so it has to be compressed there too: compressing after copying it down would mean the uncompressed archive had already crossed the network, which is the cost this is meant to avoid. The only thing we can do on that host is run a command, so the matching compressor is invoked over ssh between the save and the copy. That is also why the set of formats is limited to algorithms available as a command of the same name. Two details worth stating. A shell reports 127 when it cannot find the command, which is worth reporting plainly as a host without the compressor installed; anything else, a failure to connect included, must not be reported that way, and a probe beforehand cannot make that distinction without also costing an extra connection. And the compressor removes its input only once it succeeds and may have written part of its output before giving up, so a failure cleans up both paths. Signed-off-by: Scott Callaway --- pkg/domain/entities/scp.go | 3 + pkg/domain/utils/scp.go | 14 + pkg/domain/utils/scp_compression.go | 70 ++++ pkg/domain/utils/scp_compression_test.go | 432 +++++++++++++++++++---- 4 files changed, 455 insertions(+), 64 deletions(-) diff --git a/pkg/domain/entities/scp.go b/pkg/domain/entities/scp.go index 491ec06794..b0e4abf6c6 100644 --- a/pkg/domain/entities/scp.go +++ b/pkg/domain/entities/scp.go @@ -109,6 +109,9 @@ type ScpSaveToRemoteOptions struct { SSHMode ssh.EngineMode // Format is the save format (oci-archive or docker-archive). Empty means default of podman save (docker-archive). Format string + // ScpCompressionOptions describes how to compress the archive on the remote + // host before it is copied over the network. + ScpCompressionOptions } type ScpSaveToRemoteReport struct{} diff --git a/pkg/domain/utils/scp.go b/pkg/domain/utils/scp.go index 490704287a..53a317b2bd 100644 --- a/pkg/domain/utils/scp.go +++ b/pkg/domain/utils/scp.go @@ -103,6 +103,8 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) ( saveToRemoteOpts.Iden = sshInfo.Identities[0] saveToRemoteOpts.SSHMode = opts.SSHMode saveToRemoteOpts.Format = opts.SaveFormat + // Compress on the source host: only compressed bytes are copied down. + saveToRemoteOpts.ScpCompressionOptions = opts.ScpCompressionOptions _, err = SaveToRemote(saveToRemoteOpts) if err != nil { return nil, err @@ -115,6 +117,8 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) ( loadToRemoteOpts.URL = sshInfo.URI[1] loadToRemoteOpts.Iden = sshInfo.Identities[1] loadToRemoteOpts.SSHMode = opts.SSHMode + // ScpCompressionOptions is deliberately left unset: SaveToRemote + // already compressed this on the source host, so stream it on as it is. loadToRemoteRep, err := LoadToRemote(loadToRemoteOpts) if err != nil { return nil, err @@ -387,6 +391,16 @@ func saveToRemote(run remoteRunner, opts entities.ScpSaveToRemoteOptions) (*enti return nil, err } + if opts.CompressionFormat != "" { + // Compress it where it is, so only compressed bytes are copied over the + // network. + compressedFile, err := compressRemoteFile(run.exec, execOpts, opts.SSHMode, remoteFile, opts.ScpCompressionOptions) + if err != nil { + return nil, err + } + remoteFile = compressedFile + } + scpConnOpts := ssh.ConnectionScpOptions{User: opts.URL.User, Identity: opts.Iden, Port: port, Source: "ssh://" + opts.URL.User.String() + "@" + opts.URL.Hostname() + ":" + remoteFile, Destination: opts.LocalFile} scpRep, err := run.scp(&scpConnOpts, opts.SSHMode) if err != nil { diff --git a/pkg/domain/utils/scp_compression.go b/pkg/domain/utils/scp_compression.go index 2b896bc39e..41a3b0f1f0 100644 --- a/pkg/domain/utils/scp_compression.go +++ b/pkg/domain/utils/scp_compression.go @@ -1,12 +1,15 @@ package utils import ( + "errors" "fmt" "io" "maps" "slices" + "strconv" "strings" + "go.podman.io/common/pkg/ssh" "go.podman.io/image/v5/pkg/compression" "go.podman.io/podman/v6/libpod/define" "go.podman.io/podman/v6/pkg/domain/entities" @@ -111,3 +114,70 @@ func compressReader(input io.Reader, opts entities.ScpCompressionOptions) (io.Re return reader, nil } + +// remoteCompressCommand returns the compressor binary, the argv compressing +// remoteFile in place, and the path the compressed archive ends up at. +func remoteCompressCommand(remoteFile string, opts entities.ScpCompressionOptions) (bin string, argv []string, compressedFile string, err error) { + format, err := scpCompressionFormatByName(opts.CompressionFormat) + if err != nil { + return "", nil, "", err + } + + argv = make([]string, 0, len(format.args)+3) + argv = append(argv, format.bin) + argv = append(argv, format.args...) + if opts.CompressionLevel != nil { + argv = append(argv, "-"+strconv.Itoa(*opts.CompressionLevel)) + } + argv = append(argv, remoteFile) + + return format.bin, argv, remoteFile + format.ext, nil +} + +// cmdNotFoundStatus is what a POSIX shell exits with for an unknown command. +const cmdNotFoundStatus = 127 + +// compressRemoteFile compresses remoteFile in place on the host described by +// execOpts and returns the compressed path. The compressor removes remoteFile, so +// only the returned path needs cleaning up; a failure leaves nothing behind. +func compressRemoteFile(run remoteExec, execOpts ssh.ConnectionExecOptions, sshMode ssh.EngineMode, remoteFile string, opts entities.ScpCompressionOptions) (string, error) { + bin, argv, compressedFile, err := remoteCompressCommand(remoteFile, opts) + if err != nil { + return "", err + } + + compress := execOpts + compress.Args = argv + if _, err := run(&compress, sshMode); err != nil { + // Either path can exist: the input is only removed on success, and a + // partial output may already have been written. + removeRemoteFiles(run, execOpts, sshMode, remoteFile, compressedFile) + + // Only 127 means the host lacks the compressor. Reporting anything else + // that way would mislabel a failure to connect. + if remoteExitStatus(err) == cmdNotFoundStatus { + return "", fmt.Errorf("compressing the transfer archive with %q requires the %q command on the remote host: %w", + opts.CompressionFormat, bin, err) + } + return "", fmt.Errorf("compressing %q on the remote host: %w", remoteFile, err) + } + + return compressedFile, nil +} + +// remoteExitStatus returns the status the remote command exited with, or -1 if +// err is not a command that ran to completion. The two ssh engines return +// different error types spelling the accessor differently. Matching the accessor +// rather than the type also keeps this testable: crypto/ssh's status field is +// unexported, so its ExitError cannot be built with a chosen status. +func remoteExitStatus(err error) int { + var sshExit interface{ ExitStatus() int } + if errors.As(err, &sshExit) { + return sshExit.ExitStatus() + } + var cmdExit interface{ ExitCode() int } + if errors.As(err, &cmdExit) { + return cmdExit.ExitCode() + } + return -1 +} diff --git a/pkg/domain/utils/scp_compression_test.go b/pkg/domain/utils/scp_compression_test.go index b5d058845d..caa60d105a 100644 --- a/pkg/domain/utils/scp_compression_test.go +++ b/pkg/domain/utils/scp_compression_test.go @@ -3,9 +3,11 @@ package utils import ( "bytes" "errors" + "fmt" "io" "net/url" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -17,6 +19,8 @@ import ( "go.podman.io/podman/v6/libpod/define" "go.podman.io/podman/v6/pkg/domain/entities" "go.podman.io/storage/pkg/archive" + "go.podman.io/storage/pkg/fileutils" + cryptossh "golang.org/x/crypto/ssh" ) func TestValidateScpCompression(t *testing.T) { @@ -97,6 +101,262 @@ func TestScpCompressionFormatsAreUsable(t *testing.T) { } } +// The feature rests on podman load recognising the compression unaided, and the +// two archive formats reach that through different detectors. +func TestCompressReaderIsDetectedByBothLoadPaths(t *testing.T) { + payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) + + for _, format := range ScpCompressionFormats() { + t.Run(format, func(t *testing.T) { + reader, err := compressReader(bytes.NewReader(payload), entities.ScpCompressionOptions{CompressionFormat: format}) + require.NoError(t, err) + defer reader.Close() + compressed, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Less(t, len(compressed), len(payload), "compressed output should be smaller than the input") + + // docker-archive: c/image tarfile.Reader uses AutoDecompress. + viaCImage, isCompressed, err := compression.AutoDecompress(bytes.NewReader(compressed)) + require.NoError(t, err) + require.True(t, isCompressed) + defer viaCImage.Close() + roundTripped, err := io.ReadAll(viaCImage) + require.NoError(t, err) + assert.Equal(t, payload, roundTripped) + + // oci-archive: c/storage archive.Untar uses DecompressStream. + viaCStorage, err := archive.DecompressStream(bytes.NewReader(compressed)) + require.NoError(t, err) + defer viaCStorage.Close() + roundTripped, err = io.ReadAll(viaCStorage) + require.NoError(t, err) + assert.Equal(t, payload, roundTripped) + }) + } +} + +func TestRemoteCompressCommand(t *testing.T) { + level := func(l int) *int { return &l } + + tests := []struct { + name string + opts entities.ScpCompressionOptions + wantBin string + wantArgv []string + wantCompressed string + wantErr string + }{ + { + name: "gzip without a level", + opts: entities.ScpCompressionOptions{CompressionFormat: "gzip"}, + wantBin: "gzip", + wantArgv: []string{"gzip", "-f", "-q", "/tmp/tmp.XXXX"}, + wantCompressed: "/tmp/tmp.XXXX.gz", + }, + { + name: "gzip with a level", + opts: entities.ScpCompressionOptions{CompressionFormat: "gzip", CompressionLevel: level(9)}, + wantBin: "gzip", + wantArgv: []string{"gzip", "-f", "-q", "-9", "/tmp/tmp.XXXX"}, + wantCompressed: "/tmp/tmp.XXXX.gz", + }, + { + name: "zstd keeps its input unless told otherwise", + opts: entities.ScpCompressionOptions{CompressionFormat: "zstd"}, + // --rm matters: without it the uncompressed archive is left behind + // on the remote host next to the compressed one. + wantBin: "zstd", + wantArgv: []string{"zstd", "-f", "-q", "--rm", "/tmp/tmp.XXXX"}, + wantCompressed: "/tmp/tmp.XXXX.zst", + }, + { + name: "zstd with a level", + opts: entities.ScpCompressionOptions{CompressionFormat: "zstd", CompressionLevel: level(19)}, + wantBin: "zstd", + wantArgv: []string{"zstd", "-f", "-q", "--rm", "-19", "/tmp/tmp.XXXX"}, + wantCompressed: "/tmp/tmp.XXXX.zst", + }, + { + name: "unsupported format", + opts: entities.ScpCompressionOptions{CompressionFormat: "bzip2"}, + wantErr: `unsupported compression format "bzip2"`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + bin, argv, compressedFile, err := remoteCompressCommand("/tmp/tmp.XXXX", tt.opts) + if tt.wantErr != "" { + assert.ErrorContains(t, err, tt.wantErr) + return + } + require.NoError(t, err) + assert.Equal(t, tt.wantBin, bin) + assert.Equal(t, tt.wantArgv, argv) + assert.Equal(t, tt.wantCompressed, compressedFile) + // The path has to be the last argument: the compressors take their + // flags first and everything after is treated as a file name. + assert.Equal(t, "/tmp/tmp.XXXX", argv[len(argv)-1]) + }) + } +} + +// A wrong suffix makes the scp of the compressed archive fail with "no such file". +func TestRemoteCompressCommandExtensionMatchesCompressor(t *testing.T) { + extensions := map[string]string{"gzip": ".gz", "zstd": ".zst"} + + for _, format := range ScpCompressionFormats() { + ext, known := extensions[format] + require.True(t, known, "no expected extension recorded for %q", format) + + _, _, compressedFile, err := remoteCompressCommand("/tmp/archive", entities.ScpCompressionOptions{CompressionFormat: format}) + require.NoError(t, err) + assert.Equal(t, "/tmp/archive"+ext, compressedFile) + } +} + +func TestCompressReaderWithLevel(t *testing.T) { + payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) + + level := 1 + reader, err := compressReader(bytes.NewReader(payload), entities.ScpCompressionOptions{ + CompressionFormat: "gzip", + CompressionLevel: &level, + }) + require.NoError(t, err) + defer reader.Close() + + compressed, err := io.ReadAll(reader) + require.NoError(t, err) + assert.Less(t, len(compressed), len(payload)) +} + +// c/image can compress xz and zstd:chunked, so these catch compressReader going +// straight to it. "lz4" would not: c/image does not know it either. +func TestCompressReaderRejectsFormatsOutsideTheTable(t *testing.T) { + for _, format := range []string{"xz", "zstd:chunked", "lz4"} { + t.Run(format, func(t *testing.T) { + _, err := compressReader(bytes.NewReader(nil), entities.ScpCompressionOptions{CompressionFormat: format}) + assert.ErrorContains(t, err, "unsupported compression format") + assert.ErrorIs(t, err, define.ErrInvalidArg) + }) + } +} + +func TestCompressReaderPropagatesReadError(t *testing.T) { + reader, err := compressReader(&failingReader{}, entities.ScpCompressionOptions{CompressionFormat: "gzip"}) + require.NoError(t, err) + defer reader.Close() + + _, err = io.ReadAll(reader) + assert.ErrorContains(t, err, "read failed") +} + +// Run the argv that would be sent to the remote host against the real +// compressors: the only check that the flags do what compressRemoteFile assumes. +func TestRemoteCompressCommandAgainstRealCompressors(t *testing.T) { + payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) + + for _, format := range ScpCompressionFormats() { + t.Run(format, func(t *testing.T) { + opts := entities.ScpCompressionOptions{CompressionFormat: format} + bin, argv, compressedFile, err := remoteCompressCommand(filepath.Join(t.TempDir(), "archive"), opts) + require.NoError(t, err) + + if _, err := exec.LookPath(bin); err != nil { + t.Skipf("%s is not installed", bin) + } + + archiveFile := argv[len(argv)-1] + require.NoError(t, os.WriteFile(archiveFile, payload, 0o600)) + + out, err := exec.Command(argv[0], argv[1:]...).CombinedOutput() + require.NoError(t, err, "%s failed: %s", bin, out) + + compressed, err := os.ReadFile(compressedFile) + require.NoError(t, err, "%s did not produce %q", bin, compressedFile) + assert.Less(t, len(compressed), len(payload)) + + // The uncompressed archive must be gone: it can be gigabytes, and + // only the compressed path gets cleaned up afterwards. + assert.ErrorIs(t, fileutils.Lexists(archiveFile), os.ErrNotExist, + "%s left the uncompressed archive behind", bin) + + decompressed, err := archive.DecompressStream(bytes.NewReader(compressed)) + require.NoError(t, err) + defer decompressed.Close() + roundTripped, err := io.ReadAll(decompressed) + require.NoError(t, err) + assert.Equal(t, payload, roundTripped) + }) + } +} + +// Where the compression happens is the whole point of the feature: the archive +// has to be compressed on the host that produced it, before it is copied. +func TestSaveToRemoteCompressesBeforeCopying(t *testing.T) { + url, err := url.Parse("ssh://root@example.test:22") + require.NoError(t, err) + level := 5 + + baseOpts := entities.ScpSaveToRemoteOptions{ + Image: "alpine", + LocalFile: "/local/archive", + URL: url, + SSHMode: ssh.GolangMode, + } + + t.Run("the compressed archive is what gets copied and cleaned up", func(t *testing.T) { + // mktemp prints the path with a newline, as the real one does. + remote := &fakeRemote{out: []string{"/tmp/tmp.XXXX\n"}} + opts := baseOpts + opts.ScpCompressionOptions = entities.ScpCompressionOptions{CompressionFormat: "zstd", CompressionLevel: &level} + + _, err := saveToRemote(remote.runner(), opts) + require.NoError(t, err) + + assert.Equal(t, [][]string{ + {"mktemp"}, + {"podman", "image", "save", "alpine", "--output", "/tmp/tmp.XXXX"}, + {"zstd", "-f", "-q", "--rm", "-5", "/tmp/tmp.XXXX"}, + {"rm", "-f", "/tmp/tmp.XXXX.zst"}, + }, remote.argv) + + require.Len(t, remote.scpOpts, 1) + assert.Equal(t, "ssh://root@example.test:/tmp/tmp.XXXX.zst", remote.scpOpts[0].Source) + assert.Equal(t, "/local/archive", remote.scpOpts[0].Destination) + }) + + t.Run("without a format the archive is copied as saved", func(t *testing.T) { + remote := &fakeRemote{out: []string{"/tmp/tmp.XXXX\n"}} + + _, err := saveToRemote(remote.runner(), baseOpts) + require.NoError(t, err) + + assert.Equal(t, [][]string{ + {"mktemp"}, + {"podman", "image", "save", "alpine", "--output", "/tmp/tmp.XXXX"}, + {"rm", "-f", "/tmp/tmp.XXXX"}, + }, remote.argv) + + require.Len(t, remote.scpOpts, 1) + assert.Equal(t, "ssh://root@example.test:/tmp/tmp.XXXX", remote.scpOpts[0].Source) + }) + + t.Run("a compressor that fails stops the transfer before anything is copied", func(t *testing.T) { + remote := &fakeRemote{ + out: []string{"/tmp/tmp.XXXX\n"}, + errs: []error{nil, nil, exitStatusError{status: cmdNotFoundStatus}}, + } + opts := baseOpts + opts.ScpCompressionOptions = entities.ScpCompressionOptions{CompressionFormat: "zstd"} + + _, err := saveToRemote(remote.runner(), opts) + assert.ErrorContains(t, err, `requires the "zstd" command on the remote host`) + assert.Empty(t, remote.scpOpts, "nothing should be copied after the compressor failed") + }) +} + // The API path does not go through flag parsing. Ordering is checked by its // consequence: ExecuteTransfer creates its temporary file straight after // validating, so a later check would leave one behind on every rejected request. @@ -141,79 +401,123 @@ func TestExecuteTransferRejectsBadCompressionBeforeDoingAnything(t *testing.T) { } } -// The feature rests on podman load recognising the compression unaided, and the -// two archive formats reach that through different detectors. -func TestCompressReaderIsDetectedByBothLoadPaths(t *testing.T) { - payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) +func TestCompressRemoteFile(t *testing.T) { + execOpts := ssh.ConnectionExecOptions{Host: "ssh://root@example.test"} + opts := entities.ScpCompressionOptions{CompressionFormat: "gzip"} - for _, format := range ScpCompressionFormats() { - t.Run(format, func(t *testing.T) { - reader, err := compressReader(bytes.NewReader(payload), entities.ScpCompressionOptions{CompressionFormat: format}) - require.NoError(t, err) - defer reader.Close() - compressed, err := io.ReadAll(reader) - require.NoError(t, err) - assert.Less(t, len(compressed), len(payload), "compressed output should be smaller than the input") - - // docker-archive: c/image tarfile.Reader uses AutoDecompress. - viaCImage, isCompressed, err := compression.AutoDecompress(bytes.NewReader(compressed)) - require.NoError(t, err) - require.True(t, isCompressed) - defer viaCImage.Close() - roundTripped, err := io.ReadAll(viaCImage) - require.NoError(t, err) - assert.Equal(t, payload, roundTripped) - - // oci-archive: c/storage archive.Untar uses DecompressStream. - viaCStorage, err := archive.DecompressStream(bytes.NewReader(compressed)) - require.NoError(t, err) - defer viaCStorage.Close() - roundTripped, err = io.ReadAll(viaCStorage) - require.NoError(t, err) - assert.Equal(t, payload, roundTripped) - }) - } -} - -func TestCompressReaderWithLevel(t *testing.T) { - payload := []byte(strings.Repeat("podman image scp compression payload\n", 512)) - - level := 1 - reader, err := compressReader(bytes.NewReader(payload), entities.ScpCompressionOptions{ - CompressionFormat: "gzip", - CompressionLevel: &level, + t.Run("success returns the compressed path and cleans up nothing", func(t *testing.T) { + remote := &fakeRemote{} + got, err := compressRemoteFile(remote.exec, execOpts, ssh.GolangMode, "/tmp/tmp.XXXX", opts) + require.NoError(t, err) + assert.Equal(t, "/tmp/tmp.XXXX.gz", got) + assert.Equal(t, [][]string{{"gzip", "-f", "-q", "/tmp/tmp.XXXX"}}, remote.argv) }) - require.NoError(t, err) - defer reader.Close() - compressed, err := io.ReadAll(reader) - require.NoError(t, err) - assert.Less(t, len(compressed), len(payload)) + t.Run("a missing compressor is named as such", func(t *testing.T) { + remote := &fakeRemote{errs: []error{exitStatusError{status: cmdNotFoundStatus}}} + _, err := compressRemoteFile(remote.exec, execOpts, ssh.GolangMode, "/tmp/tmp.XXXX", opts) + assert.ErrorContains(t, err, `requires the "gzip" command on the remote host`) + }) + + t.Run("a compressor that fails for its own reasons is not blamed on the host", func(t *testing.T) { + // zstd exits 1 when it cannot write its output. + remote := &fakeRemote{errs: []error{exitStatusError{status: 1}}} + _, err := compressRemoteFile(remote.exec, execOpts, ssh.GolangMode, "/tmp/tmp.XXXX", opts) + assert.ErrorContains(t, err, `compressing "/tmp/tmp.XXXX" on the remote host`) + assert.NotContains(t, err.Error(), "requires the") + }) + + t.Run("any other failure is not blamed on a missing compressor", func(t *testing.T) { + remote := &fakeRemote{errs: []error{errors.New("failed to connect: no route to host")}} + _, err := compressRemoteFile(remote.exec, execOpts, ssh.GolangMode, "/tmp/tmp.XXXX", opts) + assert.ErrorContains(t, err, `compressing "/tmp/tmp.XXXX" on the remote host`) + assert.NotContains(t, err.Error(), "requires the") + }) + + t.Run("a failure leaves neither the archive nor a partial output behind", func(t *testing.T) { + remote := &fakeRemote{errs: []error{errors.New("no space left on device")}} + _, err := compressRemoteFile(remote.exec, execOpts, ssh.GolangMode, "/tmp/tmp.XXXX", opts) + require.Error(t, err) + require.Len(t, remote.argv, 2) + assert.Equal(t, []string{"rm", "-f", "/tmp/tmp.XXXX", "/tmp/tmp.XXXX.gz"}, remote.argv[1]) + }) + + t.Run("an unsupported format never reaches the remote host", func(t *testing.T) { + remote := &fakeRemote{} + _, err := compressRemoteFile(remote.exec, execOpts, ssh.GolangMode, "/tmp/tmp.XXXX", + entities.ScpCompressionOptions{CompressionFormat: "xz"}) + assert.ErrorContains(t, err, `unsupported compression format "xz"`) + assert.Empty(t, remote.argv) + }) } -// c/image can compress xz and zstd:chunked, so these catch compressReader going -// straight to it. "lz4" would not: c/image does not know it either. -func TestCompressReaderRejectsFormatsOutsideTheTable(t *testing.T) { - for _, format := range []string{"xz", "zstd:chunked", "lz4"} { - t.Run(format, func(t *testing.T) { - _, err := compressReader(bytes.NewReader(nil), entities.ScpCompressionOptions{CompressionFormat: format}) - assert.ErrorContains(t, err, "unsupported compression format") - assert.ErrorIs(t, err, define.ErrInvalidArg) +// The fakes below prove the accessor matching works; these prove it still matches +// the types the ssh engines actually return. +var ( + _ interface{ ExitStatus() int } = (*cryptossh.ExitError)(nil) + _ interface{ ExitCode() int } = (*exec.ExitError)(nil) +) + +// crypto/ssh's ExitError has an unexported status, so it cannot be built here. +type exitStatusError struct{ status int } + +func (e exitStatusError) ExitStatus() int { return e.status } +func (e exitStatusError) Error() string { + return fmt.Sprintf("Process exited with status %d", e.status) +} + +// os/exec's ExitError, as the native ssh engine returns for the local ssh binary. +type exitCodeError struct{ code int } + +func (e exitCodeError) ExitCode() int { return e.code } +func (e exitCodeError) Error() string { return fmt.Sprintf("exit status %d", e.code) } + +// The case that matters most: an error with no status, such as a failure to +// connect, must not be mistaken for a command that ran. +func TestRemoteExitStatus(t *testing.T) { + tests := []struct { + name string + err error + want int + }{ + { + name: "golang engine, command not found", + err: exitStatusError{status: cmdNotFoundStatus}, + want: cmdNotFoundStatus, + }, + { + name: "native engine, command not found", + err: exitCodeError{code: cmdNotFoundStatus}, + want: cmdNotFoundStatus, + }, + { + name: "wrapped, as the golang engine returns it alongside remote stderr", + err: fmt.Errorf("sh: gzip: command not found: %w", exitStatusError{status: cmdNotFoundStatus}), + want: cmdNotFoundStatus, + }, + { + name: "the command ran and failed for its own reasons", + err: exitStatusError{status: 1}, + want: 1, + }, + { + name: "never ran: no route to the host", + err: errors.New("failed to connect: dial tcp: no route to host"), + want: -1, + }, + { + name: "never ran, wrapped", + err: fmt.Errorf("ssh: %w", errors.New("handshake failed")), + want: -1, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, remoteExitStatus(tt.err)) }) } } -func TestCompressReaderPropagatesReadError(t *testing.T) { - reader, err := compressReader(&failingReader{}, entities.ScpCompressionOptions{CompressionFormat: "gzip"}) - require.NoError(t, err) - defer reader.Close() - - _, err = io.ReadAll(reader) - assert.ErrorContains(t, err, "read failed") -} - -// The path itself needs two hosts, so without this the flag could stop being -// honoured and every other test would still pass. // A level the compressor will not take has to fail before anything is streamed; // only the API path can reach this, as the flag path validates the range first. func TestCompressReaderRejectsUnusableLevel(t *testing.T) { From 8cec4284017dae9428e8d45c11161054b54f051f Mon Sep 17 00:00:00 2001 From: Scott Callaway Date: Wed, 5 Aug 2026 16:16:31 +0100 Subject: [PATCH 5/7] image scp: add --compression-format and --compression-level Expose the compression the transfer already knows how to do, and document what each option means on each path. --compression-format takes gzip or zstd, matching the vocabulary --compression-format already uses on podman push, minus the algorithms this cannot produce or detect. --compression-level takes the level, and is rejected without a format to apply it to rather than being silently ignored. The level needs one caveat spelling out in the man page. A remote source passes it to the command line compressor, where every value is distinct. A local source compresses through c/image, which groups zstd levels into four bands, so 10 and above are the same there. The accepted zstd range also stops at 19 rather than podman push's 20, because the command line compressor needs --ultra past that. The flags are validated before the engine is reached, so podman --remote reports a bad combination without a round trip; the transfer validates again for callers arriving over the API. Fixes: #23192 Signed-off-by: Scott Callaway --- cmd/podman/common/completion.go | 6 ++++ cmd/podman/images/scp.go | 32 +++++++++++++++++++--- docs/source/markdown/podman-image-scp.1.md | 32 ++++++++++++++++++++++ test/e2e/image_scp_test.go | 31 +++++++++++++++++++++ 4 files changed, 97 insertions(+), 4 deletions(-) diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 9c0371ea3d..62741b5d92 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -24,6 +24,7 @@ import ( "go.podman.io/podman/v6/libpod/define" "go.podman.io/podman/v6/libpod/events" "go.podman.io/podman/v6/pkg/domain/entities" + "go.podman.io/podman/v6/pkg/domain/utils" "go.podman.io/podman/v6/pkg/inspect" "go.podman.io/podman/v6/pkg/signal" systemdDefine "go.podman.io/podman/v6/pkg/systemd/define" @@ -1704,6 +1705,11 @@ func AutocompleteImageScpFormat(_ *cobra.Command, _ []string, _ string) ([]strin return ValidScpFormats, cobra.ShellCompDirectiveNoFileComp } +// AutocompleteImageScpCompressionFormat - Autocomplete image scp compression-format options. +func AutocompleteImageScpCompressionFormat(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { + return utils.ScpCompressionFormats(), cobra.ShellCompDirectiveNoFileComp +} + // AutocompleteWaitCondition - Autocomplete wait condition options. // -> "unknown", "configured", "created", "running", "stopped", "paused", "exited", "removing" func AutocompleteWaitCondition(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { diff --git a/cmd/podman/images/scp.go b/cmd/podman/images/scp.go index c87d3b2033..281e9604f8 100644 --- a/cmd/podman/images/scp.go +++ b/cmd/podman/images/scp.go @@ -5,11 +5,13 @@ import ( "strings" "github.com/spf13/cobra" + "go.podman.io/common/pkg/completion" "go.podman.io/common/pkg/ssh" "go.podman.io/podman/v6/cmd/podman/common" "go.podman.io/podman/v6/cmd/podman/registry" "go.podman.io/podman/v6/cmd/podman/validate" "go.podman.io/podman/v6/pkg/domain/entities" + "go.podman.io/podman/v6/pkg/domain/utils" ) var ( @@ -29,9 +31,11 @@ var ( ) var ( - parentFlags []string - quiet bool - format string + parentFlags []string + quiet bool + format string + scpCompressFormat string + scpCompressLevel int ) func init() { @@ -49,11 +53,30 @@ func scpFlags(cmd *cobra.Command) { formatChoice := validate.Value(&format, common.ValidScpFormats...) flags.Var(formatChoice, "format", "Format for `podman save` when creating the transfer archive ("+formatChoice.Choices()+"). Default is docker-archive when omitted.") _ = cmd.RegisterFlagCompletionFunc("format", common.AutocompleteImageScpFormat) + + compFormatFlagName := "compression-format" + compFormatChoice := validate.Value(&scpCompressFormat, utils.ScpCompressionFormats()...) + flags.Var(compFormatChoice, compFormatFlagName, "Compress the transfer archive with the given algorithm ("+compFormatChoice.Choices()+"). Default is no compression.") + _ = cmd.RegisterFlagCompletionFunc(compFormatFlagName, common.AutocompleteImageScpCompressionFormat) + + compLevelFlagName := "compression-level" + flags.IntVar(&scpCompressLevel, compLevelFlagName, 0, "Compression level to use") + _ = cmd.RegisterFlagCompletionFunc(compLevelFlagName, completion.AutocompleteNone) } -func scp(_ *cobra.Command, args []string) (finalErr error) { +func scp(cmd *cobra.Command, args []string) (finalErr error) { var err error + compressOpts := entities.ScpCompressionOptions{CompressionFormat: scpCompressFormat} + if cmd.Flags().Changed("compression-level") { + compressOpts.CompressionLevel = &scpCompressLevel + } + // Report a bad combination before anything expensive happens. The transfer + // validates again for the sake of callers coming in over the API. + if err := utils.ValidateScpCompression(compressOpts); err != nil { + return err + } + containerConfig := registry.PodmanConfig() sshType := containerConfig.SSHMode @@ -83,6 +106,7 @@ func scp(_ *cobra.Command, args []string) (finalErr error) { scpOpts.Quiet = quiet scpOpts.SSHMode = sshEngine scpOpts.SaveFormat = format + scpOpts.ScpCompressionOptions = compressOpts _, err = registry.ImageEngine().Scp(registry.Context(), src, dst, scpOpts) if err != nil { return err diff --git a/docs/source/markdown/podman-image-scp.1.md b/docs/source/markdown/podman-image-scp.1.md index 8a000a9623..e0ccba22e8 100644 --- a/docs/source/markdown/podman-image-scp.1.md +++ b/docs/source/markdown/podman-image-scp.1.md @@ -22,6 +22,26 @@ This is not a direct storage-to-storage copy. The image is saved to an archive ( ## OPTIONS +#### **--compression-format**=**gzip** | **zstd** + +Compress the transfer archive with the given algorithm before it is sent over the network. Supported values are `gzip` and `zstd`. The default is no compression: the archive is transferred as **podman save** wrote it. + +Because **podman save** writes docker-archive layers uncompressed, compressing the archive typically cuts the transferred data to around half its original size or less. The receiving **podman load** detects the compression and decompresses the archive itself, so nothing has to be configured on the destination. + +How much is gained depends on **--format**. An **oci-archive** keeps the layers in the compression they already have, so there is little left to compress and the transfer is barely smaller; the saving applies to the default **docker-archive**. + +Compression is applied on the host that produces the archive, so that only compressed bytes cross the network. When the source is a remote host, the archive is compressed there and the matching command line compressor (**gzip** or **zstd**) must be installed on that host. When the source is local, Podman compresses the archive itself and no extra tooling is needed. + +This option has no effect on a transfer between two users on the same machine, because no data crosses a network. + +#### **--compression-level**=*level* + +Compression level to use, **1**-**9** for **gzip** and **1**-**19** for **zstd**. The default is whatever the chosen algorithm defaults to, as no level is passed to it. Requires **--compression-format**. + +The accepted range for **zstd** stops at **19** rather than the **20** accepted by **podman push**, because levels above that need the compressor's *--ultra* mode when the archive is compressed on a remote host. + +Note that **zstd** levels are only fully distinct when the source is a remote host, where the level is passed to the command line compressor. When the source is local, Podman compresses through the same library used elsewhere, which groups the level into four bands (**1**-**2**, **3**-**5**, **6**-**9**, **10** and above), so any level of **10** or more produces the same output. + #### **--format**=*format* Format passed to **podman save** when creating the transfer archive. Allowed values are **oci-archive** and **docker-archive**. If omitted, **podman save** uses its default (docker-archive). @@ -113,6 +133,18 @@ Copy image to remote host (uses default format when **--format** is omitted): $ podman image scp alpine root@myserver:: ``` +Copy specified image to a remote host, compressing the transfer archive with zstd: +``` +$ podman image scp --compression-format zstd alpine root@myserver:: +Loaded image: docker.io/library/alpine:latest +``` + +Copy specified image to a remote host, trading CPU time for the smallest transfer: +``` +$ podman image scp --compression-format zstd --compression-level 19 alpine root@myserver:: +Loaded image: docker.io/library/alpine:latest +``` + ## SEE ALSO **[podman(1)](podman.1.md)**, **[podman-load(1)](podman-load.1.md)**, **[podman-save(1)](podman-save.1.md)**, **[podman-remote(1)](podman-remote.1.md)**, **[podman-system-connection-add(1)](podman-system-connection-add.1.md)**, **[containers.conf(5)](https://github.com/containers/container-libs/blob/main/common/docs/containers.conf.5.md)**, **[containers-transports(5)](https://github.com/containers/image/blob/main/docs/containers-transports.5.md)** diff --git a/test/e2e/image_scp_test.go b/test/e2e/image_scp_test.go index 4a9c49d191..b98ac3ccf2 100644 --- a/test/e2e/image_scp_test.go +++ b/test/e2e/image_scp_test.go @@ -63,4 +63,35 @@ var _ = Describe("podman image scp", func() { scp.WaitWithDefaultTimeout() Expect(scp).Should(ExitWithError(125, "unknown user user@domain")) }) + + It("podman image scp rejects an unsupported compression format", func() { + scp := podmanTest.Podman([]string{"image", "scp", "--compression-format", "bzip2", ALPINE, "QA::"}) + scp.WaitWithDefaultTimeout() + // Single space: ErrorToString collapses whitespace, the message has two. + Expect(scp).Should(ExitWithError(125, `"bzip2" is not a valid value. Choose from: "gzip, zstd"`)) + }) + + It("podman image scp rejects a compression level without a format", func() { + scp := podmanTest.Podman([]string{"image", "scp", "--compression-level", "9", ALPINE, "QA::"}) + scp.WaitWithDefaultTimeout() + Expect(scp).Should(ExitWithError(125, "a compression level requires a compression format: invalid argument")) + }) + + It("podman image scp rejects an out of range compression level", func() { + scp := podmanTest.Podman([]string{"image", "scp", "--compression-format", "gzip", "--compression-level", "10", ALPINE, "QA::"}) + scp.WaitWithDefaultTimeout() + Expect(scp).Should(ExitWithError(125, `compression level 10 is out of range for "gzip", must be between 1 and 9: invalid argument`)) + }) + + It("podman image scp ignores compression for a local user to user transfer", func() { + SkipIfRootless("the local user lookup only happens during a rootful transfer") + + // Asking for compression here is a no-op, not an error. The bogus user + // makes the transfer fail after the warning is emitted. + scp := podmanTest.Podman([]string{"image", "scp", "--compression-format", "zstd", "user@domain@localhost::" + ALPINE}) + scp.WaitWithDefaultTimeout() + Expect(scp).Should(ExitWithError(125, "unknown user user@domain")) + // Loose around the format name: logrus escapes the quotes it puts round it. + Expect(scp.ErrorToString()).To(MatchRegexp(`Ignoring compression format .*zstd.*: it only applies to transfers over ssh`)) + }) }) From 8d5b091a450480ab2149306f0ab2432ed8f04276 Mon Sep 17 00:00:00 2001 From: Scott Callaway Date: Wed, 5 Aug 2026 16:16:31 +0100 Subject: [PATCH 6/7] image scp: pass the compression options over the remote API The tunnel engine builds its own ScpOptions, so without this the flags parse fine under podman --remote and are then dropped, transferring uncompressed with no indication that anything was ignored. Carry both options through the bindings to the libpod ImageScp handler, which hands them to ExecuteTransfer the same way the local path does, and document them on the endpoint. This is also the point at which the transfer's own validation becomes reachable over HTTP, so map it accordingly: a rejected format or level is the caller's mistake and answers 400, not the 500 every error from the transfer used to produce. Signed-off-by: Scott Callaway --- pkg/api/handlers/libpod/images.go | 14 ++++++++++++-- pkg/api/server/register_images.go | 10 ++++++++++ pkg/bindings/images/types.go | 4 ++++ pkg/bindings/test/types_test.go | 21 +++++++++++++++++++++ pkg/domain/infra/tunnel/images.go | 4 ++++ test/apiv2/12-imagesMore.at | 10 ++++++++++ 6 files changed, 61 insertions(+), 2 deletions(-) diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index 3ab226cacd..41100e0ef2 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -746,8 +746,10 @@ func ImagesRemove(w http.ResponseWriter, r *http.Request) { func ImageScp(w http.ResponseWriter, r *http.Request) { decoder := r.Context().Value(api.DecoderKey).(*schema.Decoder) query := struct { - Destination string `schema:"destination"` - Quiet bool `schema:"quiet"` + Destination string `schema:"destination"` + Quiet bool `schema:"quiet"` + CompressionFormat string `schema:"compressionFormat"` + CompressionLevel *int `schema:"compressionLevel"` }{ // This is where you can override the golang default value for one of fields } @@ -761,8 +763,16 @@ func ImageScp(w http.ResponseWriter, r *http.Request) { opts := entities.ScpExecuteTransferOptions{} opts.Quiet = query.Quiet opts.SSHMode = ssh.GolangMode + opts.CompressionFormat = query.CompressionFormat + opts.CompressionLevel = query.CompressionLevel report, err := domainUtils.ExecuteTransfer(sourceArg, query.Destination, opts) if err != nil { + // The transfer validates its options, so a rejected compression format or + // level is the caller's mistake rather than a fault on this end. + if errors.Is(err, define.ErrInvalidArg) { + utils.Error(w, http.StatusBadRequest, err) + return + } utils.Error(w, http.StatusInternalServerError, err) return } diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index d973e2f323..4614ded79e 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -2286,6 +2286,16 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // description: quiet output // type: boolean // default: false + // - in: query + // name: compressionFormat + // required: false + // description: compress the transfer archive with this algorithm (gzip, zstd) + // type: string + // - in: query + // name: compressionLevel + // required: false + // description: compression level to use + // type: integer // produces: // - application/json // responses: diff --git a/pkg/bindings/images/types.go b/pkg/bindings/images/types.go index b8c118ae2a..682abc1f93 100644 --- a/pkg/bindings/images/types.go +++ b/pkg/bindings/images/types.go @@ -243,4 +243,8 @@ type ExistsOptions struct{} type ScpOptions struct { Quiet *bool Destination *string + // CompressionFormat is the algorithm used to compress the transfer archive. + CompressionFormat *string `schema:"compressionFormat"` + // CompressionLevel is the level handed to the compressor. + CompressionLevel *int `schema:"compressionLevel"` } diff --git a/pkg/bindings/test/types_test.go b/pkg/bindings/test/types_test.go index 9e89bf4f65..8467c99bc8 100644 --- a/pkg/bindings/test/types_test.go +++ b/pkg/bindings/test/types_test.go @@ -39,6 +39,27 @@ var _ = Describe("Binding types", func() { Expect(params.Has("skiptlsverify")).To(BeFalse()) }) + It("serialize image scp options", func() { + // The names here are what the libpod ImageScp handler decodes, so a + // rename on either side silently drops compression on the remote client. + format := "zstd" + level := 3 + opts := &images.ScpOptions{CompressionFormat: &format, CompressionLevel: &level} + params, err := opts.ToParams() + Expect(err).ToNot(HaveOccurred()) + Expect(params.Get("compressionFormat")).To(Equal("zstd")) + Expect(params.Get("compressionLevel")).To(Equal("3")) + }) + + It("serialize image scp options without compression", func() { + // An unset level must not be sent as 0: the server would reject it. + opts := &images.ScpOptions{} + params, err := opts.ToParams() + Expect(err).ToNot(HaveOccurred()) + Expect(params.Has("compressionFormat")).To(BeFalse()) + Expect(params.Has("compressionLevel")).To(BeFalse()) + }) + It("serialize manifest modify options", func() { opts := new(manifests.ModifyOptions).WithOS("foo").WithSkipTLSVerify(true) params, err := opts.ToParams() diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 7b34120f8a..0d926af282 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -466,6 +466,10 @@ func (ir *ImageEngine) Scp(_ context.Context, src, dst string, opts entities.Ima } options.Quiet = &opts.Quiet options.Destination = destination + if opts.CompressionFormat != "" { + options.CompressionFormat = &opts.CompressionFormat + } + options.CompressionLevel = opts.CompressionLevel rep, err := images.Scp(ir.ClientCtx, &src, destination, *options) if err != nil { diff --git a/test/apiv2/12-imagesMore.at b/test/apiv2/12-imagesMore.at index c35be5860f..5b67d8ce50 100644 --- a/test/apiv2/12-imagesMore.at +++ b/test/apiv2/12-imagesMore.at @@ -93,6 +93,16 @@ podman system connection add --default $conn \ # cirrus weirdness with exec.Command. All of the args have been parsed successfully. t POST "libpod/images/scp/$IMAGE?destination=QA::" 500 \ .cause="exit status 125" + +# Bad compression options are the caller's mistake, so they must come back as 400 +# rather than 500, and be rejected before the transfer is attempted at all. +t POST "libpod/images/scp/$IMAGE?destination=QA::&compressionFormat=bzip2" 400 \ + .cause="invalid argument" +t POST "libpod/images/scp/$IMAGE?destination=QA::&compressionFormat=gzip&compressionLevel=10" 400 \ + .cause="invalid argument" +t POST "libpod/images/scp/$IMAGE?destination=QA::&compressionLevel=9" 400 \ + .cause="invalid argument" + t DELETE libpod/images/$IMAGE 200 \ .ExitCode=0 From 79b35ad0d9f2bf7477da6a5711c446aefdbe9282 Mon Sep 17 00:00:00 2001 From: Scott Callaway Date: Thu, 10 Sep 2026 16:45:30 +0100 Subject: [PATCH 7/7] image scp: accept --compression-format=none Until now the default could only be expressed by leaving the option off, which reads as an omission rather than a choice and gives a script no way to say it wants the archive transferred as podman save wrote it. Accept none as a format meaning exactly that. It is taken on the API path too, so both interfaces share one vocabulary, and it is treated as the absence of a format throughout: nothing is compressed, a level attached to it is rejected the same way a level with no format is, and the local user to user transfer has nothing to warn about ignoring. The remote client still leaves it off the request, so naming the default does not make a transfer fail against a service that predates these options. Signed-off-by: Scott Callaway --- cmd/podman/common/completion.go | 2 +- cmd/podman/images/scp.go | 4 +- docs/source/markdown/podman-image-scp.1.md | 6 +- pkg/api/server/register_images.go | 2 +- pkg/domain/entities/scp.go | 3 +- pkg/domain/infra/tunnel/images.go | 4 +- pkg/domain/utils/scp.go | 6 +- pkg/domain/utils/scp_compression.go | 24 +++++++- pkg/domain/utils/scp_compression_test.go | 72 ++++++++++++++++------ test/e2e/image_scp_test.go | 21 ++++++- 10 files changed, 109 insertions(+), 35 deletions(-) diff --git a/cmd/podman/common/completion.go b/cmd/podman/common/completion.go index 62741b5d92..74b5c3fa50 100644 --- a/cmd/podman/common/completion.go +++ b/cmd/podman/common/completion.go @@ -1707,7 +1707,7 @@ func AutocompleteImageScpFormat(_ *cobra.Command, _ []string, _ string) ([]strin // AutocompleteImageScpCompressionFormat - Autocomplete image scp compression-format options. func AutocompleteImageScpCompressionFormat(_ *cobra.Command, _ []string, _ string) ([]string, cobra.ShellCompDirective) { - return utils.ScpCompressionFormats(), cobra.ShellCompDirectiveNoFileComp + return utils.ScpCompressionValues(), cobra.ShellCompDirectiveNoFileComp } // AutocompleteWaitCondition - Autocomplete wait condition options. diff --git a/cmd/podman/images/scp.go b/cmd/podman/images/scp.go index 281e9604f8..0ce2f7acf7 100644 --- a/cmd/podman/images/scp.go +++ b/cmd/podman/images/scp.go @@ -55,8 +55,8 @@ func scpFlags(cmd *cobra.Command) { _ = cmd.RegisterFlagCompletionFunc("format", common.AutocompleteImageScpFormat) compFormatFlagName := "compression-format" - compFormatChoice := validate.Value(&scpCompressFormat, utils.ScpCompressionFormats()...) - flags.Var(compFormatChoice, compFormatFlagName, "Compress the transfer archive with the given algorithm ("+compFormatChoice.Choices()+"). Default is no compression.") + compFormatChoice := validate.Value(&scpCompressFormat, utils.ScpCompressionValues()...) + flags.Var(compFormatChoice, compFormatFlagName, "Compress the transfer archive with the given algorithm ("+compFormatChoice.Choices()+"). Default is none.") _ = cmd.RegisterFlagCompletionFunc(compFormatFlagName, common.AutocompleteImageScpCompressionFormat) compLevelFlagName := "compression-level" diff --git a/docs/source/markdown/podman-image-scp.1.md b/docs/source/markdown/podman-image-scp.1.md index e0ccba22e8..d732439ad3 100644 --- a/docs/source/markdown/podman-image-scp.1.md +++ b/docs/source/markdown/podman-image-scp.1.md @@ -22,9 +22,9 @@ This is not a direct storage-to-storage copy. The image is saved to an archive ( ## OPTIONS -#### **--compression-format**=**gzip** | **zstd** +#### **--compression-format**=**gzip** | **zstd** | **none** -Compress the transfer archive with the given algorithm before it is sent over the network. Supported values are `gzip` and `zstd`. The default is no compression: the archive is transferred as **podman save** wrote it. +Compress the transfer archive with the given algorithm before it is sent over the network. Supported values are `gzip` and `zstd`. The default is **none**, which transfers the archive as **podman save** wrote it. Because **podman save** writes docker-archive layers uncompressed, compressing the archive typically cuts the transferred data to around half its original size or less. The receiving **podman load** detects the compression and decompresses the archive itself, so nothing has to be configured on the destination. @@ -36,7 +36,7 @@ This option has no effect on a transfer between two users on the same machine, b #### **--compression-level**=*level* -Compression level to use, **1**-**9** for **gzip** and **1**-**19** for **zstd**. The default is whatever the chosen algorithm defaults to, as no level is passed to it. Requires **--compression-format**. +Compression level to use, **1**-**9** for **gzip** and **1**-**19** for **zstd**. The default is whatever the chosen algorithm defaults to, as no level is passed to it. Requires **--compression-format** to name an algorithm. The accepted range for **zstd** stops at **19** rather than the **20** accepted by **podman push**, because levels above that need the compressor's *--ultra* mode when the archive is compressed on a remote host. diff --git a/pkg/api/server/register_images.go b/pkg/api/server/register_images.go index 4614ded79e..32d68c148f 100644 --- a/pkg/api/server/register_images.go +++ b/pkg/api/server/register_images.go @@ -2289,7 +2289,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error { // - in: query // name: compressionFormat // required: false - // description: compress the transfer archive with this algorithm (gzip, zstd) + // description: compress the transfer archive with this algorithm (gzip, zstd, none) // type: string // - in: query // name: compressionLevel diff --git a/pkg/domain/entities/scp.go b/pkg/domain/entities/scp.go index b0e4abf6c6..f344539849 100644 --- a/pkg/domain/entities/scp.go +++ b/pkg/domain/entities/scp.go @@ -9,7 +9,8 @@ import ( // ScpCompressionOptions describes how the transfer archive should be compressed. type ScpCompressionOptions struct { // CompressionFormat is the algorithm used to compress the archive before it - // is sent over the network. An empty string disables compression. + // is sent over the network. An empty string and "none" both disable + // compression. CompressionFormat string `json:"compressionFormat,omitempty"` // CompressionLevel is the level handed to the compressor. A nil value uses // the algorithm's default. diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 0d926af282..23c837877e 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -466,7 +466,9 @@ func (ir *ImageEngine) Scp(_ context.Context, src, dst string, opts entities.Ima } options.Quiet = &opts.Quiet options.Destination = destination - if opts.CompressionFormat != "" { + // "none" is only spelled out for the CLI; leaving it off the request keeps a + // service that predates these options from rejecting it. + if utils.ScpCompressionRequested(opts.CompressionFormat) { options.CompressionFormat = &opts.CompressionFormat } options.CompressionLevel = opts.CompressionLevel diff --git a/pkg/domain/utils/scp.go b/pkg/domain/utils/scp.go index 53a317b2bd..4f1cdb3cf2 100644 --- a/pkg/domain/utils/scp.go +++ b/pkg/domain/utils/scp.go @@ -184,7 +184,7 @@ func ExecuteTransfer(src, dst string, opts entities.ScpExecuteTransferOptions) ( return nil, err } default: // else native load, both source and dest are local and transferring between users - if opts.CompressionFormat != "" { + if ScpCompressionRequested(opts.CompressionFormat) { // Nothing crosses the network here, so compressing would only burn CPU. logrus.Warnf("Ignoring compression format %q: it only applies to transfers over ssh", opts.CompressionFormat) } @@ -285,7 +285,7 @@ func loadToRemote(run remoteRunner, opts entities.ScpLoadToRemoteOptions) (*enti defer input.Close() var stream io.Reader = input - if opts.CompressionFormat != "" { + if ScpCompressionRequested(opts.CompressionFormat) { // The remote podman load detects the compression itself. compressed, err := compressReader(input, opts.ScpCompressionOptions) if err != nil { @@ -391,7 +391,7 @@ func saveToRemote(run remoteRunner, opts entities.ScpSaveToRemoteOptions) (*enti return nil, err } - if opts.CompressionFormat != "" { + if ScpCompressionRequested(opts.CompressionFormat) { // Compress it where it is, so only compressed bytes are copied over the // network. compressedFile, err := compressRemoteFile(run.exec, execOpts, opts.SSHMode, remoteFile, opts.ScpCompressionOptions) diff --git a/pkg/domain/utils/scp_compression.go b/pkg/domain/utils/scp_compression.go index 41a3b0f1f0..4daa75eb7a 100644 --- a/pkg/domain/utils/scp_compression.go +++ b/pkg/domain/utils/scp_compression.go @@ -40,17 +40,35 @@ var scpCompressionFormats = map[string]scpCompressionFormat{ "zstd": {bin: "zstd", args: []string{"-f", "-q", "--rm"}, ext: ".zst", minLevel: 1, maxLevel: 19}, } -// ScpCompressionFormats lists the accepted --compression-format values. +// ScpCompressionNone asks for no compression, which is what omitting the format +// does too. It exists so the default can be spelled out rather than only +// expressed by leaving the option off. +const ScpCompressionNone = "none" + +// ScpCompressionFormats lists the algorithms the archive can be compressed with. func ScpCompressionFormats() []string { return slices.Sorted(maps.Keys(scpCompressionFormats)) } +// ScpCompressionValues lists the accepted --compression-format values: every +// algorithm plus the explicit opt out, kept last so it reads as an aside to the +// algorithms rather than one of them. +func ScpCompressionValues() []string { + return append(ScpCompressionFormats(), ScpCompressionNone) +} + +// ScpCompressionRequested reports whether format asks for the archive to be +// compressed. An empty format and ScpCompressionNone both say it does not. +func ScpCompressionRequested(format string) bool { + return format != "" && format != ScpCompressionNone +} + // scpCompressionFormatByName gives every caller the same rejection wording. func scpCompressionFormatByName(name string) (scpCompressionFormat, error) { format, ok := scpCompressionFormats[name] if !ok { return scpCompressionFormat{}, fmt.Errorf("unsupported compression format %q, choose from: %s: %w", - name, strings.Join(ScpCompressionFormats(), ", "), define.ErrInvalidArg) + name, strings.Join(ScpCompressionValues(), ", "), define.ErrInvalidArg) } return format, nil } @@ -59,7 +77,7 @@ func scpCompressionFormatByName(name string) (scpCompressionFormat, error) { // that the level, if any, is in range. The errors avoid flag names because this // also runs on the API path. func ValidateScpCompression(opts entities.ScpCompressionOptions) error { - if opts.CompressionFormat == "" { + if !ScpCompressionRequested(opts.CompressionFormat) { if opts.CompressionLevel != nil { return fmt.Errorf("a compression level requires a compression format: %w", define.ErrInvalidArg) } diff --git a/pkg/domain/utils/scp_compression_test.go b/pkg/domain/utils/scp_compression_test.go index caa60d105a..064288b920 100644 --- a/pkg/domain/utils/scp_compression_test.go +++ b/pkg/domain/utils/scp_compression_test.go @@ -35,6 +35,17 @@ func TestValidateScpCompression(t *testing.T) { name: "no compression requested", opts: entities.ScpCompressionOptions{}, }, + { + name: "none is an explicit no compression", + opts: entities.ScpCompressionOptions{CompressionFormat: ScpCompressionNone}, + }, + { + name: "level with none", + opts: entities.ScpCompressionOptions{CompressionFormat: ScpCompressionNone, CompressionLevel: level(9)}, + // none compresses nothing, so a level attached to it is as pointless + // as one with no format at all. + wantErr: "a compression level requires a compression format", + }, { name: "level without a format", opts: entities.ScpCompressionOptions{CompressionLevel: level(9)}, @@ -93,14 +104,27 @@ func TestValidateScpCompression(t *testing.T) { } } -// The list is part of the command's interface: it drives the flag's choices. func TestScpCompressionFormatsAreUsable(t *testing.T) { assert.Equal(t, []string{"gzip", "zstd"}, ScpCompressionFormats()) for _, format := range ScpCompressionFormats() { assert.NoError(t, ValidateScpCompression(entities.ScpCompressionOptions{CompressionFormat: format})) + assert.True(t, ScpCompressionRequested(format)) } } +// The list is part of the command's interface: it drives the flag's choices. +func TestScpCompressionValuesAreAccepted(t *testing.T) { + assert.Equal(t, []string{"gzip", "zstd", "none"}, ScpCompressionValues()) + for _, value := range ScpCompressionValues() { + assert.NoError(t, ValidateScpCompression(entities.ScpCompressionOptions{CompressionFormat: value})) + } +} + +func TestScpCompressionRequested(t *testing.T) { + assert.False(t, ScpCompressionRequested("")) + assert.False(t, ScpCompressionRequested(ScpCompressionNone)) +} + // The feature rests on podman load recognising the compression unaided, and the // two archive formats reach that through different detectors. func TestCompressReaderIsDetectedByBothLoadPaths(t *testing.T) { @@ -327,21 +351,27 @@ func TestSaveToRemoteCompressesBeforeCopying(t *testing.T) { assert.Equal(t, "/local/archive", remote.scpOpts[0].Destination) }) - t.Run("without a format the archive is copied as saved", func(t *testing.T) { - remote := &fakeRemote{out: []string{"/tmp/tmp.XXXX\n"}} + // The flag's "none" has to end up where the omission an empty format stands + // for does: the archive copied exactly as podman save wrote it. + for _, format := range []string{"", ScpCompressionNone} { + t.Run(fmt.Sprintf("format %q copies the archive as saved", format), func(t *testing.T) { + remote := &fakeRemote{out: []string{"/tmp/tmp.XXXX\n"}} + opts := baseOpts + opts.ScpCompressionOptions = entities.ScpCompressionOptions{CompressionFormat: format} - _, err := saveToRemote(remote.runner(), baseOpts) - require.NoError(t, err) + _, err := saveToRemote(remote.runner(), opts) + require.NoError(t, err) - assert.Equal(t, [][]string{ - {"mktemp"}, - {"podman", "image", "save", "alpine", "--output", "/tmp/tmp.XXXX"}, - {"rm", "-f", "/tmp/tmp.XXXX"}, - }, remote.argv) + assert.Equal(t, [][]string{ + {"mktemp"}, + {"podman", "image", "save", "alpine", "--output", "/tmp/tmp.XXXX"}, + {"rm", "-f", "/tmp/tmp.XXXX"}, + }, remote.argv) - require.Len(t, remote.scpOpts, 1) - assert.Equal(t, "ssh://root@example.test:/tmp/tmp.XXXX", remote.scpOpts[0].Source) - }) + require.Len(t, remote.scpOpts, 1) + assert.Equal(t, "ssh://root@example.test:/tmp/tmp.XXXX", remote.scpOpts[0].Source) + }) + } t.Run("a compressor that fails stops the transfer before anything is copied", func(t *testing.T) { remote := &fakeRemote{ @@ -568,13 +598,17 @@ func TestLoadToRemoteCompressesTheStream(t *testing.T) { // The remote to remote path will rely on this: once the archive has been // compressed on the source host, this leg has to stream it untouched rather // than compress it a second time. - t.Run("without a format the file is streamed as it is", func(t *testing.T) { - remote := &fakeRemote{out: []string{"Loaded image: quay.io/libpod/alpine:latest"}} + for _, format := range []string{"", ScpCompressionNone} { + t.Run(fmt.Sprintf("format %q streams the file as it is", format), func(t *testing.T) { + remote := &fakeRemote{out: []string{"Loaded image: quay.io/libpod/alpine:latest"}} + opts := baseOpts + opts.ScpCompressionOptions = entities.ScpCompressionOptions{CompressionFormat: format} - _, err := loadToRemote(remote.runner(), baseOpts) - require.NoError(t, err) - assert.Equal(t, payload, remote.input) - }) + _, err := loadToRemote(remote.runner(), opts) + require.NoError(t, err) + assert.Equal(t, payload, remote.input) + }) + } } var errRead = errors.New("read failed") diff --git a/test/e2e/image_scp_test.go b/test/e2e/image_scp_test.go index b98ac3ccf2..76d015c4f0 100644 --- a/test/e2e/image_scp_test.go +++ b/test/e2e/image_scp_test.go @@ -68,7 +68,7 @@ var _ = Describe("podman image scp", func() { scp := podmanTest.Podman([]string{"image", "scp", "--compression-format", "bzip2", ALPINE, "QA::"}) scp.WaitWithDefaultTimeout() // Single space: ErrorToString collapses whitespace, the message has two. - Expect(scp).Should(ExitWithError(125, `"bzip2" is not a valid value. Choose from: "gzip, zstd"`)) + Expect(scp).Should(ExitWithError(125, `"bzip2" is not a valid value. Choose from: "gzip, zstd, none"`)) }) It("podman image scp rejects a compression level without a format", func() { @@ -77,6 +77,14 @@ var _ = Describe("podman image scp", func() { Expect(scp).Should(ExitWithError(125, "a compression level requires a compression format: invalid argument")) }) + It("podman image scp rejects a compression level with the none format", func() { + // none is accepted as a format but compresses nothing, so a level with it + // is as pointless as one on its own. + scp := podmanTest.Podman([]string{"image", "scp", "--compression-format", "none", "--compression-level", "9", ALPINE, "QA::"}) + scp.WaitWithDefaultTimeout() + Expect(scp).Should(ExitWithError(125, "a compression level requires a compression format: invalid argument")) + }) + It("podman image scp rejects an out of range compression level", func() { scp := podmanTest.Podman([]string{"image", "scp", "--compression-format", "gzip", "--compression-level", "10", ALPINE, "QA::"}) scp.WaitWithDefaultTimeout() @@ -94,4 +102,15 @@ var _ = Describe("podman image scp", func() { // Loose around the format name: logrus escapes the quotes it puts round it. Expect(scp.ErrorToString()).To(MatchRegexp(`Ignoring compression format .*zstd.*: it only applies to transfers over ssh`)) }) + + It("podman image scp says nothing about compression for the none format", func() { + SkipIfRootless("the local user lookup only happens during a rootful transfer") + + // none asks for the default, so it has nothing to ignore and nothing to + // warn about. The bogus user fails the transfer as above. + scp := podmanTest.Podman([]string{"image", "scp", "--compression-format", "none", "user@domain@localhost::" + ALPINE}) + scp.WaitWithDefaultTimeout() + Expect(scp).Should(ExitWithError(125, "unknown user user@domain")) + Expect(scp.ErrorToString()).NotTo(ContainSubstring("Ignoring compression format")) + }) })