From 80defed4a9e13a471fa27462b71a423f97580db3 Mon Sep 17 00:00:00 2001 From: danishprakash Date: Tue, 21 Mar 2023 18:51:17 +0530 Subject: [PATCH] build: add support for `--push` * pass systemCtx to push util * fix test Signed-off-by: Danish Prakash --- cmd/buildah/push.go | 21 +---------- define/build.go | 1 + define/types.go | 33 +++++++++++++++- docs/buildah-build.1.md | 6 +++ imagebuildah/executor.go | 3 ++ imagebuildah/stage_executor.go | 16 ++++++-- internal/util/util.go | 33 +++++++++++++++- pkg/cli/build.go | 12 +++++- pkg/cli/common.go | 2 + pkg/parse/parse.go | 69 ++++++++++++++++++++++------------ tests/bud.bats | 22 +++++++++++ util/util.go | 26 +++++++++++++ 12 files changed, 192 insertions(+), 52 deletions(-) diff --git a/cmd/buildah/push.go b/cmd/buildah/push.go index 7cfff350bbb..d894fac5338 100644 --- a/cmd/buildah/push.go +++ b/cmd/buildah/push.go @@ -16,7 +16,6 @@ import ( "github.com/containers/image/v5/manifest" "github.com/containers/image/v5/pkg/compression" "github.com/containers/image/v5/transports" - "github.com/containers/image/v5/transports/alltransports" "github.com/containers/storage" imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" @@ -148,25 +147,9 @@ func pushCmd(c *cobra.Command, args []string, iopts pushOptions) error { return err } - dest, err := alltransports.ParseImageName(destSpec) - // add the docker:// transport to see if they neglected it. + dest, err := util.ImageStringToImageReference(destSpec) if err != nil { - destTransport := strings.Split(destSpec, ":")[0] - if t := transports.Get(destTransport); t != nil { - return err - } - - if strings.Contains(destSpec, "://") { - return err - } - - destSpec = "docker://" + destSpec - dest2, err2 := alltransports.ParseImageName(destSpec) - if err2 != nil { - return err - } - dest = dest2 - logrus.Debugf("Assuming docker:// as the transport method for DESTINATION: %s", destSpec) + return fmt.Errorf("generating image reference: %w", err) } systemContext, err := parse.SystemContextFromOptions(c) diff --git a/define/build.go b/define/build.go index 68f3455b34c..96e2ef1cb03 100644 --- a/define/build.go +++ b/define/build.go @@ -171,6 +171,7 @@ type BuildOptions struct { // image that's meant to be run using krun as a VM instead of a conventional // process-type container. ConfidentialWorkload ConfidentialWorkloadOptions + Push bool // Additional tags to add to the image that we write, if we know of a // way to add them. AdditionalTags []string diff --git a/define/types.go b/define/types.go index 4f917b7db4a..63e6c74d094 100644 --- a/define/types.go +++ b/define/types.go @@ -15,6 +15,7 @@ import ( "strings" "github.com/containers/image/v5/manifest" + imageTypes "github.com/containers/image/v5/types" "github.com/containers/storage/pkg/archive" "github.com/containers/storage/pkg/chrootarchive" "github.com/containers/storage/pkg/ioutils" @@ -111,9 +112,37 @@ type Secret struct { // 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 + ImageRef imageTypes.ImageReference + Image string IsStdout bool + Path string // Only valid if !IsStdout + Push bool + Type BuildOutputType + + // Deprecated: Use Type instead to determine output type + IsDir bool +} + +type BuildOutputType int + +const ( + _ BuildOutputType = iota + BuildOutputImage + BuildOutputLocal + BuildOutputTar +) + +// String converts a BuildOutputType into a string. +func (t BuildOutputType) String() string { + switch t { + case BuildOutputImage: + return "image" + case BuildOutputLocal: + return "local" + case BuildOutputTar: + return "tar" + } + return fmt.Sprintf("unrecognized build output type %d", t) } // ConfidentialWorkloadOptions encapsulates options which control whether or not diff --git a/docs/buildah-build.1.md b/docs/buildah-build.1.md index 6820a2e9d39..68e49a371d3 100644 --- a/docs/buildah-build.1.md +++ b/docs/buildah-build.1.md @@ -717,6 +717,8 @@ Supported _keys_ are: Valid _type_ values are: - **local**: write the resulting build files to a directory on the client-side. +- **image**: writes the build results as an image to local storage. +- **registry**: pushes the resulting build image to the registry. Shorthand for `type=image,push=true`. - **tar**: write the resulting files as a single tarball (.tar). If no type is specified, the value defaults to **local**. @@ -774,6 +776,10 @@ registries.conf if newer. Raise an error if a base or SBOM scanner image is not found in the registries when image with the same name is not present locally. +**--push** + +Shorthand for "--output=type=registry" + **--quiet**, **-q** Suppress output messages which indicate which instruction is being processed, diff --git a/imagebuildah/executor.go b/imagebuildah/executor.go index f78b191e232..565df51cb9a 100644 --- a/imagebuildah/executor.go +++ b/imagebuildah/executor.go @@ -74,6 +74,7 @@ type Executor struct { registry string ignoreUnrecognizedInstructions bool quiet bool + push bool runtime string runtimeArgs []string transientMounts []Mount @@ -245,6 +246,7 @@ func newExecutor(logger *logrus.Logger, logPrefix string, store storage.Store, o registry: options.Registry, ignoreUnrecognizedInstructions: options.IgnoreUnrecognizedInstructions, quiet: options.Quiet, + push: options.Push, // TODO: not needed if planning to update buildOutput in cli/build runtime: options.Runtime, runtimeArgs: options.RuntimeArgs, transientMounts: transientMounts, @@ -950,6 +952,7 @@ func (b *Executor) Build(ctx context.Context, stages imagebuilder.Stages) (image } } stageID, stageRef, stageOnlyBaseImage, stageErr := b.buildStage(ctx, cleanupStages, stages, index) + if stageErr != nil { cancel = true ch <- Result{ diff --git a/imagebuildah/stage_executor.go b/imagebuildah/stage_executor.go index 60d5a9a9ead..e971eea1f67 100644 --- a/imagebuildah/stage_executor.go +++ b/imagebuildah/stage_executor.go @@ -1268,7 +1268,7 @@ func (s *StageExecutor) Execute(ctx context.Context, base string) (imgID string, canGenerateBuildOutput := (s.executor.buildOutput != "" && lastStage) if canGenerateBuildOutput { logrus.Debugf("Generating custom build output with options %q", s.executor.buildOutput) - buildOutputOption, err = parse.GetBuildOutput(s.executor.buildOutput) + buildOutputOption, err = parse.GetBuildOutput(s.executor.buildOutput, s.executor.output) if err != nil { return "", nil, false, fmt.Errorf("failed to parse build output: %w", err) } @@ -2363,7 +2363,17 @@ func (s *StageExecutor) commit(ctx context.Context, createdBy string, emptyLayer return imgID, ref, nil } -func (s *StageExecutor) generateBuildOutput(buildOutputOpts define.BuildOutputOption) error { +func (s *StageExecutor) generateBuildOutput(opts define.BuildOutputOption) error { + if opts.Type == define.BuildOutputImage { + if opts.Push { + err := internalUtil.PushImage(s.executor.store, opts, s.executor.systemContext) + if err != nil { + return fmt.Errorf("failed to export build output: %w", err) + } + } + return nil + } + extractRootfsOpts := buildah.ExtractRootfsOptions{} if unshare.IsRootless() { // In order to maintain as much parity as possible @@ -2383,7 +2393,7 @@ func (s *StageExecutor) generateBuildOutput(buildOutputOpts define.BuildOutputOp return fmt.Errorf("failed to extract rootfs from given container image: %w", err) } defer rc.Close() - err = internalUtil.ExportFromReader(rc, buildOutputOpts) + err = internalUtil.ExportFromReader(rc, opts) if err != nil { return fmt.Errorf("failed to export build output: %w", err) } diff --git a/internal/util/util.go b/internal/util/util.go index 6643d97ffbd..c0f998f1684 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -1,12 +1,14 @@ package util import ( + "context" "fmt" "io" "os" "path/filepath" "github.com/containers/buildah/define" + "github.com/containers/buildah/util" "github.com/containers/common/libimage" lplatform "github.com/containers/common/libimage/platform" "github.com/containers/image/v5/types" @@ -50,6 +52,29 @@ func NormalizePlatform(platform v1.Platform) v1.Platform { } } +// PushImage copies contents of the image to a new location +func PushImage(store storage.Store, opts define.BuildOutputOption, systemCtx *types.SystemContext) error { + runtime, err := libimage.RuntimeFromStore(store, &libimage.RuntimeOptions{SystemContext: systemCtx}) + if err != nil { + return err + } + + imageRef, err := util.ImageStringToImageReference(opts.Image) + if err != nil { + return fmt.Errorf("failed to convert image to ImageReference") + } + + libimageOptions := &libimage.PushOptions{} + libimageOptions.Writer = os.Stdout + dest := fmt.Sprintf("%s:%s", imageRef.Transport().Name(), imageRef.StringWithinTransport()) + _, err = runtime.Push(context.Background(), opts.Image, dest, libimageOptions) + if err != nil { + return fmt.Errorf("failed while pushing image %+q: %w", opts.ImageRef, err) + } + + return nil +} + // ExportFromReader reads bytes from given reader and exports to external tar, directory or stdout. func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error { var err error @@ -59,7 +84,8 @@ func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error { return err } } - if opts.IsDir { + switch opts.Type { + case define.BuildOutputLocal: // 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 @@ -81,7 +107,7 @@ func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error { if err != nil { return fmt.Errorf("failed while performing untar at %q: %w", opts.Path, err) } - } else { + case define.BuildOutputTar: outFile := os.Stdout if !opts.IsStdout { outFile, err = os.Create(opts.Path) @@ -94,7 +120,10 @@ func ExportFromReader(input io.Reader, opts define.BuildOutputOption) error { if err != nil { return fmt.Errorf("failed while performing copy to %q: %w", opts.Path, err) } + default: + return fmt.Errorf("build output type %s not supported", opts.Type) } + return nil } diff --git a/pkg/cli/build.go b/pkg/cli/build.go index 18f9ff2834d..4a1a4e1a412 100644 --- a/pkg/cli/build.go +++ b/pkg/cli/build.go @@ -265,7 +265,7 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) ( timestamp = &t } if c.Flag("output").Changed { - buildOption, err := parse.GetBuildOutput(iopts.BuildOutput) + buildOption, err := parse.GetBuildOutput(iopts.BuildOutput, output) if err != nil { return options, nil, nil, err } @@ -280,6 +280,15 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) ( return options, nil, nil, err } } + + if c.Flag("push").Changed { + if len(iopts.BuildOutput) == 0 { + iopts.BuildOutput = "type=registry" + } else { + return options, nil, nil, fmt.Errorf("cannot set both --push and --output") + } + } + var cacheTo []reference.Named var cacheFrom []reference.Named cacheTo = nil @@ -399,6 +408,7 @@ func GenBuildOptions(c *cobra.Command, inputArgs []string, iopts BuildOptions) ( OutputFormat: format, Platforms: platforms, PullPolicy: pullPolicy, + Push: iopts.Push, Quiet: iopts.Quiet, RemoveIntermediateCtrs: iopts.Rm, ReportWriter: reporter, diff --git a/pkg/cli/common.go b/pkg/cli/common.go index c7e4f5439c3..69b3134b034 100644 --- a/pkg/cli/common.go +++ b/pkg/cli/common.go @@ -85,6 +85,7 @@ type BudResults struct { Pull string PullAlways bool PullNever bool + Push bool Quiet bool IdentityLabel bool Rm bool @@ -305,6 +306,7 @@ newer: only pull base and SBOM scanner images when newer images exist on the r fs.BoolVar(&flags.Stdin, "stdin", false, "pass stdin into containers") fs.StringArrayVarP(&flags.Tag, "tag", "t", []string{}, "tagged `name` to apply to the built image") fs.StringVarP(&flags.BuildOutput, "output", "o", "", "output destination (format: type=local,dest=path)") + fs.BoolVar(&flags.Push, "push", false, "Shorthand for `--output=type=registry`") fs.StringVar(&flags.Target, "target", "", "set the target build stage to build") fs.Int64Var(&flags.Timestamp, "timestamp", 0, "set created timestamp to the specified epoch seconds to allow for deterministic builds, defaults to current time") fs.BoolVar(&flags.TLSVerify, "tls-verify", true, "require HTTPS and verify certificates when accessing the registry") diff --git a/pkg/parse/parse.go b/pkg/parse/parse.go index 0b5d92892db..98d32f1c847 100644 --- a/pkg/parse/parse.go +++ b/pkg/parse/parse.go @@ -656,29 +656,34 @@ 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) { +func GetBuildOutput(buildOutput, image string) (define.BuildOutputOption, error) { if len(buildOutput) == 1 && 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, + Type: define.BuildOutputTar, }, nil } - if !strings.Contains(buildOutput, ",") { - // expect default --output - return define.BuildOutputOption{ - Path: buildOutput, - IsDir: true, - IsStdout: false, - }, nil + + // TODO: breaking change if we remove this; flag syntax change + // see: https://github.com/containers/buildah/pull/4677/files#r1267278612 + // if !strings.Contains(buildOutput, ",") { + // // expect default --output + // return define.BuildOutputOption{ + // Path: buildOutput, + // IsDir: true, + // IsStdout: false, + // }, nil + // } + + out := define.BuildOutputOption{ + IsStdout: false, } - isDir := true - isStdout := false + typeSelected := false pathSelected := false - path := "" tokens := strings.Split(buildOutput, ",") for _, option := range tokens { arr := strings.SplitN(option, "=", 2) @@ -691,40 +696,54 @@ func GetBuildOutput(buildOutput string) (define.BuildOutputOption, error) { return define.BuildOutputOption{}, fmt.Errorf("duplicate %q not supported", arr[0]) } typeSelected = true - if arr[1] == "local" { - isDir = true - } else if arr[1] == "tar" { - isDir = false - } else { - return define.BuildOutputOption{}, fmt.Errorf("invalid type %q selected for build output options %q", arr[1], buildOutput) + switch exportType := arr[1]; exportType { + case "local": + out.Type = define.BuildOutputLocal + case "tar": + out.Type = define.BuildOutputTar + case "image", "registry": + // --type=registry ==> --type=image,push=true + out.Type = define.BuildOutputImage + out.Image = image + + if exportType == "registry" { + out.Push = true + } + default: + return define.BuildOutputOption{}, fmt.Errorf("invalid type %q selected for build output options %q", exportType, buildOutput) } case "dest": if pathSelected { return define.BuildOutputOption{}, fmt.Errorf("duplicate %q not supported", arr[0]) } pathSelected = true - path = arr[1] + out.Path = arr[1] + case "push": + if out.Type != define.BuildOutputImage { + return define.BuildOutputOption{}, fmt.Errorf("push can only be used with type=image") + } + out.Push = true default: return define.BuildOutputOption{}, fmt.Errorf("unrecognized key %q in build output option: %q", arr[0], buildOutput) } } - if !typeSelected || !pathSelected { - return define.BuildOutputOption{}, fmt.Errorf("invalid build output option %q, accepted keys are type and dest must be present", buildOutput) + if !typeSelected && !pathSelected { + return define.BuildOutputOption{}, fmt.Errorf("invalid build output option %q, accepted keys type and dest must be present", buildOutput) } - if path == "-" { - if isDir { + if out.Path == "-" { + if out.Type == define.BuildOutputLocal { return define.BuildOutputOption{}, fmt.Errorf("invalid build output option %q, type=local and dest=- is not supported", buildOutput) } return define.BuildOutputOption{ Path: "", - IsDir: false, IsStdout: true, + Type: define.BuildOutputTar, }, nil } - return define.BuildOutputOption{Path: path, IsDir: isDir, IsStdout: isStdout}, nil + return out, nil } // TeeType parses a string value and returns a TeeType diff --git a/tests/bud.bats b/tests/bud.bats index dea9ccb21a0..b2498c69cc6 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -6902,3 +6902,25 @@ _EOF run_buildah run testctr -- sh -c 'cd podman-tag && git ls-remote --tags origin v5.0.0^{} | cut -f1' assert "$output" = "$local_head_hash" } + +@test "bud with --output=type=registry" { + _prefetch alpine + local contextdir=${TEST_SCRATCH_DIR}/bud/platform + mkdir -p $contextdir + + cat > $contextdir/Containerfile << _EOF +FROM alpine +_EOF + + testuser="testuser$RANDOM" + testpassword="testpassword$RANDOM" + start_registry "$testuser" "$testpassword" + + run_buildah build $WITH_POLICY_JSON \ + --cert-dir $REGISTRY_DIR \ + --creds="$testuser":"$testpassword" \ + --tls-verify=false \ + --output=type=registry \ + -t localhost:${REGISTRY_PORT}/image1:latest \ + -f $contextdir/Containerfile +} diff --git a/util/util.go b/util/util.go index 40002d503ef..90230edffa1 100644 --- a/util/util.go +++ b/util/util.go @@ -17,6 +17,7 @@ import ( "github.com/containers/image/v5/docker/reference" "github.com/containers/image/v5/pkg/shortnames" "github.com/containers/image/v5/signature" + "github.com/containers/image/v5/transports" "github.com/containers/image/v5/transports/alltransports" "github.com/containers/image/v5/types" "github.com/containers/storage" @@ -47,6 +48,31 @@ func StringInSlice(s string, slice []string) bool { return slices.Contains(slice, s) } +func ImageStringToImageReference(image string) (types.ImageReference, error) { + dest, err := alltransports.ParseImageName(image) + // add the docker:// transport to see if they neglected it. + if err != nil { + destTransport := strings.Split(image, ":")[0] + if t := transports.Get(destTransport); t != nil { + return nil, err + } + + if strings.Contains(image, "://") { + return nil, err + } + + image = "docker://" + image + dest2, err2 := alltransports.ParseImageName(image) + if err2 != nil { + return nil, err + } + dest = dest2 + logrus.Debugf("Assuming docker:// as the transport method for DESTINATION: %s", image) + } + + return dest, nil +} + // resolveName checks if name is a valid image name, and if that name doesn't // include a domain portion, returns a list of the names which it might // correspond to in the set of configured registries, and the transport used to