Support non-distributable layers in Write and MultiWrite (#930)

* Support non-distributable layers in Write and MultiWrite

Signed-off-by: Dennis Leon <[email protected]>

* Rename the func option to WithNonDistributable

* Also update the docstring

* WithNondistributable directly accepts *options

pkg/v1/remote/multi_write.go

@@ -45,23 +45,27 @@ func MultiWrite(m map[name.Reference]Taggable, options ...Option) error {
 		}
 	}
 
+	o, err := makeOptions(repo, options...)
+	if err != nil {
+		return err
+	}
+
 	// Collect unique blobs (layers and config blobs).
 	blobs := map[v1.Hash]v1.Layer{}
 	newManifests := []map[name.Reference]Taggable{}
 	// Separate originally requested images and indexes, so we can push images first.
 	images, indexes := map[name.Reference]Taggable{}, map[name.Reference]Taggable{}
-	var err error
 	for ref, i := range m {
 		if img, ok := i.(v1.Image); ok {
 			images[ref] = i
-			if err := addImageBlobs(img, blobs); err != nil {
+			if err := addImageBlobs(img, blobs, o.allowNondistributableArtifacts); err != nil {
 				return err
 			}
 			continue
 		}
 		if idx, ok := i.(v1.ImageIndex); ok {
 			indexes[ref] = i
-			newManifests, err = addIndexBlobs(idx, blobs, repo, newManifests, 0)
+			newManifests, err = addIndexBlobs(idx, blobs, repo, newManifests, 0, o.allowNondistributableArtifacts)
 			if err != nil {
 				return err
 			}
@@ -70,10 +74,6 @@ func MultiWrite(m map[name.Reference]Taggable, options ...Option) error {
 		return fmt.Errorf("pushable resource was not Image or ImageIndex: %T", i)
 	}
 
-	o, err := makeOptions(repo, options...)
-	if err != nil {
-		return err
-	}
 	// Determine if any of the layers are Mountable, because if so we need
 	// to request Pull scope too.
 	ls := []v1.Layer{}
@@ -161,7 +161,7 @@ func MultiWrite(m map[name.Reference]Taggable, options ...Option) error {
 
 // addIndexBlobs adds blobs to the set of blobs we intend to upload, and
 // returns the latest copy of the ordered collection of manifests to upload.
-func addIndexBlobs(idx v1.ImageIndex, blobs map[v1.Hash]v1.Layer, repo name.Repository, newManifests []map[name.Reference]Taggable, lvl int) ([]map[name.Reference]Taggable, error) {
+func addIndexBlobs(idx v1.ImageIndex, blobs map[v1.Hash]v1.Layer, repo name.Repository, newManifests []map[name.Reference]Taggable, lvl int, allowNondistributableArtifacts bool) ([]map[name.Reference]Taggable, error) {
 	if lvl > len(newManifests)-1 {
 		newManifests = append(newManifests, map[name.Reference]Taggable{})
 	}
@@ -177,7 +177,7 @@ func addIndexBlobs(idx v1.ImageIndex, blobs map[v1.Hash]v1.Layer, repo name.Repo
 			if err != nil {
 				return nil, err
 			}
-			newManifests, err = addIndexBlobs(idx, blobs, repo, newManifests, lvl+1)
+			newManifests, err = addIndexBlobs(idx, blobs, repo, newManifests, lvl+1, allowNondistributableArtifacts)
 			if err != nil {
 				return nil, err
 			}
@@ -189,7 +189,7 @@ func addIndexBlobs(idx v1.ImageIndex, blobs map[v1.Hash]v1.Layer, repo name.Repo
 			if err != nil {
 				return nil, err
 			}
-			if err := addImageBlobs(img, blobs); err != nil {
+			if err := addImageBlobs(img, blobs, allowNondistributableArtifacts); err != nil {
 				return nil, err
 			}
 
@@ -202,7 +202,7 @@ func addIndexBlobs(idx v1.ImageIndex, blobs map[v1.Hash]v1.Layer, repo name.Repo
 	return newManifests, nil
 }
 
-func addImageBlobs(img v1.Image, blobs map[v1.Hash]v1.Layer) error {
+func addImageBlobs(img v1.Image, blobs map[v1.Hash]v1.Layer, allowNondistributableArtifacts bool) error {
 	ls, err := img.Layers()
 	if err != nil {
 		return err
@@ -219,8 +219,7 @@ func addImageBlobs(img v1.Image, blobs map[v1.Hash]v1.Layer) error {
 		if err != nil {
 			return err
 		}
-		if !mt.IsDistributable() {
-			// TODO(jonjohnsonjr): Add "allow-nondistributable-artifacts" option.
+		if !mt.IsDistributable() && !allowNondistributableArtifacts {
 			continue
 		}
 

pkg/v1/remote/multi_write_test.go

@@ -107,6 +107,47 @@ func TestMultiWrite(t *testing.T) {
 	}
 }
 
+func TestMultiWriteWithNondistributableLayer(t *testing.T) {
+	// Create a random image.
+	img1, err := random.Image(1024, 2)
+	if err != nil {
+		t.Fatal("random.Image:", err)
+	}
+
+	// Create another image that's based on the first.
+	rl, err := random.Layer(1024, types.OCIRestrictedLayer)
+	if err != nil {
+		t.Fatal("random.Layer:", err)
+	}
+	img, err := mutate.AppendLayers(img1, rl)
+	if err != nil {
+		t.Fatal("mutate.AppendLayers:", err)
+	}
+
+	// Set up a fake registry.
+	s := httptest.NewServer(registry.New())
+	defer s.Close()
+	u, err := url.Parse(s.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Write the image.
+	tag1 := mustNewTag(t, u.Host+"/repo:tag1")
+	if err := MultiWrite(map[name.Reference]Taggable{tag1: img}, WithNondistributable); err != nil {
+		t.Error("Write:", err)
+	}
+
+	// Check that tagged image is present.
+	got, err := Image(tag1)
+	if err != nil {
+		t.Error(err)
+	}
+	if err := validate.Image(got); err != nil {
+		t.Error("Validate() =", err)
+	}
+}
+
 // TestMultiWrite_Deep tests that a deeply nested tree of manifest lists gets
 // pushed in the correct order (i.e., each level in sequence).
 func TestMultiWrite_Deep(t *testing.T) {

pkg/v1/remote/options.go

@@ -29,13 +29,14 @@ import (
 type Option func(*options) error
 
 type options struct {
-	auth      authn.Authenticator
-	keychain  authn.Keychain
-	transport http.RoundTripper
-	platform  v1.Platform
-	context   context.Context
-	jobs      int
-	userAgent string
+	auth                           authn.Authenticator
+	keychain                       authn.Keychain
+	transport                      http.RoundTripper
+	platform                       v1.Platform
+	context                        context.Context
+	jobs                           int
+	userAgent                      string
+	allowNondistributableArtifacts bool
 }
 
 var defaultPlatform = v1.Platform{
@@ -173,3 +174,13 @@ func WithUserAgent(ua string) Option {
 		return nil
 	}
 }
+
+// WithNondistributable includes non-distributable (foreign) layers
+// when writing images, see:
+// https://github.com/opencontainers/image-spec/blob/master/layer.md#non-distributable-layers
+//
+// The default behaviour is to skip these layers
+func WithNondistributable(o *options) error {
+	o.allowNondistributableArtifacts = true
+	return nil
+}

pkg/v1/remote/write.go

@@ -77,8 +77,7 @@ func Write(ref name.Reference, img v1.Image, options ...Option) error {
 		if err != nil {
 			return err
 		}
-		if !mt.IsDistributable() {
-			// TODO(jonjohnsonjr): Add "allow-nondistributable-artifacts" option.
+		if !mt.IsDistributable() && !o.allowNondistributableArtifacts {
 			continue
 		}
 

pkg/v1/remote/write_test.go

@@ -1276,7 +1276,7 @@ func (l *fakeForeignLayer) Uncompressed() (io.ReadCloser, error) {
 	return nil, nil
 }
 
-func TestSkipForeignLayers(t *testing.T) {
+func TestSkipForeignLayersByDefault(t *testing.T) {
 	// Set up an image with a foreign layer.
 	base := setupImage(t)
 	img, err := mutate.AppendLayers(base, &fakeForeignLayer{t: t})
@@ -1302,6 +1302,78 @@ func TestSkipForeignLayers(t *testing.T) {
 	}
 }
 
+func TestWriteForeignLayerIfOptionSet(t *testing.T) {
+	// Set up an image with a foreign layer.
+	base := setupImage(t)
+	foreignLayer, err := random.Layer(1024, types.DockerForeignLayer)
+	if err != nil {
+		t.Fatal("random.Layer:", err)
+	}
+	img, err := mutate.AppendLayers(base, foreignLayer)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expectedRepo := "write/time"
+	headPathPrefix := fmt.Sprintf("/v2/%s/blobs/", expectedRepo)
+	initiatePath := fmt.Sprintf("/v2/%s/blobs/uploads/", expectedRepo)
+	manifestPath := fmt.Sprintf("/v2/%s/manifests/latest", expectedRepo)
+	uploadPath := "/upload"
+	commitPath := "/commit"
+	var numUploads int32
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodHead && strings.HasPrefix(r.URL.Path, headPathPrefix) && r.URL.Path != initiatePath {
+			http.Error(w, "NotFound", http.StatusNotFound)
+			return
+		}
+		switch r.URL.Path {
+		case "/v2/":
+			w.WriteHeader(http.StatusOK)
+		case initiatePath:
+			if r.Method != http.MethodPost {
+				t.Errorf("Method; got %v, want %v", r.Method, http.MethodPost)
+			}
+			w.Header().Set("Location", uploadPath)
+			http.Error(w, "Accepted", http.StatusAccepted)
+		case uploadPath:
+			if r.Method != http.MethodPatch {
+				t.Errorf("Method; got %v, want %v", r.Method, http.MethodPatch)
+			}
+			atomic.AddInt32(&numUploads, 1)
+			w.Header().Set("Location", commitPath)
+			http.Error(w, "Created", http.StatusCreated)
+		case commitPath:
+			http.Error(w, "Created", http.StatusCreated)
+		case manifestPath:
+			if r.Method != http.MethodPut {
+				t.Errorf("Method; got %v, want %v", r.Method, http.MethodPut)
+			}
+			http.Error(w, "Created", http.StatusCreated)
+		default:
+			t.Fatalf("Unexpected path: %v", r.URL.Path)
+		}
+	}))
+	defer server.Close()
+	u, err := url.Parse(server.URL)
+	if err != nil {
+		t.Fatalf("url.Parse(%v) = %v", server.URL, err)
+	}
+	tag, err := name.NewTag(fmt.Sprintf("%s/%s:latest", u.Host, expectedRepo), name.WeakValidation)
+	if err != nil {
+		t.Fatalf("NewTag() = %v", err)
+	}
+
+	if err := Write(tag, img, WithNondistributable); err != nil {
+		t.Errorf("Write: %v", err)
+	}
+
+	// 1 random layer, 1 foreign layer, 1 image config blob
+	wantUploads := int32(1 + 1 + 1)
+	if numUploads != wantUploads {
+		t.Fatalf("Write uploaded %d blobs, want %d", numUploads, wantUploads)
+	}
+}
+
 func TestTag(t *testing.T) {
 	idx := setupIndex(t, 3)
 	// Set up a fake registry.