Remove the duplicate http error struct (#6516)

There are two different types to represent http error in the current code. This commit updates the codes to keep only one.

Signed-off-by: Wenkai Yin <yinw@vmware.com>

src/common/api/base.go

@@ -21,7 +21,7 @@ import (
 	"strconv"
 
 	"github.com/astaxie/beego/validation"
-	http_error "github.com/goharbor/harbor/src/common/utils/error"
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/utils/log"
 
 	"github.com/astaxie/beego"
@@ -103,8 +103,8 @@ func (b *BaseAPI) ParseAndHandleError(text string, err error) {
 		return
 	}
 	log.Errorf("%s: %v", text, err)
-	if e, ok := err.(*http_error.HTTPError); ok {
+	if e, ok := err.(*commonhttp.Error); ok {
-		b.RenderError(e.StatusCode, e.Detail)
+		b.RenderError(e.Code, e.Message)
 		return
 	}
 	b.RenderError(http.StatusInternalServerError, "")

src/common/security/admiral/authcontext/authcontext.go

@@ -24,9 +24,9 @@ import (
 	"strings"
 
 	"github.com/goharbor/harbor/src/common"
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
-	http_error "github.com/goharbor/harbor/src/common/utils/error"
 	"github.com/goharbor/harbor/src/common/utils/log"
 )
 
@@ -187,9 +187,9 @@ func send(client *http.Client, req *http.Request) (*AuthContext, error) {
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, &http_error.HTTPError{
+		return nil, &commonhttp.Error{
-			StatusCode: resp.StatusCode,
+			Code:    resp.StatusCode,
-			Detail:     string(data),
+			Message: string(data),
 		}
 	}
 

src/common/utils/error/error.go

@@ -16,19 +16,7 @@ package error
 
 import (
 	"errors"
-	"fmt"
 )
 
 // ErrDupProject is the error returned when creating a duplicate project
 var ErrDupProject = errors.New("duplicate project")
-
-// HTTPError : if response is returned but the status code is not 200, an Error instance will be returned
-type HTTPError struct {
-	StatusCode int
-	Detail     string
-}
-
-// Error returns the details as string
-func (e *HTTPError) Error() string {
-	return fmt.Sprintf("%d %s", e.StatusCode, e.Detail)
-}

src/common/utils/error/error_test.go

@@ -1,30 +0,0 @@
-// Copyright Project Harbor Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//    http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-package error
-
-import (
-	"testing"
-)
-
-func TestError(t *testing.T) {
-	err := &HTTPError{
-		StatusCode: 404,
-		Detail:     "not found",
-	}
-
-	if err.Error() != "404 not found" {
-		t.Fatalf("unexpected content: %s != %s",
-			err.Error(), "404 not found")
-	}
-}

src/common/utils/registry/auth/util.go

@@ -21,8 +21,8 @@ import (
 	"net/url"
 
 	"github.com/docker/distribution/registry/auth/token"
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/models"
-	registry_error "github.com/goharbor/harbor/src/common/utils/error"
 	"github.com/goharbor/harbor/src/common/utils/registry"
 )
 
@@ -73,9 +73,9 @@ func getToken(client *http.Client, credential Credential, realm, service string,
 		return nil, err
 	}
 	if resp.StatusCode != http.StatusOK {
-		return nil, &registry_error.HTTPError{
+		return nil, &commonhttp.Error{
-			StatusCode: resp.StatusCode,
+			Code:    resp.StatusCode,
-			Detail:     string(data),
+			Message: string(data),
 		}
 	}
 

src/common/utils/registry/registry.go

@@ -23,8 +23,8 @@ import (
 	"strings"
 	// "time"
 
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/utils"
-	registry_error "github.com/goharbor/harbor/src/common/utils/error"
 )
 
 // Registry holds information of a registry entity
@@ -118,9 +118,9 @@ func (r *Registry) Catalog() ([]string, error) {
 				suffix = ""
 			}
 		} else {
-			return repos, &registry_error.HTTPError{
+			return repos, &commonhttp.Error{
-				StatusCode: resp.StatusCode,
+				Code:    resp.StatusCode,
-				Detail:     string(b),
+				Message: string(b),
 			}
 		}
 	}
@@ -149,8 +149,8 @@ func (r *Registry) Ping() error {
 		return err
 	}
 
-	return &registry_error.HTTPError{
+	return &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 }

src/common/utils/registry/repository.go

@@ -30,8 +30,8 @@ import (
 	"github.com/docker/distribution/manifest/schema1"
 	"github.com/docker/distribution/manifest/schema2"
 
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/utils"
-	registry_error "github.com/goharbor/harbor/src/common/utils/error"
 )
 
 // Repository holds information of a repository entity
@@ -61,7 +61,7 @@ func NewRepository(name, endpoint string, client *http.Client) (*Repository, err
 
 func parseError(err error) error {
 	if urlErr, ok := err.(*url.Error); ok {
-		if regErr, ok := urlErr.Err.(*registry_error.HTTPError); ok {
+		if regErr, ok := urlErr.Err.(*commonhttp.Error); ok {
 			return regErr
 		}
 	}
@@ -109,9 +109,9 @@ func (r *Repository) ListTag() ([]string, error) {
 		return tags, nil
 	}
 
-	return tags, &registry_error.HTTPError{
+	return tags, &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 
 }
@@ -149,9 +149,9 @@ func (r *Repository) ManifestExist(reference string) (digest string, exist bool,
 		return
 	}
 
-	err = &registry_error.HTTPError{
+	err = &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 	return
 }
@@ -186,9 +186,9 @@ func (r *Repository) PullManifest(reference string, acceptMediaTypes []string) (
 		return
 	}
 
-	err = &registry_error.HTTPError{
+	err = &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 
 	return
@@ -221,9 +221,9 @@ func (r *Repository) PushManifest(reference, mediaType string, payload []byte) (
 		return
 	}
 
-	err = &registry_error.HTTPError{
+	err = &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 
 	return
@@ -252,9 +252,9 @@ func (r *Repository) DeleteManifest(digest string) error {
 		return err
 	}
 
-	return &registry_error.HTTPError{
+	return &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 }
 
@@ -288,8 +288,8 @@ func (r *Repository) DeleteTag(tag string) error {
 	}
 
 	if !exist {
-		return &registry_error.HTTPError{
+		return &commonhttp.Error{
-			StatusCode: http.StatusNotFound,
+			Code: http.StatusNotFound,
 		}
 	}
 
@@ -323,9 +323,9 @@ func (r *Repository) BlobExist(digest string) (bool, error) {
 		return false, err
 	}
 
-	return false, &registry_error.HTTPError{
+	return false, &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 }
 
@@ -359,9 +359,9 @@ func (r *Repository) PullBlob(digest string) (size int64, data io.ReadCloser, er
 		return
 	}
 
-	err = &registry_error.HTTPError{
+	err = &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 
 	return
@@ -390,9 +390,9 @@ func (r *Repository) initiateBlobUpload(name string) (location, uploadUUID strin
 		return
 	}
 
-	err = &registry_error.HTTPError{
+	err = &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 
 	return
@@ -424,9 +424,9 @@ func (r *Repository) monolithicBlobUpload(location, digest string, size int64, d
 		return err
 	}
 
-	return &registry_error.HTTPError{
+	return &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 }
 
@@ -462,9 +462,9 @@ func (r *Repository) DeleteBlob(digest string) error {
 		return err
 	}
 
-	return &registry_error.HTTPError{
+	return &commonhttp.Error{
-		StatusCode: resp.StatusCode,
+		Code:    resp.StatusCode,
-		Detail:     string(b),
+		Message: string(b),
 	}
 }
 

src/common/utils/registry/repository_test.go

@@ -29,7 +29,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/docker/distribution/manifest/schema2"
-	registry_error "github.com/goharbor/harbor/src/common/utils/error"
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/utils/test"
 )
 
@@ -392,10 +392,10 @@ func TestListTag(t *testing.T) {
 
 func TestParseError(t *testing.T) {
 	err := &url.Error{
-		Err: &registry_error.HTTPError{},
+		Err: &commonhttp.Error{},
 	}
 	e := parseError(err)
-	if _, ok := e.(*registry_error.HTTPError); !ok {
+	if _, ok := e.(*commonhttp.Error); !ok {
 		t.Errorf("error type does not match registry error")
 	}
 }

src/core/api/repository.go

@@ -32,7 +32,6 @@ import (
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/common/utils/clair"
-	registry_error "github.com/goharbor/harbor/src/common/utils/error"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/common/utils/notary"
 	"github.com/goharbor/harbor/src/common/utils/registry"
@@ -265,8 +264,8 @@ func (ra *RepositoryAPI) Delete() {
 		if err != nil {
 			log.Errorf("error occurred while listing tags of %s: %v", repoName, err)
 
-			if regErr, ok := err.(*registry_error.HTTPError); ok {
+			if regErr, ok := err.(*commonhttp.Error); ok {
-				ra.CustomAbort(regErr.StatusCode, regErr.Detail)
+				ra.CustomAbort(regErr.Code, regErr.Message)
 			}
 
 			ra.CustomAbort(http.StatusInternalServerError, "internal error")
@@ -312,12 +311,12 @@ func (ra *RepositoryAPI) Delete() {
 			return
 		}
 		if err = rc.DeleteTag(t); err != nil {
-			if regErr, ok := err.(*registry_error.HTTPError); ok {
+			if regErr, ok := err.(*commonhttp.Error); ok {
-				if regErr.StatusCode == http.StatusNotFound {
+				if regErr.Code == http.StatusNotFound {
 					continue
 				}
 				log.Errorf("failed to delete tag %s: %v", t, err)
-				ra.CustomAbort(regErr.StatusCode, regErr.Detail)
+				ra.CustomAbort(regErr.Code, regErr.Message)
 			}
 			log.Errorf("error occurred while deleting tag %s:%s: %v", repoName, t, err)
 			ra.CustomAbort(http.StatusInternalServerError, "internal error")
@@ -751,8 +750,8 @@ func (ra *RepositoryAPI) GetManifests() {
 	if err != nil {
 		log.Errorf("error occurred while getting manifest of %s:%s: %v", repoName, tag, err)
 
-		if regErr, ok := err.(*registry_error.HTTPError); ok {
+		if regErr, ok := err.(*commonhttp.Error); ok {
-			ra.CustomAbort(regErr.StatusCode, regErr.Detail)
+			ra.CustomAbort(regErr.Code, regErr.Message)
 		}
 
 		ra.CustomAbort(http.StatusInternalServerError, "internal error")

src/core/api/utils.go

@@ -21,10 +21,10 @@ import (
 	"strings"
 
 	"github.com/goharbor/harbor/src/common/dao"
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	"github.com/goharbor/harbor/src/common/utils/clair"
-	registry_error "github.com/goharbor/harbor/src/common/utils/error"
 	"github.com/goharbor/harbor/src/common/utils/log"
 	"github.com/goharbor/harbor/src/common/utils/registry"
 	"github.com/goharbor/harbor/src/common/utils/registry/auth"
@@ -273,7 +273,7 @@ func buildReplicationActionURL() string {
 func repositoryExist(name string, client *registry.Repository) (bool, error) {
 	tags, err := client.ListTag()
 	if err != nil {
-		if regErr, ok := err.(*registry_error.HTTPError); ok && regErr.StatusCode == http.StatusNotFound {
+		if regErr, ok := err.(*commonhttp.Error); ok && regErr.Code == http.StatusNotFound {
 			return false, nil
 		}
 		return false, err

src/core/promgr/pmsdriver/admiral/admiral.go

@@ -26,6 +26,7 @@ import (
 	"strconv"
 	"strings"
 
+	commonhttp "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	er "github.com/goharbor/harbor/src/common/utils/error"
@@ -286,20 +287,20 @@ func (d *driver) Create(pro *models.Project) (int64, error) {
 		// Maybe a 409 error will be returned if Admiral team finds the way to
 		// return a specific code in Xenon.
 		// The following codes convert both those two errors to DupProjectErr
-		httpErr, ok := err.(*er.HTTPError)
+		httpErr, ok := err.(*commonhttp.Error)
 		if !ok {
 			return 0, err
 		}
 
-		if httpErr.StatusCode == http.StatusConflict {
+		if httpErr.Code == http.StatusConflict {
 			return 0, er.ErrDupProject
 		}
 
-		if httpErr.StatusCode != http.StatusInternalServerError {
+		if httpErr.Code != http.StatusInternalServerError {
 			return 0, err
 		}
 
-		match, e := regexp.MatchString(dupProjectPattern, httpErr.Detail)
+		match, e := regexp.MatchString(dupProjectPattern, httpErr.Message)
 		if e != nil {
 			log.Errorf("failed to match duplicate project mattern: %v", e)
 		}
@@ -397,9 +398,9 @@ func (d *driver) send(method, path string, body io.Reader) ([]byte, error) {
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, &er.HTTPError{
+		return nil, &commonhttp.Error{
-			StatusCode: resp.StatusCode,
+			Code:    resp.StatusCode,
-			Detail:     string(b),
+			Message: string(b),
 		}
 	}