Skip to content

Commit a6e3853

Browse files
iTrooziTrooz
committed
feat(build): make --output=tar,dest=- work + print error on build flag --output=type=INVALID
1 parent 96f4ac3 commit a6e3853

File tree

8 files changed

+171
-9
lines changed

8 files changed

+171
-9
lines changed

‎define/types.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ type Secret struct {
110110
}
111111

112112
// BuildOutputOptions contains the the outcome of parsing the value of a build --output flag
113+
// Deprecated: This structure is now internal
113114
type BuildOutputOption struct {
114115
Path string // Only valid if !IsStdout
115116
IsDir bool

‎imagebuildah/stage_executor.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
buildahdocker "github.com/containers/buildah/docker"
2121
"github.com/containers/buildah/internal"
2222
"github.com/containers/buildah/internal/metadata"
23+
internalParse "github.com/containers/buildah/internal/parse"
2324
"github.com/containers/buildah/internal/sanitize"
2425
"github.com/containers/buildah/internal/tmpdir"
2526
internalUtil "github.com/containers/buildah/internal/util"
@@ -1310,11 +1311,11 @@ func (s *stageExecutor) execute(ctx context.Context, base string) (imgID string,
13101311
}
13111312

13121313
// Parse and populate buildOutputOption if needed
1313-
var buildOutputOptions []define.BuildOutputOption
1314+
var buildOutputOptions []internalParse.BuildOutputOption
13141315
if lastStage && len(s.executor.buildOutputs) > 0 {
13151316
for _, buildOutput := range s.executor.buildOutputs {
13161317
logrus.Debugf("generating custom build output with options %q", buildOutput)
1317-
buildOutputOption, err := parse.GetBuildOutput(buildOutput)
1318+
buildOutputOption, err := internalParse.GetBuildOutput(buildOutput)
13181319
if err != nil {
13191320
return "", nil, false, fmt.Errorf("failed to parse build output %q: %w", buildOutput, err)
13201321
}
@@ -2621,7 +2622,7 @@ func (s *stageExecutor) commit(ctx context.Context, createdBy string, emptyLayer
26212622
return results.ImageID, results, nil
26222623
}
26232624

2624-
func (s *stageExecutor) generateBuildOutput(buildOutputOpts define.BuildOutputOption) error {
2625+
func (s *stageExecutor) generateBuildOutput(buildOutputOpts internalParse.BuildOutputOption) error {
26252626
forceTimestamp := s.executor.timestamp
26262627
if s.executor.sourceDateEpoch != nil {
26272628
forceTimestamp = s.executor.sourceDateEpoch

‎internal/parse/build_output.go‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package parse
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
8+
type BuildOutputType int
9+
10+
const (
11+
BuildOutputStdout BuildOutputType = 0 // stream tar to stdout
12+
BuildOutputLocalDir BuildOutputType = 1
13+
BuildOutputTar BuildOutputType = 2
14+
)
15+
16+
// BuildOutputOptions contains the the outcome of parsing the value of a build --output flag
17+
type BuildOutputOption struct {
18+
Type BuildOutputType
19+
Path string // Only valid if Type is local dir or tar
20+
}
21+
22+
// GetBuildOutput is responsible for parsing custom build output argument i.e `build --output` flag.
23+
// Takes `buildOutput` as string and returns BuildOutputOption
24+
func GetBuildOutput(buildOutput string) (BuildOutputOption, error) {
25+
// Support simple values, in the form --output ./mydir
26+
if !strings.Contains(buildOutput, ",") && !strings.Contains(buildOutput, "=") {
27+
if buildOutput == "-" {
28+
// Feature parity with buildkit, output tar to stdout
29+
// Read more here: https://docs.docker.com/engine/reference/commandline/build/#custom-build-outputs
30+
return BuildOutputOption{
31+
Type: BuildOutputStdout,
32+
Path: "",
33+
}, nil
34+
}
35+
36+
return BuildOutputOption{
37+
Type: BuildOutputLocalDir,
38+
Path: buildOutput,
39+
}, nil
40+
}
41+
42+
// Support complex values, in the form --output type=local,dest=./mydir
43+
var typeSelected BuildOutputType = -1
44+
pathSelected := ""
45+
for option := range strings.SplitSeq(buildOutput, ",") {
46+
key, value, found := strings.Cut(option, "=")
47+
if !found {
48+
return BuildOutputOption{}, fmt.Errorf("invalid build output options %q, expected format key=value", buildOutput)
49+
}
50+
switch key {
51+
case "type":
52+
if typeSelected != -1 {
53+
return BuildOutputOption{}, fmt.Errorf("duplicate %q not supported", key)
54+
}
55+
switch value {
56+
case "local":
57+
typeSelected = BuildOutputLocalDir
58+
case "tar":
59+
typeSelected = BuildOutputTar
60+
default:
61+
return BuildOutputOption{}, fmt.Errorf("invalid type %q selected for build output options %q", value, buildOutput)
62+
}
63+
case "dest":
64+
if pathSelected != "" {
65+
return BuildOutputOption{}, fmt.Errorf("duplicate %q not supported", key)
66+
}
67+
pathSelected = value
68+
default:
69+
return BuildOutputOption{}, fmt.Errorf("unrecognized key %q in build output option: %q", key, buildOutput)
70+
}
71+
}
72+
73+
// Validate there is a type
74+
if typeSelected == -1 {
75+
return BuildOutputOption{}, fmt.Errorf("missing required key %q in build output option: %q", "type", buildOutput)
76+
}
77+
78+
// Validate path
79+
if typeSelected == BuildOutputLocalDir || typeSelected == BuildOutputTar {
80+
if pathSelected == "" {
81+
return BuildOutputOption{}, fmt.Errorf("missing required key %q in build output option: %q", "dest", buildOutput)
82+
}
83+
} else {
84+
// Clear path when not needed by type
85+
pathSelected = ""
86+
}
87+
88+
// Handle redirecting stdout for tar output
89+
if pathSelected == "-" {
90+
if typeSelected == BuildOutputTar {
91+
typeSelected = BuildOutputStdout
92+
pathSelected = ""
93+
} else {
94+
return BuildOutputOption{}, fmt.Errorf(`invalid build output option %q, only "type=tar" can be used with "dest=-"`, buildOutput)
95+
}
96+
}
97+
98+
return BuildOutputOption{
99+
Type: typeSelected,
100+
Path: pathSelected,
101+
}, nil
102+
}

‎internal/parse/build_output_test.go‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package parse
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestGetBuildOutput(t *testing.T) {
11+
testCases := []struct {
12+
description string
13+
input string
14+
output BuildOutputOption
15+
}{
16+
{
17+
description: "hyphen",
18+
input: "-",
19+
output: BuildOutputOption{
20+
Type: BuildOutputStdout,
21+
},
22+
},
23+
{
24+
description: "just-a-path",
25+
input: "/tmp",
26+
output: BuildOutputOption{
27+
Type: BuildOutputLocalDir,
28+
Path: "/tmp",
29+
},
30+
},
31+
{
32+
description: "normal-path",
33+
input: "type=local,dest=/tmp",
34+
output: BuildOutputOption{
35+
Type: BuildOutputLocalDir,
36+
Path: "/tmp",
37+
},
38+
},
39+
}
40+
for _, testCase := range testCases {
41+
t.Run(testCase.description, func(t *testing.T) {
42+
result, err := GetBuildOutput(testCase.input)
43+
require.NoErrorf(t, err, "expected to be able to parse %q", testCase.input)
44+
assert.Equal(t, testCase.output, result)
45+
})
46+
}
47+
}

‎internal/util/util.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66
"os"
77
"path/filepath"
88

9-
"github.com/containers/buildah/define"
9+
"github.com/containers/buildah/internal/parse"
1010
v1 "github.com/opencontainers/image-spec/specs-go/v1"
1111
"go.podman.io/common/libimage"
1212
lplatform "go.podman.io/common/libimage/platform"
@@ -51,14 +51,14 @@ func NormalizePlatform(platform v1.Platform) v1.Platform {
5151
}
5252

5353
// ExportFromReader reads bytes from given reader and exports to external tar, directory or stdout.
54-
func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error {
54+
func ExportFromReader(input io.Reader, opts parse.BuildOutputOption) error {
5555
var err error
5656
if !filepath.IsAbs(opts.Path) {
5757
if opts.Path, err = filepath.Abs(opts.Path); err != nil {
5858
return err
5959
}
6060
}
61-
if opts.IsDir {
61+
if opts.Type == parse.BuildOutputLocalDir {
6262
// In order to keep this feature as close as possible to
6363
// buildkit it was decided to preserve ownership when
6464
// invoked as root since caller already has access to artifacts
@@ -80,7 +80,7 @@ func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error {
8080
}
8181
} else {
8282
outFile := os.Stdout
83-
if !opts.IsStdout {
83+
if opts.Type != parse.BuildOutputStdout {
8484
if outFile, err = os.Create(opts.Path); err != nil {
8585
return fmt.Errorf("failed while creating destination tar at %q: %w", opts.Path, err)
8686
}

‎pkg/cli/build.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"time"
1919

2020
"github.com/containers/buildah/define"
21+
internalParse "github.com/containers/buildah/internal/parse"
2122
"github.com/containers/buildah/pkg/parse"
2223
"github.com/containers/buildah/pkg/util"
2324
"github.com/opencontainers/runtime-spec/specs-go"
@@ -278,11 +279,11 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) (
278279
for _, buildOutput := range iopts.BuildOutputs {
279280
// if any of these go to stdout, we need to avoid
280281
// interspersing our random output in with it
281-
buildOption, err := parse.GetBuildOutput(buildOutput)
282+
buildOption, err := internalParse.GetBuildOutput(buildOutput)
282283
if err != nil {
283284
return options, nil, nil, err
284285
}
285-
if buildOption.IsStdout {
286+
if buildOption.Type == internalParse.BuildOutputStdout {
286287
iopts.Quiet = true
287288
}
288289
}

‎pkg/parse/parse.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,7 @@ func AuthConfig(creds string) (*types.DockerAuthConfig, error) {
734734

735735
// GetBuildOutput is responsible for parsing custom build output argument i.e `build --output` flag.
736736
// Takes `buildOutput` as string and returns BuildOutputOption
737+
// Deprecated: This function is now internal
737738
func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) {
738739
if buildOutput == "-" {
739740
// Feature parity with buildkit, output tar to stdout

‎tests/bud.bats‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2755,6 +2755,13 @@ _EOF
27552755
mkdir $mytmpdir/rootfs
27562756
tar -C $mytmpdir/rootfs -xvf $mytmpdir/rootfs.tar
27572757
ls $mytmpdir/rootfs/hello
2758+
2759+
# test with long syntax as well
2760+
buildah build $WITH_POLICY_JSON --output type=tar,dest=- -t test-bud -f $mytmpdir/Containerfile . > $mytmpdir/rootfs2.tar
2761+
# explode tar
2762+
mkdir $mytmpdir/rootfs2
2763+
tar -C $mytmpdir/rootfs2 -xvf $mytmpdir/rootfs2.tar
2764+
ls $mytmpdir/rootfs2/hello
27582765
}
27592766

27602767
@test "build with custom build output and output rootfs to tar with no additional step" {
@@ -2785,6 +2792,8 @@ _EOF
27852792
FROM alpine
27862793
RUN echo 'hello'> hello
27872794
_EOF
2795+
run_buildah 125 build --output type=tar $WITH_POLICY_JSON -t test-bud -f $mytmpdir/Containerfile .
2796+
expect_output --substring 'missing required key "dest"'
27882797
run_buildah 125 build --output type=tar, $WITH_POLICY_JSON -t test-bud -f $mytmpdir/Containerfile .
27892798
expect_output --substring 'invalid'
27902799
run_buildah 125 build --output type=wrong,dest=hello $WITH_POLICY_JSON -t test-bud -f $mytmpdir/Containerfile .

0 commit comments

Comments
 (0)