From a05b0ac923cb18949296c81575bebafddc93c258 Mon Sep 17 00:00:00 2001 From: Avi Deitcher Date: Mon, 21 Jun 2021 23:33:34 +0300 Subject: [PATCH 1/2] check for arch when pulling to cache, push by descriptor Signed-off-by: Avi Deitcher --- src/cmd/linuxkit/cache/pull.go | 18 ++++++++++++++++++ src/cmd/linuxkit/cache/push.go | 8 ++------ src/cmd/linuxkit/cache/write.go | 7 ++----- 3 files changed, 22 insertions(+), 11 deletions(-) diff --git a/src/cmd/linuxkit/cache/pull.go b/src/cmd/linuxkit/cache/pull.go index ef3618367..28853ccf3 100644 --- a/src/cmd/linuxkit/cache/pull.go +++ b/src/cmd/linuxkit/cache/pull.go @@ -2,6 +2,7 @@ package cache import ( "errors" + "fmt" "github.com/containerd/containerd/reference" "github.com/google/go-containerregistry/pkg/v1" @@ -28,6 +29,9 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts if desc, err = partial.Descriptor(img); err != nil { return ImageSource{}, errors.New("image could not create valid descriptor") } + if desc.Platform == nil || desc.Platform.Architecture != architecture || desc.Platform.OS != "linux" { + return ImageSource{}, fmt.Errorf("image was not for requested architecture: linux/%s", architecture) + } } else { ii, err := root.ImageIndex() if err == nil { @@ -36,6 +40,20 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts return ImageSource{}, errors.New("index could not create valid descriptor") } } + // check that the index has a manifest for our arch + im, err := imageIndex.IndexManifest() + if err != nil { + return ImageSource{}, fmt.Errorf("could not get index manifest: %v", err) + } + for _, m := range im.Manifests { + if m.Platform != nil && m.Platform.Architecture == architecture && m.Platform.OS == "linux" { + return p.NewSource( + ref, + architecture, + desc, + ), nil + } + } } } // three possibilities now: diff --git a/src/cmd/linuxkit/cache/push.go b/src/cmd/linuxkit/cache/push.go index cf9c44598..fc49bc414 100644 --- a/src/cmd/linuxkit/cache/push.go +++ b/src/cmd/linuxkit/cache/push.go @@ -58,13 +58,9 @@ func (p *Provider) Push(name string) error { if err != nil { return fmt.Errorf("could not create a valid arch-specific tag %s: %v", archTag, err) } - image, err := p.FindRoot(archTag) + img, err := p.cache.Image(m.Digest) if err != nil { - return fmt.Errorf("could not find arch-specific image in cache %s: %v", archTag, err) - } - img, err := image.Image() - if err != nil { - return fmt.Errorf("found arch-specific image in cache %s, but could not resolve to actual image: %v", archTag, err) + return fmt.Errorf("could not find arch-specific image in cache %s: %v", m.Digest, err) } log.Debugf("pushing image %s", tag) if err := remote.Tag(tag, img, options...); err != nil { diff --git a/src/cmd/linuxkit/cache/write.go b/src/cmd/linuxkit/cache/write.go index f90534e31..4d485d2cb 100644 --- a/src/cmd/linuxkit/cache/write.go +++ b/src/cmd/linuxkit/cache/write.go @@ -82,11 +82,8 @@ func (p *Provider) ImagePull(ref *reference.Spec, trustedRef, architecture strin if err != nil { return ImageSource{}, fmt.Errorf("unable to save image to cache: %v", err) } - return p.NewSource( - ref, - architecture, - &desc.Descriptor, - ), nil + // ensure it includes our architecture + return p.ValidateImage(ref, architecture) } // ImageLoad takes an OCI format image tar stream and writes it locally. It should be From ebbb1281f395d4d599688840283a32fce369f818 Mon Sep 17 00:00:00 2001 From: Avi Deitcher Date: Mon, 21 Jun 2021 23:44:25 +0300 Subject: [PATCH 2/2] one-more Signed-off-by: Avi Deitcher --- src/cmd/linuxkit/cache/pull.go | 57 +++++++------- src/cmd/linuxkit/cache/push.go | 16 +++- src/cmd/linuxkit/cache/write.go | 102 ++++---------------------- src/cmd/linuxkit/pkglib/build.go | 8 +- src/cmd/linuxkit/pkglib/build_test.go | 4 +- src/cmd/linuxkit/spec/cache.go | 2 +- 6 files changed, 63 insertions(+), 126 deletions(-) diff --git a/src/cmd/linuxkit/cache/pull.go b/src/cmd/linuxkit/cache/pull.go index 28853ccf3..9af136623 100644 --- a/src/cmd/linuxkit/cache/pull.go +++ b/src/cmd/linuxkit/cache/pull.go @@ -29,9 +29,6 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts if desc, err = partial.Descriptor(img); err != nil { return ImageSource{}, errors.New("image could not create valid descriptor") } - if desc.Platform == nil || desc.Platform.Architecture != architecture || desc.Platform.OS != "linux" { - return ImageSource{}, fmt.Errorf("image was not for requested architecture: linux/%s", architecture) - } } else { ii, err := root.ImageIndex() if err == nil { @@ -40,20 +37,6 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts return ImageSource{}, errors.New("index could not create valid descriptor") } } - // check that the index has a manifest for our arch - im, err := imageIndex.IndexManifest() - if err != nil { - return ImageSource{}, fmt.Errorf("could not get index manifest: %v", err) - } - for _, m := range im.Manifests { - if m.Platform != nil && m.Platform.Architecture == architecture && m.Platform.OS == "linux" { - return p.NewSource( - ref, - architecture, - desc, - ), nil - } - } } } // three possibilities now: @@ -66,25 +49,35 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts // or because it was not available - so get it from the remote return ImageSource{}, errors.New("no such image") case imageIndex != nil: + // check that the index has a manifest for our arch + im, err := imageIndex.IndexManifest() + if err != nil { + return ImageSource{}, fmt.Errorf("could not get index manifest: %v", err) + } // we found a local index, just make sure it is up to date and, if not, download it - if err := validate.Index(imageIndex); err == nil { - return p.NewSource( - ref, - architecture, - desc, - ), nil + if err := validate.Index(imageIndex); err != nil { + return ImageSource{}, errors.New("invalid index") } - return ImageSource{}, errors.New("invalid index") + for _, m := range im.Manifests { + if m.Platform != nil && m.Platform.Architecture == architecture && m.Platform.OS == "linux" { + return p.NewSource( + ref, + architecture, + desc, + ), nil + } + } + return ImageSource{}, fmt.Errorf("index for %s did not contain image for platform linux/%s", imageName, architecture) case image != nil: - // we found a local image, just make sure it is up to date - if err := validate.Image(image); err == nil { - return p.NewSource( - ref, - architecture, - desc, - ), nil + // we found a local image, make sure it is up to date, and that it matches our platform + if err := validate.Image(image); err != nil { + return ImageSource{}, errors.New("invalid image") } - return ImageSource{}, errors.New("invalid image") + return p.NewSource( + ref, + architecture, + desc, + ), nil } // if we made it to here, we had some strange error return ImageSource{}, errors.New("should not have reached this point, image index and image were both empty and not-empty") diff --git a/src/cmd/linuxkit/cache/push.go b/src/cmd/linuxkit/cache/push.go index fc49bc414..aeaca0de5 100644 --- a/src/cmd/linuxkit/cache/push.go +++ b/src/cmd/linuxkit/cache/push.go @@ -7,6 +7,7 @@ import ( namepkg "github.com/google/go-containerregistry/pkg/name" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/linuxkit/linuxkit/src/cmd/linuxkit/registry" + imagespec "github.com/opencontainers/image-spec/specs-go/v1" log "github.com/sirupsen/logrus" ) @@ -60,7 +61,20 @@ func (p *Provider) Push(name string) error { } img, err := p.cache.Image(m.Digest) if err != nil { - return fmt.Errorf("could not find arch-specific image in cache %s: %v", m.Digest, err) + // it might not have existed, so we can add it locally + // use the original image name in the annotation + desc := m.DeepCopy() + if desc.Annotations == nil { + desc.Annotations = map[string]string{} + } + desc.Annotations[imagespec.AnnotationRefName] = archTag + if err := p.cache.AppendDescriptor(*desc); err != nil { + return fmt.Errorf("error appending descriptor for %s to layout index: %v", archTag, err) + } + img, err = p.cache.Image(m.Digest) + if err != nil { + return fmt.Errorf("could not find or create arch-specific image for %s: %v", archTag, err) + } } log.Debugf("pushing image %s", tag) if err := remote.Tag(tag, img, options...); err != nil { diff --git a/src/cmd/linuxkit/cache/write.go b/src/cmd/linuxkit/cache/write.go index 4d485d2cb..172f6fdfd 100644 --- a/src/cmd/linuxkit/cache/write.go +++ b/src/cmd/linuxkit/cache/write.go @@ -4,6 +4,7 @@ import ( "archive/tar" "bytes" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -301,99 +302,26 @@ func (p *Provider) IndexWrite(ref *reference.Spec, descriptors ...v1.Descriptor) ), nil } -// DescriptorWrite writes a name for a given descriptor -func (p *Provider) DescriptorWrite(ref *reference.Spec, descriptors ...v1.Descriptor) (lktspec.ImageSource, error) { +// DescriptorWrite writes a descriptor to the cache index; it validates that it has a name +// and replaces any existing one +func (p *Provider) DescriptorWrite(ref *reference.Spec, desc v1.Descriptor) (lktspec.ImageSource, error) { + if ref == nil { + return ImageSource{}, errors.New("cannot write descriptor without reference name") + } image := ref.String() - log.Debugf("writing descriptors for image %s: %v", image, descriptors) + if desc.Annotations == nil { + desc.Annotations = map[string]string{} + } + desc.Annotations[imagespec.AnnotationRefName] = image + log.Debugf("writing descriptor for image %s", image) - ii, err := p.cache.ImageIndex() - if err != nil { - return ImageSource{}, fmt.Errorf("unable to get root index: %v", err) - } - images, err := partial.FindImages(ii, match.Name(image)) - if err != nil { - return ImageSource{}, fmt.Errorf("error parsing index: %v", err) - } - if err == nil && len(images) > 0 { - return ImageSource{}, fmt.Errorf("image named %s already exists in cache and is not an index", image) - } - indexes, err := partial.FindIndexes(ii, match.Name(image)) - if err != nil { - return ImageSource{}, fmt.Errorf("error parsing index: %v", err) - } - var im v1.IndexManifest // do we update an existing one? Or create a new one? - if len(indexes) > 0 { - // we already had one, so update just the referenced index and return - im, err := indexes[0].IndexManifest() - if err != nil { - return ImageSource{}, fmt.Errorf("unable to convert index for %s into its manifest: %v", image, err) - } - oldhash, err := indexes[0].Digest() - if err != nil { - return ImageSource{}, fmt.Errorf("unable to get hash of existing index: %v", err) - } - // we only care about avoiding duplicate arch/OS/Variant - descReplace := map[string]v1.Descriptor{} - for _, desc := range descriptors { - descReplace[fmt.Sprintf("%s/%s/%s", desc.Platform.OS, desc.Platform.Architecture, desc.Platform.OSVersion)] = desc - } - // now we can go through each one and see if it already exists, and, if so, replace it - var manifests []v1.Descriptor - for _, m := range im.Manifests { - lookup := fmt.Sprintf("%s/%s/%s", m.Platform.OS, m.Platform.Architecture, m.Platform.OSVersion) - if desc, ok := descReplace[lookup]; ok { - manifests = append(manifests, desc) - // already added, so do not need it in the lookup list any more - delete(descReplace, lookup) - continue - } - manifests = append(manifests, m) - } - // any left get added - for _, desc := range descReplace { - manifests = append(manifests, desc) - } - im.Manifests = manifests - - if err := p.cache.RemoveBlob(oldhash); err != nil { - return ImageSource{}, fmt.Errorf("unable to remove old index blob: %v", err) - } - } else { - // we did not have one, so create an index, store it, update the root index.json, and return - im = v1.IndexManifest{ - MediaType: types.OCIImageIndex, - Manifests: descriptors, - SchemaVersion: 2, - } - } - - // write the updated index, remove the old one - b, err := json.Marshal(im) - if err != nil { - return ImageSource{}, fmt.Errorf("unable to marshal new index to json: %v", err) - } - hash, size, err := v1.SHA256(bytes.NewReader(b)) - if err != nil { - return ImageSource{}, fmt.Errorf("error calculating hash of index json: %v", err) - } - if err := p.cache.WriteBlob(hash, ioutil.NopCloser(bytes.NewReader(b))); err != nil { - return ImageSource{}, fmt.Errorf("error writing new index to json: %v", err) - } - // finally update the descriptor in the root if err := p.cache.RemoveDescriptors(match.Name(image)); err != nil { - return ImageSource{}, fmt.Errorf("unable to remove old descriptor from index.json: %v", err) - } - desc := v1.Descriptor{ - MediaType: types.OCIImageIndex, - Size: size, - Digest: hash, - Annotations: map[string]string{ - imagespec.AnnotationRefName: image, - }, + return ImageSource{}, fmt.Errorf("unable to remove old descriptors for %s: %v", image, err) } + if err := p.cache.AppendDescriptor(desc); err != nil { - return ImageSource{}, fmt.Errorf("unable to append new descriptor to index.json: %v", err) + return ImageSource{}, fmt.Errorf("unable to append new descriptor for %s: %v", image, err) } return p.NewSource( diff --git a/src/cmd/linuxkit/pkglib/build.go b/src/cmd/linuxkit/pkglib/build.go index 7f7eeffc9..aaf536614 100644 --- a/src/cmd/linuxkit/pkglib/build.go +++ b/src/cmd/linuxkit/pkglib/build.go @@ -15,6 +15,7 @@ import ( registry "github.com/google/go-containerregistry/pkg/v1" "github.com/linuxkit/linuxkit/src/cmd/linuxkit/cache" lktspec "github.com/linuxkit/linuxkit/src/cmd/linuxkit/spec" + "github.com/linuxkit/linuxkit/src/cmd/linuxkit/util" "github.com/linuxkit/linuxkit/src/cmd/linuxkit/version" imagespec "github.com/opencontainers/image-spec/specs-go/v1" log "github.com/sirupsen/logrus" @@ -321,21 +322,22 @@ func (p Pkg) Build(bos ...BuildOpt) error { if err != nil { return err } + fullRelTag := util.ReferenceExpand(relTag) - ref, err = reference.Parse(relTag) + ref, err = reference.Parse(fullRelTag) if err != nil { return err } if _, err := c.DescriptorWrite(&ref, *desc); err != nil { return err } - if err := c.Push(relTag); err != nil { + if err := c.Push(fullRelTag); err != nil { return err } // tag in docker, if requested if bo.targetDocker { - if err := d.tag(p.FullTag(), relTag); err != nil { + if err := d.tag(p.FullTag(), fullRelTag); err != nil { return err } } diff --git a/src/cmd/linuxkit/pkglib/build_test.go b/src/cmd/linuxkit/pkglib/build_test.go index 8fa6c8776..708f832df 100644 --- a/src/cmd/linuxkit/pkglib/build_test.go +++ b/src/cmd/linuxkit/pkglib/build_test.go @@ -201,7 +201,7 @@ func (c *cacheMocker) Push(name string) error { return nil } -func (c *cacheMocker) DescriptorWrite(ref *reference.Spec, descriptors ...registry.Descriptor) (lktspec.ImageSource, error) { +func (c *cacheMocker) DescriptorWrite(ref *reference.Spec, desc registry.Descriptor) (lktspec.ImageSource, error) { if !c.enabledDescriptorWrite { return nil, errors.New("descriptor disabled") } @@ -209,7 +209,7 @@ func (c *cacheMocker) DescriptorWrite(ref *reference.Spec, descriptors ...regist image = ref.String() im = registry.IndexManifest{ MediaType: types.OCIImageIndex, - Manifests: descriptors, + Manifests: []registry.Descriptor{desc}, SchemaVersion: 2, } ) diff --git a/src/cmd/linuxkit/spec/cache.go b/src/cmd/linuxkit/spec/cache.go index 692f58acb..2a91112ca 100644 --- a/src/cmd/linuxkit/spec/cache.go +++ b/src/cmd/linuxkit/spec/cache.go @@ -13,7 +13,7 @@ type CacheProvider interface { ImagePull(ref *reference.Spec, trustedRef, architecture string, alwaysPull bool) (ImageSource, error) IndexWrite(ref *reference.Spec, descriptors ...v1.Descriptor) (ImageSource, error) ImageLoad(ref *reference.Spec, architecture string, r io.Reader) (ImageSource, error) - DescriptorWrite(ref *reference.Spec, descriptors ...v1.Descriptor) (ImageSource, error) + DescriptorWrite(ref *reference.Spec, descriptors v1.Descriptor) (ImageSource, error) Push(name string) error NewSource(ref *reference.Spec, architecture string, descriptor *v1.Descriptor) ImageSource }