forked from TrueCloudLab/distribution
Change some incorrect error types in proxy stores from API errors to
distribution errors. Fill in missing checks for mutations on a registry pull-through cache. Add unit tests and update documentation. Also, give v2.ErrorCodeUnsupported an HTTP status code, previously it was defaulting to 500, now its 405 Method Not Allowed. Signed-off-by: Richard Scothern <richard.scothern@gmail.com>
This commit is contained in:
parent
ed3ecfdccb
commit
43fc9a195d
7 changed files with 114 additions and 15 deletions
|
@ -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.
|
||||
|
|
|
@ -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",
|
||||
|
|
|
@ -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)
|
||||
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
return
|
||||
case distribution.ErrBlobUnknown:
|
||||
bh.Errors = append(bh.Errors, v2.ErrorCodeBlobUnknown)
|
||||
return
|
||||
default:
|
||||
bh.Errors = append(bh.Errors, errcode.ErrorCodeUnknown)
|
||||
bh.Errors = append(bh.Errors, err)
|
||||
context.GetLogger(bh).Errorf("Unknown error deleting blob: %s", err.Error())
|
||||
return
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
w.Header().Set("Content-Length", "0")
|
||||
|
|
|
@ -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 {
|
||||
buh.Errors = append(buh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
|
||||
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:
|
||||
|
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue