From 26b7fe4a9161bc99b43a398f275b1e460449ca34 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Fri, 17 Jul 2015 17:07:11 -0700 Subject: [PATCH] Use "Size" field to describe blobs over "Length" After consideration, we've changed the main descriptor field name to for number of bytes to "size" to match convention. While this may be a subjective argument, commonly we refer to files by their "size" rather than their "length". This will match other conventions, like `(FileInfo).Size()` and methods on `io.SizeReaderAt`. Under more broad analysis, this argument doesn't necessarily hold up. If anything, "size" is shorter than "length". Signed-off-by: Stephen J Day --- blobs.go | 4 ++-- docs/notifications.md | 8 +++++++- notifications/bridge.go | 2 ++ notifications/event.go | 4 ++++ notifications/event_test.go | 14 ++++++++++---- registry/client/repository.go | 6 +++--- registry/client/repository_test.go | 16 ++++++++-------- registry/storage/blob_test.go | 10 +++++----- registry/storage/blobserver.go | 4 ++-- registry/storage/blobstore.go | 6 +++--- registry/storage/blobwriter.go | 10 +++++----- registry/storage/cache/cache.go | 4 ++-- registry/storage/cache/redis/redis.go | 11 +++++++---- registry/storage/cache/suite.go | 6 +++--- 14 files changed, 63 insertions(+), 42 deletions(-) diff --git a/blobs.go b/blobs.go index c606d9149f..b0c89d1f33 100644 --- a/blobs.go +++ b/blobs.go @@ -49,8 +49,8 @@ type Descriptor struct { // encoded as utf-8. MediaType string `json:"mediaType,omitempty"` - // Length in bytes of content. - Length int64 `json:"length,omitempty"` + // Size in bytes of content. + Size int64 `json:"size,omitempty"` // Digest uniquely identifies the content. A byte stream can be verified // against against this digest. diff --git a/docs/notifications.md b/docs/notifications.md index 09f41d8eeb..4875cf17b5 100644 --- a/docs/notifications.md +++ b/docs/notifications.md @@ -84,8 +84,9 @@ manifest: "action": "push", "target": { "mediaType": "application/vnd.docker.distribution.manifest.v1+json", - "length": 1, + "size": 1, "digest": "sha256:0123456789abcdef0", + "length": 1, "repository": "library/test", "url": "http://example.com/v2/library/test/manifests/latest" }, @@ -105,6 +106,11 @@ manifest: } ``` +> __NOTE(stevvooe):__ As of version 2.1, the `length` field for event targets +> is being deprecated for the `size` field, bringing the target in line with +> common nomenclature. Both will continue to be set for the foreseeable +> future. Newer code should favor `size` but accept either. + ## Envelope The envelope contains one or more events, with the following json structure: diff --git a/notifications/bridge.go b/notifications/bridge.go index fff45a4fc2..b97925a514 100644 --- a/notifications/bridge.go +++ b/notifications/bridge.go @@ -97,6 +97,7 @@ func (b *bridge) createManifestEvent(action string, repo string, sm *manifest.Si } event.Target.Length = int64(len(p)) + event.Target.Size = int64(len(p)) event.Target.Digest, err = digest.FromBytes(p) if err != nil { return nil, err @@ -122,6 +123,7 @@ func (b *bridge) createBlobEventAndWrite(action string, repo string, desc distri func (b *bridge) createBlobEvent(action string, repo string, desc distribution.Descriptor) (*Event, error) { event := b.createEvent(action) event.Target.Descriptor = desc + event.Target.Length = desc.Size event.Target.Repository = repo var err error diff --git a/notifications/event.go b/notifications/event.go index a898021b97..9703002649 100644 --- a/notifications/event.go +++ b/notifications/event.go @@ -54,6 +54,10 @@ type Event struct { distribution.Descriptor + // Length in bytes of content. Same as Size field in Descriptor. + // Provided for backwards compatibility. + Length int64 `json:"length,omitempty"` + // Repository identifies the named repository. Repository string `json:"repository,omitempty"` diff --git a/notifications/event_test.go b/notifications/event_test.go index c413ce79bd..ac4dfd93f0 100644 --- a/notifications/event_test.go +++ b/notifications/event_test.go @@ -22,8 +22,9 @@ func TestEventEnvelopeJSONFormat(t *testing.T) { "action": "push", "target": { "mediaType": "application/vnd.docker.distribution.manifest.v1+json", - "length": 1, + "size": 1, "digest": "sha256:0123456789abcdef0", + "length": 1, "repository": "library/test", "url": "http://example.com/v2/library/test/manifests/latest" }, @@ -47,8 +48,9 @@ func TestEventEnvelopeJSONFormat(t *testing.T) { "action": "push", "target": { "mediaType": "application/vnd.docker.container.image.rootfs.diff+x-gtar", - "length": 2, + "size": 2, "digest": "tarsum.v2+sha256:0123456789abcdef1", + "length": 2, "repository": "library/test", "url": "http://example.com/v2/library/test/manifests/latest" }, @@ -72,8 +74,9 @@ func TestEventEnvelopeJSONFormat(t *testing.T) { "action": "push", "target": { "mediaType": "application/vnd.docker.container.image.rootfs.diff+x-gtar", - "length": 3, + "size": 3, "digest": "tarsum.v2+sha256:0123456789abcdef2", + "length": 3, "repository": "library/test", "url": "http://example.com/v2/library/test/manifests/latest" }, @@ -115,7 +118,8 @@ func TestEventEnvelopeJSONFormat(t *testing.T) { manifestPush = prototype manifestPush.ID = "asdf-asdf-asdf-asdf-0" manifestPush.Target.Digest = "sha256:0123456789abcdef0" - manifestPush.Target.Length = int64(1) + manifestPush.Target.Length = 1 + manifestPush.Target.Size = 1 manifestPush.Target.MediaType = manifest.ManifestMediaType manifestPush.Target.Repository = "library/test" manifestPush.Target.URL = "http://example.com/v2/library/test/manifests/latest" @@ -125,6 +129,7 @@ func TestEventEnvelopeJSONFormat(t *testing.T) { layerPush0.ID = "asdf-asdf-asdf-asdf-1" layerPush0.Target.Digest = "tarsum.v2+sha256:0123456789abcdef1" layerPush0.Target.Length = 2 + layerPush0.Target.Size = 2 layerPush0.Target.MediaType = layerMediaType layerPush0.Target.Repository = "library/test" layerPush0.Target.URL = "http://example.com/v2/library/test/manifests/latest" @@ -134,6 +139,7 @@ func TestEventEnvelopeJSONFormat(t *testing.T) { layerPush1.ID = "asdf-asdf-asdf-asdf-2" layerPush1.Target.Digest = "tarsum.v2+sha256:0123456789abcdef2" layerPush1.Target.Length = 3 + layerPush1.Target.Size = 3 layerPush1.Target.MediaType = layerMediaType layerPush1.Target.Repository = "library/test" layerPush1.Target.URL = "http://example.com/v2/library/test/manifests/latest" diff --git a/registry/client/repository.go b/registry/client/repository.go index 840a7af6f1..fc90cb6e4d 100644 --- a/registry/client/repository.go +++ b/registry/client/repository.go @@ -343,7 +343,7 @@ func (bs *blobs) Open(ctx context.Context, dgst digest.Digest) (distribution.Rea return nil, err } - return transport.NewHTTPReadSeeker(bs.client, blobURL, stat.Length), nil + return transport.NewHTTPReadSeeker(bs.client, blobURL, stat.Size), nil } func (bs *blobs) ServeBlob(ctx context.Context, w http.ResponseWriter, r *http.Request, dgst digest.Digest) error { @@ -366,7 +366,7 @@ func (bs *blobs) Put(ctx context.Context, mediaType string, p []byte) (distribut desc := distribution.Descriptor{ MediaType: mediaType, - Length: int64(len(p)), + Size: int64(len(p)), Digest: dgstr.Digest(), } @@ -435,7 +435,7 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi return distribution.Descriptor{ MediaType: resp.Header.Get("Content-Type"), - Length: length, + Size: length, Digest: dgst, }, nil case http.StatusNotFound: diff --git a/registry/client/repository_test.go b/registry/client/repository_test.go index 642ef99810..3a91be9800 100644 --- a/registry/client/repository_test.go +++ b/registry/client/repository_test.go @@ -127,8 +127,8 @@ func TestBlobExists(t *testing.T) { t.Fatalf("Unexpected digest: %s, expected %s", stat.Digest, d1) } - if stat.Length != int64(len(b1)) { - t.Fatalf("Unexpected length: %d, expected %d", stat.Length, len(b1)) + if stat.Size != int64(len(b1)) { + t.Fatalf("Unexpected length: %d, expected %d", stat.Size, len(b1)) } // TODO(dmcgowan): Test error cases and ErrBlobUnknown case @@ -244,14 +244,14 @@ func TestBlobUploadChunked(t *testing.T) { blob, err := upload.Commit(ctx, distribution.Descriptor{ Digest: dgst, - Length: int64(len(b1)), + Size: int64(len(b1)), }) if err != nil { t.Fatal(err) } - if blob.Length != int64(len(b1)) { - t.Fatalf("Unexpected blob size: %d; expected: %d", blob.Length, len(b1)) + if blob.Size != int64(len(b1)) { + t.Fatalf("Unexpected blob size: %d; expected: %d", blob.Size, len(b1)) } } @@ -352,14 +352,14 @@ func TestBlobUploadMonolithic(t *testing.T) { blob, err := upload.Commit(ctx, distribution.Descriptor{ Digest: dgst, - Length: int64(len(b1)), + Size: int64(len(b1)), }) if err != nil { t.Fatal(err) } - if blob.Length != int64(len(b1)) { - t.Fatalf("Unexpected blob size: %d; expected: %d", blob.Length, len(b1)) + if blob.Size != int64(len(b1)) { + t.Fatalf("Unexpected blob size: %d; expected: %d", blob.Size, len(b1)) } } diff --git a/registry/storage/blob_test.go b/registry/storage/blob_test.go index 569f756dae..0dbfe81055 100644 --- a/registry/storage/blob_test.go +++ b/registry/storage/blob_test.go @@ -178,7 +178,7 @@ func TestSimpleBlobRead(t *testing.T) { t.Fatalf("error getting seeker size for random layer: %v", err) } - descBefore := distribution.Descriptor{Digest: dgst, MediaType: "application/octet-stream", Length: randomLayerSize} + descBefore := distribution.Descriptor{Digest: dgst, MediaType: "application/octet-stream", Size: randomLayerSize} t.Logf("desc: %v", descBefore) desc, err = addBlob(ctx, bs, descBefore, randomLayerReader) @@ -186,8 +186,8 @@ func TestSimpleBlobRead(t *testing.T) { t.Fatalf("error adding blob to blobservice: %v", err) } - if desc.Length != randomLayerSize { - t.Fatalf("committed blob has incorrect length: %v != %v", desc.Length, randomLayerSize) + if desc.Size != randomLayerSize { + t.Fatalf("committed blob has incorrect length: %v != %v", desc.Size, randomLayerSize) } rc, err = bs.Open(ctx, desc.Digest) // note that we are opening with original digest. @@ -330,8 +330,8 @@ func addBlob(ctx context.Context, bs distribution.BlobIngester, desc distributio if nn, err := io.Copy(wr, rd); err != nil { return distribution.Descriptor{}, err - } else if nn != desc.Length { - return distribution.Descriptor{}, fmt.Errorf("incorrect number of bytes copied: %v != %v", nn, desc.Length) + } else if nn != desc.Size { + return distribution.Descriptor{}, fmt.Errorf("incorrect number of bytes copied: %v != %v", nn, desc.Size) } return wr.Commit(ctx, desc) diff --git a/registry/storage/blobserver.go b/registry/storage/blobserver.go index a7b42681d7..d0b3204cf1 100644 --- a/registry/storage/blobserver.go +++ b/registry/storage/blobserver.go @@ -41,7 +41,7 @@ func (bs *blobServer) ServeBlob(ctx context.Context, w http.ResponseWriter, r *h http.Redirect(w, r, redirectURL, http.StatusTemporaryRedirect) case driver.ErrUnsupportedMethod: // Fallback to serving the content directly. - br, err := newFileReader(ctx, bs.driver, path, desc.Length) + br, err := newFileReader(ctx, bs.driver, path, desc.Size) if err != nil { return err } @@ -61,7 +61,7 @@ func (bs *blobServer) ServeBlob(ctx context.Context, w http.ResponseWriter, r *h if w.Header().Get("Content-Length") == "" { // Set the content length if not already set. - w.Header().Set("Content-Length", fmt.Sprint(desc.Length)) + w.Header().Set("Content-Length", fmt.Sprint(desc.Size)) } http.ServeContent(w, r, desc.Digest.String(), time.Time{}, br) diff --git a/registry/storage/blobstore.go b/registry/storage/blobstore.go index afe428479b..484e2106ab 100644 --- a/registry/storage/blobstore.go +++ b/registry/storage/blobstore.go @@ -50,7 +50,7 @@ func (bs *blobStore) Open(ctx context.Context, dgst digest.Digest) (distribution return nil, err } - return newFileReader(ctx, bs.driver, path, desc.Length) + return newFileReader(ctx, bs.driver, path, desc.Size) } // Put stores the content p in the blob store, calculating the digest. If the @@ -81,7 +81,7 @@ func (bs *blobStore) Put(ctx context.Context, mediaType string, p []byte) (distr // TODO(stevvooe): Write out mediatype here, as well. return distribution.Descriptor{ - Length: int64(len(p)), + Size: int64(len(p)), // NOTE(stevvooe): The central blob store firewalls media types from // other users. The caller should look this up and override the value @@ -179,7 +179,7 @@ func (bs *blobStatter) Stat(ctx context.Context, dgst digest.Digest) (distributi // mediatype that overrides the main one. return distribution.Descriptor{ - Length: fi.Size(), + Size: fi.Size(), // NOTE(stevvooe): The central blob store firewalls media types from // other users. The caller should look this up and override the value diff --git a/registry/storage/blobwriter.go b/registry/storage/blobwriter.go index 4189d5178e..b39c851e51 100644 --- a/registry/storage/blobwriter.go +++ b/registry/storage/blobwriter.go @@ -148,7 +148,7 @@ func (bw *blobWriter) validateBlob(ctx context.Context, desc distribution.Descri // NOTE(stevvooe): We really don't care if the file is // not actually present for the reader. We now assume // that the desc length is zero. - desc.Length = 0 + desc.Size = 0 default: // Any other error we want propagated up the stack. return distribution.Descriptor{}, err @@ -161,14 +161,14 @@ func (bw *blobWriter) validateBlob(ctx context.Context, desc distribution.Descri bw.size = fi.Size() } - if desc.Length > 0 { - if desc.Length != bw.size { + if desc.Size > 0 { + if desc.Size != bw.size { return distribution.Descriptor{}, distribution.ErrBlobInvalidLength } } else { // if provided 0 or negative length, we can assume caller doesn't know or // care about length. - desc.Length = bw.size + desc.Size = bw.size } // TODO(stevvooe): This section is very meandering. Need to be broken down @@ -216,7 +216,7 @@ func (bw *blobWriter) validateBlob(ctx context.Context, desc distribution.Descri } // Read the file from the backend driver and validate it. - fr, err := newFileReader(ctx, bw.bufferedFileWriter.driver, bw.path, desc.Length) + fr, err := newFileReader(ctx, bw.bufferedFileWriter.driver, bw.path, desc.Size) if err != nil { return distribution.Descriptor{}, err } diff --git a/registry/storage/cache/cache.go b/registry/storage/cache/cache.go index 79e6d9c84b..10a3909197 100644 --- a/registry/storage/cache/cache.go +++ b/registry/storage/cache/cache.go @@ -23,8 +23,8 @@ func ValidateDescriptor(desc distribution.Descriptor) error { return err } - if desc.Length < 0 { - return fmt.Errorf("cache: invalid length in descriptor: %v < 0", desc.Length) + if desc.Size < 0 { + return fmt.Errorf("cache: invalid length in descriptor: %v < 0", desc.Size) } if desc.MediaType == "" { diff --git a/registry/storage/cache/redis/redis.go b/registry/storage/cache/redis/redis.go index 29bbe3bc3b..64010a0928 100644 --- a/registry/storage/cache/redis/redis.go +++ b/registry/storage/cache/redis/redis.go @@ -66,17 +66,20 @@ func (rbds *redisBlobDescriptorService) Stat(ctx context.Context, dgst digest.Di // stat provides an internal stat call that takes a connection parameter. This // allows some internal management of the connection scope. func (rbds *redisBlobDescriptorService) stat(ctx context.Context, conn redis.Conn, dgst digest.Digest) (distribution.Descriptor, error) { - reply, err := redis.Values(conn.Do("HMGET", rbds.blobDescriptorHashKey(dgst), "digest", "length", "mediatype")) + reply, err := redis.Values(conn.Do("HMGET", rbds.blobDescriptorHashKey(dgst), "digest", "size", "mediatype")) if err != nil { return distribution.Descriptor{}, err } - if len(reply) < 2 || reply[0] == nil || reply[1] == nil { // don't care if mediatype is nil + // NOTE(stevvooe): The "size" field used to be "length". We treat a + // missing "size" field here as an unknown blob, which causes a cache + // miss, effectively migrating the field. + if len(reply) < 3 || reply[0] == nil || reply[1] == nil { // don't care if mediatype is nil return distribution.Descriptor{}, distribution.ErrBlobUnknown } var desc distribution.Descriptor - if _, err := redis.Scan(reply, &desc.Digest, &desc.Length, &desc.MediaType); err != nil { + if _, err := redis.Scan(reply, &desc.Digest, &desc.Size, &desc.MediaType); err != nil { return distribution.Descriptor{}, err } @@ -104,7 +107,7 @@ func (rbds *redisBlobDescriptorService) SetDescriptor(ctx context.Context, dgst func (rbds *redisBlobDescriptorService) setDescriptor(ctx context.Context, conn redis.Conn, dgst digest.Digest, desc distribution.Descriptor) error { if _, err := conn.Do("HMSET", rbds.blobDescriptorHashKey(dgst), "digest", desc.Digest, - "length", desc.Length); err != nil { + "size", desc.Size); err != nil { return err } diff --git a/registry/storage/cache/suite.go b/registry/storage/cache/suite.go index ceefab9723..f74d9f9e70 100644 --- a/registry/storage/cache/suite.go +++ b/registry/storage/cache/suite.go @@ -35,14 +35,14 @@ func checkBlobDescriptorCacheEmptyRepository(t *testing.T, ctx context.Context, if err := cache.SetDescriptor(ctx, "", distribution.Descriptor{ Digest: "sha384:abc", - Length: 10, + Size: 10, MediaType: "application/octet-stream"}); err != digest.ErrDigestInvalidFormat { t.Fatalf("expected error with invalid digest: %v", err) } if err := cache.SetDescriptor(ctx, "sha384:abc", distribution.Descriptor{ Digest: "", - Length: 10, + Size: 10, MediaType: "application/octet-stream"}); err == nil { t.Fatalf("expected error setting value on invalid descriptor") } @@ -60,7 +60,7 @@ func checkBlobDescriptorCacheSetAndRead(t *testing.T, ctx context.Context, provi localDigest := digest.Digest("sha384:abc") expected := distribution.Descriptor{ Digest: "sha256:abc", - Length: 10, + Size: 10, MediaType: "application/octet-stream"} cache, err := provider.RepositoryScoped("foo/bar")