fix: argocd-cm empty data label nil pointer fix (#3443)

* config map data nil pointer fix

* check for nil config map

* creating secret for public chart repo

* delete chart repo based on it's presence in argocd-cm

* code for unique chart name while adding a chart repo

* secret creation doesn't support uppercase letters hence adding checks

* minor change

* minor fix

Author: prakash100198

pkg/chartRepo/ChartRepositoryService.go

@@ -20,6 +20,7 @@ package chartRepo
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"github.com/devtron-labs/devtron/internal/sql/repository"
 	"github.com/devtron-labs/devtron/internal/util"
@@ -99,22 +100,42 @@ func NewChartRepositoryServiceImpl(logger *zap.SugaredLogger, repoRepository cha
 }
 
 // Private helm charts credentials are saved as secrets
-func (impl *ChartRepositoryServiceImpl) CreateSecretDataForPrivateHelmChart(request *ChartRepoDto) (secretData map[string]string) {
+func (impl *ChartRepositoryServiceImpl) CreateSecretDataForHelmChart(request *ChartRepoDto, isPrivateChart bool) (secretData map[string]string) {
 	secretData = make(map[string]string)
-	secretData[NAME] = fmt.Sprintf("%s-%s", request.Name, uuid.New().String()) // making repo name unique so that "helm repp add" command in argo-repo-server doesn't give error
-	secretData[USERNAME] = request.UserName
-	secretData[PASSWORD] = request.Password
+	secretData[NAME] = fmt.Sprintf("%s-%s", request.Name, uuid.New().String()) // making repo name unique so that "helm repo add" command in argo-repo-server doesn't give error
 	secretData[TYPE] = HELM
 	secretData[URL] = request.Url
-	isInsecureConnection := "true"
-	if !request.AllowInsecureConnection {
-		isInsecureConnection = "false"
+	if isPrivateChart {
+		secretData[USERNAME] = request.UserName
+		secretData[PASSWORD] = request.Password
+		isInsecureConnection := "true"
+		if !request.AllowInsecureConnection {
+			isInsecureConnection = "false"
+		}
+		secretData[INSECRUE] = isInsecureConnection
 	}
-	secretData[INSECRUE] = isInsecureConnection
+
 	return secretData
 }
 
 func (impl *ChartRepositoryServiceImpl) CreateChartRepo(request *ChartRepoDto) (*chartRepoRepository.ChartRepo, error) {
+	//metadata.name label in Secret doesn't support uppercase hence returning if user enters uppercase letters
+	if strings.ToLower(request.Name) != request.Name {
+		return nil, errors.New("invalid repo name: please use lowercase")
+	}
+	allChartsRepos, err := impl.repoRepository.FindAll()
+	if err != nil {
+		impl.logger.Errorw("error in getting list of all chart repos", "err", err)
+		return nil, err
+	}
+	if len(allChartsRepos) == 0 {
+		return nil, nil
+	}
+	for _, chart := range allChartsRepos {
+		if chart.Name == request.Name {
+			return nil, errors.New("repo with chart name already exists")
+		}
+	}
 	dbConnection := impl.repoRepository.GetConnection()
 	tx, err := dbConnection.Begin()
 	if err != nil {
@@ -166,24 +187,10 @@ func (impl *ChartRepositoryServiceImpl) CreateChartRepo(request *ChartRepoDto) (
 	for !updateSuccess && retryCount < 3 {
 		retryCount = retryCount + 1
 
-		if !isPrivateChart {
-			cm, err := impl.K8sUtil.GetConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, impl.aCDAuthConfig.ACDConfigMapName, client)
-			if err != nil {
-				return nil, err
-			}
-			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)
-		} else {
-			secretLabel := make(map[string]string)
-			secretLabel[LABEL] = REPOSITORY
-			secretData := impl.CreateSecretDataForPrivateHelmChart(request)
-			_, err = impl.K8sUtil.CreateSecret(impl.aCDAuthConfig.ACDConfigMapNamespace, nil, chartRepo.Name, "", client, secretLabel, secretData)
-		}
+		secretLabel := make(map[string]string)
+		secretLabel[LABEL] = REPOSITORY
+		secretData := impl.CreateSecretDataForHelmChart(request, isPrivateChart)
+		_, err = impl.K8sUtil.CreateSecret(impl.aCDAuthConfig.ACDConfigMapNamespace, nil, chartRepo.Name, "", client, secretLabel, secretData)
 		if err != nil {
 			continue
 		}
@@ -215,7 +222,11 @@ func (impl *ChartRepositoryServiceImpl) UpdateData(request *ChartRepoDto) (*char
 		return nil, err
 	}
 	previousName := chartRepo.Name
-
+	previousUrl := chartRepo.Url
+	//metadata.name label in Secret doesn't support uppercase hence returning if user enters uppercase letters in repo name
+	if request.Name != previousName && strings.ToLower(request.Name) != request.Name {
+		return nil, errors.New("invalid repo name: please use lowercase")
+	}
 	chartRepo.Url = request.Url
 	chartRepo.Name = request.Name
 	chartRepo.AuthMode = request.AuthMode
@@ -257,31 +268,55 @@ func (impl *ChartRepositoryServiceImpl) UpdateData(request *ChartRepoDto) (*char
 	for !updateSuccess && retryCount < 3 {
 		retryCount = retryCount + 1
 
-		if !isPrivateChart {
-			cm, err := impl.K8sUtil.GetConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, impl.aCDAuthConfig.ACDConfigMapName, client)
+		var isFoundInArgoCdCm bool
+		cm, err := impl.K8sUtil.GetConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, impl.aCDAuthConfig.ACDConfigMapName, client)
+		if err != nil {
+			return nil, err
+		}
+		var repositories []*AcdConfigMapRepositoriesDto
+		if cm != nil && cm.Data != nil {
+			repoStr := cm.Data["repositories"]
+			repoByte, err := yaml.YAMLToJSON([]byte(repoStr))
 			if err != nil {
+				impl.logger.Errorw("error in json patch", "err", err)
 				return nil, err
 			}
-			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
+			err = json.Unmarshal(repoByte, &repositories)
 			if err != nil {
-				impl.logger.Warnw(" config map update failed", "err", err)
-				continue
-			}
-			if previousName != request.Name {
-				data, err = impl.removeRepoData(cm.Data, previousName)
+				impl.logger.Errorw("error in unmarshal", "err", err)
+				return nil, err
 			}
-			if err != nil {
-				impl.logger.Warnw(" config map update failed", "err", err)
-				continue
+			for _, repo := range repositories {
+				if repo.Name == previousName && repo.Url == previousUrl {
+					//chart repo is present in argocd-cm
+					isFoundInArgoCdCm = true
+					break
+				}
 			}
+		}
 
+		if isFoundInArgoCdCm {
+			var data map[string]string
+			// if the repo name has been updated then, create a new repo
+			if cm != nil && cm.Data != nil {
+				data, err = impl.updateRepoData(cm.Data, request)
+				// if the repo name has been updated then, delete the previous repo
+				if err != nil {
+					impl.logger.Warnw(" config map update failed", "err", err)
+					continue
+				}
+				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)
 		} else {
-			secretData := impl.CreateSecretDataForPrivateHelmChart(request)
+			secretData := impl.CreateSecretDataForHelmChart(request, isPrivateChart)
 			secret, err := impl.K8sUtil.GetSecret(impl.aCDAuthConfig.ACDConfigMapNamespace, previousName, client)
 			if err != nil {
 				impl.logger.Errorw("error in fetching secret", "err", err)
@@ -380,23 +415,45 @@ func (impl *ChartRepositoryServiceImpl) DeleteChartRepo(request *ChartRepoDto) e
 	retryCount := 0
 	//request.Url = ""
 
-	isPrivateChart := false
-	if len(chartRepo.UserName) > 0 && len(chartRepo.Password) > 0 {
-		isPrivateChart = true
-	}
-
 	for !updateSuccess && retryCount < 3 {
 		retryCount = retryCount + 1
 
-		if !isPrivateChart {
-			cm, err := impl.K8sUtil.GetConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, impl.aCDAuthConfig.ACDConfigMapName, client)
+		var isFoundInArgoCdCm bool
+		cm, err := impl.K8sUtil.GetConfigMap(impl.aCDAuthConfig.ACDConfigMapNamespace, impl.aCDAuthConfig.ACDConfigMapName, client)
+		if err != nil {
+			return err
+		}
+		var repositories []*AcdConfigMapRepositoriesDto
+		if cm != nil && cm.Data != nil {
+			repoStr := cm.Data["repositories"]
+			repoByte, err := yaml.YAMLToJSON([]byte(repoStr))
 			if err != nil {
+				impl.logger.Errorw("error in json patch", "err", err)
 				return err
 			}
-			data, err := impl.removeRepoData(cm.Data, request.Name)
+			err = json.Unmarshal(repoByte, &repositories)
 			if err != nil {
-				impl.logger.Warnw(" config map update failed", "err", err)
-				continue
+				impl.logger.Errorw("error in unmarshal", "err", err)
+				return err
+			}
+			for _, repo := range repositories {
+				if repo.Name == request.Name && repo.Url == request.Url {
+					//chart repo is present in argocd-cm
+					isFoundInArgoCdCm = true
+					break
+				}
+			}
+		}
+
+		if isFoundInArgoCdCm {
+			var data map[string]string
+
+			if cm != nil && cm.Data != nil {
+				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)
@@ -572,6 +629,10 @@ func (impl *ChartRepositoryServiceImpl) ValidateAndCreateChartRepo(request *Char
 	if err != nil {
 		return nil, err, validationResult
 	}
+	if chartRepo == nil && err == nil {
+		//case when no entry for chart in chart_repo table and no need to trigger chart sync
+		return nil, nil, validationResult
+	}
 
 	// Trigger chart sync job, ignore error
 	err = impl.TriggerChartSyncManual()