commit: make target image names optional

Make the name of the image to create an optional parameter.  If none is
specified, use a temporary mostly-random name that can't be interpreted
as an ID, so that the image copying logic will compute the correct ID to
assign to the new image, and remove the temporary name before returning.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

Closes: #1486
Approved by: rhatdan
This commit is contained in:
Nalin Dahyabhai 2019-03-20 13:06:29 -04:00 committed by Atomic Bot
parent 29f306c4b3
commit 610eb7a0b2
5 changed files with 106 additions and 54 deletions

View File

@ -12,6 +12,7 @@ import (
"github.com/containers/buildah/util"
"github.com/containers/image/storage"
"github.com/containers/image/transports/alltransports"
"github.com/containers/image/types"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
@ -46,7 +47,8 @@ func init() {
RunE: func(cmd *cobra.Command, args []string) error {
return commitCmd(cmd, args, opts)
},
Example: `buildah commit containerID newImageName
Example: `buildah commit containerID
buildah commit containerID newImageName
buildah commit containerID docker://localhost:5000/imageId`,
}
commitCommand.SetUsageTemplate(UsageTemplate())
@ -80,6 +82,7 @@ func init() {
}
func commitCmd(c *cobra.Command, args []string, iopts commitInputOptions) error {
var dest types.ImageReference
if len(args) == 0 {
return errors.Errorf("container ID must be specified")
}
@ -88,13 +91,13 @@ func commitCmd(c *cobra.Command, args []string, iopts commitInputOptions) error
}
name := args[0]
args = Tail(args)
if len(args) == 0 {
return errors.Errorf("an image name must be specified")
}
if len(args) > 1 {
return errors.Errorf("too many arguments specified")
}
image := args[0]
image := ""
if len(args) > 0 {
image = args[0]
}
compress := imagebuildah.Gzip
if iopts.disableCompression {
compress = imagebuildah.Uncompressed
@ -130,20 +133,21 @@ func commitCmd(c *cobra.Command, args []string, iopts commitInputOptions) error
return errors.Wrapf(err, "error building system context")
}
dest, err := alltransports.ParseImageName(image)
if err != nil {
candidates, _, _, err := util.ResolveName(image, "", systemContext, store)
if err != nil {
return errors.Wrapf(err, "error parsing target image name %q", image)
if image != "" {
if dest, err = alltransports.ParseImageName(image); err != nil {
candidates, _, _, err := util.ResolveName(image, "", systemContext, store)
if err != nil {
return errors.Wrapf(err, "error parsing target image name %q", image)
}
if len(candidates) == 0 {
return errors.Errorf("error parsing target image name %q", image)
}
dest2, err2 := storage.Transport.ParseStoreReference(store, candidates[0])
if err2 != nil {
return errors.Wrapf(err, "error parsing target image name %q", image)
}
dest = dest2
}
if len(candidates) == 0 {
return errors.Errorf("error parsing target image name %q", image)
}
dest2, err2 := storage.Transport.ParseStoreReference(store, candidates[0])
if err2 != nil {
return errors.Wrapf(err, "error parsing target image name %q", image)
}
dest = dest2
}
options := buildah.CommitOptions{

View File

@ -5,6 +5,7 @@ import (
"fmt"
"io"
"io/ioutil"
"strings"
"time"
"github.com/containers/buildah/pkg/blobcache"
@ -18,6 +19,7 @@ import (
"github.com/containers/image/types"
"github.com/containers/storage"
"github.com/containers/storage/pkg/archive"
"github.com/containers/storage/pkg/stringid"
digest "github.com/opencontainers/go-digest"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
@ -110,10 +112,28 @@ type PushOptions struct {
// Commit writes the contents of the container, along with its updated
// configuration, to a new image in the specified location, and if we know how,
// add any additional tags that were specified. Returns the ID of the new image
// if commit was successful and the image destination was local
// if commit was successful and the image destination was local.
func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options CommitOptions) (string, reference.Canonical, digest.Digest, error) {
var imgID string
// If we weren't given a name, build a destination reference using a
// temporary name that we'll remove later. The correct thing to do
// would be to read the manifest and configuration blob, and ask the
// manifest for the ID that we'd give the image, but that computation
// requires that we know the digests of the layer blobs, which we don't
// want to compute here because we'll have to do it again when
// cp.Image() instantiates a source image, and we don't want to do the
// work twice.
nameToRemove := ""
if dest == nil {
nameToRemove = stringid.GenerateRandomID() + "-tmp"
dest2, err := is.Transport.ParseStoreReference(b.store, nameToRemove)
if err != nil {
return imgID, nil, "", errors.Wrapf(err, "error creating temporary destination reference for image")
}
dest = dest2
}
systemContext := getSystemContext(b.store, options.SystemContext, options.SignaturePolicyPath)
blocked, err := isReferenceBlocked(dest, systemContext)
@ -148,10 +168,13 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
}
}
}
// Build an image reference from which we can copy the finished image.
src, err := b.makeImageRef(options.PreferredManifestType, options.Parent, exportBaseLayers, options.Squash, options.BlobDirectory, options.Compression, options.HistoryTimestamp, options.OmitTimestamp)
if err != nil {
return imgID, nil, "", errors.Wrapf(err, "error computing layer digests and building metadata for container %q", b.ContainerID)
}
// In case we're using caching, decide how to handle compression for a cache.
// If we're using blob caching, set it up for the source.
var maybeCachedSrc = types.ImageReference(src)
var maybeCachedDest = types.ImageReference(dest)
if options.BlobDirectory != "" {
@ -181,6 +204,8 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
if manifestBytes, err = cp.Image(ctx, policyContext, maybeCachedDest, maybeCachedSrc, getCopyOptions(b.store, options.ReportWriter, maybeCachedSrc, nil, maybeCachedDest, systemContext, "")); err != nil {
return imgID, nil, "", errors.Wrapf(err, "error copying layers and metadata for container %q", b.ContainerID)
}
// If we've got more names to attach, and we know how to do that for
// the transport that we're writing the new image to, add them now.
if len(options.AdditionalTags) > 0 {
switch dest.Transport().Name() {
case is.Transport.Name():
@ -201,10 +226,25 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
if err != nil && err != storage.ErrImageUnknown {
return imgID, nil, "", errors.Wrapf(err, "error locating image %q in local storage", transports.ImageName(dest))
}
if err == nil {
imgID = img.ID
prunedNames := make([]string, 0, len(img.Names))
for _, name := range img.Names {
if !(nameToRemove != "" && strings.Contains(name, nameToRemove)) {
prunedNames = append(prunedNames, name)
}
}
if len(prunedNames) < len(img.Names) {
if err = b.store.SetNames(imgID, prunedNames); err != nil {
return imgID, nil, "", errors.Wrapf(err, "failed to prune temporary name from image %q", imgID)
}
logrus.Debugf("reassigned names %v to image %q", prunedNames, img.ID)
dest2, err := is.Transport.ParseStoreReference(b.store, "@"+imgID)
if err != nil {
return imgID, nil, "", errors.Wrapf(err, "error creating unnamed destination reference for image")
}
dest = dest2
}
if options.IIDFile != "" {
if err = ioutil.WriteFile(options.IIDFile, []byte(img.ID), 0644); err != nil {
return imgID, nil, "", errors.Wrapf(err, "failed to write image ID to file %q", options.IIDFile)

View File

@ -27,7 +27,6 @@ import (
"github.com/containers/image/types"
"github.com/containers/storage"
"github.com/containers/storage/pkg/archive"
"github.com/containers/storage/pkg/stringid"
docker "github.com/fsouza/go-dockerclient"
"github.com/opencontainers/image-spec/specs-go/v1"
"github.com/opencontainers/runtime-spec/specs-go"
@ -826,33 +825,22 @@ func (s *StageExecutor) Delete() (err error) {
return err
}
// resolveNameToImageRef creates a types.ImageReference from b.output
// resolveNameToImageRef creates a types.ImageReference for the output name in local storage
func (b *Executor) resolveNameToImageRef(output string) (types.ImageReference, error) {
var (
imageRef types.ImageReference
err error
)
if output != "" {
imageRef, err = alltransports.ParseImageName(output)
if err != nil {
candidates, _, _, err := util.ResolveName(output, "", b.systemContext, b.store)
if err != nil {
return nil, errors.Wrapf(err, "error parsing target image name %q", output)
}
if len(candidates) == 0 {
return nil, errors.Errorf("error parsing target image name %q", output)
}
imageRef2, err2 := is.Transport.ParseStoreReference(b.store, candidates[0])
if err2 != nil {
return nil, errors.Wrapf(err, "error parsing target image name %q", output)
}
return imageRef2, nil
}
return imageRef, nil
}
imageRef, err = is.Transport.ParseStoreReference(b.store, "@"+stringid.GenerateRandomID())
imageRef, err := alltransports.ParseImageName(output)
if err != nil {
return nil, errors.Wrapf(err, "error parsing reference for image to be written")
candidates, _, _, err := util.ResolveName(output, "", b.systemContext, b.store)
if err != nil {
return nil, errors.Wrapf(err, "error parsing target image name %q", output)
}
if len(candidates) == 0 {
return nil, errors.Errorf("error parsing target image name %q", output)
}
imageRef2, err2 := is.Transport.ParseStoreReference(b.store, candidates[0])
if err2 != nil {
return nil, errors.Wrapf(err, "error parsing target image name %q", output)
}
return imageRef2, nil
}
return imageRef, nil
}
@ -1147,7 +1135,12 @@ func (s *StageExecutor) Execute(ctx context.Context, stage imagebuilder.Stage, b
// copyExistingImage creates a copy of an image already in the store
func (s *StageExecutor) copyExistingImage(ctx context.Context, cacheID, output string) (string, reference.Canonical, error) {
// Get the destination Image Reference
// If we don't need to attach a name to the image, just return the cache ID.
if output == "" {
return cacheID, nil, nil
}
// Get the destination image reference.
dest, err := s.executor.resolveNameToImageRef(output)
if err != nil {
return "", nil, err
@ -1356,9 +1349,13 @@ func urlContentModified(url string, historyTime *time.Time) (bool, error) {
// commit writes the container's contents to an image, using a passed-in tag as
// the name if there is one, generating a unique ID-based one otherwise.
func (s *StageExecutor) commit(ctx context.Context, ib *imagebuilder.Builder, createdBy, output string) (string, reference.Canonical, error) {
imageRef, err := s.executor.resolveNameToImageRef(output)
if err != nil {
return "", nil, err
var imageRef types.ImageReference
if output != "" {
imageRef2, err := s.executor.resolveNameToImageRef(output)
if err != nil {
return "", nil, err
}
imageRef = imageRef2
}
if ib.Author != "" {
@ -1449,9 +1446,11 @@ func (s *StageExecutor) commit(ctx context.Context, ib *imagebuilder.Builder, cr
return "", nil, err
}
var ref reference.Canonical
if dref := imageRef.DockerReference(); dref != nil {
if ref, err = reference.WithDigest(dref, manifestDigest); err != nil {
return "", nil, errors.Wrapf(err, "error computing canonical reference for new image %q", imgID)
if imageRef != nil {
if dref := imageRef.DockerReference(); dref != nil {
if ref, err = reference.WithDigest(dref, manifestDigest); err != nil {
return "", nil, errors.Wrapf(err, "error computing canonical reference for new image %q", imgID)
}
}
}
return imgID, ref, nil

View File

@ -1046,3 +1046,7 @@ load helpers
buildah umount ${cid}
buildah rm ${cid}
}
@test "bud-no-target-name" {
run_buildah bud --signature-policy ${TESTSDIR}/policy.json ${TESTSDIR}/bud/maintainer
}

View File

@ -91,3 +91,8 @@ load helpers
[ "${status}" -eq 0 ]
[ "$output" == "/bin/sh" ]
}
@test "commit-no-name" {
cid=$(buildah from --pull --signature-policy ${TESTSDIR}/policy.json alpine)
run_buildah commit --signature-policy ${TESTSDIR}/policy.json $cid
}