From d21c82b38059158ecc3e6dd955e4d76df7e7167d Mon Sep 17 00:00:00 2001 From: Jon Johnson Date: Mon, 5 Jun 2023 10:03:07 -0700 Subject: [PATCH] Split publishing and loading This untangles daemon from remote when publishing a bit more, which will make it easier to move towards lazily producing these images, which will make it easier to avoid building things entirely. Signed-off-by: Jon Johnson --- go.mod | 1 - go.sum | 2 - internal/cli/publish.go | 35 +++-- pkg/build/oci/publish.go | 322 ++++++++++++++++++--------------------- 4 files changed, 172 insertions(+), 188 deletions(-) diff --git a/go.mod b/go.mod index e525ce20b..46e326862 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,6 @@ module chainguard.dev/apko go 1.19 require ( - github.com/avast/retry-go v3.0.0+incompatible github.com/awslabs/amazon-ecr-credential-helper/ecr-login v0.0.0-20220920003936-cd2dbcbbab49 github.com/chainguard-dev/go-apk v0.0.0-20230605180416-2829525a7136 github.com/chrismellard/docker-credential-acr-env v0.0.0-20220327082430-c57b701bfc08 diff --git a/go.sum b/go.sum index e942b50da..abe94c2c3 100644 --- a/go.sum +++ b/go.sum @@ -47,8 +47,6 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkY github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2 h1:DklsrG3dyBCFEj5IhUbnKptjxatkF07cF2ak3yi77so= github.com/asaskevich/govalidator v0.0.0-20230301143203-a9d515a09cc2/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw= -github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHSxpiH9JdtuBj0= -github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY= github.com/aws/aws-sdk-go-v2 v1.16.15/go.mod h1:SwiyXi/1zTUZ6KIAmLK5V5ll8SiURNUYOqTerZPaF9k= github.com/aws/aws-sdk-go-v2 v1.16.16/go.mod h1:SwiyXi/1zTUZ6KIAmLK5V5ll8SiURNUYOqTerZPaF9k= github.com/aws/aws-sdk-go-v2 v1.17.8 h1:GMupCNNI7FARX27L7GjCJM8NgivWbRgpjNI/hOQjFS8= diff --git a/internal/cli/publish.go b/internal/cli/publish.go index 885d66ef5..cad4903ca 100644 --- a/internal/cli/publish.go +++ b/internal/cli/publish.go @@ -27,6 +27,7 @@ import ( "github.com/chrismellard/docker-credential-acr-env/pkg/credhelper" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/authn/github" + "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/google" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/spf13/cobra" @@ -220,8 +221,25 @@ func PublishCmd(ctx context.Context, outputRefs string, archs []types.Architectu if logger == nil { logger = log.NewLogger(os.Stderr) } + + if local { + // TODO: We shouldn't even need to build the index if we're loading a single image. + ref, err := oci.LoadIndex(ctx, idx, logger, tags) + if err != nil { + return fmt.Errorf("loading index: %w", err) + } + logger.Printf("using local option, exiting early") + fmt.Println(ref.String()) + return nil + } + // publish each arch-specific image - refs, err := oci.PublishImagesFromIndex(ctx, idx, local, shouldPushTags, logger, tags, ropt...) + // TODO: This should just happen as part of PublishIndex. + ref, err := name.ParseReference(tags[0]) + if err != nil { + return fmt.Errorf("parsing %q as tag: %w", tags[0], err) + } + refs, err := oci.PublishImagesFromIndex(ctx, idx, logger, ref.Context(), ropt...) if err != nil { return fmt.Errorf("publishing images from index: %w", err) } @@ -230,7 +248,7 @@ func PublishCmd(ctx context.Context, outputRefs string, archs []types.Architectu } // publish the index - finalDigest, _, err := oci.PublishIndex(ctx, idx, logger, local, shouldPushTags, tags, ropt...) + finalDigest, err := oci.PublishIndex(ctx, idx, logger, shouldPushTags, tags, ropt...) if err != nil { return fmt.Errorf("publishing image index: %w", err) } @@ -245,13 +263,6 @@ func PublishCmd(ctx context.Context, outputRefs string, archs []types.Architectu } } - // If saving local, exit early (no SBOMs etc.) - if local { - logger.Printf("using local option, exiting early") - fmt.Println(strings.Split(finalDigest.String(), "@")[0]) - return nil - } - if !shouldPushTags { allTags := tags allTags = append(allTags, additionalTags...) @@ -274,11 +285,13 @@ func PublishCmd(ctx context.Context, outputRefs string, archs []types.Architectu return fmt.Errorf("failed to write tags: %w", err) } } else { + // TODO: Why does this happen separately from PublishIndex? skipLocalCopy := strings.HasPrefix(finalDigest.Name(), fmt.Sprintf("%s/", oci.LocalDomain)) - var g errgroup.Group + g, ctx := errgroup.WithContext(ctx) for _, at := range additionalTags { at := at if skipLocalCopy { + // TODO: We probably don't need this now that we return early. logger.Warnf("skipping local domain tag %s", at) continue } @@ -294,6 +307,8 @@ func PublishCmd(ctx context.Context, outputRefs string, archs []types.Architectu // publish each arch-specific sbom // publish the index sbom if wantSBOM { + // TODO: Why aren't these just attached to idx? + // all sboms will be in the same directory if err := oci.PostAttachSBOMsFromIndex( ctx, idx, sboms, logger, tags, ropt..., diff --git a/pkg/build/oci/publish.go b/pkg/build/oci/publish.go index 9f2c0659a..78211affe 100644 --- a/pkg/build/oci/publish.go +++ b/pkg/build/oci/publish.go @@ -19,10 +19,7 @@ import ( "fmt" "os" "strings" - "sync" - "time" - "github.com/avast/retry-go" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/daemon" @@ -32,7 +29,6 @@ import ( "github.com/sigstore/cosign/v2/pkg/oci/walk" "golang.org/x/sync/errgroup" - "chainguard.dev/apko/pkg/build/types" "chainguard.dev/apko/pkg/log" ) @@ -40,40 +36,90 @@ import ( // `local` determines if it should push to the local docker daemon or to the actual registry. // `shouldPushTags` determines whether to push the tags provided in the `tags` parameter, or whether // to treat the first tag as a digest and push that instead. -func PublishImage(ctx context.Context, image oci.SignedImage, local, shouldPushTags bool, logger log.Logger, tags []string, remoteOpts ...remote.Option) (name.Digest, error) { - h, err := image.Digest() +func PublishImage(ctx context.Context, image oci.SignedImage, shouldPushTags bool, logger log.Logger, tags []string, remoteOpts ...remote.Option) (name.Digest, error) { + ref, err := name.ParseReference(tags[0]) + if err != nil { + return name.Digest{}, fmt.Errorf("parsing tag %q: %w", tags[0], err) + } + + hash, err := image.Digest() if err != nil { return name.Digest{}, fmt.Errorf("failed to compute digest: %w", err) } - digest := name.Digest{} + dig := ref.Context().Digest(hash.String()) + toPublish := tags msg := "publish image tag" + if !shouldPushTags { - toPublish = []string{fmt.Sprintf("%s@%s", strings.Split(tags[0], ":")[0], h.String())} + toPublish = []string{dig.String()} msg = "publishing image without tag (digest only)" } + + g, ctx := errgroup.WithContext(ctx) for _, tag := range toPublish { - logger.Printf("%s %v", msg, tag) - digest, err = publishTagFromImage(ctx, image, tag, h, local, logger, remoteOpts...) - if err != nil { - return name.Digest{}, err - } + tag := tag + g.Go(func() error { + logger.Printf("%s %v", msg, tag) + ref, err := name.ParseReference(tag) + if err != nil { + return fmt.Errorf("unable to parse reference: %w", err) + } + + // Write any attached SBOMs/signatures. + wp := writePeripherals(ref, logger, remoteOpts...) + g.Go(func() error { + return wp(ctx, image) + }) + + g.Go(func() error { + return remote.Write(ref, image, remoteOpts...) + }) + + return nil + }) + } + + if err := g.Wait(); err != nil { + return name.Digest{}, fmt.Errorf("failed to publish: %w", err) } - return digest, nil + return dig, nil } -// PublishImageFromLayer convenience function that creates an image from a v1.Layer, and then publishes that. -// Just wraps BuildImageFromLayer and PublishImage. -// Options provided are applied either to BuildImageFromlayer or PublishImage. -func PublishImageFromLayer(ctx context.Context, layer v1.Layer, ic types.ImageConfiguration, created time.Time, arch types.Architecture, logger log.Logger, local, shouldPushTags bool, tags []string, remoteOpts ...remote.Option) (name.Digest, oci.SignedImage, error) { - v1Image, err := BuildImageFromLayer(layer, ic, created, arch, logger) +func LoadImage(ctx context.Context, image oci.SignedImage, logger log.Logger, tags []string) (name.Reference, error) { + hash, err := image.Digest() + if err != nil { + return name.Digest{}, err + } + localSrcTagStr := fmt.Sprintf("%s/%s:%s", LocalDomain, LocalRepo, hash.Hex) + localSrcTag, err := name.NewTag(localSrcTagStr) + if err != nil { + return name.Digest{}, err + } + logger.Infof("saving OCI image locally: %s", localSrcTag.Name()) + resp, err := daemon.Write(localSrcTag, image) if err != nil { - return name.Digest{}, nil, err + logger.Errorf("docker daemon error: %s", strings.ReplaceAll(resp, "\n", "\\n")) + return name.Digest{}, fmt.Errorf("failed to save OCI image locally: %w", err) } - dig, err := PublishImage(ctx, v1Image, local, shouldPushTags, logger, tags, remoteOpts...) - return dig, v1Image, err + logger.Debugf("docker daemon response: %s", strings.ReplaceAll(resp, "\n", "\\n")) + for _, tag := range tags { + localDstTag, err := name.NewTag(tag) + if err != nil { + return name.Digest{}, err + } + if strings.HasPrefix(localSrcTag.Name(), fmt.Sprintf("%s/", LocalDomain)) { + logger.Warnf("skipping local domain tagging %s as %s", localSrcTag.Name(), localDstTag.Name()) + } else { + logger.Printf("tagging local image %s as %s", localSrcTag.Name(), localDstTag.Name()) + if err := daemon.Tag(localSrcTag, localDstTag); err != nil { + return name.Digest{}, err + } + } + } + return localSrcTag, nil } // PublishIndex given an oci.SignedImageIndex, publish it to a registry. @@ -81,207 +127,135 @@ func PublishImageFromLayer(ctx context.Context, layer v1.Layer, ic types.ImageCo // Note that docker, when provided with a multi-architecture index, will load just the image inside for the provided // platform, defaulting to the one on which the docker daemon is running. // PublishIndex will determine that platform and use it to publish the updated index. -func PublishIndex(ctx context.Context, idx oci.SignedImageIndex, logger log.Logger, local bool, shouldPushTags bool, tags []string, remoteOpts ...remote.Option) (name.Digest, oci.SignedImageIndex, error) { +func PublishIndex(ctx context.Context, idx oci.SignedImageIndex, logger log.Logger, shouldPushTags bool, tags []string, remoteOpts ...remote.Option) (name.Digest, error) { // TODO(jason): Also set annotations on the index. ggcr's // pkg/v1/mutate.Annotations will drop the interface methods from // oci.SignedImageIndex, so we may need to reimplement // mutate.Annotations in ocimutate to keep it for now. - // If attempting to save locally, pick the native architecture - // and use that cached image for local tags - // Ported from https://github.com/ko-build/ko/blob/main/pkg/publish/daemon.go#L92-L168 - if local { - im, err := idx.IndexManifest() - if err != nil { - return name.Digest{}, nil, err - } - goos, goarch := os.Getenv("GOOS"), os.Getenv("GOARCH") - if goos == "" { - goos = "linux" - } - if goarch == "" { - goarch = "amd64" - } - // Default to just using the first one in the list if we cannot match - useManifest := im.Manifests[0] - for _, manifest := range im.Manifests { - if manifest.Platform == nil { - continue - } - if manifest.Platform.OS != goos { - continue - } - if manifest.Platform.Architecture != goarch { - continue - } - useManifest = manifest - } - localSrcTagStr := fmt.Sprintf("%s/%s:%s", LocalDomain, LocalRepo, useManifest.Digest.Hex) - logger.Printf("using best guess single-arch image for local tags: %s (%s/%s)", localSrcTagStr, goos, goarch) - localSrcTag, err := name.NewTag(localSrcTagStr) - if err != nil { - return name.Digest{}, nil, err - } - for _, tag := range tags { - localDstTag, err := name.NewTag(tag) - if err != nil { - return name.Digest{}, nil, err - } - if strings.HasPrefix(localSrcTag.Name(), fmt.Sprintf("%s/", LocalDomain)) { - logger.Warnf("skipping local domain tagging %s as %s", localSrcTag.Name(), localDstTag.Name()) - } else { - logger.Printf("tagging local image %s as %s", localSrcTag.Name(), localDstTag.Name()) - if err := daemon.Tag(localSrcTag, localDstTag); err != nil { - return name.Digest{}, nil, err - } - } - } - digest, err := name.NewDigest(fmt.Sprintf("%s@%s", localSrcTag.Name(), useManifest.Digest.String())) - if err != nil { - return name.Digest{}, nil, err - } - return digest, idx, nil + ref, err := name.ParseReference(tags[0]) + if err != nil { + return name.Digest{}, fmt.Errorf("parsing tag %q: %w", tags[0], err) } h, err := idx.Digest() if err != nil { - return name.Digest{}, nil, err + return name.Digest{}, err } - digest := name.Digest{} - msg := "publishing index tag" + dig := ref.Context().Digest(h.String()) + toPublish := tags + msg := "publishing index tag" + if !shouldPushTags { - toPublish = []string{fmt.Sprintf("%s@%s", strings.Split(tags[0], ":")[0], h.String())} + toPublish = []string{dig.String()} msg = "publishing index without tag (digest only)" } + + g, ctx := errgroup.WithContext(ctx) for _, tag := range toPublish { logger.Printf("%s %v", msg, tag) - digest, err = publishTagFromIndex(ctx, idx, tag, h, logger, remoteOpts...) + + ref, err := name.ParseReference(tag) if err != nil { - return name.Digest{}, nil, err + return name.Digest{}, fmt.Errorf("unable to parse reference: %w", err) } - } - - return digest, idx, nil -} - -// publishTagFromIndex publishes a single tag from an oci.SignedImageIndex, -// as well as any attached signatures/SBoMs. -func publishTagFromIndex(ctx context.Context, index oci.SignedImageIndex, imageRef string, hash v1.Hash, logger log.Logger, remoteOpts ...remote.Option) (name.Digest, error) { - ref, err := name.ParseReference(imageRef) - if err != nil { - return name.Digest{}, fmt.Errorf("unable to parse reference: %w", err) - } - - var g errgroup.Group - // Write any attached SBOMs/signatures (recursively) - wp := writePeripherals(ref, logger, remoteOpts...) - if err := walk.SignedEntity(ctx, index, func(ctx context.Context, se oci.SignedEntity) error { + // Write any attached SBOMs/signatures (recursively) g.Go(func() error { - return wp(ctx, se) + wp := writePeripherals(ref, logger, remoteOpts...) + return walk.SignedEntity(ctx, idx, func(ctx context.Context, se oci.SignedEntity) error { + g.Go(func() error { + return wp(ctx, se) + }) + return nil + }) }) - return nil - }); err != nil { - return name.Digest{}, err - } - g.Go(func() error { - return retry.Do(func() error { - return remote.WriteIndex(ref, index, remoteOpts...) + g.Go(func() error { + return remote.WriteIndex(ref, idx, remoteOpts...) }) - }) + } if err := g.Wait(); err != nil { return name.Digest{}, fmt.Errorf("failed to publish: %w", err) } - return ref.Context().Digest(hash.String()), nil + return dig, nil } -// publishTagFromImage publishes a single tag from an oci.SignedImage, -// as well as any attached signatures/SBoMs. -// Supports pushing to local docker daemon via the `local` flag. -func publishTagFromImage(ctx context.Context, image oci.SignedImage, imageRef string, hash v1.Hash, local bool, logger log.Logger, remoteOpts ...remote.Option) (name.Digest, error) { - imgRef, err := name.ParseReference(imageRef) +// If attempting to save locally, pick the native architecture +// and use that cached image for local tags +// Ported from https://github.com/ko-build/ko/blob/main/pkg/publish/daemon.go#L92-L168 +func LoadIndex(ctx context.Context, idx oci.SignedImageIndex, logger log.Logger, tags []string) (name.Reference, error) { + im, err := idx.IndexManifest() if err != nil { - return name.Digest{}, fmt.Errorf("unable to parse reference: %w", err) + return name.Digest{}, err } - - if local { - localSrcTagStr := fmt.Sprintf("%s/%s:%s", LocalDomain, LocalRepo, hash.Hex) - localSrcTag, err := name.NewTag(localSrcTagStr) - if err != nil { - return name.Digest{}, err - } - logger.Infof("saving OCI image locally: %s", localSrcTag.Name()) - resp, err := daemon.Write(localSrcTag, image) - if err != nil { - logger.Errorf("docker daemon error: %s", strings.ReplaceAll(resp, "\n", "\\n")) - return name.Digest{}, fmt.Errorf("failed to save OCI image locally: %w", err) + goos, goarch := os.Getenv("GOOS"), os.Getenv("GOARCH") + if goos == "" { + goos = "linux" + } + if goarch == "" { + goarch = "amd64" + } + // Default to just using the first one in the list if we cannot match + useManifest := im.Manifests[0] + for _, manifest := range im.Manifests { + if manifest.Platform == nil { + continue } - logger.Debugf("docker daemon response: %s", strings.ReplaceAll(resp, "\n", "\\n")) - localDstTag, err := name.NewTag(imageRef) - if err != nil { - return name.Digest{}, err + if manifest.Platform.OS != goos { + continue } - if strings.HasPrefix(localSrcTag.Name(), fmt.Sprintf("%s/", LocalDomain)) { - logger.Warnf("skipping local domain tagging %s as %s", localSrcTag.Name(), localDstTag.Name()) - } else { - logger.Printf("tagging local image %s as %s", localSrcTag.Name(), localDstTag.Name()) - if err := daemon.Tag(localSrcTag, localDstTag); err != nil { - return name.Digest{}, err - } + if manifest.Platform.Architecture != goarch { + continue } - return name.NewDigest(fmt.Sprintf("%s@%s", localSrcTag.Name(), hash)) + useManifest = manifest } - - var g errgroup.Group - - // Write any attached SBOMs/signatures. - wp := writePeripherals(imgRef, logger, remoteOpts...) - g.Go(func() error { - return wp(ctx, image) - }) - - g.Go(func() error { - return retry.Do(func() error { - return remote.Write(imgRef, image, remoteOpts...) - }) - }) - - if err := g.Wait(); err != nil { - return name.Digest{}, fmt.Errorf("failed to publish: %w", err) + img, err := idx.SignedImage(useManifest.Digest) + if err != nil { + return name.Digest{}, fmt.Errorf("reading child image %q", useManifest.Digest.String()) } - return imgRef.Context().Digest(hash.String()), nil + + logger.Printf("using best guess single-arch image for local tags (%s/%s)", goos, goarch) + return LoadImage(ctx, img, logger, tags) } // PublishImagesFromIndex publishes all images from an index to a remote registry. // The only difference between this and PublishIndex is that PublishIndex pushes out all blobs and referenced manifests // from within the index. This adds pushing the referenced SignedImage artifacts along with appropriate tags. -func PublishImagesFromIndex(ctx context.Context, idx oci.SignedImageIndex, local, shouldPushTags bool, logger log.Logger, tags []string, remoteOpts ...remote.Option) (digests []name.Digest, err error) { +func PublishImagesFromIndex(ctx context.Context, idx oci.SignedImageIndex, logger log.Logger, repo name.Repository, remoteOpts ...remote.Option) ([]name.Digest, error) { manifest, err := idx.IndexManifest() if err != nil { return nil, fmt.Errorf("failed to get index manifest: %w", err) } - var ( - g errgroup.Group - mtx sync.Mutex - ) - for _, m := range manifest.Manifests { - m := m + + digests := make([]name.Digest, len(manifest.Manifests)) + + g, ctx := errgroup.WithContext(ctx) + for i, m := range manifest.Manifests { + i, m := i, m + + dig := repo.Digest(m.Digest.String()) + digests[i] = dig + g.Go(func() error { img, err := idx.SignedImage(m.Digest) if err != nil { return fmt.Errorf("failed to get image for %v from index: %w", m, err) } - if dig, err := PublishImage(ctx, img, local, shouldPushTags, logger, tags, remoteOpts...); err != nil { - return err - } else { - mtx.Lock() - digests = append(digests, dig) - mtx.Unlock() - } + + g.Go(func() error { + // Write any attached SBOMs/signatures. + wp := writePeripherals(dig, logger, remoteOpts...) + return wp(ctx, img) + }) + + g.Go(func() error { + return remote.Write(dig, img, remoteOpts...) + }) + return nil }) } @@ -325,9 +299,7 @@ func writePeripherals(tag name.Reference, logger log.Logger, opt ...remote.Optio return nil } - if err := retry.Do(func() error { - return remote.Write(ref, f, opt...) - }); err != nil { + if err := remote.Write(ref, f, opt...); err != nil { return fmt.Errorf("writing sbom: %w", err) }