Merge pull request #6887 from devtron-labs/fix/cluster-update-validation

fix: enhance validation and error handling in cluster update process

Author: Ash-exp

client/argocdServer/repoCredsK8sClient/repoCreds.go

@@ -105,7 +105,8 @@ func (impl *RepositoryCredsK8sClientImpl) createOrUpdateArgoRepoSecret(argoK8sCo
 		argoK8sConfig.AcdNamespace,
 		uniqueSecretName,
 		secretLabel,
-		secretData)
+		secretData,
+		nil)
 	if err != nil {
 		impl.logger.Errorw("error in create/update argocd secret by name", "secretName", uniqueSecretName, "err", err)
 		return err

cmd/external-app/wire_gen.go

@@ -19,8 +19,10 @@ package cluster
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log"
+	"net/http"
 	"net/url"
 	"sync"
 	"time"
@@ -198,8 +200,8 @@ func (impl *ClusterServiceImpl) Save(parent context.Context, bean *bean.ClusterB
 	}
 
 	existingModel, err := impl.clusterRepository.FindOne(bean.ClusterName)
-	if err != nil && err != pg.ErrNoRows {
-		impl.logger.Error(err)
+	if err != nil && !errors.Is(err, pg.ErrNoRows) {
+		impl.logger.Errorw("error in finding cluster for create", "clusterName", bean.ClusterName, "err", err)
 		return nil, err
 	}
 	if existingModel.Id > 0 {
@@ -376,17 +378,23 @@ func (impl *ClusterServiceImpl) FindByIdsWithoutConfig(ids []int) ([]*bean.Clust
 func (impl *ClusterServiceImpl) Update(ctx context.Context, bean *bean.ClusterBean, userId int32) (*bean.ClusterBean, error) {
 	model, err := impl.clusterRepository.FindById(bean.Id)
 	if err != nil {
-		impl.logger.Error(err)
+		impl.logger.Errorw("error in fetching cluster", "clusterId", bean.Id, "err", err)
 		return nil, err
 	}
 	existingModel, err := impl.clusterRepository.FindOne(bean.ClusterName)
-	if err != nil && err != pg.ErrNoRows {
-		impl.logger.Error(err)
+	if err != nil && !errors.Is(err, pg.ErrNoRows) {
+		impl.logger.Errorw("error in fetching cluster", "clusterName", bean.ClusterName, "err", err)
 		return nil, err
 	}
 	if existingModel.Id > 0 && model.Id != existingModel.Id {
 		impl.logger.Errorw("error on fetching cluster, duplicate", "name", bean.ClusterName)
-		return nil, fmt.Errorf("cluster already exists")
+		errMsg := fmt.Sprintf("cluster already exists with name %q", bean.ClusterName)
+		return nil, util.NewApiError(http.StatusBadRequest, errMsg, errMsg)
+	}
+	if bean.ClusterName != model.ClusterName {
+		impl.logger.Errorw("cluster name cannot be changed", "oldName", model.ClusterName, "newName", bean.ClusterName)
+		errMsg := fmt.Sprintf("cluster name cannot be changed")
+		return nil, util.NewApiError(http.StatusBadRequest, errMsg, errMsg)
 	}
 
 	// check whether config modified or not, if yes create informer with updated config
@@ -417,7 +425,8 @@ func (impl *ClusterServiceImpl) Update(ctx context.Context, bean *bean.ClusterBe
 	if bean.ServerUrl != model.ServerUrl || bean.InsecureSkipTLSVerify != model.InsecureSkipTlsVerify || dbConfigBearerToken != requestConfigBearerToken || dbConfigTlsKey != requestConfigTlsKey || dbConfigCertData != requestConfigCertData || dbConfigCAData != requestConfigCAData {
 		if bean.ClusterName == clusterBean.DefaultCluster {
 			impl.logger.Errorw("default_cluster is reserved by the system and cannot be updated, default_cluster", "name", bean.ClusterName)
-			return nil, fmt.Errorf("default_cluster is reserved by the system and cannot be updated")
+			errMsg := fmt.Sprintf("default_cluster is reserved by the system and cannot be updated")
+			return nil, util.NewApiError(http.StatusBadRequest, errMsg, errMsg)
 		}
 		bean.HasConfigOrUrlChanged = true
 		//validating config