Repository: google/go-containerregistry
Pull request #930 (2021-02-04 00:46:19)

Support non-distributable layers in Write and MultiWrite
Implements #741

google-cla[bot]: 
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: **Please visit <https://cla.developers.google.com/> to sign.**

Once you've signed (or fixed any issues), please reply here with `@googlebot I signed it!` and we'll verify it.

----

#### What to do if you already signed the CLA

##### Individual signers

*   It's possible we don't have your GitHub username or you're using a different email address on your commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).

##### Corporate signers

*   Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to [go/cla#troubleshoot](http://go/cla#troubleshoot) ([Public version](https://opensource.google/docs/cla/#troubleshoot)).
*   The email used to register you as an authorized contributor must be the email used for the Git commit. Check [your existing CLA data](https://cla.developers.google.com/clas) and verify that your [email is set on your git commits](https://help.github.com/articles/setting-your-email-in-git/).
*   The email used to register you as an authorized contributor must also be [attached to your GitHub account](https://github.com/settings/emails).
		

ℹ️ **Googlers: [Go here](https://goto.google.com/prinfo/https%3A%2F%2Fgithub.com%2Fgoogle%2Fgo-containerregistry%2Fpull%2F930) for more info**.

<!-- need_sender_cla -->

Patch:
diff --git a/.github/workflows/knative-releasability.yaml b/.github/workflows/knative-releasability.yaml
index ca0820a6..cecf0a52 100644
--- a/.github/workflows/knative-releasability.yaml
+++ b/.github/workflows/knative-releasability.yaml
@@ -36,8 +36,8 @@ jobs:
     env:
       #########################################
       #   Update this section each release.   #
-      RELEASE: 'v0.19'
-      SLACK_CHANNEL: 'release-19'
+      RELEASE: 'v0.20'
+      SLACK_CHANNEL: 'release-20'
       #########################################
 
     steps:
@@ -50,6 +50,9 @@ jobs:
           if [[ "${{ github.event.inputs.slackChannel }}" != "" ]]; then
             echo "SLACK_CHANNEL=${{ github.event.inputs.slackChannel }}" >> $GITHUB_ENV
           fi
+          if [[ "${{ secrets.SLACK_WEBHOOK }}" != "" ]]; then
+            echo "SLACK_WEBHOOK=exists" >> $GITHUB_ENV
+          fi
 
       - name: Set up Go 1.15.x
         uses: actions/setup-go@v2
@@ -142,7 +145,8 @@ jobs:
           echo 'SLACK_TITLE=Releasability for ${{ github.repository }} @ ${{ env.RELEASE }} is NO-GO!' >> $GITHUB_ENV
 
       - name: Post status to Slack
-        if: ${{ secrets.SLACK_WEBHOOK != '' && steps.exists.outputs.release-branch == 'false' }}
+        # Note: using env.SLACK_WEBHOOK here because secrets are not allowed in the if block.
+        if: env.SLACK_WEBHOOK != '' && steps.exists.outputs.release-branch == 'false'
         uses: rtCamp/action-slack-notify@v2.1.0
         env:
           SLACK_ICON: http://github.com/knative.png?size=48
diff --git a/.github/workflows/knative-style.yaml b/.github/workflows/knative-style.yaml
index 3c7f28dd..4e0947a9 100644
--- a/.github/workflows/knative-style.yaml
+++ b/.github/workflows/knative-style.yaml
@@ -127,7 +127,7 @@ jobs:
         if: steps.golangci_configuration.outputs.files_exists == 'true'
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.32
+          version: v1.35
 
       - name: misspell
         shell: bash
@@ -141,7 +141,12 @@ jobs:
           echo '::group:: Running github.com/client9/misspell with reviewdog 🐶 ...'
           # Don't fail because of misspell
           set +o pipefail
-          find . -type f -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*' |
+          # Exclude generated and vendored files, plus some legacy
+          # paths until we update all .gitattributes
+          git ls-files |
+          git check-attr --stdin linguist-generated | grep -Ev ': (set|true)$' | cut -d: -f1 |
+          git check-attr --stdin linguist-vendored | grep -Ev ': (set|true)$' | cut -d: -f1 |
+          grep -Ev '^(vendor/|third_party/|.git)' |
           xargs misspell -error |
           reviewdog -efm="%f:%l:%c: %m" \
                 -name="github.com/client9/misspell" \
@@ -164,7 +169,13 @@ jobs:
           echo '::group:: Flagging trailing whitespace with reviewdog 🐶 ...'
           # Don't fail because of grep
           set +o pipefail
-          find . -type f -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*' |
+          
+          # Exclude generated and vendored files, plus some legacy
+          # paths until we update all .gitattributes
+          git ls-files |
+          git check-attr --stdin linguist-generated | grep -Ev ': (set|true)$' | cut -d: -f1 |
+          git check-attr --stdin linguist-vendored | grep -Ev ': (set|true)$' | cut -d: -f1 |
+          grep -Ev '^(vendor/|third_party/|.git)' |
           xargs grep -nE " +$" |
           reviewdog -efm="%f:%l:%m" \
                 -name="trailing whitespace" \
@@ -192,7 +203,13 @@ jobs:
           #  - nothing in third_party
           #  - nothing in .git/
           #  - no *.ai (Adobe Illustrator) files.
-          for x in $(find . -type f -not -name '*.ai' -not -path './vendor/*' -not -path './third_party/*' -not -path './.git/*'); do
+          LINT_FILES=$(git ls-files |
+          git check-attr --stdin linguist-generated | grep -Ev ': (set|true)$' | cut -d: -f1 |
+          git check-attr --stdin linguist-vendored | grep -Ev ': (set|true)$' | cut -d: -f1 |
+          grep -Ev '^(vendor/|third_party/|.git)' |
+          grep -v '\.ai$')
+
+          for x in $LINT_FILES; do
             # Based on https://stackoverflow.com/questions/34943632/linux-check-if-there-is-an-empty-line-at-the-end-of-a-file
             if [[ -f $x && ! ( -s "$x" && -z "$(tail -c 1 $x)" ) ]]; then
               # We add 1 to `wc -l` here because of this limitation (from the man page):
diff --git a/pkg/v1/remote/multi_write.go b/pkg/v1/remote/multi_write.go
index 803c2293..796b2248 100644
--- a/pkg/v1/remote/multi_write.go
+++ b/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
 		}
 
diff --git a/pkg/v1/remote/options.go b/pkg/v1/remote/options.go
index 5544fabf..04ff5082 100644
--- a/pkg/v1/remote/options.go
+++ b/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
+}
diff --git a/pkg/v1/remote/write.go b/pkg/v1/remote/write.go
index d850203d..55fc1afe 100644
--- a/pkg/v1/remote/write.go
+++ b/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
 		}
 


Test patch:
diff --git a/pkg/v1/remote/multi_write_test.go b/pkg/v1/remote/multi_write_test.go
index 2e6fef7f..6abe8b68 100644
--- a/pkg/v1/remote/multi_write_test.go
+++ b/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) {
diff --git a/pkg/v1/remote/write_test.go b/pkg/v1/remote/write_test.go
index a79d2ebb..caf0ec1e 100644
--- a/pkg/v1/remote/write_test.go
+++ b/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.