Merge pull request #848 from RichardScothern/error-type

Error handling fixes

blobs.go

@@ -27,9 +27,6 @@ var (
 	// ErrBlobInvalidLength returned when the blob has an expected length on
 	// commit, meaning mismatched with the descriptor or an invalid value.
 	ErrBlobInvalidLength = errors.New("blob invalid length")
-
-	// ErrUnsupported returned when an unsupported operation is attempted
-	ErrUnsupported = errors.New("unsupported operation")
 )
 
 // ErrBlobInvalidDigest returned when digest check fails.

docs/spec/api.md

@@ -1731,6 +1731,24 @@ The error codes that may be included in the response body are enumerated below:
 
 
 
+###### On Failure: Not allowed
+
+```
+405 Method Not Allowed
+```
+
+Manifest put is not allowed because the registry is configured as a pull-through cache or for some other reason
+
+
+
+The error codes that may be included in the response body are enumerated below:
+
+|Code|Message|Description|
+|----|-------|-----------|
+| `UNSUPPORTED` | The operation is unsupported. | The operation was unsupported due to a missing implementation or invalid set of parameters. |
+
+
+
 
 #### DELETE Manifest
 
@@ -1871,6 +1889,24 @@ The error codes that may be included in the response body are enumerated below:
 
 
 
+###### On Failure: Not allowed
+
+```
+405 Method Not Allowed
+```
+
+Manifest delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled.
+
+
+
+The error codes that may be included in the response body are enumerated below:
+
+|Code|Message|Description|
+|----|-------|-----------|
+| `UNSUPPORTED` | The operation is unsupported. | The operation was unsupported due to a missing implementation or invalid set of parameters. |
+
+
+
 
 
 ### Blob
@@ -2323,7 +2359,7 @@ Content-Type: application/json; charset=utf-8
 }
 ```
 
-Delete is not enabled on the registry
+Blob delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled
 
 
 
@@ -2456,6 +2492,24 @@ The error codes that may be included in the response body are enumerated below:
 
 
 
+###### On Failure: Not allowed
+
+```
+405 Method Not Allowed
+```
+
+Blob upload is not allowed because the registry is configured as a pull-through cache or for some other reason
+
+
+
+The error codes that may be included in the response body are enumerated below:
+
+|Code|Message|Description|
+|----|-------|-----------|
+| `UNSUPPORTED` | The operation is unsupported. | The operation was unsupported due to a missing implementation or invalid set of parameters. |
+
+
+
 ##### Initiate Resumable Blob Upload
 
 ```
@@ -3129,6 +3183,7 @@ The error codes that may be included in the response body are enumerated below:
 | `DIGEST_INVALID` | provided digest did not match uploaded content | When a blob is uploaded, the registry will check that the content matches the digest provided by the client. The error may include a detail structure with the key "digest", including the invalid digest string. This error may also be returned when a manifest includes an invalid layer digest. |
 | `NAME_INVALID` | invalid repository name | Invalid repository name encountered either during manifest validation or any API operation. |
 | `BLOB_UPLOAD_INVALID` | blob upload invalid | The blob upload encountered an error and can no longer proceed. |
+| `UNSUPPORTED` | The operation is unsupported. | The operation was unsupported due to a missing implementation or invalid set of parameters. |
 
 
 

errors.go

@@ -7,6 +7,10 @@ import (
 	"github.com/docker/distribution/digest"
 )
 
+// ErrUnsupported is returned when an unimplemented or unsupported action is
+// performed
+var ErrUnsupported = fmt.Errorf("operation unsupported")
+
 // ErrRepositoryUnknown is returned if the named repository is not known by
 // the registry.
 type ErrRepositoryUnknown struct {

registry/api/errcode/register.go

@@ -30,7 +30,7 @@ var (
 		Message: "The operation is unsupported.",
 		Description: `The operation was unsupported due to a missing
 		implementation or invalid set of parameters.`,
-		HTTPStatusCode: http.StatusBadRequest,
+		HTTPStatusCode: http.StatusMethodNotAllowed,
 	})
 
 	// ErrorCodeUnauthorized is returned if a request is not authorized.

registry/api/v2/descriptors.go

@@ -689,6 +689,14 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 							},
+							{
+								Name:        "Not allowed",
+								Description: "Manifest put is not allowed because the registry is configured as a pull-through cache or for some other reason",
+								StatusCode:  http.StatusMethodNotAllowed,
+								ErrorCodes: []errcode.ErrorCode{
+									errcode.ErrorCodeUnsupported,
+								},
+							},
 						},
 					},
 				},
