Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --all-tags to push #1380

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 68 additions & 3 deletions cmd/buildah/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,17 @@ import (
"github.com/containers/image/manifest"
"github.com/containers/image/transports"
"github.com/containers/image/transports/alltransports"
"github.com/containers/storage"
"github.com/containers/storage/pkg/archive"
multierror "github.com/hashicorp/go-multierror"
imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)

type pushResults struct {
allTags bool
authfile string
blobCache string
certDir string
Expand Down Expand Up @@ -61,6 +65,7 @@ func init() {

flags := pushCommand.Flags()
flags.SetInterspersed(false)
flags.BoolVarP(&opts.allTags, "all-tags", "a", false, "push all tagged images to the repository")
flags.StringVar(&opts.authfile, "authfile", "", "path of the authentication file. Default is ${XDG_RUNTIME_DIR}/containers/auth.json")
flags.StringVar(&opts.blobCache, "blob-cache", "", "assume image blobs in the specified directory will be available for pushing")
flags.StringVar(&opts.certDir, "cert-dir", "", "use certificates at the specified path to access the registry")
Expand Down Expand Up @@ -98,16 +103,47 @@ func pushCmd(c *cobra.Command, args []string, iopts pushResults) error {
return errors.New("Only two arguments are necessary to push: source and destination")
}

store, err := getStore(c)
if err != nil {
return err
}

// Map of image tags found for the specified image.
// tags stored in the key
var imageTags []string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a map.

if iopts.allTags {
if err := getAllTags(src, &imageTags, store); err != nil {
return err
}
logrus.Debugf("For the image: [%s] found tags: %v", src, imageTags)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail with a nice error message if len(imageTags) == 0.

}

var errs *multierror.Error
imageName := src
destTarget := destSpec
compress := imagebuildah.Gzip
if iopts.disableCompression {
compress = imagebuildah.Uncompressed
}

store, err := getStore(c)
if err != nil {
return err
if len(imageTags) > 1 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be opts.allTags; even if getAllTags finds only a single one, the tagKey must be appended to source/destination.

(And then I think it would be possible to merge the two if opts.allTags branches into a single block.)

for _, tagKey := range imageTags {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(The Key seems both unnecessary and confusing to me.)

src = imageName + ":" + tagKey
destSpec = destTarget + ":" + tagKey
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(*shrug* This looks fragile, but if dest can be an arbitrary destination, it’s a practical compromise I guess.)

if err = pushImage(c, src, destSpec, store, compress, iopts); err != nil {
errs = multierror.Append(errs, err)
}
}
} else {
if err = pushImage(c, src, destSpec, store, compress, iopts); err != nil {
errs = multierror.Append(errs, err)
}
}

return errs.ErrorOrNil()
}

func pushImage(c *cobra.Command, src string, destSpec string, store storage.Store, compress archive.Compression, iopts pushResults) error {
dest, err := alltransports.ParseImageName(destSpec)
// add the docker:// transport to see if they neglected it.
if err != nil {
Expand Down Expand Up @@ -179,3 +215,32 @@ func getListOfTransports() string {
allTransports := strings.Join(transports.ListNames(), ",")
return strings.Replace(allTransports, ",tarball", "", 1)
}

// getAllTags gets all of the locally tagged images and adds them
// to the passed in slice.
func getAllTags(imageName string, imageTags *[]string, store storage.Store) error {

imageName = "/" + imageName
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

??? I can’t see how buildah push --all-tags hostname.com/foo/bar can work with this.

images, err := store.Images()
if err != nil {
return errors.Wrapf(err, "error reading images")
}
for _, image := range images {

if len(image.Names) < 1 {
continue
}
// Name should look like: "docker.io/library/alpine:latest"
for _, name := range image.Names {
splitName := strings.Split(name, ":")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref, err := reference.ParseNormalizedNamed(name)
if err { /* I don't know; fail or ignore? */ }
if tagged, isTagged := ref.(reference.Tagged); isTagged {
   if digested, hasDigest := ref.(reference.Digested); hasDigest { // a c/storage image name can have both at the same time!
    // No idea; maybe ignore this, or the ":+tagKey editing step would have to be more complex
  }
   tag := tagged.Tag()
   name = ref.Name()
   …
} // else {
  // no-tag, digest-only name, ignore
}

(@mrunalp Exhibit N+1 why claiming to support name:tag@sha256:digest would be painful.)

if !strings.HasSuffix(splitName[0], imageName) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this enough? What happens if I am looking for image frank, and I have an image named ballparkfrank?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @mtrmac might have a much better way of doing this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I put in the image name 'frank' it won't match up to 'ballparkfrank' as I force add a '/' to the imageName up at line 223. So I think this is OK, but happy to rework it if there's a better way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn’t this just a simple equality check? That must be intentional, but what’s the reason for that? buildah push does not, AFAIK, do fuzzy matching that way.

… oh, it’s the registry search list again, isn’t it. Well, then, please actually do the work to match only using the search registry host names, and fully interpret sysregistriesv2.FindUnqualifiedSearchRegistries the way ResolveNames does (probably extracting its initRegistries, or more of it, into a helper).

Yes, that’s a lot of work. Have I ever mentioned that I don’t like the search mechanism?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(BTW FindImage just loops over the result of ResolveNames. That’s not obviously correct for transport-qualified names, or for storage IDs, but a similar loop here would at least be consistent with that code.)

break
}
if len(splitName) > 1 {
*imageTags = append(*imageTags, splitName[1])
}
}

}
return nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I would find it a bit more natural for imageTags to be a return value instead of a reference parameter.)

}
5 changes: 3 additions & 2 deletions docs/buildah-pull.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ The image ID of the image that was pulled. On error 1 is returned.

