feat(build): print error on build flag --output=type=something

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

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)
 			}

pkg/cli/build.go

@@ -282,7 +282,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
 			}
 		}

pkg/parse/parse.go

@@ -735,26 +735,25 @@ 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{
-			Path:     "",
-			IsDir:    false,
-			IsStdout: true,
-		}, nil
-	}
-	if !strings.Contains(buildOutput, ",") {
-		// expect default --output <dirname>
+	// Support simple values, in the form --output ./mydir
+	if !strings.Contains(buildOutput, ",") && !strings.Contains(buildOutput, "=") {
+		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{
-			Path:     buildOutput,
-			IsDir:    true,
-			IsStdout: false,
+			Type: define.BuildOutputLocalDir,
+			Path: buildOutput,
 		}, nil
 	}
-	isDir := true
-	isStdout := false
-	typeSelected := ""
+
+	// Support complex values, in the form --output type=local,dest=./mydir
+	var typeSelected define.BuildOutputType = -1
 	pathSelected := ""
 	for option := range strings.SplitSeq(buildOutput, ",") {
 		key, value, found := strings.Cut(option, "=")
@@ -763,15 +762,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)
 			}
@@ -785,17 +783,35 @@ 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)
+	// 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
+	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 when not needed by type
+		pathSelected = ""
+	}
+
+	// Handle redirecting stdout for tar output
 	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.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{Path: pathSelected, IsDir: isDir, IsStdout: isStdout}, nil
+	return define.BuildOutputOption{
+		Type: typeSelected,
+		Path: pathSelected,
+	}, nil
 }
 
 // TeeType parses a string value and returns a TeeType

pkg/parse/parse_test.go

@@ -271,23 +271,23 @@ func TestGetBuildOutput(t *testing.T) {
 			description: "hyphen",
 			input:       "-",
 			output: define.BuildOutputOption{
-				IsStdout: true,
+				Type: define.BuildOutputStdout,
 			},
 		},
 		{
 			description: "just-a-path",
 			input:       "/tmp",
 			output: define.BuildOutputOption{
-				IsDir: true,
-				Path:  "/tmp",
+				Type: define.BuildOutputLocalDir,
+				Path: "/tmp",
 			},
 		},
 		{
 			description: "normal-path",
 			input:       "type=local,dest=/tmp",
 			output: define.BuildOutputOption{
-				IsDir: true,
-				Path:  "/tmp",
+				Type: define.BuildOutputLocalDir,
+				Path: "/tmp",
 			},
 		},
 	}

tests/bud.bats

@@ -2755,6 +2755,13 @@ _EOF
   mkdir $mytmpdir/rootfs
   tar -C $mytmpdir/rootfs -xvf $mytmpdir/rootfs.tar
   ls $mytmpdir/rootfs/hello
+
+  # test with long syntax as well
+  buildah build $WITH_POLICY_JSON --output type=tar,dest=- -t test-bud -f $mytmpdir/Containerfile . > $mytmpdir/rootfs2.tar
+  # explode tar
+  mkdir $mytmpdir/rootfs2
+  tar -C $mytmpdir/rootfs2 -xvf $mytmpdir/rootfs2.tar
+  ls $mytmpdir/rootfs2/hello
 }
 
 @test "build with custom build output and output rootfs to tar with no additional step" {
@@ -2785,6 +2792,8 @@ _EOF
 FROM alpine
 RUN echo 'hello'> hello
 _EOF
+  run_buildah 125 build --output type=tar $WITH_POLICY_JSON -t test-bud -f $mytmpdir/Containerfile .
+  expect_output --substring 'missing required key "dest"'
   run_buildah 125 build --output type=tar, $WITH_POLICY_JSON -t test-bud -f $mytmpdir/Containerfile .
   expect_output --substring 'invalid'
   run_buildah 125 build --output type=wrong,dest=hello $WITH_POLICY_JSON -t test-bud -f $mytmpdir/Containerfile .