Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support non-distributable layers in Write and MultiWrite #930

Merged
merged 3 commits into from
Feb 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 12 additions & 13 deletions pkg/v1/remote/multi_write.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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{}
Expand Down Expand Up @@ -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{})
}
Expand All @@ -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
}
Expand All @@ -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
}

Expand All @@ -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
Expand All @@ -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
}

Expand Down
41 changes: 41 additions & 0 deletions pkg/v1/remote/multi_write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
25 changes: 18 additions & 7 deletions pkg/v1/remote/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
3 changes: 1 addition & 2 deletions pkg/v1/remote/write.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
74 changes: 73 additions & 1 deletion pkg/v1/remote/write_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand All @@ -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.
Expand Down