From c4c279089a40173515475772eea1aa0b8a9e4aa8 Mon Sep 17 00:00:00 2001
From: wang yan <wangyan@vmware.com>
Date: Sun, 26 Apr 2020 17:30:20 +0800
Subject: [PATCH] fix return code on getting non exist manifest

It's found by conformance test, it should be 404 instead of 500 when to get a non exist manifest

Signed-off-by: wang yan <wangyan@vmware.com>
---
 src/lib/context.go                                      | 4 +++-
 src/server/middleware/contenttrust/contenttrust.go      | 3 +--
 src/server/middleware/contenttrust/contenttrust_test.go | 8 ++++++++
 src/server/middleware/immutable/pushmf.go               | 2 +-
 src/server/middleware/vulnerable/vulnerable.go          | 2 +-
 src/server/middleware/vulnerable/vulnerable_test.go     | 2 +-
 6 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/lib/context.go b/src/lib/context.go
index 20f7f5ebd..9e8e2f274 100644
--- a/src/lib/context.go
+++ b/src/lib/context.go
@@ -14,7 +14,9 @@
 
 package lib
 
-import "context"
+import (
+	"context"
+)
 
 type contextKey string
 
diff --git a/src/server/middleware/contenttrust/contenttrust.go b/src/server/middleware/contenttrust/contenttrust.go
index ebd0551da..70792e844 100644
--- a/src/server/middleware/contenttrust/contenttrust.go
+++ b/src/server/middleware/contenttrust/contenttrust.go
@@ -1,7 +1,6 @@
 package contenttrust
 
 import (
-	"fmt"
 	"net/http"
 
 	"github.com/goharbor/harbor/src/controller/artifact"
@@ -36,7 +35,7 @@ func Middleware() func(http.Handler) http.Handler {
 		none := lib.ArtifactInfo{}
 		af := lib.GetArtifactInfo(ctx)
 		if af == none {
-			return fmt.Errorf("artifactinfo middleware required before this middleware")
+			return errors.New("artifactinfo middleware required before this middleware").WithCode(errors.NotFoundCode)
 		}
 		if len(af.Digest) == 0 {
 			art, err := artifact.Ctl.GetByReference(ctx, af.Repository, af.Reference, nil)
diff --git a/src/server/middleware/contenttrust/contenttrust_test.go b/src/server/middleware/contenttrust/contenttrust_test.go
index 9b79f84b0..637642428 100644
--- a/src/server/middleware/contenttrust/contenttrust_test.go
+++ b/src/server/middleware/contenttrust/contenttrust_test.go
@@ -131,6 +131,14 @@ func (suite *MiddlewareTestSuite) TestContentTrustDisabled() {
 	suite.Equal(rr.Code, http.StatusOK)
 }
 
+func (suite *MiddlewareTestSuite) TestNoneArtifact() {
+	req := httptest.NewRequest("GET", "/v1/library/photon/manifests/nonexist", nil)
+	rr := httptest.NewRecorder()
+
+	Middleware()(suite.next).ServeHTTP(rr, req)
+	suite.Equal(rr.Code, http.StatusNotFound)
+}
+
 func (suite *MiddlewareTestSuite) TestAuthenticatedUserPulling() {
 	mock.OnAnything(suite.artifactController, "GetByReference").Return(suite.artifact, nil)
 	mock.OnAnything(suite.projectController, "GetByName").Return(suite.project, nil)
diff --git a/src/server/middleware/immutable/pushmf.go b/src/server/middleware/immutable/pushmf.go
index 18c1e865b..cbc007bea 100644
--- a/src/server/middleware/immutable/pushmf.go
+++ b/src/server/middleware/immutable/pushmf.go
@@ -40,7 +40,7 @@ func handlePush(req *http.Request) error {
 	none := lib.ArtifactInfo{}
 	art := lib.GetArtifactInfo(req.Context())
 	if art == none {
-		return errors.New("cannot get the manifest information from request context")
+		return errors.New("cannot get the manifest information from request context").WithCode(errors.NotFoundCode)
 	}
 
 	af, err := artifact.Ctl.GetByReference(req.Context(), art.Repository, art.Tag, &artifact.Option{
diff --git a/src/server/middleware/vulnerable/vulnerable.go b/src/server/middleware/vulnerable/vulnerable.go
index 5162bddd6..5a8692189 100644
--- a/src/server/middleware/vulnerable/vulnerable.go
+++ b/src/server/middleware/vulnerable/vulnerable.go
@@ -48,7 +48,7 @@ func Middleware() func(http.Handler) http.Handler {
 		none := lib.ArtifactInfo{}
 		info := lib.GetArtifactInfo(ctx)
 		if info == none {
-			return fmt.Errorf("artifactinfo middleware required before this middleware")
+			return errors.New("artifactinfo middleware required before this middleware").WithCode(errors.NotFoundCode)
 		}
 
 		art, err := artifactController.GetByReference(ctx, info.Repository, info.Reference, nil)
diff --git a/src/server/middleware/vulnerable/vulnerable_test.go b/src/server/middleware/vulnerable/vulnerable_test.go
index 551b84edf..5abd57e6d 100644
--- a/src/server/middleware/vulnerable/vulnerable_test.go
+++ b/src/server/middleware/vulnerable/vulnerable_test.go
@@ -127,7 +127,7 @@ func (suite *MiddlewareTestSuite) TestNoArtifactInfo() {
 	rr := httptest.NewRecorder()
 
 	Middleware()(suite.next).ServeHTTP(rr, req)
-	suite.Equal(rr.Code, http.StatusInternalServerError)
+	suite.Equal(rr.Code, http.StatusNotFound)
 }
 
 func (suite *MiddlewareTestSuite) TestGetArtifactFailed() {