From bb958a7f4b373d2729b81ff194289ce5b65d55dc Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Tue, 22 Aug 2017 13:34:06 +0800 Subject: [PATCH 1/3] convert 500 error returned by Admiral to duplicate project error when creating duplicate project --- src/common/utils/error/error.go | 3 +++ src/ui/api/project.go | 5 ++--- src/ui/projectmanager/db/pm.go | 21 ++++++++++++++++++++- src/ui/projectmanager/pms/pm.go | 30 ++++++++++++++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/common/utils/error/error.go b/src/common/utils/error/error.go index 2f7ec33cf..f005a7c76 100644 --- a/src/common/utils/error/error.go +++ b/src/common/utils/error/error.go @@ -15,9 +15,12 @@ package error import ( + "errors" "fmt" ) +var DupProjectErr = 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 diff --git a/src/ui/api/project.go b/src/ui/api/project.go index b41110dec..17390d92d 100644 --- a/src/ui/api/project.go +++ b/src/ui/api/project.go @@ -23,6 +23,7 @@ import ( "github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/models" "github.com/vmware/harbor/src/common/utils" + errutil "github.com/vmware/harbor/src/common/utils/error" "github.com/vmware/harbor/src/common/utils/log" "github.com/vmware/harbor/src/ui/config" @@ -44,7 +45,6 @@ type ProjectAPI struct { const projectNameMaxLen int = 30 const projectNameMinLen int = 2 const restrictedNameChars = `[a-z0-9]+(?:[._-][a-z0-9]+)*` -const dupProjectPattern = `Duplicate entry '\w+' for key 'name'` // Prepare validates the URL and the user func (p *ProjectAPI) Prepare() { @@ -130,8 +130,7 @@ func (p *ProjectAPI) Post() { AutomaticallyScanImagesOnPush: pro.AutomaticallyScanImagesOnPush, }) if err != nil { - dup, _ := regexp.MatchString(dupProjectPattern, err.Error()) - if dup { + if err == errutil.DupProjectErr { log.Debugf("conflict %s", pro.Name) p.RenderError(http.StatusConflict, "") } else { diff --git a/src/ui/projectmanager/db/pm.go b/src/ui/projectmanager/db/pm.go index 4f659d6a1..9c0534ab4 100644 --- a/src/ui/projectmanager/db/pm.go +++ b/src/ui/projectmanager/db/pm.go @@ -16,12 +16,17 @@ package db import ( "fmt" + "regexp" "time" "github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/models" + errutil "github.com/vmware/harbor/src/common/utils/error" + "github.com/vmware/harbor/src/common/utils/log" ) +const dupProjectPattern = `Duplicate entry '\w+' for key 'name'` + // ProjectManager implements pm.PM interface based on database type ProjectManager struct{} @@ -105,7 +110,21 @@ func (p *ProjectManager) Create(project *models.Project) (int64, error) { UpdateTime: t, } - return dao.AddProject(*pro) + id, err := dao.AddProject(*pro) + if err != nil { + dup, e := regexp.MatchString(dupProjectPattern, err.Error()) + if e != nil { + log.Errorf("failed to match duplicate project pattern: %v", e) + } + + if dup { + err = errutil.DupProjectErr + } + + return 0, err + } + + return id, nil } // Delete ... diff --git a/src/ui/projectmanager/pms/pm.go b/src/ui/projectmanager/pms/pm.go index b61d62f19..29bba92ae 100644 --- a/src/ui/projectmanager/pms/pm.go +++ b/src/ui/projectmanager/pms/pm.go @@ -22,6 +22,7 @@ import ( "io" "io/ioutil" "net/http" + "regexp" "strconv" "strings" @@ -31,6 +32,8 @@ import ( "github.com/vmware/harbor/src/common/utils/log" ) +const dupProjectPattern = `Project name '\w+' is already used` + // ProjectManager implements projectmanager.ProjecdtManager interface // base on project management service type ProjectManager struct { @@ -366,6 +369,33 @@ func (p *ProjectManager) Create(pro *models.Project) (int64, error) { b, err := p.send(http.MethodPost, "/projects", bytes.NewBuffer(data)) if err != nil { + // when creating a project with a duplicate name in Admiral, a 500 error + // with a specific message will be returned for now. + // 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) + if !ok { + return 0, err + } + + if httpErr.StatusCode == http.StatusConflict { + return 0, er.DupProjectErr + } + + if httpErr.StatusCode != http.StatusInternalServerError { + return 0, err + } + + match, e := regexp.MatchString(dupProjectPattern, httpErr.Detail) + if e != nil { + log.Errorf("failed to match duplicate project mattern: %v", e) + } + + if match { + err = er.DupProjectErr + } + return 0, err } From ffb2f4201b2dbaefed12ebee16587e65d472ac6f Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Tue, 22 Aug 2017 14:28:45 +0800 Subject: [PATCH 2/3] update --- src/common/utils/error/error.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/utils/error/error.go b/src/common/utils/error/error.go index f005a7c76..6fea13a21 100644 --- a/src/common/utils/error/error.go +++ b/src/common/utils/error/error.go @@ -19,6 +19,7 @@ import ( "fmt" ) +// DupProjectErr is the error returned when creating a duplicate project var DupProjectErr = errors.New("duplicate project") // HTTPError : if response is returned but the status code is not 200, an Error instance will be returned From 599d94be0cd0c5162807f91538ded437f48ac18d Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Tue, 22 Aug 2017 15:22:25 +0800 Subject: [PATCH 3/3] update --- src/common/utils/error/error.go | 4 ++-- src/ui/api/project.go | 2 +- src/ui/projectmanager/db/pm.go | 2 +- src/ui/projectmanager/db/pm_test.go | 14 ++++++++++++++ src/ui/projectmanager/pms/pm.go | 4 ++-- src/ui/projectmanager/pms/pm_test.go | 7 +++++++ 6 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/common/utils/error/error.go b/src/common/utils/error/error.go index 6fea13a21..3f2574f87 100644 --- a/src/common/utils/error/error.go +++ b/src/common/utils/error/error.go @@ -19,8 +19,8 @@ import ( "fmt" ) -// DupProjectErr is the error returned when creating a duplicate project -var DupProjectErr = errors.New("duplicate project") +// 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 { diff --git a/src/ui/api/project.go b/src/ui/api/project.go index 17390d92d..1b7a6f2a8 100644 --- a/src/ui/api/project.go +++ b/src/ui/api/project.go @@ -130,7 +130,7 @@ func (p *ProjectAPI) Post() { AutomaticallyScanImagesOnPush: pro.AutomaticallyScanImagesOnPush, }) if err != nil { - if err == errutil.DupProjectErr { + if err == errutil.ErrDupProject { log.Debugf("conflict %s", pro.Name) p.RenderError(http.StatusConflict, "") } else { diff --git a/src/ui/projectmanager/db/pm.go b/src/ui/projectmanager/db/pm.go index 9c0534ab4..f723d4516 100644 --- a/src/ui/projectmanager/db/pm.go +++ b/src/ui/projectmanager/db/pm.go @@ -118,7 +118,7 @@ func (p *ProjectManager) Create(project *models.Project) (int64, error) { } if dup { - err = errutil.DupProjectErr + err = errutil.ErrDupProject } return 0, err diff --git a/src/ui/projectmanager/db/pm_test.go b/src/ui/projectmanager/db/pm_test.go index 227ed6063..8e7148fe0 100644 --- a/src/ui/projectmanager/db/pm_test.go +++ b/src/ui/projectmanager/db/pm_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/models" + errutil "github.com/vmware/harbor/src/common/utils/error" "github.com/vmware/harbor/src/common/utils/log" ) @@ -166,6 +167,19 @@ func TestCreateAndDelete(t *testing.T) { }) assert.Nil(t, err) assert.Nil(t, pm.Delete(id)) + + // duplicate project name + id, err = pm.Create(&models.Project{ + Name: "test", + OwnerName: "admin", + }) + assert.Nil(t, err) + defer pm.Delete(id) + _, err = pm.Create(&models.Project{ + Name: "test", + OwnerName: "admin", + }) + assert.Equal(t, errutil.ErrDupProject, err) } func TestUpdate(t *testing.T) { diff --git a/src/ui/projectmanager/pms/pm.go b/src/ui/projectmanager/pms/pm.go index 29bba92ae..14944248b 100644 --- a/src/ui/projectmanager/pms/pm.go +++ b/src/ui/projectmanager/pms/pm.go @@ -380,7 +380,7 @@ func (p *ProjectManager) Create(pro *models.Project) (int64, error) { } if httpErr.StatusCode == http.StatusConflict { - return 0, er.DupProjectErr + return 0, er.ErrDupProject } if httpErr.StatusCode != http.StatusInternalServerError { @@ -393,7 +393,7 @@ func (p *ProjectManager) Create(pro *models.Project) (int64, error) { } if match { - err = er.DupProjectErr + err = er.ErrDupProject } return 0, err diff --git a/src/ui/projectmanager/pms/pm_test.go b/src/ui/projectmanager/pms/pm_test.go index 66103ef0c..c8965699a 100644 --- a/src/ui/projectmanager/pms/pm_test.go +++ b/src/ui/projectmanager/pms/pm_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/vmware/harbor/src/common/models" + errutil "github.com/vmware/harbor/src/common/utils/error" ) var ( @@ -344,6 +345,12 @@ func TestCreate(t *testing.T) { assert.True(t, project.PreventVulnerableImagesFromRunning) assert.Equal(t, "medium", project.PreventVulnerableImagesFromRunningSeverity) assert.True(t, project.AutomaticallyScanImagesOnPush) + + // duplicate project name + _, err = pm.Create(&models.Project{ + Name: name, + }) + assert.Equal(t, errutil.ErrDupProject, err) } func TestDelete(t *testing.T) {