From e1d3a099764ce9cbb596c3dcacf3c098a5f5dce3 Mon Sep 17 00:00:00 2001 From: Avi Deitcher Date: Thu, 30 Nov 2023 14:31:17 +0200 Subject: [PATCH 1/2] when filling cache, ensure we include attestations Signed-off-by: Avi Deitcher --- src/cmd/linuxkit/cache/pull.go | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/cmd/linuxkit/cache/pull.go b/src/cmd/linuxkit/cache/pull.go index 72ea27e4d..796444959 100644 --- a/src/cmd/linuxkit/cache/pull.go +++ b/src/cmd/linuxkit/cache/pull.go @@ -5,12 +5,16 @@ import ( "fmt" "github.com/containerd/containerd/reference" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/partial" "github.com/google/go-containerregistry/pkg/v1/validate" lktspec "github.com/linuxkit/linuxkit/src/cmd/linuxkit/spec" ) +const ( + unknown = "unknown" +) + // ValidateImage given a reference, validate that it is complete. If not, pull down missing // components as necessary. It also calculates the hash of each component. func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lktspec.ImageSource, error) { @@ -49,13 +53,17 @@ 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 + // check that the index has a manifest for our arch, as well as any non-arch-specific ones im, err := imageIndex.IndexManifest() if err != nil { return ImageSource{}, fmt.Errorf("could not get index manifest: %v", err) } + var found bool for _, m := range im.Manifests { - if m.Platform != nil && m.Platform.Architecture == architecture && m.Platform.OS == linux { + if m.Platform == nil { + continue + } + if m.Platform.Architecture == architecture && m.Platform.OS == linux { img, err := imageIndex.Image(m.Digest) if err != nil { return ImageSource{}, fmt.Errorf("unable to get image: %v", err) @@ -63,6 +71,18 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts if err := validate.Image(img); err != nil { return ImageSource{}, fmt.Errorf("invalid image: %s", err) } + found = true + } + if m.Platform.Architecture == unknown && m.Platform.OS == unknown { + img, err := imageIndex.Image(m.Digest) + if err != nil { + return ImageSource{}, fmt.Errorf("unable to get image: %v", err) + } + if err := validate.Image(img); err != nil { + return ImageSource{}, fmt.Errorf("invalid image: %s", err) + } + } + if found { return p.NewSource( ref, architecture, From 6e54a7bd6e4a298b5698061c6b344d2ca3e64fc7 Mon Sep 17 00:00:00 2001 From: Avi Deitcher Date: Thu, 30 Nov 2023 15:28:26 +0200 Subject: [PATCH 2/2] properly write index when pulling image, including all manifests Signed-off-by: Avi Deitcher --- src/cmd/linuxkit/cache/errors.go | 13 ++++++ src/cmd/linuxkit/cache/pull.go | 77 +++++++++++++++++++------------- src/cmd/linuxkit/cache/write.go | 53 ++++++++++------------ src/cmd/linuxkit/moby/build.go | 2 +- src/cmd/linuxkit/moby/images.go | 15 ++----- 5 files changed, 86 insertions(+), 74 deletions(-) create mode 100644 src/cmd/linuxkit/cache/errors.go diff --git a/src/cmd/linuxkit/cache/errors.go b/src/cmd/linuxkit/cache/errors.go new file mode 100644 index 000000000..b6d8207f5 --- /dev/null +++ b/src/cmd/linuxkit/cache/errors.go @@ -0,0 +1,13 @@ +package cache + +import ( + "fmt" +) + +type noReferenceError struct { + reference string +} + +func (e *noReferenceError) Error() string { + return fmt.Sprintf("no such reference: %s", e.reference) +} diff --git a/src/cmd/linuxkit/cache/pull.go b/src/cmd/linuxkit/cache/pull.go index 796444959..61aeef57e 100644 --- a/src/cmd/linuxkit/cache/pull.go +++ b/src/cmd/linuxkit/cache/pull.go @@ -15,8 +15,13 @@ const ( unknown = "unknown" ) -// ValidateImage given a reference, validate that it is complete. If not, pull down missing -// components as necessary. It also calculates the hash of each component. +// ValidateImage given a reference, validate that it is complete. If not, it will *not* +// pull down missing content. This function is network-free. If you wish to validate +// and fill in missing content, use PullImage. +// If the reference is to an index, it will check the index for a manifest for the given +// architecture, and any manifests that have no architecture at all. It will ignore manifests +// for other architectures. If no architecture is provided, it will validate all manifests. +// It also calculates the hash of each component. func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lktspec.ImageSource, error) { var ( imageIndex v1.ImageIndex @@ -51,48 +56,39 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts case imageIndex == nil && image == nil: // we did not find it yet - either because we were told not to look locally, // or because it was not available - so get it from the remote - return ImageSource{}, errors.New("no such image") + return ImageSource{}, &noReferenceError{reference: imageName} case imageIndex != nil: // check that the index has a manifest for our arch, as well as any non-arch-specific ones im, err := imageIndex.IndexManifest() if err != nil { - return ImageSource{}, fmt.Errorf("could not get index manifest: %v", err) + return ImageSource{}, fmt.Errorf("could not get index manifest: %w", err) } - var found bool + var architectures = make(map[string]bool) + // ignore only other architectures; manifest entries that have no architectures at all + // are going to be additional metadata, so we need to check them for _, m := range im.Manifests { - if m.Platform == nil { - continue - } - if m.Platform.Architecture == architecture && m.Platform.OS == linux { - img, err := imageIndex.Image(m.Digest) - if err != nil { - return ImageSource{}, fmt.Errorf("unable to get image: %v", err) - } - if err := validate.Image(img); err != nil { - return ImageSource{}, fmt.Errorf("invalid image: %s", err) - } - found = true - } - if m.Platform.Architecture == unknown && m.Platform.OS == unknown { - img, err := imageIndex.Image(m.Digest) - if err != nil { - return ImageSource{}, fmt.Errorf("unable to get image: %v", err) - } - if err := validate.Image(img); err != nil { - return ImageSource{}, fmt.Errorf("invalid image: %s", err) + if m.Platform == nil || (m.Platform.Architecture == unknown && m.Platform.OS == unknown) { + if err := validateManifestContents(imageIndex, m.Digest); err != nil { + return ImageSource{}, fmt.Errorf("invalid image: %w", err) } } - if found { - return p.NewSource( - ref, - architecture, - desc, - ), nil + if architecture != "" && m.Platform.Architecture == architecture && m.Platform.OS == linux { + if err := validateManifestContents(imageIndex, m.Digest); err != nil { + return ImageSource{}, fmt.Errorf("invalid image: %w", err) + } + architectures[architecture] = true } } + if architecture == "" || architectures[architecture] { + 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, make sure it is up to date, and that it matches our platform + // we found a local image, make sure it is up to date if err := validate.Image(image); err != nil { return ImageSource{}, fmt.Errorf("invalid image, %s", err) } @@ -105,3 +101,20 @@ func (p *Provider) ValidateImage(ref *reference.Spec, architecture string) (lkts // 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") } + +// validateManifestContents given an index and a digest, validate that the contents of the +// manifest are a valid image. This function is network-free. +// The only validation it does is checks that all of the parts of the image exist. +func validateManifestContents(index v1.ImageIndex, digest v1.Hash) error { + img, err := index.Image(digest) + if err != nil { + return fmt.Errorf("unable to get image: %w", err) + } + if _, err := img.ConfigFile(); err != nil { + return fmt.Errorf("unable to get config: %w", err) + } + if _, err := img.Layers(); err != nil { + return fmt.Errorf("unable to get layers: %w", err) + } + return nil +} diff --git a/src/cmd/linuxkit/cache/write.go b/src/cmd/linuxkit/cache/write.go index e8008a610..03fce0e52 100644 --- a/src/cmd/linuxkit/cache/write.go +++ b/src/cmd/linuxkit/cache/write.go @@ -32,11 +32,14 @@ const ( // ImagePull takes an image name and ensures that the image manifest or index to which it refers // exists in local cache and, if not, pulls it from the registry and writes it locally. It should be // efficient and only write missing blobs, based on their content hash. -// It will only pull the actual blobs, config and manifest for the requested architectures, even if ref -// points to an index with multiple architectures. If the ref and all of the content for the requested +// If the ref and all of the content for the requested // architectures already exist in the cache, it will not pull anything, unless alwaysPull is set to true. // If you call it multiple times, even with different architectures, the ref will continue to point to the same index. // Only the underlying content will be added. +// However, do note that it *always* reaches out to the remote registry to check the content. +// If you just want to check the status of a local ref, use ValidateImage. +// Note that ImagePull does try ValidateImage first, so if the image is already in the cache, it will not +// do any network activity at all. func (p *Provider) ImagePull(ref *reference.Spec, trustedRef, architecture string, alwaysPull bool) (lktspec.ImageSource, error) { image := ref.String() pullImageName := image @@ -47,15 +50,21 @@ func (p *Provider) ImagePull(ref *reference.Spec, trustedRef, architecture strin log.Debugf("ImagePull to cache %s trusted reference %s", image, pullImageName) // unless alwaysPull is set to true, check locally first - if !alwaysPull { + if alwaysPull { + log.Printf("Instructed always to pull, so pulling image %s arch %s", image, architecture) + } else { imgSrc, err := p.ValidateImage(ref, architecture) - if err == nil && imgSrc != nil { + switch { + case err == nil && imgSrc != nil: log.Printf("Image %s arch %s found in local cache, not pulling", image, architecture) return imgSrc, nil + case err != nil && errors.Is(err, &noReferenceError{}): + log.Printf("Image %s arch %s not found in local cache, pulling", image, architecture) + default: + log.Printf("Image %s arch %s incomplete or invalid in local cache, error %v, pulling", image, architecture, err) } // there was an error, so try to pull } - log.Printf("Image %s arch %s not found in local cache, pulling", image, architecture) remoteRef, err := name.ParseReference(pullImageName) if err != nil { return ImageSource{}, fmt.Errorf("invalid image name %s: %v", pullImageName, err) @@ -75,28 +84,12 @@ func (p *Provider) ImagePull(ref *reference.Spec, trustedRef, architecture strin ii, err := desc.ImageIndex() if err == nil { log.Debugf("ImageWrite retrieved %s is index, saving", pullImageName) - im, err := ii.IndexManifest() - if err != nil { - return ImageSource{}, fmt.Errorf("unable to get IndexManifest: %v", err) - } - // write the index blob and the descriptor - if err := p.cache.WriteBlob(desc.Digest, io.NopCloser(bytes.NewReader(desc.Manifest))); err != nil { - return ImageSource{}, fmt.Errorf("unable to write index content to cache: %v", err) + if err := p.cache.WriteIndex(ii); err != nil { + return ImageSource{}, fmt.Errorf("unable to write index: %v", err) } if _, err := p.DescriptorWrite(ref, desc.Descriptor); err != nil { return ImageSource{}, fmt.Errorf("unable to write index descriptor to cache: %v", err) } - for _, m := range im.Manifests { - if m.MediaType.IsImage() && (m.Platform == nil || m.Platform.Architecture == architecture) { - img, err := ii.Image(m.Digest) - if err != nil { - return ImageSource{}, fmt.Errorf("unable to get image: %v", err) - } - if err := p.cache.WriteImage(img); err != nil { - return ImageSource{}, fmt.Errorf("unable to write image: %v", err) - } - } - } } else { var im v1.Image // try an image @@ -105,13 +98,15 @@ func (p *Provider) ImagePull(ref *reference.Spec, trustedRef, architecture strin return ImageSource{}, fmt.Errorf("provided image is neither an image nor an index: %s", image) } log.Debugf("ImageWrite retrieved %s is image, saving", pullImageName) - err = p.cache.ReplaceImage(im, match.Name(image), layout.WithAnnotations(annotations)) + if err = p.cache.ReplaceImage(im, match.Name(image), layout.WithAnnotations(annotations)); err != nil { + return ImageSource{}, fmt.Errorf("unable to save image to cache: %v", err) + } } - if err != nil { - return ImageSource{}, fmt.Errorf("unable to save image to cache: %v", err) - } - // ensure it includes our architecture - return p.ValidateImage(ref, architecture) + return p.NewSource( + ref, + architecture, + &desc.Descriptor, + ), nil } // ImageLoad takes an OCI format image tar stream and writes it locally. It should be diff --git a/src/cmd/linuxkit/moby/build.go b/src/cmd/linuxkit/moby/build.go index 9e6afcf47..600f5518a 100644 --- a/src/cmd/linuxkit/moby/build.go +++ b/src/cmd/linuxkit/moby/build.go @@ -173,7 +173,7 @@ func Build(m Moby, w io.Writer, opts BuildOpts) error { log.Infof("Process init image: %s", ii) err := ImageTar(ii, "", apkTar, resolvconfSymlink, opts) if err != nil { - return fmt.Errorf("Failed to build init tarball from %s: %v", ii, err) + return fmt.Errorf("failed to build init tarball from %s: %v", ii, err) } } if err := apkTar.WriteAPKDB(); err != nil { diff --git a/src/cmd/linuxkit/moby/images.go b/src/cmd/linuxkit/moby/images.go index 76b3ef61e..46a6fafe4 100644 --- a/src/cmd/linuxkit/moby/images.go +++ b/src/cmd/linuxkit/moby/images.go @@ -24,21 +24,12 @@ func imagePull(ref *reference.Spec, alwaysPull bool, cacheDir string, dockerCach // docker is not required, so any error - image not available, no docker, whatever - just gets ignored } - // next try the local cache - if !alwaysPull { - c, err := cache.NewProvider(cacheDir) - if err != nil { - return nil, err - } - if image, err := c.ValidateImage(ref, architecture); err == nil { - return image, nil - } - } - - // if we made it here, we either did not have the image, or it was incomplete + // get a reference to the local cache; we either will find the ref there or will pull to it c, err := cache.NewProvider(cacheDir) if err != nil { return nil, err } + + // if we made it here, we either did not have the image, or it was incomplete return c.ImagePull(ref, ref.String(), architecture, alwaysPull) }