Merge pull request #2138 from yuwaMSFT2/master

closes issue#2135 image pull returns 404 on manifest request if there is storage error

registry/handlers/api_test.go

@@ -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}

registry/handlers/manifests.go

@@ -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 {
-			imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err))
+			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 {
-		imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err))
+		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 {
-			imh.Errors = append(imh.Errors, v2.ErrorCodeManifestUnknown.WithDetail(err))
+			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 {
-		imh.Errors = append(imh.Errors, v2.ErrorCodeManifestInvalid.WithDetail(err))
+		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
 	}