From ea095bd64d02fb9bcca6729d1fbcfcdfe7c2aa71 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Tue, 4 Nov 2025 12:44:26 +0100 Subject: [PATCH 1/5] print error on output arguments like `type=invalid` Signed-off-by: iTrooz --- vendor/github.com/containers/buildah/pkg/parse/parse.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/vendor/github.com/containers/buildah/pkg/parse/parse.go b/vendor/github.com/containers/buildah/pkg/parse/parse.go index 911d5dedac1..a7ffa4b8d00 100644 --- a/vendor/github.com/containers/buildah/pkg/parse/parse.go +++ b/vendor/github.com/containers/buildah/pkg/parse/parse.go @@ -721,7 +721,9 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { IsStdout: true, }, nil } - if !strings.Contains(buildOutput, ",") { + + // Support simple values, in the form --output ./mydir + if !strings.Contains(buildOutput, ",") && !strings.Contains(buildOutput, "=") { // expect default --output return define.BuildOutputOption{ Path: buildOutput, @@ -729,6 +731,8 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { IsStdout: false, }, nil } + + // Support complex values, in the form --output type=local,dest=./mydir isDir := true isStdout := false typeSelected := "" From bafcaa31c64f770a6a27fc7b991fd3205145e82a Mon Sep 17 00:00:00 2001 From: iTrooz Date: Tue, 4 Nov 2025 13:42:50 +0100 Subject: [PATCH 2/5] refactor: make BuildOutputType store type of export requested Signed-off-by: iTrooz --- .../containers/buildah/define/types.go | 13 ++++-- .../containers/buildah/internal/util/util.go | 4 +- .../containers/buildah/pkg/cli/build.go | 2 +- .../containers/buildah/pkg/parse/parse.go | 46 ++++++++++--------- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/vendor/github.com/containers/buildah/define/types.go b/vendor/github.com/containers/buildah/define/types.go index 022634ce551..0c254c9491e 100644 --- a/vendor/github.com/containers/buildah/define/types.go +++ b/vendor/github.com/containers/buildah/define/types.go @@ -109,11 +109,18 @@ type Secret struct { SourceType string } +type BuildOutputType int + +const ( + BuildOutputStdout BuildOutputType = 0 // stream tar to stdout + BuildOutputLocalDir BuildOutputType = 1 + BuildOutputTar BuildOutputType = 2 +) + // BuildOutputOptions contains the the outcome of parsing the value of a build --output flag type BuildOutputOption struct { - Path string // Only valid if !IsStdout - IsDir bool - IsStdout bool + Type BuildOutputType + Path string // Only valid if Type is local dir or tar } // ConfidentialWorkloadOptions encapsulates options which control whether or not diff --git a/vendor/github.com/containers/buildah/internal/util/util.go b/vendor/github.com/containers/buildah/internal/util/util.go index b230d0e3055..ac10ee0af24 100644 --- a/vendor/github.com/containers/buildah/internal/util/util.go +++ b/vendor/github.com/containers/buildah/internal/util/util.go @@ -58,7 +58,7 @@ func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error { return err } } - if opts.IsDir { + if opts.Type == define.BuildOutputLocalDir { // In order to keep this feature as close as possible to // buildkit it was decided to preserve ownership when // invoked as root since caller already has access to artifacts @@ -80,7 +80,7 @@ func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error { } } else { outFile := os.Stdout - if !opts.IsStdout { + if opts.Type != define.BuildOutputStdout { if outFile, err = os.Create(opts.Path); err != nil { return fmt.Errorf("failed while creating destination tar at %q: %w", opts.Path, err) } diff --git a/vendor/github.com/containers/buildah/pkg/cli/build.go b/vendor/github.com/containers/buildah/pkg/cli/build.go index 9449ac883a7..1ae57b3546f 100644 --- a/vendor/github.com/containers/buildah/pkg/cli/build.go +++ b/vendor/github.com/containers/buildah/pkg/cli/build.go @@ -279,7 +279,7 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) ( if err != nil { return options, nil, nil, err } - if buildOption.IsStdout { + if buildOption.Type == define.BuildOutputStdout { iopts.Quiet = true } } diff --git a/vendor/github.com/containers/buildah/pkg/parse/parse.go b/vendor/github.com/containers/buildah/pkg/parse/parse.go index a7ffa4b8d00..634e2038a9f 100644 --- a/vendor/github.com/containers/buildah/pkg/parse/parse.go +++ b/vendor/github.com/containers/buildah/pkg/parse/parse.go @@ -716,9 +716,8 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { // Feature parity with buildkit, output tar to stdout // Read more here: https://docs.docker.com/engine/reference/commandline/build/#custom-build-outputs return define.BuildOutputOption{ - Path: "", - IsDir: false, - IsStdout: true, + Type: define.BuildOutputStdout, + Path: "", }, nil } @@ -726,16 +725,13 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { if !strings.Contains(buildOutput, ",") && !strings.Contains(buildOutput, "=") { // expect default --output return define.BuildOutputOption{ - Path: buildOutput, - IsDir: true, - IsStdout: false, + Type: define.BuildOutputLocalDir, + Path: buildOutput, }, nil } // Support complex values, in the form --output type=local,dest=./mydir - isDir := true - isStdout := false - typeSelected := "" + var typeSelected define.BuildOutputType = -1 pathSelected := "" for option := range strings.SplitSeq(buildOutput, ",") { key, value, found := strings.Cut(option, "=") @@ -744,15 +740,14 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { } switch key { case "type": - if typeSelected != "" { + if typeSelected != -1 { return define.BuildOutputOption{}, fmt.Errorf("duplicate %q not supported", key) } - typeSelected = value - switch typeSelected { + switch value { case "local": - isDir = true + typeSelected = define.BuildOutputLocalDir case "tar": - isDir = false + typeSelected = define.BuildOutputTar default: return define.BuildOutputOption{}, fmt.Errorf("invalid type %q selected for build output options %q", value, buildOutput) } @@ -766,17 +761,26 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { } } - if typeSelected == "" || pathSelected == "" { - return define.BuildOutputOption{}, fmt.Errorf(`invalid build output option %q, accepted keys are "type" and "dest" must be present`, buildOutput) + // Validation + if typeSelected == -1 { + return define.BuildOutputOption{}, fmt.Errorf("missing required key %q in build output option: %q", "type", buildOutput) } - - if pathSelected == "-" { - if isDir { - return define.BuildOutputOption{}, fmt.Errorf(`invalid build output option %q, "type=local" can not be used with "dest=-"`, buildOutput) + if typeSelected == define.BuildOutputLocalDir || typeSelected == define.BuildOutputTar { + if pathSelected == "" { + return define.BuildOutputOption{}, fmt.Errorf("missing required key %q in build output option: %q", "dest", buildOutput) } + } else { + // Clear path selected when not needed by type + pathSelected = "" + } + if typeSelected == define.BuildOutputLocalDir && pathSelected == "-" { + return define.BuildOutputOption{}, fmt.Errorf(`invalid build output option %q, "type=local" can not be used with "dest=-"`, buildOutput) } - return define.BuildOutputOption{Path: pathSelected, IsDir: isDir, IsStdout: isStdout}, nil + return define.BuildOutputOption{ + Type: typeSelected, + Path: pathSelected, + }, nil } // TeeType parses a string value and returns a TeeType From 9b6b4ef5c833600d80f4c5a39524a54475188525 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Tue, 4 Nov 2025 14:14:44 +0100 Subject: [PATCH 3/5] fix: handle `--output=type=tar,dest=-` Signed-off-by: iTrooz --- .../containers/buildah/pkg/parse/parse.go | 36 +++++++++++-------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/vendor/github.com/containers/buildah/pkg/parse/parse.go b/vendor/github.com/containers/buildah/pkg/parse/parse.go index 634e2038a9f..ddb58cee4b4 100644 --- a/vendor/github.com/containers/buildah/pkg/parse/parse.go +++ b/vendor/github.com/containers/buildah/pkg/parse/parse.go @@ -712,18 +712,17 @@ func AuthConfig(creds string) (*types.DockerAuthConfig, error) { // GetBuildOutput is responsible for parsing custom build output argument i.e `build --output` flag. // Takes `buildOutput` as string and returns BuildOutputOption func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { - if buildOutput == "-" { - // Feature parity with buildkit, output tar to stdout - // Read more here: https://docs.docker.com/engine/reference/commandline/build/#custom-build-outputs - return define.BuildOutputOption{ - Type: define.BuildOutputStdout, - Path: "", - }, nil - } - // Support simple values, in the form --output ./mydir if !strings.Contains(buildOutput, ",") && !strings.Contains(buildOutput, "=") { - // expect default --output + if buildOutput == "-" { + // Feature parity with buildkit, output tar to stdout + // Read more here: https://docs.docker.com/engine/reference/commandline/build/#custom-build-outputs + return define.BuildOutputOption{ + Type: define.BuildOutputStdout, + Path: "", + }, nil + } + return define.BuildOutputOption{ Type: define.BuildOutputLocalDir, Path: buildOutput, @@ -761,20 +760,29 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { } } - // Validation + // Validate there is a type if typeSelected == -1 { return define.BuildOutputOption{}, fmt.Errorf("missing required key %q in build output option: %q", "type", buildOutput) } + + // Validate path is only set when needed if typeSelected == define.BuildOutputLocalDir || typeSelected == define.BuildOutputTar { if pathSelected == "" { return define.BuildOutputOption{}, fmt.Errorf("missing required key %q in build output option: %q", "dest", buildOutput) } } else { - // Clear path selected when not needed by type + // Clear path when not needed by type pathSelected = "" } - if typeSelected == define.BuildOutputLocalDir && pathSelected == "-" { - return define.BuildOutputOption{}, fmt.Errorf(`invalid build output option %q, "type=local" can not be used with "dest=-"`, buildOutput) + + // Handle redirecting stdout for tar output + if pathSelected == "-" { + if typeSelected == define.BuildOutputTar { + typeSelected = define.BuildOutputStdout + pathSelected = "" + } else { + return define.BuildOutputOption{}, fmt.Errorf(`invalid build output option %q, only "type=tar" can be used with "dest=-"`, buildOutput) + } } return define.BuildOutputOption{ From 4f66dd52784046f85fa7c44a338d50305865ecb3 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Tue, 4 Nov 2025 15:17:15 +0100 Subject: [PATCH 4/5] add tests Signed-off-by: iTrooz --- test/e2e/build_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index f85875cce97..12940e14341 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -5,6 +5,7 @@ package integration import ( "bytes" "fmt" + "io" "os" "os/exec" "path/filepath" @@ -12,6 +13,7 @@ import ( "strings" "github.com/containers/buildah/define" + "github.com/containers/podman/v6/test/utils" . "github.com/containers/podman/v6/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -1378,4 +1380,59 @@ COPY --from=img2 /etc/alpine-release /prefix-test/container-prefix.txt` session.WaitWithDefaultTimeout() Expect(session).Should(ExitCleanly()) }) + + It("podman build --output ./folder", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile", "--output", podmanTest.TempDir}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + files, err := os.ReadDir(podmanTest.TempDir) + Expect(err).ToNot(HaveOccurred()) + Expect(len(files)).To(BeNumerically(">", 1)) + }) + + It("podman build --output type=local,dest=./folder", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile", "--output", fmt.Sprintf("type=local,dest=%v", podmanTest.TempDir)}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + files, err := os.ReadDir(podmanTest.TempDir) + Expect(err).ToNot(HaveOccurred()) + Expect(len(files)).To(BeNumerically(">", 1)) + }) + + It("podman build --output type=tar,dest=./folder/file.tar", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile", "--output", fmt.Sprintf("type=tar,dest=%v/file.tar", podmanTest.TempDir)}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + tarFile := filepath.Join(podmanTest.TempDir, "file.tar") + _, err := os.Stat(tarFile) + Expect(err).ToNot(HaveOccurred()) + }) + + It("podman build --output -", func() { + // Capture output to buffer manually, to avoid binary output leaking into test logs + session := podmanTest.PodmanWithOptions(utils.PodmanExecOptions{ + FullOutputWriter: io.Discard, + }, "build", "-f", "build/basicalpine/Containerfile", "--output", "-") + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + // Check for tar header magic number + Expect(session.OutputToString()).To(ContainSubstring("ustar")) + }) + + It("podman build --output type=tar,dest=-", func() { + // Capture output to buffer manually, to avoid binary output leaking into test logs + session := podmanTest.PodmanWithOptions(utils.PodmanExecOptions{ + FullOutputWriter: io.Discard, + }, "build", "-f", "build/basicalpine/Containerfile", "--output", "type=tar,dest=-") + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + + // Check for tar header magic number + Expect(session.OutputToString()).To(ContainSubstring("ustar")) + }) + }) From 25f902d332bde9e09dc495a4e59c011ec5da6887 Mon Sep 17 00:00:00 2001 From: iTrooz Date: Tue, 4 Nov 2025 15:37:54 +0100 Subject: [PATCH 5/5] add tests --- test/e2e/build_test.go | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index 12940e14341..f0939160dfb 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -13,7 +13,6 @@ import ( "strings" "github.com/containers/buildah/define" - "github.com/containers/podman/v6/test/utils" . "github.com/containers/podman/v6/test/utils" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -1413,7 +1412,7 @@ COPY --from=img2 /etc/alpine-release /prefix-test/container-prefix.txt` It("podman build --output -", func() { // Capture output to buffer manually, to avoid binary output leaking into test logs - session := podmanTest.PodmanWithOptions(utils.PodmanExecOptions{ + session := podmanTest.PodmanWithOptions(PodmanExecOptions{ FullOutputWriter: io.Discard, }, "build", "-f", "build/basicalpine/Containerfile", "--output", "-") session.WaitWithDefaultTimeout() @@ -1425,7 +1424,7 @@ COPY --from=img2 /etc/alpine-release /prefix-test/container-prefix.txt` It("podman build --output type=tar,dest=-", func() { // Capture output to buffer manually, to avoid binary output leaking into test logs - session := podmanTest.PodmanWithOptions(utils.PodmanExecOptions{ + session := podmanTest.PodmanWithOptions(PodmanExecOptions{ FullOutputWriter: io.Discard, }, "build", "-f", "build/basicalpine/Containerfile", "--output", "type=tar,dest=-") session.WaitWithDefaultTimeout() @@ -1435,4 +1434,32 @@ COPY --from=img2 /etc/alpine-release /prefix-test/container-prefix.txt` Expect(session.OutputToString()).To(ContainSubstring("ustar")) }) + // Should error because no type + It("podman build --output dest=./folder", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile", "--output", fmt.Sprintf("dest=%v", podmanTest.TempDir)}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, `missing required key "type"`)) + }) + + // Should error because invalid type + It("podman build --output type=INVALID", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile", "--output", "type=INVALID"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, `invalid type "INVALID"`)) + }) + + // Should error because no dest specified + It("podman build --output type=local", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile", "--output", "type=local"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, `missing required key "dest"`)) + }) + + // Should error because invalid dest for local type + It("podman build --output type=local,dest=-", func() { + session := podmanTest.Podman([]string{"build", "-f", "build/basicalpine/Containerfile", "--output", "type=local,dest=-"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, `only "type=tar" can be used with "dest=-"`)) + }) + })