update code per review comments

Signed-off-by: wang yan <wangyan@vmware.com>

src/lib/errors/const.go

@@ -11,6 +11,8 @@ const (
 	BadRequestCode = "BAD_REQUEST"
 	// ForbiddenCode ...
 	ForbiddenCode = "FORBIDDEN"
+	// MethodNotAllowedCode ...
+	MethodNotAllowedCode = "METHOD_NOT_ALLOWED"
 	// PreconditionCode ...
 	PreconditionCode = "PRECONDITION"
 	// GeneralCode ...
@@ -59,6 +61,11 @@ func ForbiddenError(err error) *Error {
 	return New("forbidden").WithCode(ForbiddenCode).WithCause(err)
 }
 
+// MethodNotAllowedError is error for the case of forbidden
+func MethodNotAllowedError(err error) *Error {
+	return New("method not allowed").WithCode(MethodNotAllowedCode).WithCause(err)
+}
+
 // PreconditionFailedError is error for the case of precondition failed
 func PreconditionFailedError(err error) *Error {
 	return New("precondition failed").WithCode(PreconditionCode).WithCause(err)

src/registryctl/api/base.go

@@ -26,9 +26,9 @@ func HandleInternalServerError(w http.ResponseWriter, err error) {
 	HandleError(w, errors.UnknownError(err))
 }
 
-// HandleForbidden ...
-func HandleForbidden(w http.ResponseWriter) {
-	HandleError(w, errors.ForbiddenError(nil))
+// HandleNotMethodAllowed ...
+func HandleNotMethodAllowed(w http.ResponseWriter) {
+	HandleError(w, errors.MethodNotAllowedError(nil))
 }
 
 // HandleBadRequest ...

src/registryctl/api/base_test.go

@@ -36,9 +36,9 @@ func TestHandleError(t *testing.T) {
 	}
 
 	w = httptest.NewRecorder()
-	HandleForbidden(w)
-	if w.Code != http.StatusForbidden {
-		t.Errorf("unexpected status code: %d != %d", w.Code, http.StatusForbidden)
+	HandleNotMethodAllowed(w)
+	if w.Code != http.StatusMethodNotAllowed {
+		t.Errorf("unexpected status code: %d != %d", w.Code, http.StatusMethodNotAllowed)
 	}
 
 	w = httptest.NewRecorder()

src/registryctl/api/registry/blob/blob.go

@@ -3,6 +3,7 @@ package blob
 import (
 	"errors"
 	"github.com/docker/distribution/registry/storage"
+	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/registryctl/api"
 	regConf "github.com/goharbor/harbor/src/registryctl/config/registry"
 	"github.com/gorilla/mux"
@@ -22,7 +23,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	case http.MethodDelete:
 		h.delete(w, req)
 	default:
-		api.HandleForbidden(w)
+		api.HandleNotMethodAllowed(w)
 	}
 }
 
@@ -36,6 +37,7 @@ func (h *handler) delete(w http.ResponseWriter, r *http.Request) {
 	// don't parse the reference here as RemoveBlob does.
 	cleaner := storage.NewVacuum(r.Context(), regConf.StorageDriver)
 	if err := cleaner.RemoveBlob(ref); err != nil {
+		log.Info("failed to remove blob: %s, with error:%v", ref, err)
 		api.HandleError(w, err)
 		return
 	}

src/registryctl/api/registry/gc/gc.go

@@ -26,7 +26,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	case http.MethodPost:
 		h.start(w, req)
 	default:
-		api.HandleForbidden(w)
+		api.HandleNotMethodAllowed(w)
 	}
 }
 

src/registryctl/api/registry/manifest/manifest.go

@@ -3,6 +3,7 @@ package manifest
 import (
 	"github.com/docker/distribution/registry/storage"
 	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/registryctl/api"
 	regConf "github.com/goharbor/harbor/src/registryctl/config/registry"
 	"github.com/gorilla/mux"
@@ -24,7 +25,7 @@ func (h *handler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	case http.MethodDelete:
 		h.delete(w, req)
 	default:
-		api.HandleForbidden(w)
+		api.HandleNotMethodAllowed(w)
 	}
 }
 
@@ -52,6 +53,7 @@ func (h *handler) delete(w http.ResponseWriter, r *http.Request) {
 
 	cleaner := storage.NewVacuum(r.Context(), regConf.StorageDriver)
 	if err := cleaner.RemoveManifest(repoName, dgst, tags); err != nil {
+		log.Info("failed to remove manifest: %s, with error:%v", ref, err)
 		api.HandleInternalServerError(w, err)
 		return
 	}

src/registryctl/config/registry/config.go

@@ -7,10 +7,10 @@ import (
 	"os"
 )
 
-// StorageDriver ...
+// StorageDriver the storage driver bases on the registry configurations, like filesystem, oss, gcs, S3, and etc.
 var StorageDriver storagedriver.StorageDriver
 
-// ResolveConfiguration ...
+// ResolveConfiguration loads the mounted registry configuration file, which is shared with the registry controller.
 func ResolveConfiguration(configPath string) (*configuration.Configuration, error) {
 	fp, err := os.Open(configPath)
 	if err != nil {

src/server/error/error.go

@@ -34,6 +34,7 @@ var (
 		errors.UNSUPPORTED:                     http.StatusBadRequest,
 		errors.UnAuthorizedCode:                http.StatusUnauthorized,
 		errors.ForbiddenCode:                   http.StatusForbidden,
+		errors.MethodNotAllowedCode:            http.StatusMethodNotAllowed,
 		errors.DENIED:                          http.StatusForbidden,
 		errors.NotFoundCode:                    http.StatusNotFound,
 		errors.ConflictCode:                    http.StatusConflict,