From b6270d9c14caf4627e9ca42e15c0f573c428cee6 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 2 Feb 2015 13:01:49 -0800 Subject: [PATCH] Handle empty blob files more appropriately Several API tests were added to ensure correct acceptance of zero-size and empty tar files. This led to several changes in the storage backend around the guarantees of remote file reading, which backs the layer and layer upload type. In support of these changes, zero-length and empty checks have been added to the digest package. These provide a sanity check against upstream tarsum changes. The fileReader has been modified to be more robust when reading and seeking on zero-length or non-existent files. The file no longer needs to exist for the reader to be created. Seeks can now move beyond the end of the file, causing reads to issue an io.EOF. This eliminates errors during certain race conditions for reading files which should be detected by stat calls. As a part of this, a few error types were factored out and the read buffer size was increased to something more reasonable. Signed-off-by: Stephen J Day --- docs/api_test.go | 35 ++++++++++++++++++++++------------- docs/layerupload.go | 7 ------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/docs/api_test.go b/docs/api_test.go index 68254920..5f9e6c38 100644 --- a/docs/api_test.go +++ b/docs/api_test.go @@ -144,7 +144,7 @@ func TestLayerAPI(t *testing.T) { checkResponse(t, "status of deleted upload", resp, http.StatusNotFound) // ----------------------------------------- - // Do layer push with an empty body + // Do layer push with an empty body and different digest uploadURLBase = startPushLayer(t, builder, imageName) resp, err = doPushLayer(t, builder, imageName, layerDigest, uploadURLBase, bytes.NewReader([]byte{})) if err != nil { @@ -152,21 +152,30 @@ func TestLayerAPI(t *testing.T) { } checkResponse(t, "bad layer push", resp, http.StatusBadRequest) - checkBodyHasErrorCodes(t, "bad layer push", resp, v2.ErrorCodeBlobUploadInvalid) + checkBodyHasErrorCodes(t, "bad layer push", resp, v2.ErrorCodeDigestInvalid) // ----------------------------------------- - // Do layer push with an invalid body - - // This is a valid but empty tarfile! - badTar := bytes.Repeat([]byte("\x00"), 1024) - uploadURLBase = startPushLayer(t, builder, imageName) - resp, err = doPushLayer(t, builder, imageName, layerDigest, uploadURLBase, bytes.NewReader(badTar)) + // Do layer push with an empty body and correct digest + zeroDigest, err := digest.FromTarArchive(bytes.NewReader([]byte{})) if err != nil { - t.Fatalf("unexpected error doing bad layer push: %v", err) + t.Fatalf("unexpected error digesting empty buffer: %v", err) } - checkResponse(t, "bad layer push", resp, http.StatusBadRequest) - checkBodyHasErrorCodes(t, "bad layer push", resp, v2.ErrorCodeDigestInvalid) + uploadURLBase = startPushLayer(t, builder, imageName) + pushLayer(t, builder, imageName, zeroDigest, uploadURLBase, bytes.NewReader([]byte{})) + + // ----------------------------------------- + // Do layer push with an empty body and correct digest + + // This is a valid but empty tarfile! + emptyTar := bytes.Repeat([]byte("\x00"), 1024) + emptyDigest, err := digest.FromTarArchive(bytes.NewReader(emptyTar)) + if err != nil { + t.Fatalf("unexpected error digesting empty tar: %v", err) + } + + uploadURLBase = startPushLayer(t, builder, imageName) + pushLayer(t, builder, imageName, emptyDigest, uploadURLBase, bytes.NewReader(emptyTar)) // ------------------------------------------ // Now, actually do successful upload. @@ -517,7 +526,7 @@ func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, error for _, err := range errs.Errors { if _, ok := expected[err.Code]; !ok { - t.Fatalf("unexpected error code %v encountered: %s ", err.Code, string(p)) + t.Fatalf("unexpected error code %v encountered during %s: %s ", err.Code, msg, string(p)) } counts[err.Code]++ } @@ -525,7 +534,7 @@ func checkBodyHasErrorCodes(t *testing.T, msg string, resp *http.Response, error // Ensure that counts of expected errors were all non-zero for code := range expected { if counts[code] == 0 { - t.Fatalf("expected error code %v not encounterd: %s", code, string(p)) + t.Fatalf("expected error code %v not encounterd during %s: %s", code, msg, string(p)) } } diff --git a/docs/layerupload.go b/docs/layerupload.go index e9585b0e..cfce98f3 100644 --- a/docs/layerupload.go +++ b/docs/layerupload.go @@ -198,13 +198,6 @@ func (luh *layerUploadHandler) PutLayerUploadComplete(w http.ResponseWriter, r * layer, err := luh.Upload.Finish(dgst) if err != nil { switch err := err.(type) { - case storage.ErrLayerUploadUnavailable: - w.WriteHeader(http.StatusBadRequest) - // TODO(stevvooe): Arguably, we may want to add an error code to - // cover this condition. It is not always a client error but it - // may be. For now, we effectively throw out the upload and have - // them start over. - luh.Errors.Push(v2.ErrorCodeBlobUploadInvalid, err.Err) case storage.ErrLayerInvalidDigest: w.WriteHeader(http.StatusBadRequest) luh.Errors.Push(v2.ErrorCodeDigestInvalid, err)