Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 7, 2023
Merged

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

Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
10d246c
config map data nil pointer fix
prakash100198 May 29, 2023
85c284d
Merge branch 'main' into argocd-cm-data-nil-pointer-check-added
prakash100198 May 29, 2023
9628039
check for nil config map
prakash100198 May 30, 2023
3509c03
creating secret for public chart repo
prakash100198 Jun 5, 2023
3cd13c6
Merge branch 'main' into argocd-cm-data-nil-pointer-check-added
prakash100198 Jun 5, 2023
3a0b6f3
delete chart repo based on it's presence in argocd-cm
prakash100198 Jun 6, 2023
4e7dc98
code for unique chart name while adding a chart repo
prakash100198 Jun 6, 2023
cf43284
secret creation doesn't support uppercase letters hence adding checks
prakash100198 Jun 7, 2023
9315887
Merge branch 'main' into argocd-cm-data-nil-pointer-check-added
prakash100198 Jun 7, 2023
ae4b1f6
minor change
prakash100198 Jun 7, 2023
8402952
minor fix
prakash100198 Jun 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 113 additions & 52 deletions pkg/chartRepo/ChartRepositoryService.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand Down