Support pull-through for manifest lists

pkg/imagestream should use layers API when it needs to know about
sub-manifests.

pkg/errors/errors.go

@@ -45,7 +45,10 @@ func NewError(code, msg string, err error) Error {
 }
 
 func (e registryError) Error() string {
-	return fmt.Sprintf("%s: %s: %s", e.code, e.message, e.err.Error())
+	if e.err != nil {
+		return fmt.Sprintf("%s: %s: %s", e.code, e.message, e.err.Error())
+	}
+	return fmt.Sprintf("%s: %s", e.code, e.message)
 }
 
 func (e registryError) Code() string {

pkg/imagestream/imagestream.go

@@ -143,7 +143,7 @@ func (is *imageStream) ResolveImageID(ctx context.Context, dgst digest.Digest) (
 	return tagEvent, nil
 }
 
-// GetStoredImageOfImageStream retrieves the Image with digest `dgst` and
+// getStoredImageOfImageStream retrieves the Image with digest `dgst` and
 // ensures that the image belongs to the image stream `is`. It uses two
 // queries to master API:
 //
@@ -168,15 +168,7 @@ func (is *imageStream) getStoredImageOfImageStream(ctx context.Context, dgst dig
 	return image, tagEvent, nil
 }
 
-// GetImageOfImageStream retrieves the Image with digest `dgst` for the image
-// stream. The image's field DockerImageReference is modified on the fly to
-// pretend that we've got the image from the source from which the image was
-// tagged to match tag's DockerImageReference.
-//
-// NOTE: due to on the fly modification, the returned image object should
-// not be sent to the master API. If you need unmodified version of the
-// image object, please use getStoredImageOfImageStream.
-func (is *imageStream) GetImageOfImageStream(ctx context.Context, dgst digest.Digest) (*imageapiv1.Image, rerrors.Error) {
+func (is *imageStream) getImageOfImageStream(ctx context.Context, dgst digest.Digest) (*imageapiv1.Image, rerrors.Error) {
 	image, tagEvent, err := is.getStoredImageOfImageStream(ctx, dgst)
 	if err != nil {
 		return nil, err
@@ -189,6 +181,109 @@ func (is *imageStream) GetImageOfImageStream(ctx context.Context, dgst digest.Di
 	return &img, nil
 }
 
+// GetImageOfImageStream retrieves the Image with digest `dgst` for the image
+// stream. The image's field DockerImageReference is modified on the fly to
+// pretend that we've got the image from the source from which the image was
+// tagged to match tag's DockerImageReference.
+//
+// The layers API is also searched, as a manifest which is part of a manifest
+// list in an image stream will not be available in the image stream history,
+// only its parent manifest list will be found there.
+//
+// If the Image with the given digest is not part of the image stream, a not found
+// error is returned.
+//
+// NOTE: due to on the fly modification, the returned image object should
+// not be sent to the master API. If you need unmodified version of the
+// image object, please use getStoredImageOfImageStream.
+func (is *imageStream) GetImageOfImageStream(ctx context.Context, dgst digest.Digest) (*imageapiv1.Image, rerrors.Error) {
+	isImage, err := is.getImageOfImageStream(ctx, dgst)
+	if err == nil {
+		return isImage, nil
+	}
+
+	ref, err := is.resolveUpstreamRef(ctx, dgst)
+	if err != nil {
+		return nil, err
+	}
+
+	image, err := is.getImage(ctx, dgst)
+	if err != nil {
+		return nil, err
+	}
+
+	// We don't want to mutate the origial image object, which we've got by reference.
+	img := *image
+	img.DockerImageReference = ref.String()
+
+	return &img, nil
+}
+
+// resolveUpstreamRef returns an image reference for an image with the given
+// digest that can be used to pull the image from the upstream repository.
+//
+// It uses the image layers API to find the parent image, and then finds the
+// upstream repository for the parent image in the image stream.
+//
+// It works only for sub-manifests, for which the image stream usually does not
+// have a history entry. For the main manifest, the image stream should have a
+// history entry that can be found by ResolveImageID.
+func (is *imageStream) resolveUpstreamRef(ctx context.Context, dgst digest.Digest) (reference.DockerImageReference, rerrors.Error) {
+	layers, rErr := is.imageStreamGetter.layers()
+	if rErr != nil {
+		return reference.DockerImageReference{}, rerrors.NewError(
+			ErrImageStreamUnknownErrorCode,
+			fmt.Sprintf("resolveUpstreamRef: failed to get layers for image stream %s", is.Reference()),
+			rErr,
+		)
+	}
+
+	parent := ""
+	for image, ibr := range layers.Images {
+		found := false
+		for _, m := range ibr.Manifests {
+			if m == dgst.String() {
+				found = true
+				break
+			}
+		}
+		if found {
+			parent = image
+			break
+		}
+	}
+	if parent == "" {
+		return reference.DockerImageReference{}, rerrors.NewError(
+			ErrImageStreamImageNotFoundCode,
+			fmt.Sprintf("resolveUpstreamRef: unable to find parent for image %s in image stream %s", dgst.String(), is.Reference()),
+			nil,
+		)
+	}
+
+	parentTagEvent, rErr := is.ResolveImageID(ctx, digest.Digest(parent))
+	if rErr != nil {
+		return reference.DockerImageReference{}, rerrors.NewError(
+			ErrImageStreamUnknownErrorCode,
+			fmt.Sprintf("resolveUpstreamRef: unable to get parent event %s in image stream %s", parent, is.Reference()),
+			rErr,
+		)
+	}
+
+	ref, err := reference.Parse(parentTagEvent.DockerImageReference)
+	if err != nil {
+		return reference.DockerImageReference{}, rerrors.NewError(
+			ErrImageStreamUnknownErrorCode,
+			fmt.Sprintf("resolveUpstreamRef: unable to parse parent image reference %s in image stream %s", parentTagEvent.DockerImageReference, is.Reference()),
+			err,
+		)
+	}
+
+	ref.Tag = ""
+	ref.ID = dgst.String()
+
+	return ref, nil
+}
+
 func (is *imageStream) GetSecrets() ([]corev1.Secret, rerrors.Error) {
 	secrets, err := is.registryOSClient.ImageStreamSecrets(is.namespace).Secrets(context.TODO(), is.name, metav1.GetOptions{})
 	if err != nil {

pkg/testframework/manifest.go

@@ -74,6 +74,42 @@ func NewSchema2ImageData() (Schema2ImageData, error) {
 	return data, nil
 }
 
+type ImageIndexData struct {
+	ManifestMediaType string
+	Manifest          []byte
+	ManifestDigest    digest.Digest
+}
+
+func NewImageIndexData(images ...Schema2ImageData) (ImageIndexData, error) {
+	manifests := make([]map[string]interface{}, 0, len(images))
+	for _, image := range images {
+		manifests = append(manifests, map[string]interface{}{
+			"mediaType": image.ManifestMediaType,
+			"size":      len(image.Manifest),
+			"digest":    image.ManifestDigest.String(),
+			"platform": map[string]interface{}{
+				"architecture": "amd64",
+				"os":           "linux",
+			},
+		})
+	}
+
+	manifest, err := json.Marshal(map[string]interface{}{
+		"schemaVersion": 2,
+		"mediaType":     "application/vnd.docker.distribution.manifest.list.v2+json",
+		"manifests":     manifests,
+	})
+	if err != nil {
+		return ImageIndexData{}, fmt.Errorf("unable to create image index: %v", err)
+	}
+
+	return ImageIndexData{
+		ManifestMediaType: "application/vnd.docker.distribution.manifest.list.v2+json",
+		Manifest:          manifest,
+		ManifestDigest:    digest.FromBytes(manifest),
+	}, nil
+}
+
 func ServeV2(w http.ResponseWriter, r *http.Request) bool {
 	if r.Method == "GET" && r.URL.Path == "/v2/" {
 		_, _ = w.Write([]byte(`{}`))
@@ -151,3 +187,16 @@ func PushSchema2ImageData(ctx context.Context, repo distribution.Repository, tag
 
 	return manifest, nil
 }
+
+func PushImageIndexData(ctx context.Context, repo distribution.Repository, tag string, data ImageIndexData) (distribution.Manifest, error) {
+	manifest, _, err := distribution.UnmarshalManifest(data.ManifestMediaType, data.Manifest)
+	if err != nil {
+		return manifest, fmt.Errorf("parse manifest: %w", err)
+	}
+
+	if err := testutil.UploadManifest(ctx, repo, tag, manifest); err != nil {
+		return manifest, fmt.Errorf("upload manifest: %w", err)
+	}
+
+	return manifest, nil
+}

pkg/testutil/manifests.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/manifest"
+	"github.com/docker/distribution/manifest/manifestlist"
 	"github.com/docker/distribution/manifest/ocischema"
 	"github.com/docker/distribution/manifest/schema1"
 	"github.com/docker/distribution/manifest/schema2"
@@ -114,7 +115,7 @@ func CanonicalManifest(m distribution.Manifest) ([]byte, error) {
 	switch m := m.(type) {
 	case *schema1.SignedManifest:
 		return m.Canonical, nil
-	case *schema2.DeserializedManifest, *ocischema.DeserializedManifest:
+	case *schema2.DeserializedManifest, *ocischema.DeserializedManifest, *manifestlist.DeserializedManifestList:
 		_, payload, err := m.Payload()
 		if err != nil {
 			return nil, err

test/integration/pullthrough/pullthrough_test.go

@@ -90,6 +90,16 @@ func TestPullThroughInsecure(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	submanifestData, err := testframework.NewSchema2ImageData()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	imageIndexData, err := testframework.NewImageIndexData(submanifestData)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	master := testframework.NewMaster(t)
 	defer master.Close()
 
@@ -100,6 +110,7 @@ func TestPullThroughInsecure(t *testing.T) {
 	// start regular HTTP server
 	reponame := "testrepo"
 	repotag := "testtag"
+	imageindextag := "manifestlist"
 	isname := "test/" + reponame
 
 	descriptors := map[string]int64{
@@ -123,6 +134,16 @@ func TestPullThroughInsecure(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	_, err = testframework.PushSchema2ImageData(context.TODO(), remoteRepo, "redundant-tag-to-make-testutil-happy", submanifestData)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	_, err = testframework.PushImageIndexData(context.TODO(), remoteRepo, imageindextag, imageIndexData)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	stream := imageapiv1.ImageStreamImport{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: namespace,
@@ -141,6 +162,13 @@ func TestPullThroughInsecure(t *testing.T) {
 					},
 					ImportPolicy: imageapiv1.TagImportPolicy{Insecure: true},
 				},
+				{
+					From: corev1.ObjectReference{
+						Kind: "DockerImage",
+						Name: remoteRegistryAddr + "/" + isname + ":" + imageindextag,
+					},
+					ImportPolicy: imageapiv1.TagImportPolicy{Insecure: true},
+				},
 			},
 		},
 	}
@@ -152,8 +180,8 @@ func TestPullThroughInsecure(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if len(isi.Status.Images) != 1 {
-		t.Fatalf("imported unexpected number of images (%d != 1)", len(isi.Status.Images))
+	if len(isi.Status.Images) != 2 {
+		t.Fatalf("imported unexpected number of images (%d != 2)", len(isi.Status.Images))
 	}
 	for i, image := range isi.Status.Images {
 		if image.Status.Status != metav1.StatusSuccess {
@@ -163,11 +191,6 @@ func TestPullThroughInsecure(t *testing.T) {
 		if image.Image == nil {
 			t.Fatalf("unexpected empty image %d", i)
 		}
-
-		// the image name is always the sha256, and size is calculated
-		if image.Image.Name != imageData.ManifestDigest.String() {
-			t.Fatalf("unexpected image %d: %#v (expect %q)", i, image.Image.Name, imageData.ManifestDigest.String())
-		}
 	}
 
 	istream, err := adminImageClient.ImageStreams(stream.Namespace).Get(context.Background(), stream.Name, metav1.GetOptions{})
@@ -199,6 +222,11 @@ func TestPullThroughInsecure(t *testing.T) {
 		t.Fatal(err)
 	}
 
+	t.Logf("Run testPullThroughGetManifest with submanifest digest...")
+	if err := testPullThroughGetManifest(registry.BaseURL(), &stream, testuser.Name, testuser.Token, submanifestData.ManifestDigest.String()); err != nil {
+		t.Fatal(err)
+	}
+
 	t.Logf("Run testPullThroughStatBlob (%s == true, spec.tags[%q].importPolicy.insecure == true)...", imageapiv1.InsecureRepositoryAnnotation, repotag)
 	for digest := range descriptors {
 		if err := testPullThroughStatBlob(registry.BaseURL(), &stream, testuser.Name, testuser.Token, digest); err != nil {
@@ -228,11 +256,13 @@ func TestPullThroughInsecure(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	for i, tag := range istream.Spec.Tags {
-		if tag.Name == repotag {
-			istream.Spec.Tags[i].ImportPolicy.Insecure = false
-			break
-		}
+	for i := range istream.Spec.Tags {
+		// Ideally we need to set insecure to false only for repotag.
+		// But if there is at least one tag with insecure set to true,
+		// its upstream registry is allowed to be accessed insecurely
+		// for all tags, so we set it to false for all tags to forbid
+		// insecure access.
+		istream.Spec.Tags[i].ImportPolicy.Insecure = false
 	}
 	_, err = adminImageClient.ImageStreams(istream.Namespace).Update(context.Background(), istream, metav1.UpdateOptions{})
 	if err != nil {