diff --git a/pkg/chartRepo/ChartRepositoryService.go b/pkg/chartRepo/ChartRepositoryService.go index e45c14cd26..a057b5add7 100644 --- a/pkg/chartRepo/ChartRepositoryService.go +++ b/pkg/chartRepo/ChartRepositoryService.go @@ -44,7 +44,7 @@ import ( type ChartRepositoryService interface { CreateChartRepo(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error) - UpdateChartRepo(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error) + UpdateDataInArgocdCm(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error) GetChartRepoById(id int) (*ChartRepoDto, error) GetChartRepoByName(name string) (*ChartRepoDto, error) GetChartRepoList() ([]*ChartRepoDto, error) @@ -52,7 +52,7 @@ type ChartRepositoryService interface { ValidateAndCreateChartRepo(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error, *DetailedErrorHelmRepoValidation) ValidateAndUpdateChartRepo(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error, *DetailedErrorHelmRepoValidation) TriggerChartSyncManual() error - DeleteChartRepo(request *ChartRepoDto) error + DeleteChartRepoFromArgocdCM(request *ChartRepoDto) error } type ChartRepositoryServiceImpl struct { @@ -127,7 +127,11 @@ func (impl *ChartRepositoryServiceImpl) CreateChartRepo(request *ChartRepoDto) ( if err != nil { return nil, err } - data := impl.updateData(cm.Data, request) + data, err := impl.updateRepoData(cm.Data, request) + if err != nil { + impl.logger.Warnw(" config map update failed", "err", err) + continue + } cm.Data = data _, err = impl.K8sUtil.UpdateConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, cm, client) if err != nil { @@ -148,7 +152,7 @@ func (impl *ChartRepositoryServiceImpl) CreateChartRepo(request *ChartRepoDto) ( return chartRepo, nil } -func (impl *ChartRepositoryServiceImpl) UpdateChartRepo(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error) { +func (impl *ChartRepositoryServiceImpl) UpdateDataInArgocdCm(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error) { dbConnection := impl.repoRepository.GetConnection() tx, err := dbConnection.Begin() if err != nil { @@ -160,8 +164,10 @@ func (impl *ChartRepositoryServiceImpl) UpdateChartRepo(request *ChartRepoDto) ( if err != nil && !util.IsErrNoRows(err) { return nil, err } + previousName := chartRepo.Name chartRepo.Url = request.Url + chartRepo.Name = request.Name chartRepo.AuthMode = request.AuthMode chartRepo.UserName = request.UserName chartRepo.Password = request.Password @@ -186,7 +192,6 @@ func (impl *ChartRepositoryServiceImpl) UpdateChartRepo(request *ChartRepoDto) ( if err != nil { return nil, err } - client, err := impl.K8sUtil.GetClient(cfg) if err != nil { return nil, err @@ -200,7 +205,18 @@ func (impl *ChartRepositoryServiceImpl) UpdateChartRepo(request *ChartRepoDto) ( if err != nil { return nil, err } - data := impl.updateData(cm.Data, request) + var data map[string]string + // if the repo name has been updated then, create a new repo + data, err = impl.updateRepoData(cm.Data, request) + // if the repo name has been updated then, delete the previous repo + if previousName != request.Name { + data, err = impl.removeRepoData(cm.Data, previousName) + } + if err != nil { + impl.logger.Warnw(" config map update failed", "err", err) + continue + } + cm.Data = data _, err = impl.K8sUtil.UpdateConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, cm, client) if err != nil { @@ -223,6 +239,88 @@ func (impl *ChartRepositoryServiceImpl) UpdateChartRepo(request *ChartRepoDto) ( return chartRepo, nil } +// DeleteChartRepoFromArgocdCM update the active state from DB and modify the argo-cm with repo URL to null +func (impl *ChartRepositoryServiceImpl) DeleteChartRepoFromArgocdCM(request *ChartRepoDto) error { + dbConnection := impl.repoRepository.GetConnection() + tx, err := dbConnection.Begin() + if err != nil { + impl.logger.Errorw("error in establishing db connection, DeleteChartRepo", "err", err) + return err + } + // Rollback tx on error. + defer tx.Rollback() + + chartRepo, err := impl.repoRepository.FindById(request.Id) + if err != nil && !util.IsErrNoRows(err) { + impl.logger.Errorw("error in finding chart repo by id", "err", err, "id", request.Id) + return err + } + chartRepo.Url = request.Url + chartRepo.AuthMode = request.AuthMode + chartRepo.UserName = request.UserName + chartRepo.Password = request.Password + chartRepo.Active = request.Active + chartRepo.AccessToken = request.AccessToken + chartRepo.SshKey = request.SshKey + chartRepo.Active = request.Active + chartRepo.UpdatedBy = request.UserId + chartRepo.UpdatedOn = time.Now() + err = impl.repoRepository.MarkChartRepoDeleted(chartRepo, tx) + if err != nil { + impl.logger.Errorw("error in deleting chart repo", "err", err) + return err + } + + // modify configmap + clusterBean, err := impl.clusterService.FindOne(cluster.DefaultClusterName) + if err != nil { + return err + } + cfg, err := impl.clusterService.GetClusterConfig(clusterBean) + if err != nil { + return err + } + client, err := impl.K8sUtil.GetClient(cfg) + if err != nil { + return err + } + updateSuccess := false + retryCount := 0 + //request.Url = "" + for !updateSuccess && retryCount < 3 { + retryCount = retryCount + 1 + + cm, err := impl.K8sUtil.GetConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, impl.aCDAuthConfig.ACDConfigMapName, client) + if err != nil { + return err + } + data, err := impl.removeRepoData(cm.Data, request.Name) + if err != nil { + impl.logger.Warnw(" config map update failed", "err", err) + continue + } + cm.Data = data + _, err = impl.K8sUtil.UpdateConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, cm, client) + if err != nil { + impl.logger.Warnw(" config map failed", "err", err) + continue + } + if err == nil { + impl.logger.Warnw(" config map apply succeeded", "on retryCount", retryCount) + updateSuccess = true + } + } + if !updateSuccess { + return fmt.Errorf("resouce version not matched with config map attempted 3 times") + } + err = tx.Commit() + if err != nil { + impl.logger.Errorw("error in tx commit, DeleteChartRepo", "err", err) + return err + } + return nil +} + func (impl *ChartRepositoryServiceImpl) GetChartRepoById(id int) (*ChartRepoDto, error) { model, err := impl.repoRepository.FindById(id) if err != nil && !util.IsErrNoRows(err) { @@ -357,7 +455,7 @@ func (impl *ChartRepositoryServiceImpl) ValidateAndUpdateChartRepo(request *Char if validationResult.CustomErrMsg != ValidationSuccessMsg { return nil, nil, validationResult } - chartRepo, err := impl.UpdateChartRepo(request) + chartRepo, err := impl.UpdateDataInArgocdCm(request) if err != nil { return nil, err, validationResult } @@ -465,25 +563,30 @@ func (impl *ChartRepositoryServiceImpl) createRepoElement(request *ChartRepoDto) return repoData } -func (impl *ChartRepositoryServiceImpl) updateData(data map[string]string, request *ChartRepoDto) map[string]string { +// updateRepoData update the request field in the argo-cm +func (impl *ChartRepositoryServiceImpl) updateRepoData(data map[string]string, request *ChartRepoDto) (map[string]string, error) { helmRepoStr := data["helm.repositories"] helmRepoByte, err := yaml.YAMLToJSON([]byte(helmRepoStr)) if err != nil { - panic(err) + impl.logger.Errorw("error in json patch", "err", err) + return nil, err } var helmRepositories []*AcdConfigMapRepositoriesDto err = json.Unmarshal(helmRepoByte, &helmRepositories) if err != nil { - panic(err) + impl.logger.Errorw("error in unmarshal", "err", err) + return nil, err } rb, err := json.Marshal(helmRepositories) if err != nil { - panic(err) + impl.logger.Errorw("error in marshal", "err", err) + return nil, err } helmRepositoriesYamlByte, err := yaml.JSONToYAML(rb) if err != nil { - panic(err) + impl.logger.Errorw("error in yaml patch", "err", err) + return nil, err } //SETUP for repositories @@ -491,16 +594,18 @@ func (impl *ChartRepositoryServiceImpl) updateData(data map[string]string, reque repoStr := data["repositories"] repoByte, err := yaml.YAMLToJSON([]byte(repoStr)) if err != nil { - panic(err) + impl.logger.Errorw("error in json patch", "err", err) + return nil, err } err = json.Unmarshal(repoByte, &repositories) if err != nil { - panic(err) + impl.logger.Errorw("error in unmarshal", "err", err) + return nil, err } found := false for _, item := range repositories { - //if request chart repo found, than update its values + //if request chart repo found, then update its values if item.Name == request.Name { if request.AuthMode == repository.AUTH_MODE_USERNAME_PASSWORD { usernameSecret := &KeyDto{Name: request.UserName, Key: "username"} @@ -526,11 +631,13 @@ func (impl *ChartRepositoryServiceImpl) updateData(data map[string]string, reque rb, err = json.Marshal(repositories) if err != nil { - panic(err) + impl.logger.Errorw("error in marshal", "err", err) + return nil, err } repositoriesYamlByte, err := yaml.JSONToYAML(rb) if err != nil { - panic(err) + impl.logger.Errorw("error in yaml patch", "err", err) + return nil, err } if len(helmRepositoriesYamlByte) > 0 { @@ -542,44 +649,84 @@ func (impl *ChartRepositoryServiceImpl) updateData(data map[string]string, reque //dex config copy as it is dexConfigStr := data["dex.config"] data["dex.config"] = string([]byte(dexConfigStr)) - return data + return data, nil } -func (impl *ChartRepositoryServiceImpl) DeleteChartRepo(request *ChartRepoDto) error { - dbConnection := impl.repoRepository.GetConnection() - tx, err := dbConnection.Begin() +// removeRepoData delete the request field from the argo-cm +func (impl *ChartRepositoryServiceImpl) removeRepoData(data map[string]string, name string) (map[string]string, error) { + helmRepoStr := data["helm.repositories"] + helmRepoByte, err := yaml.YAMLToJSON([]byte(helmRepoStr)) if err != nil { - impl.logger.Errorw("error in establishing db connection, DeleteChartRepo", "err", err) - return err + impl.logger.Errorw("error in json patch", "err", err) + return nil, err + } + var helmRepositories []*AcdConfigMapRepositoriesDto + err = json.Unmarshal(helmRepoByte, &helmRepositories) + if err != nil { + impl.logger.Errorw("error in unmarshal", "err", err) + return nil, err } - // Rollback tx on error. - defer tx.Rollback() - chartRepo, err := impl.repoRepository.FindById(request.Id) - if err != nil && !util.IsErrNoRows(err) { - impl.logger.Errorw("error in finding chart repo by id", "err", err, "id", request.Id) - return err + rb, err := json.Marshal(helmRepositories) + if err != nil { + impl.logger.Errorw("error in marshal", "err", err) + return nil, err + } + helmRepositoriesYamlByte, err := yaml.JSONToYAML(rb) + if err != nil { + impl.logger.Errorw("error in yaml patch", "err", err) + return nil, err } - chartRepo.Url = request.Url - chartRepo.AuthMode = request.AuthMode - chartRepo.UserName = request.UserName - chartRepo.Password = request.Password - chartRepo.Active = request.Active - chartRepo.AccessToken = request.AccessToken - chartRepo.SshKey = request.SshKey - chartRepo.Active = false - chartRepo.UpdatedBy = request.UserId - chartRepo.UpdatedOn = time.Now() - err = impl.repoRepository.MarkChartRepoDeleted(chartRepo, tx) + //SETUP for repositories + var repositories []*AcdConfigMapRepositoriesDto + repoStr := data["repositories"] + repoByte, err := yaml.YAMLToJSON([]byte(repoStr)) if err != nil { - impl.logger.Errorw("error in deleting chart repo", "err", err) - return err + impl.logger.Errorw("error in json patch", "err", err) + return nil, err } - err = tx.Commit() + err = json.Unmarshal(repoByte, &repositories) if err != nil { - impl.logger.Errorw("error in tx commit, DeleteChartRepo", "err", err) - return err + impl.logger.Errorw("error in unmarshal", "err", err) + return nil, err } - return nil + + found := false + for index, item := range repositories { + //if request chart repo found, then delete its values + if item.Name == name { + repositories = append(repositories[:index], repositories[index+1:]...) + found = true + break + } + } + + // if request chart repo not found, add new one + if !found { + impl.logger.Errorw("Repo not found", "err", err) + return nil, fmt.Errorf("Repo not found in config-map") + } + + rb, err = json.Marshal(repositories) + if err != nil { + impl.logger.Errorw("error in marshal", "err", err) + return nil, err + } + repositoriesYamlByte, err := yaml.JSONToYAML(rb) + if err != nil { + impl.logger.Errorw("error in yaml patch", "err", err) + return nil, err + } + + if len(helmRepositoriesYamlByte) > 0 { + data["helm.repositories"] = string(helmRepositoriesYamlByte) + } + if len(repositoriesYamlByte) > 0 { + data["repositories"] = string(repositoriesYamlByte) + } + //dex config copy as it is + dexConfigStr := data["dex.config"] + data["dex.config"] = string([]byte(dexConfigStr)) + return data, nil } diff --git a/pkg/chartRepo/ChartRepositoryService_test.go b/pkg/chartRepo/ChartRepositoryService_test.go index 871b53f835..221b25823b 100644 --- a/pkg/chartRepo/ChartRepositoryService_test.go +++ b/pkg/chartRepo/ChartRepositoryService_test.go @@ -1,63 +1,147 @@ package chartRepo import ( + "encoding/json" + "testing" + "github.com/devtron-labs/devtron/internal/sql/repository" "github.com/devtron-labs/devtron/internal/util" + "github.com/ghodss/yaml" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "testing" ) type ChartRepositoryServiceMock struct { mock.Mock } -func TestChartRepositoryServiceImpl_ValidateChartDetails(t *testing.T) { - sugaredLogger, _ := util.NewSugardLogger() +func TestChartRepositoryServiceRemoveRepoData(t *testing.T) { + sugaredLogger, err := util.NewSugardLogger() + assert.Nil(t, err) + impl := &ChartRepositoryServiceImpl{ - logger: sugaredLogger, - repoRepository: new(ChartRepoRepositoryImplMock), - K8sUtil: nil, - clusterService: new(ClusterServiceImplMock), - aCDAuthConfig: nil, - client: nil, + logger: sugaredLogger, } + t.Run("Invalid JSON byte array", func(t *testing.T) { + data := map[string]string{ + "helm.repositories": "invalid json", + } + _, err := impl.removeRepoData(data, "test-repo") + if err == nil { + t.Errorf("Expected an error but got nil") + } + }) + + t.Run("Error in unmarshal helmRepositories", func(t *testing.T) { + data := map[string]string{ + "helm.repositories": "- name: test-1\n type: helm\n url: https://localhost\n- name: test-repo\n type: helm\n url: https://localhost\n- name: test-2\n type: helm\n url: https://localhost\n", + } + _, err := impl.removeRepoData(data, "test-repo") + if err == nil { + t.Errorf("Expected an error but got nil") + } + }) + + t.Run("Error in unmarshal repositories", func(t *testing.T) { + data := map[string]string{ + "helm.repositories": "null\n", + "repositories": "invalid json", + } + _, err := impl.removeRepoData(data, "test-repo") + if err == nil { + t.Errorf("Expected an error but got nil") + } + }) + + t.Run("Repository is found and removed from repositories", func(t *testing.T) { + data := map[string]string{ + "helm.repositories": "null\n", + "repositories": "- name: test-1\n type: helm\n url: https://localhost\n- name: test-repo\n type: helm\n url: https://localhost\n- name: test-2\n type: helm\n url: https://localhost\n", + "dex.config": "", + } + expected := map[string]string{ + "helm.repositories": "null\n", + "repositories": "- name: test-1\n type: helm\n url: https://localhost\n- name: test-2\n type: helm\n url: https://localhost\n", + "dex.config": "", + } + result, err := impl.removeRepoData(data, "test-repo") + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if !assert.Equal(t, result, expected) { + t.Errorf("Expected %v, but got %v", expected, result) + } + }) + + t.Run("Repository is not found", func(t *testing.T) { + data := map[string]string{ + "helm.repositories": "null\n", + "repositories": "- name: test-1\n type: helm\n url: https://localhost\n- name: test-2\n type: helm\n url: https://localhost\n", + } + _, err := impl.removeRepoData(data, "test-repo") + if err == nil { + t.Errorf("Expected an error but got nil") + } + }) +} - type args struct { - FileName string - chartVersion string +func TestUpdateRepository_NewRepository(t *testing.T) { + // setup + sugaredLogger, err := util.NewSugardLogger() + assert.Nil(t, err) + s := ChartRepositoryServiceImpl{logger: sugaredLogger} + req := &ChartRepoDto{ + Name: "myrepo2", + Url: "https://github.com/devtron/myrepo2/", + AuthMode: repository.AUTH_MODE_USERNAME_PASSWORD, + UserName: "myuser", + Password: "mypass", } - tests := []struct { - name string - args args - want string - wantErr bool - }{ - { - name: "Test file format", - args: struct { - FileName string - chartVersion string - }{FileName: "test.tar.gz", chartVersion: "1.0.0"}, - want: "test_1-0-0", - }, - { - name: "Test file format", - args: struct { - FileName string - chartVersion string - }{FileName: "test.pdf", chartVersion: "1.0.0"}, - wantErr: true, - }, + oldData := map[string]string{ + "helm.repositories": "null\n", + "repositories": "- name: myrepo\n url: https://github.com/devtron/myrepo/\n username:\n key: \"username\"\n name: \"myuser\"\n password:\n key: \"password\"\n name: \"mypass\"", + "dex.config": "", } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := impl.ValidateChartDetails(tt.args.FileName, tt.args.chartVersion) - if (err != nil) != tt.wantErr { - t.Errorf("ValidateChartDetails() error = %v, wantErr %v", err, tt.wantErr) - return - } - if got != tt.want { - t.Errorf("ValidateChartDetails() got = %v, want %v", got, tt.want) - } - }) + + // execute + newData, _ := s.updateRepoData(oldData, req) + newYaml := newData["repositories"] + var newRepositories []*AcdConfigMapRepositoriesDto + repoByte, _ := yaml.YAMLToJSON([]byte(newYaml)) + json.Unmarshal(repoByte, &newRepositories) + + // verify + assert.Equal(t, "https://github.com/devtron/myrepo2/", newRepositories[1].Url) + assert.Equal(t, "myuser", newRepositories[1].UsernameSecret.Name) + assert.Equal(t, "mypass", newRepositories[1].PasswordSecret.Name) +} + +func TestUpdateRepository_ExistingRepository(t *testing.T) { + // setup + sugaredLogger, err := util.NewSugardLogger() + assert.Nil(t, err) + s := ChartRepositoryServiceImpl{logger: sugaredLogger} + req := &ChartRepoDto{ + Name: "myrepo", + Url: "https://github.com/devtron/myrepo2/", + AuthMode: repository.AUTH_MODE_USERNAME_PASSWORD, + UserName: "myuser2", + Password: "mypass2", } + oldData := map[string]string{ + "helm.repositories": "null\n", + "repositories": "- name: myrepo\n url: https://github.com/devtron/myrepo/\n username:\n key: \"username\"\n name: \"myuser\"\n password:\n key: \"password\"\n name: \"mypass\"", + "dex.config": "", + } + + // execute + newData, _ := s.updateRepoData(oldData, req) + newYaml := newData["repositories"] + var newRepositories []*AcdConfigMapRepositoriesDto + repoByte, _ := yaml.YAMLToJSON([]byte(newYaml)) + json.Unmarshal(repoByte, &newRepositories) + + // verify + assert.Equal(t, "https://github.com/devtron/myrepo2/", newRepositories[0].Url) + assert.Equal(t, "myuser2", newRepositories[0].UsernameSecret.Name) + assert.Equal(t, "mypass2", newRepositories[0].PasswordSecret.Name) } diff --git a/pkg/delete/DeleteService.go b/pkg/delete/DeleteService.go index 43abf7fd0a..d91db95eca 100644 --- a/pkg/delete/DeleteService.go +++ b/pkg/delete/DeleteService.go @@ -78,7 +78,7 @@ func (impl DeleteServiceImpl) DeleteChartRepo(deleteRequest *chartRepo.ChartRepo impl.logger.Errorw("err in deleting repo, found charts deployed using this repo", "deleteRequest", deployedCharts) return fmt.Errorf("cannot delete repo, found charts deployed in this repo") } - err = impl.chartRepositoryService.DeleteChartRepo(deleteRequest) + err = impl.chartRepositoryService.DeleteChartRepoFromArgocdCM(deleteRequest) if err != nil { impl.logger.Errorw("error in deleting chart repo", "err", err, "deleteRequest", deleteRequest) return err diff --git a/pkg/delete/DeleteServiceExtended.go b/pkg/delete/DeleteServiceExtended.go index 0ffc6987aa..d9f08be5c2 100644 --- a/pkg/delete/DeleteServiceExtended.go +++ b/pkg/delete/DeleteServiceExtended.go @@ -127,7 +127,7 @@ func (impl DeleteServiceExtendedImpl) DeleteChartRepo(deleteRequest *chartRepo.C impl.logger.Errorw("err in deleting repo, found charts deployed using this repo", "deleteRequest", deployedCharts) return fmt.Errorf("cannot delete repo, found charts deployed in this repo") } - err = impl.chartRepositoryService.DeleteChartRepo(deleteRequest) + err = impl.chartRepositoryService.DeleteChartRepoFromArgocdCM(deleteRequest) if err != nil { impl.logger.Errorw("error in deleting chart repo", "err", err, "deleteRequest", deleteRequest) return err