@@ -757,6 +765,14 @@ var routeDescriptors = []RouteDescriptor{
 									Format:      errorsBody,
 								},
 							},
+							{
+								Name:        "Not allowed",
+								Description: "Manifest delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled.",
+								StatusCode:  http.StatusMethodNotAllowed,
+								ErrorCodes: []errcode.ErrorCode{
+									errcode.ErrorCodeUnsupported,
+								},
+							},
 						},
 					},
 				},
@@ -967,7 +983,7 @@ var routeDescriptors = []RouteDescriptor{
 								},
 							},
 							{
-								Description: "Delete is not enabled on the registry",
+								Description: "Blob delete is not allowed because the registry is configured as a pull-through cache or `delete` has been disabled",
 								StatusCode:  http.StatusMethodNotAllowed,
 								Body: BodyDescriptor{
 									ContentType: "application/json; charset=utf-8",
@@ -1051,6 +1067,14 @@ var routeDescriptors = []RouteDescriptor{
 								},
 							},
 							unauthorizedResponsePush,
+							{
+								Name:        "Not allowed",
+								Description: "Blob upload is not allowed because the registry is configured as a pull-through cache or for some other reason",
+								StatusCode:  http.StatusMethodNotAllowed,
+								ErrorCodes: []errcode.ErrorCode{
+									errcode.ErrorCodeUnsupported,
+								},
+							},
 						},
 					},
 					{
@@ -1389,6 +1413,7 @@ var routeDescriptors = []RouteDescriptor{
 									ErrorCodeDigestInvalid,
 									ErrorCodeNameInvalid,
 									ErrorCodeBlobUploadInvalid,
+									errcode.ErrorCodeUnsupported,
 								},
 								Body: BodyDescriptor{
 									ContentType: "application/json; charset=utf-8",

registry/handlers/api_test.go

@@ -1001,6 +1001,21 @@ type testEnv struct {
 	builder *v2.URLBuilder
 }
 
+func newTestEnvMirror(t *testing.T, deleteEnabled bool) *testEnv {
+	config := configuration.Configuration{
+		Storage: configuration.Storage{
+			"inmemory": configuration.Parameters{},
+			"delete":   configuration.Parameters{"enabled": deleteEnabled},
+		},
+		Proxy: configuration.Proxy{
+			RemoteURL: "http://example.com",
+		},
+	}
+
+	return newTestEnvWithConfig(t, &config)
+
+}
+
 func newTestEnv(t *testing.T, deleteEnabled bool) *testEnv {
 	config := configuration.Configuration{
 		Storage: configuration.Storage{
@@ -1378,3 +1393,53 @@ func createRepository(env *testEnv, t *testing.T, imageName string, tag string)
 		"Docker-Content-Digest": []string{dgst.String()},
 	})
 }
+
+// Test mutation operations on a registry configured as a cache.  Ensure that they return
+// appropriate errors.
+func TestRegistryAsCacheMutationAPIs(t *testing.T) {
+	deleteEnabled := true
+	env := newTestEnvMirror(t, deleteEnabled)
+
+	imageName := "foo/bar"
+	tag := "latest"
+	manifestURL, err := env.builder.BuildManifestURL(imageName, tag)
+	if err != nil {
+		t.Fatalf("unexpected error building base url: %v", err)
+	}
+
+	// Manifest upload
+	unsignedManifest := &manifest.Manifest{
+		Versioned: manifest.Versioned{
+			SchemaVersion: 1,
+		},
+		Name:     imageName,
+		Tag:      tag,
+		FSLayers: []manifest.FSLayer{},
+	}
+	resp := putManifest(t, "putting unsigned manifest", manifestURL, unsignedManifest)
+	checkResponse(t, "putting signed manifest to cache", resp, errcode.ErrorCodeUnsupported.Descriptor().HTTPStatusCode)
+
+	// Manifest Delete
+	resp, err = httpDelete(manifestURL)
+	checkResponse(t, "deleting signed manifest from cache", resp, errcode.ErrorCodeUnsupported.Descriptor().HTTPStatusCode)
+
+	// Blob upload initialization
+	layerUploadURL, err := env.builder.BuildBlobUploadURL(imageName)
+	if err != nil {
+		t.Fatalf("unexpected error building layer upload url: %v", err)
+	}
+
+	resp, err = http.Post(layerUploadURL, "", nil)
+	if err != nil {
+		t.Fatalf("unexpected error starting layer push: %v", err)
+	}
+	defer resp.Body.Close()
+
+	checkResponse(t, fmt.Sprintf("starting layer push to cache %v", imageName), resp, errcode.ErrorCodeUnsupported.Descriptor().HTTPStatusCode)
+
+	// Blob Delete
+	blobURL, err := env.builder.BuildBlobURL(imageName, digest.DigestSha256EmptyTar)
+	resp, err = httpDelete(blobURL)
+	checkResponse(t, "deleting blob from cache", resp, errcode.ErrorCodeUnsupported.Descriptor().HTTPStatusCode)
+
+}

registry/handlers/blob.go

@@ -76,16 +76,17 @@ func (bh *blobHandler) DeleteBlob(w http.ResponseWriter, r *http.Request) {
 	err := blobs.Delete(bh, bh.Digest)
 	if err != nil {
 		switch err {
-		case distribution.ErrBlobUnknown:
-			w.WriteHeader(http.StatusNotFound)
-			bh.Errors = append(bh.Errors, v2.ErrorCodeBlobUnknown)
 		case distribution.ErrUnsupported:
-			w.WriteHeader(http.StatusMethodNotAllowed)
 			bh.Errors = append(bh.Errors, errcode.ErrorCodeUnsupported)
-		default:
-			bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown)
-		}
 			return
+		case distribution.ErrBlobUnknown:
+			bh.Errors = append(bh.Errors, v2.ErrorCodeBlobUnknown)
+			return
+		default:
+			bh.Errors = append(bh.Errors, err)
+			context.GetLogger(bh).Errorf("Unknown error deleting blob: %s", err.Error())
+			return
+		}
 	}
 
 	w.Header().Set("Content-Length", "0")

registry/handlers/blobupload.go

@@ -117,8 +117,13 @@ type blobUploadHandler struct {
 func (buh *blobUploadHandler) StartBlobUpload(w http.ResponseWriter, r *http.Request) {
 	blobs := buh.Repository.Blobs(buh)
 	upload, err := blobs.Create(buh)
+
 	if err != nil {
+		if err == distribution.ErrUnsupported {
+			buh.Errors = append(buh.Errors, errcode.ErrorCodeUnsupported)
+		} else {
 			buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
+		}
 		return
 	}
 
@@ -227,6 +232,8 @@ func (buh *blobUploadHandler) PutBlobUploadComplete(w http.ResponseWriter, r *ht
 			buh.Errors = append(buh.Errors, v2.ErrorCodeDigestInvalid.WithDetail(err))
 		default:
 			switch err {
+			case distribution.ErrUnsupported:
+				buh.Errors = append(buh.Errors, errcode.ErrorCodeUnsupported)
 			case distribution.ErrBlobInvalidLength, distribution.ErrBlobDigestUnsupported:
 				buh.Errors = append(buh.Errors, v2.ErrorCodeBlobUploadInvalid.WithDetail(err))
 			default:

registry/handlers/images.go

@@ -154,6 +154,10 @@ func (imh *imageManifestHandler) PutImageManifest(w http.ResponseWriter, r *http
 	if err := manifests.Put(&manifest); err != nil {
 		// TODO(stevvooe): These error handling switches really need to be
 		// handled by an app global mapper.
+		if err == distribution.ErrUnsupported {
+			imh.Errors = append(imh.Errors, errcode.ErrorCodeUnsupported)
+			return
+		}
 		switch err := err.(type) {
 		case distribution.ErrManifestVerification:
 			for _, verificationError := range err {
@@ -210,14 +214,12 @@ func (imh *imageManifestHandler) DeleteImageManifest(w http.ResponseWriter, r *h
 			return
 		case distribution.ErrBlobUnknown:
 			imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown)
-			w.WriteHeader(http.StatusNotFound)
 			return
 		case distribution.ErrUnsupported:
 			imh.Errors = append(imh.Errors, errcode.ErrorCodeUnsupported)
-			w.WriteHeader(http.StatusMethodNotAllowed)
+			return
 		default:
 			imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown)
-			w.WriteHeader(http.StatusBadRequest)
 			return
 		}
 	}

registry/proxy/proxymanifeststore.go

@@ -7,7 +7,6 @@ import (
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
-	"github.com/docker/distribution/registry/api/errcode"
 	"github.com/docker/distribution/registry/client"
 	"github.com/docker/distribution/registry/proxy/scheduler"
 )
@@ -147,9 +146,9 @@ func manifestDigest(sm *manifest.SignedManifest) (digest.Digest, error) {
 }
 
 func (pms proxyManifestStore) Put(manifest *manifest.SignedManifest) error {
-	return errcode.ErrorCodeUnsupported
+	return distribution.ErrUnsupported
 }
 
 func (pms proxyManifestStore) Delete(dgst digest.Digest) error {
-	return errcode.ErrorCodeUnsupported
+	return distribution.ErrUnsupported
 }