From cd666bc3d5f1d1330f5e38dd2dd9e147bcb53725 Mon Sep 17 00:00:00 2001 From: Ashish-devtron Date: Wed, 8 Feb 2023 09:18:41 +0530 Subject: [PATCH 1/4] Duplicate name in Charts and Chart Group --- api/appStore/InstalledAppRestHandler.go | 9 +++++++++ internal/sql/repository/app/AppRepository.go | 1 + pkg/appStore/deployment/repository/ChartGroup.go | 10 ++++++++++ .../deployment/service/ChartGroupService.go | 14 +++++++++++++- 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/api/appStore/InstalledAppRestHandler.go b/api/appStore/InstalledAppRestHandler.go index 88b2d81807..3727234ed5 100644 --- a/api/appStore/InstalledAppRestHandler.go +++ b/api/appStore/InstalledAppRestHandler.go @@ -295,7 +295,16 @@ func (handler *InstalledAppRestHandlerImpl) DeployBulk(w http.ResponseWriter, r } //RBAC block ends here + visited := make(map[string]bool) + for _, item := range request.ChartGroupInstallChartRequest { + if visited[item.AppName] { + handler.Logger.Errorw("service err, CreateInstalledApp", "err", err, "payload", request) + common.WriteJsonResp(w, errors.New("Duplicate appName found"), nil, http.StatusInternalServerError) + return + } else { + visited[item.AppName] = true + } isChartRepoActive, err := handler.appStoreDeploymentService.IsChartRepoActive(item.AppStoreVersion) if err != nil { handler.Logger.Errorw("service err, CreateInstalledApp", "err", err, "payload", request) diff --git a/internal/sql/repository/app/AppRepository.go b/internal/sql/repository/app/AppRepository.go index 85511a55e9..3c33f0f00e 100644 --- a/internal/sql/repository/app/AppRepository.go +++ b/internal/sql/repository/app/AppRepository.go @@ -135,6 +135,7 @@ func (repo AppRepositoryImpl) CheckAppExists(appNames []string) ([]*App, error) err := repo.dbConnection. Model(&apps). Where("app_name in (?)", pg.In(appNames)). + Where("active = ?", true). Select() return apps, err } diff --git a/pkg/appStore/deployment/repository/ChartGroup.go b/pkg/appStore/deployment/repository/ChartGroup.go index 057b252fac..8ca63fd2e6 100644 --- a/pkg/appStore/deployment/repository/ChartGroup.go +++ b/pkg/appStore/deployment/repository/ChartGroup.go @@ -53,6 +53,7 @@ type ChartGroupReposotory interface { FindById(chartGroupId int) (*ChartGroup, error) GetAll(max int) ([]*ChartGroup, error) MarkChartGroupDeleted(chartGroupId int, tx *pg.Tx) error + FindByName(chartGroupName string) (bool, error) } func (impl *ChartGroupReposotoryImpl) Save(model *ChartGroup) (*ChartGroup, error) { @@ -105,3 +106,12 @@ func (impl *ChartGroupReposotoryImpl) MarkChartGroupDeleted(chartGroupId int, tx Update() return err } + +func (impl *ChartGroupReposotoryImpl) FindByName(chartGroupName string) (bool, error) { + var ChartGroup ChartGroup + exists, err := impl.dbConnection.Model(&ChartGroup).Where("name = ?", chartGroupName). + Where("deleted = ?", false). + Exists() + + return exists, err +} diff --git a/pkg/appStore/deployment/service/ChartGroupService.go b/pkg/appStore/deployment/service/ChartGroupService.go index 4e0fac4075..22cacadb39 100644 --- a/pkg/appStore/deployment/service/ChartGroupService.go +++ b/pkg/appStore/deployment/service/ChartGroupService.go @@ -18,6 +18,7 @@ package service import ( + "errors" appStoreBean "github.com/devtron-labs/devtron/pkg/appStore/bean" "github.com/devtron-labs/devtron/pkg/appStore/deployment/repository" appStoreValuesRepository "github.com/devtron-labs/devtron/pkg/appStore/values/repository" @@ -69,7 +70,7 @@ type ChartGroupList struct { Groups []*ChartGroupBean `json:"groups,omitempty"` } type ChartGroupBean struct { - Name string `json:"name,omitempty" validate:"name-component,max=200"` + Name string `json:"name,omitempty" validate:"name-component,max=200,min=5"` Description string `json:"description,omitempty"` Id int `json:"id,omitempty"` ChartGroupEntries []*ChartGroupEntryBean `json:"chartGroupEntries,omitempty"` @@ -110,6 +111,17 @@ type InstalledChart struct { func (impl *ChartGroupServiceImpl) CreateChartGroup(req *ChartGroupBean) (*ChartGroupBean, error) { impl.Logger.Debugw("chart group create request", "req", req) + + exist, err := impl.chartGroupRepository.FindByName(req.Name) + if err != nil { + impl.Logger.Errorw("error in creating chart group", "req", req, "err", err) + return nil, err + } + if exist { + impl.Logger.Errorw("Chart with this name already exist", "req", req, "err", err) + return nil, errors.New("A chart with this name already exist") + } + chartGrouModel := &repository.ChartGroup{ Name: req.Name, Description: req.Description, From 8cf005fbdc7c0872c353a787cbb3c9b86a8d63d4 Mon Sep 17 00:00:00 2001 From: Ashish-devtron Date: Fri, 10 Feb 2023 18:15:22 +0530 Subject: [PATCH 2/4] Bad Request --- api/appStore/InstalledAppRestHandler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/appStore/InstalledAppRestHandler.go b/api/appStore/InstalledAppRestHandler.go index 3727234ed5..d9b5a019c0 100644 --- a/api/appStore/InstalledAppRestHandler.go +++ b/api/appStore/InstalledAppRestHandler.go @@ -300,7 +300,7 @@ func (handler *InstalledAppRestHandlerImpl) DeployBulk(w http.ResponseWriter, r for _, item := range request.ChartGroupInstallChartRequest { if visited[item.AppName] { handler.Logger.Errorw("service err, CreateInstalledApp", "err", err, "payload", request) - common.WriteJsonResp(w, errors.New("Duplicate appName found"), nil, http.StatusInternalServerError) + common.WriteJsonResp(w, errors.New("Duplicate appName found"), nil, http.StatusBadRequest) return } else { visited[item.AppName] = true From 2cbd64038262950bad11665fe8045ae54cf1f4a3 Mon Sep 17 00:00:00 2001 From: Ashish-devtron Date: Fri, 10 Feb 2023 18:26:39 +0530 Subject: [PATCH 3/4] Error message --- api/appStore/InstalledAppRestHandler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/appStore/InstalledAppRestHandler.go b/api/appStore/InstalledAppRestHandler.go index d9b5a019c0..fa5d519c8a 100644 --- a/api/appStore/InstalledAppRestHandler.go +++ b/api/appStore/InstalledAppRestHandler.go @@ -300,7 +300,7 @@ func (handler *InstalledAppRestHandlerImpl) DeployBulk(w http.ResponseWriter, r for _, item := range request.ChartGroupInstallChartRequest { if visited[item.AppName] { handler.Logger.Errorw("service err, CreateInstalledApp", "err", err, "payload", request) - common.WriteJsonResp(w, errors.New("Duplicate appName found"), nil, http.StatusBadRequest) + common.WriteJsonResp(w, errors.New("duplicate appName found"), nil, http.StatusBadRequest) return } else { visited[item.AppName] = true From ee04872e835f9cb3e4f9399a4ccb2fdea80b4107 Mon Sep 17 00:00:00 2001 From: Ashish-devtron Date: Fri, 10 Feb 2023 19:13:40 +0530 Subject: [PATCH 4/4] Status Code Update --- api/restHandler/ChartGroupRestHandler.go | 10 +++++++--- pkg/appStore/deployment/service/ChartGroupService.go | 4 +++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/api/restHandler/ChartGroupRestHandler.go b/api/restHandler/ChartGroupRestHandler.go index baedd035f1..19c95d2e09 100644 --- a/api/restHandler/ChartGroupRestHandler.go +++ b/api/restHandler/ChartGroupRestHandler.go @@ -99,7 +99,11 @@ func (impl *ChartGroupRestHandlerImpl) CreateChartGroup(w http.ResponseWriter, r res, err := impl.ChartGroupService.CreateChartGroup(&request) if err != nil { impl.Logger.Errorw("service err, CreateChartGroup", "err", err, "payload", request) - common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) + statusCode := http.StatusInternalServerError + if service.AppNameAlreadyExistsError == err.Error() { + statusCode = http.StatusBadRequest + } + common.WriteJsonResp(w, err, nil, statusCode) return } common.WriteJsonResp(w, err, res, http.StatusOK) @@ -312,7 +316,7 @@ func (impl *ChartGroupRestHandlerImpl) GetChartGroupListMin(w http.ResponseWrite common.WriteJsonResp(w, err, res, http.StatusOK) } -func(impl *ChartGroupRestHandlerImpl) DeleteChartGroup(w http.ResponseWriter, r *http.Request){ +func (impl *ChartGroupRestHandlerImpl) DeleteChartGroup(w http.ResponseWriter, r *http.Request) { userId, err := impl.userAuthService.GetLoggedInUser(r) if userId == 0 || err != nil { common.WriteJsonResp(w, err, "Unauthorized User", http.StatusUnauthorized) @@ -350,4 +354,4 @@ func(impl *ChartGroupRestHandlerImpl) DeleteChartGroup(w http.ResponseWriter, r return } common.WriteJsonResp(w, err, CHART_GROUP_DELETE_SUCCESS_RESP, http.StatusOK) -} \ No newline at end of file +} diff --git a/pkg/appStore/deployment/service/ChartGroupService.go b/pkg/appStore/deployment/service/ChartGroupService.go index 22cacadb39..0525bace24 100644 --- a/pkg/appStore/deployment/service/ChartGroupService.go +++ b/pkg/appStore/deployment/service/ChartGroupService.go @@ -109,6 +109,8 @@ type InstalledChart struct { InstalledAppId int `json:"installedAppId,omitempty"` } +const AppNameAlreadyExistsError = "A chart with this name already exist" + func (impl *ChartGroupServiceImpl) CreateChartGroup(req *ChartGroupBean) (*ChartGroupBean, error) { impl.Logger.Debugw("chart group create request", "req", req) @@ -119,7 +121,7 @@ func (impl *ChartGroupServiceImpl) CreateChartGroup(req *ChartGroupBean) (*Chart } if exist { impl.Logger.Errorw("Chart with this name already exist", "req", req, "err", err) - return nil, errors.New("A chart with this name already exist") + return nil, errors.New(AppNameAlreadyExistsError) } chartGrouModel := &repository.ChartGroup{