Merge pull request #3963 from deitch/fix-image-pull-with-attestations

Fix image pull with attestations

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)
+}

src/cmd/linuxkit/cache/pull.go

@@ -5,14 +5,23 @@ 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"
 )
 
-// ValidateImage given a reference, validate that it is complete. If not, pull down missing
+const (
-// components as necessary. It also calculates the hash of each component.
+	unknown = "unknown"
+)
+
+// 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
@@ -47,32 +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
+		// 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 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 && m.Platform.Architecture == architecture && m.Platform.OS == linux {
+			if m.Platform == nil || (m.Platform.Architecture == unknown && m.Platform.OS == unknown) {
-				img, err := imageIndex.Image(m.Digest)
+				if err := validateManifestContents(imageIndex, m.Digest); err != nil {
-				if err != nil {
+					return ImageSource{}, fmt.Errorf("invalid image: %w", err)
-					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)
-				}
-				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)
 		}
@@ -85,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
+}

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
+// If the ref and all of the content for the requested
-// points to an index with multiple architectures. 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 := p.cache.WriteIndex(ii); err != nil {
-		if err != nil {
+			return ImageSource{}, fmt.Errorf("unable to write index: %v", err)
-			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.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 p.NewSource(
-		return ImageSource{}, fmt.Errorf("unable to save image to cache: %v", err)
+		ref,
-	}
+		architecture,
-	// ensure it includes our architecture
+		&desc.Descriptor,
-	return p.ValidateImage(ref, architecture)
+	), nil
 }
 
 // ImageLoad takes an OCI format image tar stream and writes it locally. It should be

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 {

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
+	// get a reference to the local cache; we either will find the ref there or will pull to it
-	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
 	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)
 }