fix: switch ci fixes (#5358)

gireesh-naidu · web-flow · commit 5c20f35048d6 · 2024-07-12T17:44:53.000+05:30
* fix: switch ci fixes

* chore: review refactoring

* chore: refactoring

* fix: add missing validations in switching to external ci

* fix: ci pipeline id is not being updated in cd pipeline if switching from external ci to other ci types

* fix: update ci pipeline id in all the cd workflows that exists in the given workflow

* fix: checkIfNsExistsForEnvIds , empty envIds check fix
diff --git a/internal/sql/repository/appWorkflow/AppWorkflowRepository.go b/internal/sql/repository/appWorkflow/AppWorkflowRepository.go
@@ -44,6 +44,7 @@ type AppWorkflowRepository interface {
 	FindWFAllMappingByWorkflowId(workflowId int) ([]*AppWorkflowMapping, error)
 	FindWFCIMappingByCIPipelineId(ciPipelineId int) ([]*AppWorkflowMapping, error)
 	FindWFCDMappingByCIPipelineId(ciPipelineId int) ([]*AppWorkflowMapping, error)
+	FindWFCDMappingsByWorkflowId(appWorkflowId int) ([]*AppWorkflowMapping, error)
 	FindWFCDMappingByCDPipelineId(cdPipelineId int) (*AppWorkflowMapping, error)
 	GetParentDetailsByPipelineId(pipelineId int) (*AppWorkflowMapping, error)
 	DeleteAppWorkflowMapping(appWorkflow *AppWorkflowMapping, tx *pg.Tx) error
@@ -273,6 +274,17 @@ func (impl AppWorkflowRepositoryImpl) FindWFCIMappingByCIPipelineId(ciPipelineId
 	return appWorkflowsMapping, err
 }
 
+func (impl AppWorkflowRepositoryImpl) FindWFCDMappingsByWorkflowId(appWorkflowId int) ([]*AppWorkflowMapping, error) {
+	var appWorkflowsMapping []*AppWorkflowMapping
+
+	err := impl.dbConnection.Model(&appWorkflowsMapping).
+		Where("app_workflow_id = ?", appWorkflowId).
+		Where("type = ?", CDPIPELINE).
+		Where("active = ?", true).
+		Select()
+	return appWorkflowsMapping, err
+}
+
 func (impl AppWorkflowRepositoryImpl) FindWFCDMappingByCIPipelineId(ciPipelineId int) ([]*AppWorkflowMapping, error) {
 	var appWorkflowsMapping []*AppWorkflowMapping
 
diff --git a/internal/sql/repository/pipelineConfig/PipelineRepository.go b/internal/sql/repository/pipelineConfig/PipelineRepository.go
@@ -122,6 +122,7 @@ type PipelineRepository interface {
 	FindAppAndEnvironmentAndProjectByPipelineIds(pipelineIds []int) (pipelines []*Pipeline, err error)
 	FilterDeploymentDeleteRequestedPipelineIds(cdPipelineIds []int) (map[int]bool, error)
 	FindDeploymentTypeByPipelineIds(cdPipelineIds []int) (map[int]DeploymentObject, error)
+	UpdateCiPipelineId(tx *pg.Tx, pipelineIds []int, ciPipelineId int) error
 	UpdateOldCiPipelineIdToNewCiPipelineId(tx *pg.Tx, oldCiPipelineId, newCiPipelineId int) error
 	// FindWithEnvironmentByCiIds Possibility of duplicate environment names when filtered by unique pipeline ids
 	FindWithEnvironmentByCiIds(ctx context.Context, cIPipelineIds []int) ([]*Pipeline, error)
@@ -770,6 +771,17 @@ func (impl PipelineRepositoryImpl) UpdateOldCiPipelineIdToNewCiPipelineId(tx *pg
 		Where("deleted = ?", false).Update()
 	return err
 }
+
+func (impl PipelineRepositoryImpl) UpdateCiPipelineId(tx *pg.Tx, pipelineIds []int, ciPipelineId int) error {
+	if len(pipelineIds) == 0 {
+		return nil
+	}
+	_, err := tx.Model((*Pipeline)(nil)).Set("ci_pipeline_id = ?", ciPipelineId).
+		Where("id IN (?) ", pg.In(pipelineIds)).
+		Where("deleted = ?", false).Update()
+	return err
+}
+
 func (impl PipelineRepositoryImpl) FindWithEnvironmentByCiIds(ctx context.Context, cIPipelineIds []int) ([]*Pipeline, error) {
 	_, span := otel.Tracer("orchestrator").Start(ctx, "FindWithEnvironmentByCiIds")
 	defer span.End()
diff --git a/pkg/bean/app.go b/pkg/bean/app.go
@@ -141,6 +141,10 @@ type CiPipeline struct {
 	EnableCustomTag          bool                     `json:"enableCustomTag"`
 }
 
+func (ciPipeline *CiPipeline) IsLinkedCi() bool {
+	return ciPipeline.IsExternal
+}
+
 type DockerConfigOverride struct {
 	DockerRegistry   string                         `json:"dockerRegistry,omitempty"`
 	DockerRepository string                         `json:"dockerRepository,omitempty"`
diff --git a/pkg/pipeline/BuildPipelineSwitchService.go b/pkg/pipeline/BuildPipelineSwitchService.go
@@ -19,6 +19,7 @@ package pipeline
 import (
 	"github.com/devtron-labs/devtron/internal/sql/repository/appWorkflow"
 	"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig"
+	"github.com/devtron-labs/devtron/internal/util"
 	"github.com/devtron-labs/devtron/pkg/bean"
 	"github.com/devtron-labs/devtron/pkg/pipeline/adapter"
 	pipelineConfigBean "github.com/devtron-labs/devtron/pkg/pipeline/bean/CiPipeline"
@@ -28,6 +29,7 @@ import (
 	"github.com/go-pg/pg"
 	"github.com/juju/errors"
 	"go.uber.org/zap"
+	"net/http"
 	"time"
 )
 
@@ -81,7 +83,12 @@ func NewBuildPipelineSwitchServiceImpl(logger *zap.SugaredLogger,
 
 func (impl *BuildPipelineSwitchServiceImpl) SwitchToExternalCi(tx *pg.Tx, appWorkflowMapping *appWorkflow.AppWorkflowMapping, switchFromCiPipelineId int, userId int32) error {
 
-	err := impl.deleteCiAndItsWorkflowMappings(tx, switchFromCiPipelineId, userId)
+	err := impl.validateSwitchPreConditions(switchFromCiPipelineId)
+	if err != nil {
+		return err
+	}
+
+	err = impl.deleteCiAndItsWorkflowMappings(tx, switchFromCiPipelineId, userId)
 	if err != nil {
 		impl.logger.Errorw("error in deleting old ci-pipeline and getting the appWorkflow mapping of that", "err", err, "userId", userId)
 		return err
@@ -97,7 +104,7 @@ func (impl *BuildPipelineSwitchServiceImpl) SwitchToExternalCi(tx *pg.Tx, appWor
 		return err
 	}
 
-	//setting new ci_pipeline_id to 0 because we dont store ci_pipeline_id if the ci_pipeline is external/webhook type.
+	// setting new ci_pipeline_id to 0 because we dont store ci_pipeline_id if the ci_pipeline is external/webhook type.
 	err = impl.pipelineRepository.UpdateOldCiPipelineIdToNewCiPipelineId(tx, switchFromCiPipelineId, 0)
 	if err != nil {
 		impl.logger.Errorw("error in updating pipelines ci_pipeline_ids with new ci_pipelineId", "oldCiPipelineId", switchFromCiPipelineId)
@@ -130,11 +137,18 @@ func (impl *BuildPipelineSwitchServiceImpl) SwitchToCiPipelineExceptExternal(req
 		return nil, err
 	}
 
-	//delete old pipeline and it's appworkflow mapping
+	// delete old pipeline and it's appworkflow mapping
 	return impl.createNewPipelineAndReplaceOldPipelineLinks(request.CiPipeline, ciConfig, switchFromPipelineId, switchFromType, request.UserId)
 }
 
 func (impl *BuildPipelineSwitchServiceImpl) createNewPipelineAndReplaceOldPipelineLinks(ciPipelineReq *bean.CiPipeline, ciConfig *bean.CiConfigRequest, switchFromPipelineId int, switchFromType pipelineConfigBean.PipelineType, userId int32) (*bean.CiConfigRequest, error) {
+
+	isSelfLinkedCiPipeline := switchFromType != pipelineConfigBean.EXTERNAL && ciPipelineReq.IsLinkedCi() && ciPipelineReq.ParentCiPipeline == switchFromPipelineId
+	if isSelfLinkedCiPipeline {
+		errMsg := "cannot create linked ci pipeline from the same source"
+		return nil, util.NewApiError().WithInternalMessage(errMsg).WithUserMessage(errMsg).WithHttpStatusCode(http.StatusBadRequest)
+	}
+
 	tx, err := impl.ciPipelineRepository.StartTx()
 	if err != nil {
 		impl.logger.Errorw("error in starting transaction", "switchFromPipelineId", switchFromPipelineId, "switchFromType", switchFromType, "userId", userId, "err", err)
@@ -161,12 +175,21 @@ func (impl *BuildPipelineSwitchServiceImpl) createNewPipelineAndReplaceOldPipeli
 		return nil, err
 	}
 
-	//we don't store ci-pipeline-id in pipeline table for external ci's
-	if switchFromPipelineId > 0 && switchFromType != pipelineConfigBean.EXTERNAL {
-		// ciPipeline id is being set in res object in the addpipelineToTemplate method.
-		err = impl.pipelineRepository.UpdateOldCiPipelineIdToNewCiPipelineId(tx, switchFromPipelineId, res.CiPipelines[0].Id)
+	if switchFromPipelineId > 0 {
+		// get all the cd workflow mappings whose parent component is our old pipeline
+		cdwfmappings, err := impl.appWorkflowRepository.FindWFCDMappingsByWorkflowId(oldAppWorkflowMapping.AppWorkflowId)
+		if err != nil {
+			impl.logger.Errorw("error in finding parent cd workflowMappings using parent component details", "parentComponentType", oldAppWorkflowMapping.Type, "parentComponentId", oldAppWorkflowMapping.ComponentId, "err", err)
+			return nil, err
+		}
+		pipelineIds := make([]int, 0, len(cdwfmappings))
+		for _, cdwfMapping := range cdwfmappings {
+			pipelineIds = append(pipelineIds, cdwfMapping.ComponentId)
+		}
+
+		err = impl.pipelineRepository.UpdateCiPipelineId(tx, pipelineIds, res.CiPipelines[0].Id)
 		if err != nil {
-			impl.logger.Errorw("error in updating pipelines ci_pipeline_ids with new ci_pipelineId", "oldCiPipelineId", switchFromPipelineId, "newCiPipelineId", res.CiPipelines[0].Id)
+			impl.logger.Errorw("error in updating pipelines ci_pipeline_ids with new ci_pipelineId", "oldCiPipelineId", switchFromPipelineId, "newCiPipelineId", res.CiPipelines[0].Id, "err", err)
 			return nil, err
 		}
 	}
@@ -200,30 +223,10 @@ func (impl *BuildPipelineSwitchServiceImpl) validateCiPipelineSwitch(switchFromC
 	// we should not check the below logic for external_ci type as builds are not built in devtron and
 	// linked pipelines won't be there as per current external-ci-pipeline architecture
 	if switchFromCiPipelineId > 0 && switchFromType != pipelineConfigBean.EXTERNAL {
-		// old ci_pipeline should not contain any linked ci_pipelines.
-		linkedCiPipelines, err := impl.ciPipelineRepository.FindLinkedCiCount(switchFromCiPipelineId)
+		err := impl.validateSwitchPreConditions(switchFromCiPipelineId)
 		if err != nil {
-			return nil
-		}
-		if linkedCiPipelines > 0 {
-			return errors.New(string(cannotConvertIfLinkedCiFound))
-		}
-
-		// note: ideally we should have found any builds running on old ci_pipeline, if yes block this conversion with proper message.
-		// but checking only latest wf for now.
-		ciWorkflow, err := impl.ciWorkflowRepository.FindLastTriggeredWorkflow(switchFromCiPipelineId)
-		// no build is triggered case
-		if err == pg.ErrNoRows {
-			return nil
-		}
-		if err != nil {
-			impl.logger.Errorw("error in finding latest ciwokflow by ciPipelineId", "ciPipelineId", switchFromCiPipelineId)
 			return err
 		}
-
-		if ciWorkflow.InProgress() {
-			return errors.New(string(cannotConvertIfLatestWorkflowIsInNonTerminalState))
-		}
 	}
 
 	return nil
@@ -348,3 +351,34 @@ func (impl *BuildPipelineSwitchServiceImpl) saveHistoryOfOverriddenTemplate(ciPi
 func (impl *BuildPipelineSwitchServiceImpl) updateLinkedAppWorkflowMappings(tx *pg.Tx, oldAppWorkflowMapping *appWorkflow.AppWorkflowMapping, newAppWorkflowMapping *appWorkflow.AppWorkflowMapping) error {
 	return impl.appWorkflowRepository.UpdateParentComponentDetails(tx, oldAppWorkflowMapping.ComponentId, oldAppWorkflowMapping.Type, newAppWorkflowMapping.ComponentId, newAppWorkflowMapping.Type, nil)
 }
+
+func (impl *BuildPipelineSwitchServiceImpl) validateSwitchPreConditions(switchFromCiPipelineId int) error {
+
+	// old ci_pipeline should not contain any linked ci_pipelines.
+	linkedCiPipelines, err := impl.ciPipelineRepository.FindLinkedCiCount(switchFromCiPipelineId)
+	if err != nil {
+		impl.logger.Errorw("error in finding the linkedCi count for the pipeline", "ciPipelineId", switchFromCiPipelineId, "err", err)
+		return err
+	}
+	if linkedCiPipelines > 0 {
+		return errors.New(string(cannotConvertIfLinkedCiFound))
+	}
+
+	// note: ideally we should have found any builds running on old ci_pipeline, if yes block this conversion with proper message.
+	// but checking only latest wf for now.
+	ciWorkflow, err := impl.ciWorkflowRepository.FindLastTriggeredWorkflow(switchFromCiPipelineId)
+	// no build is triggered case
+	if err == pg.ErrNoRows {
+		return nil
+	}
+	if err != nil {
+		impl.logger.Errorw("error in finding latest ciwokflow by ciPipelineId", "ciPipelineId", switchFromCiPipelineId)
+		return err
+	}
+
+	if ciWorkflow.InProgress() {
+		return errors.New(string(cannotConvertIfLatestWorkflowIsInNonTerminalState))
+	}
+
+	return nil
+}
diff --git a/pkg/pipeline/DeploymentPipelineConfigService.go b/pkg/pipeline/DeploymentPipelineConfigService.go
@@ -385,18 +385,24 @@ func (impl *CdPipelineConfigServiceImpl) CreateCdPipelines(pipelineCreateRequest
 		}
 		pipeline.DeploymentAppType = overrideDeploymentType
 	}
+
 	err = impl.checkIfNsExistsForEnvIds(envIds)
 	if err != nil {
+		impl.logger.Errorw("error in checking existence of namespace for env's", "envIds", envIds, "err", err)
 		return nil, err
 	}
+
 	isGitOpsRequiredForCD := impl.IsGitOpsRequiredForCD(pipelineCreateRequest)
 	app, err := impl.appRepo.FindById(pipelineCreateRequest.AppId)
+
 	if err != nil {
 		impl.logger.Errorw("app not found", "err", err, "appId", pipelineCreateRequest.AppId)
 		return nil, err
 	}
+
 	_, err = impl.validateCDPipelineRequest(pipelineCreateRequest)
 	if err != nil {
+		impl.logger.Errorw("error in validating cd pipeline create request", "pipelineCreateRequest", pipelineCreateRequest, "err", err)
 		return nil, err
 	}
 
@@ -1765,18 +1771,20 @@ func (impl *CdPipelineConfigServiceImpl) createCdPipeline(ctx context.Context, a
 			}
 
 		}
-	}
-	// save custom tag data
-	err = impl.CDPipelineCustomTagDBOperations(pipeline)
-	if err != nil {
-		return pipelineId, err
-	}
 
-	if pipeline.IsDigestEnforcedForPipeline {
-		_, err = impl.imageDigestPolicyService.CreatePolicyForPipeline(tx, pipelineId, pipeline.Name, userId)
+		// save custom tag data
+		err = impl.CDPipelineCustomTagDBOperations(pipeline)
 		if err != nil {
 			return pipelineId, err
 		}
+
+		if pipeline.IsDigestEnforcedForPipeline {
+			_, err = impl.imageDigestPolicyService.CreatePolicyForPipeline(tx, pipelineId, pipeline.Name, userId)
+			if err != nil {
+				return pipelineId, err
+			}
+		}
+
 	}
 
 	err = tx.Commit()
@@ -2072,6 +2080,10 @@ func (impl *CdPipelineConfigServiceImpl) BulkDeleteCdPipelines(impactedPipelines
 
 }
 func (impl *CdPipelineConfigServiceImpl) checkIfNsExistsForEnvIds(envIds []*int) error {
+
+	if len(envIds) == 0 {
+		return nil
+	}
 	//fetching environments for the given environment Ids
 	environmentList, err := impl.environmentRepository.FindByIds(envIds)
 	if err != nil {
