Merge pull request #2138 from yuwaMSFT2/master

closes issue#2135 image pull returns 404 on manifest request if there is storage error
This commit is contained in:
Stephen Day 2017-01-18 12:02:56 -08:00 committed by GitHub
commit 0111f1e3cf
2 changed files with 130 additions and 4 deletions

View file

@ -3,6 +3,7 @@ package handlers
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"io"
"io/ioutil"
@ -28,6 +29,8 @@ import (
"github.com/docker/distribution/reference"
"github.com/docker/distribution/registry/api/errcode"
"github.com/docker/distribution/registry/api/v2"
storagedriver "github.com/docker/distribution/registry/storage/driver"
"github.com/docker/distribution/registry/storage/driver/factory"
_ "github.com/docker/distribution/registry/storage/driver/testdriver"
"github.com/docker/distribution/testutil"
"github.com/docker/libtrust"
@ -815,6 +818,93 @@ func TestManifestAPI(t *testing.T) {
testManifestAPIManifestList(t, env2, schema2Args)
}
// storageManifestErrDriverFactory implements the factory.StorageDriverFactory interface.
type storageManifestErrDriverFactory struct{}
const (
repositoryWithManifestNotFound = "manifesttagnotfound"
repositoryWithManifestInvalidPath = "manifestinvalidpath"
repositoryWithManifestBadLink = "manifestbadlink"
repositoryWithGenericStorageError = "genericstorageerr"
)
func (factory *storageManifestErrDriverFactory) Create(parameters map[string]interface{}) (storagedriver.StorageDriver, error) {
// Initialize the mock driver
var errGenericStorage = errors.New("generic storage error")
return &mockErrorDriver{
returnErrs: []mockErrorMapping{
{
pathMatch: fmt.Sprintf("%s/_manifests/tags", repositoryWithManifestNotFound),
content: nil,
err: storagedriver.PathNotFoundError{},
},
{
pathMatch: fmt.Sprintf("%s/_manifests/tags", repositoryWithManifestInvalidPath),
content: nil,
err: storagedriver.InvalidPathError{},
},
{
pathMatch: fmt.Sprintf("%s/_manifests/tags", repositoryWithManifestBadLink),
content: []byte("this is a bad sha"),
err: nil,
},
{
pathMatch: fmt.Sprintf("%s/_manifests/tags", repositoryWithGenericStorageError),
content: nil,
err: errGenericStorage,
},
},
}, nil
}
type mockErrorMapping struct {
pathMatch string
content []byte
err error
}
// mockErrorDriver implements StorageDriver to force storage error on manifest request
type mockErrorDriver struct {
storagedriver.StorageDriver
returnErrs []mockErrorMapping
}
func (dr *mockErrorDriver) GetContent(ctx context.Context, path string) ([]byte, error) {
for _, returns := range dr.returnErrs {
if strings.Contains(path, returns.pathMatch) {
return returns.content, returns.err
}
}
return nil, errors.New("Unknown storage error")
}
func TestGetManifestWithStorageError(t *testing.T) {
factory.Register("storagemanifesterror", &storageManifestErrDriverFactory{})
config := configuration.Configuration{
Storage: configuration.Storage{
"storagemanifesterror": configuration.Parameters{},
"maintenance": configuration.Parameters{"uploadpurging": map[interface{}]interface{}{
"enabled": false,
}},
},
}
config.HTTP.Headers = headerConfig
env1 := newTestEnvWithConfig(t, &config)
defer env1.Shutdown()
repo, _ := reference.ParseNamed(repositoryWithManifestNotFound)
testManifestWithStorageError(t, env1, repo, http.StatusNotFound, v2.ErrorCodeManifestUnknown)
repo, _ = reference.ParseNamed(repositoryWithGenericStorageError)
testManifestWithStorageError(t, env1, repo, http.StatusInternalServerError, errcode.ErrorCodeUnknown)
repo, _ = reference.ParseNamed(repositoryWithManifestInvalidPath)
testManifestWithStorageError(t, env1, repo, http.StatusInternalServerError, errcode.ErrorCodeUnknown)
repo, _ = reference.ParseNamed(repositoryWithManifestBadLink)
testManifestWithStorageError(t, env1, repo, http.StatusInternalServerError, errcode.ErrorCodeUnknown)
}
func TestManifestDelete(t *testing.T) {
schema1Repo, _ := reference.ParseNamed("foo/schema1")
schema2Repo, _ := reference.ParseNamed("foo/schema2")
@ -852,6 +942,26 @@ func testManifestDeleteDisabled(t *testing.T, env *testEnv, imageName reference.
checkResponse(t, "status of disabled delete of manifest", resp, http.StatusMethodNotAllowed)
}
func testManifestWithStorageError(t *testing.T, env *testEnv, imageName reference.Named, expectedStatusCode int, expectedErrorCode errcode.ErrorCode) {
tag := "latest"
tagRef, _ := reference.WithTag(imageName, tag)
manifestURL, err := env.builder.BuildManifestURL(tagRef)
if err != nil {
t.Fatalf("unexpected error getting manifest url: %v", err)
}
// -----------------------------
// Attempt to fetch the manifest
resp, err := http.Get(manifestURL)
if err != nil {
t.Fatalf("unexpected error getting manifest: %v", err)
}
defer resp.Body.Close()
checkResponse(t, "getting non-existent manifest", resp, expectedStatusCode)
checkBodyHasErrorCodes(t, "getting non-existent manifest", resp, expectedErrorCode)
return
}
func testManifestAPISchema1(t *testing.T, env *testEnv, imageName reference.Named) manifestArgs {
tag := "thetag"
args := manifestArgs{imageName: imageName}

View file

@ -77,7 +77,11 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request)
tags := imh.Repository.Tags(imh)
desc, err := tags.Get(imh, imh.Tag)
if err != nil {
if _, ok := err.(distribution.ErrTagUnknown); ok {
imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err))
} else {
imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
}
return
}
imh.Digest = desc.Digest
@ -94,7 +98,11 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request)
}
manifest, err = manifests.Get(imh, imh.Digest, options...)
if err != nil {
if _, ok := err.(distribution.ErrManifestUnknownRevision); ok {
imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err))
} else {
imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
}
return
}
@ -161,7 +169,11 @@ func (imh *manifestHandler) GetManifest(w http.ResponseWriter, r *http.Request)
manifest, err = manifests.Get(imh, manifestDigest)
if err != nil {
if _, ok := err.(distribution.ErrManifestUnknownRevision); ok {
imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err))
} else {
imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
}
return
}
@ -191,7 +203,11 @@ func (imh *manifestHandler) convertSchema2Manifest(schema2Manifest *schema2.Dese
blobs := imh.Repository.Blobs(imh)
configJSON, err := blobs.Get(imh, targetDescriptor.Digest)
if err != nil {
if err == distribution.ErrBlobUnknown {
imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err))
} else {
imh.Errors = append(imh.Errors, errcode.ErrorCodeUnknown.WithDetail(err))
}
return nil, err
}