## OPTIONS

**--all-tags, a**
**--all-tags, -a**

All tagged images in the repository will be pulled.
All tagged images in the repository will be pulled, not just `:latest`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:latest is not the only tag one can pull, it’s only the default if the user specifies no tag and no digest.


**--authfile** *path*

Expand Down Expand Up @@ -100,6 +100,7 @@ buildah pull --creds=myusername:mypassword --cert-dir ~/auth myregistry/myreposi

buildah pull --authfile=/tmp/auths/myauths.json myregistry/myrepository/imagename:imagetag

buildah pull --all-tags alpine

## Files

Expand Down
7 changes: 7 additions & 0 deletions docs/buildah-push.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ If the transport part of DESTINATION is omitted, "docker://" is assumed.

## OPTIONS

**--all-tags, -a**

All tagged images in the repository will be pushed, not just `:latest`. A tag can not be included in the imageID or DESTINATION fields.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:latest is not the only tag one can pull, it’s only the default if the user specifies no tag and no digest.


**--authfile** *path*

Path of the authentication file. Default is ${XDG\_RUNTIME\_DIR}/containers/auth.json, which is set using `podman login`.
Expand Down Expand Up @@ -113,6 +117,9 @@ This example extracts the imageID image and puts it into the registry on the loc
This example extracts the imageID image and puts it into the registry on the localhost using credentials and certificates for authentication.
`# buildah push --cert-dir ~/auth --tls-verify=true --creds=username:password imageID docker://localhost:5000/my-imageID`

This example pushes all the tagged alpine images to the quay.io registry.
`# buildah push --all-tags alpine docker://quay.io/myrepo/alpine`

## Files

**registries.conf** (`/etc/containers/registries.conf`)
Expand Down
13 changes: 13 additions & 0 deletions tests/push.bats
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,16 @@ load helpers
echo "$output" | grep -q "docker://busybox"
buildah rmi busybox
}

@test "push-all-tags" {
buildah pull --signature-policy ${TESTSDIR}/policy.json --all-tags alpine
run buildah push --signature-policy ${TESTSDIR}/policy.json --all-tags alpine dir:/my-dir
echo "$output"
[[ "$output" =~ "my-dir:latest" ]]
[ "$status" -eq 0 ]

run ls /my-dir*
echo "$output"
[ "$status" -eq 0 ]
[ $(wc -l <<< "$output") -ge 50 ]
}