Merge pull request #6638 from devtron-labs/develop-main-sync-3jun-1

sync: Develop main sync

Author: kartik-579

internal/sql/repository/pipelineConfig/CiTemplateOverrideRepository.go

@@ -21,6 +21,7 @@ import (
 	repository2 "github.com/devtron-labs/devtron/pkg/build/git/gitMaterial/repository"
 	"github.com/devtron-labs/devtron/pkg/sql"
 	"github.com/go-pg/pg"
+	"github.com/go-pg/pg/orm"
 	"go.uber.org/zap"
 )
 
@@ -47,6 +48,7 @@ type CiTemplateOverrideRepository interface {
 	FindByAppId(appId int) ([]*CiTemplateOverride, error)
 	FindByCiPipelineIds(ciPipelineIds []int) ([]*CiTemplateOverride, error)
 	FindByCiPipelineId(ciPipelineId int) (*CiTemplateOverride, error)
+	FindIfTemplateOverrideExistsByCiPipelineIdsAndGitMaterialId(ciPipelineIds []int, gitMaterialId int) (bool, error)
 }
 
 type CiTemplateOverrideRepositoryImpl struct {
@@ -125,3 +127,22 @@ func (repo *CiTemplateOverrideRepositoryImpl) FindByCiPipelineId(ciPipelineId in
 	}
 	return ciTemplateOverride, nil
 }
+
+func (repo *CiTemplateOverrideRepositoryImpl) FindIfTemplateOverrideExistsByCiPipelineIdsAndGitMaterialId(ciPipelineIds []int, gitMaterialId int) (bool, error) {
+	if len(ciPipelineIds) == 0 {
+		return false, nil
+	}
+	count, err := repo.dbConnection.Model((*CiTemplateOverride)(nil)).
+		Where("ci_pipeline_id in (?)", pg.In(ciPipelineIds)).
+		WhereGroup(func(q *orm.Query) (*orm.Query, error) {
+			return q.Where("git_material_id = ?", gitMaterialId).WhereOr("build_context_git_material_id = ?", gitMaterialId), nil
+		}).
+		Where("active = ?", true).
+		Count()
+	if err != nil {
+		repo.logger.Errorw("error in checking if template override exists", "ciPipelineIds", ciPipelineIds, "gitMaterialId", gitMaterialId, "err", err)
+		return false, err
+	}
+	return count > 0, nil
+
+}

internal/sql/repository/pipelineConfig/PipelineRepository.go

@@ -118,7 +118,7 @@ type PipelineRepository interface {
 	FindActiveByInFilter(envId int, appIdIncludes []int) (pipelines []*Pipeline, err error)
 	FindActivePipelineAppIdsByInFilter(envId int, appIdIncludes []int) ([]int, error)
 	FindActiveByNotFilter(envId int, appIdExcludes []int) (pipelines []*Pipeline, err error)
-	FindAllPipelinesByChartsOverrideAndAppIdAndChartId(chartOverridden bool, appId int, chartId int) (pipelines []*Pipeline, err error)
+	FindAllPipelinesWithoutOverriddenCharts(appId int) (pipelineIds []int, err error)
 	FindActiveByAppIdAndPipelineId(appId int, pipelineId int) ([]*Pipeline, error)
 	FindActiveByAppIdAndEnvId(appId int, envId int) (*Pipeline, error)
 	SetDeploymentAppCreatedInPipeline(deploymentAppCreated bool, pipelineId int, userId int32) error
@@ -578,19 +578,16 @@ func (impl *PipelineRepositoryImpl) FindActiveByNotFilter(envId int, appIdExclud
 	return pipelines, err
 }
 
-func (impl *PipelineRepositoryImpl) FindAllPipelinesByChartsOverrideAndAppIdAndChartId(hasConfigOverridden bool, appId int, chartId int) (pipelines []*Pipeline, err error) {
-	err = impl.dbConnection.Model(&pipelines).
-		Column("pipeline.*").
-		Join("inner join charts on pipeline.app_id = charts.app_id").
-		Join("inner join chart_env_config_override ceco on charts.id = ceco.chart_id").
-		Where("pipeline.app_id = ?", appId).
-		Where("charts.id = ?", chartId).
-		Where("ceco.is_override = ?", hasConfigOverridden).
-		Where("pipeline.deleted = ?", false).
-		Where("ceco.active = ?", true).
-		Where("charts.active = ?", true).
-		Select()
-	return pipelines, err
+func (impl *PipelineRepositoryImpl) FindAllPipelinesWithoutOverriddenCharts(appId int) (pipelineIds []int, err error) {
+	err = impl.dbConnection.Model().Table("pipeline").Column("pipeline.id").
+		Where("pipeline.deleted = ?", false).Where("pipeline.app_id = ?", appId).
+		Where(`pipeline.environment_id NOT IN (
+				SELECT ceco.target_environment FROM chart_env_config_override ceco 
+				INNER JOIN charts ON charts.id = ceco.chart_id
+				WHERE charts.app_id = ? AND charts.active = ? AND ceco.is_override = ?
+				AND ceco.active = ? AND ceco.latest = ?)`, appId, true, true, true, true).
+		Select(&pipelineIds)
+	return pipelineIds, err
 }
 
 func (impl *PipelineRepositoryImpl) FindActiveByAppIdAndPipelineId(appId int, pipelineId int) ([]*Pipeline, error) {

pkg/build/pipeline/CiTemplateReadService.go

@@ -18,6 +18,7 @@ type CiTemplateReadService interface {
 	GetAppliedDockerConfigForCiPipeline(ciPipelineId, appId int, isOverridden bool) (*types.DockerArtifactStoreBean, error)
 	GetBaseDockerConfigForCiPipeline(appId int) (*types.DockerArtifactStoreBean, error)
 	FindByAppIds(appIds []int) (map[int]*bean.CiTemplateBean, error)
+	CheckIfTemplateOverrideExists(ciPipelineIds []int, gitMaterialId int) (bool, error)
 }
 
 type CiTemplateReadServiceImpl struct {
@@ -181,3 +182,7 @@ func (impl *CiTemplateReadServiceImpl) FindByAppIds(appIds []int) (map[int]*bean
 	}
 	return ciTemplateMap, nil
 }
+
+func (impl *CiTemplateReadServiceImpl) CheckIfTemplateOverrideExists(ciPipelineIds []int, gitMaterialId int) (bool, error) {
+	return impl.CiTemplateOverrideRepository.FindIfTemplateOverrideExistsByCiPipelineIdsAndGitMaterialId(ciPipelineIds, gitMaterialId)
+}

pkg/deployment/manifest/deploymentTemplate/DeploymentTemplateHistoryService.go

@@ -70,11 +70,16 @@ func NewDeploymentTemplateHistoryServiceImpl(logger *zap.SugaredLogger, deployme
 
 func (impl DeploymentTemplateHistoryServiceImpl) CreateDeploymentTemplateHistoryFromGlobalTemplate(chart *chartRepoRepository.Chart, tx *pg.Tx, IsAppMetricsEnabled bool) (err error) {
 	//getting all pipelines without overridden charts
-	pipelines, err := impl.pipelineRepository.FindAllPipelinesByChartsOverrideAndAppIdAndChartId(false, chart.AppId, chart.Id)
+	pipelineIds, err := impl.pipelineRepository.FindAllPipelinesWithoutOverriddenCharts(chart.AppId)
 	if err != nil && err != pg.ErrNoRows {
 		impl.logger.Errorw("err in getting pipelines, CreateDeploymentTemplateHistoryFromGlobalTemplate", "err", err, "chart", chart)
 		return err
 	}
+	/*
+		When creating base template entry, we also create entries for all pipeline whose env template is not overridden;
+		as the change in base template will also impact them. Earlier we used to create individual entries for all pipelines
+		but due to performance impact we are now saving the impacted pipelineIds in the base template entry itself (column pipeline_ids).
+	*/
 	chartRefDto, err := impl.chartRefService.FindById(chart.ChartRefId)
 	if err != nil {
 		impl.logger.Errorw("err in getting chartRef, CreateDeploymentTemplateHistoryFromGlobalTemplate", "err", err, "chart", chart)
@@ -89,6 +94,7 @@ func (impl DeploymentTemplateHistoryServiceImpl) CreateDeploymentTemplateHistory
 		TemplateName:            chartRefDto.Name,
 		TemplateVersion:         chartRefDto.Version,
 		IsAppMetricsEnabled:     IsAppMetricsEnabled,
+		PipelineIds:             pipelineIds,
 		AuditLog: sql.AuditLog{
 			CreatedOn: chart.CreatedOn,
 			CreatedBy: chart.CreatedBy,
@@ -106,34 +112,6 @@ func (impl DeploymentTemplateHistoryServiceImpl) CreateDeploymentTemplateHistory
 		impl.logger.Errorw("err in creating history entry for deployment template", "err", err, "history", historyModel)
 		return err
 	}
-	for _, pipeline := range pipelines {
-		historyModel := &repository.DeploymentTemplateHistory{
-			AppId:                   chart.AppId,
-			PipelineId:              pipeline.Id,
-			ImageDescriptorTemplate: chart.ImageDescriptorTemplate,
-			Template:                chart.GlobalOverride,
-			Deployed:                false,
-			TemplateName:            chartRefDto.Name,
-			TemplateVersion:         chartRefDto.Version,
-			IsAppMetricsEnabled:     IsAppMetricsEnabled,
-			AuditLog: sql.AuditLog{
-				CreatedOn: chart.CreatedOn,
-				CreatedBy: chart.CreatedBy,
-				UpdatedOn: chart.UpdatedOn,
-				UpdatedBy: chart.UpdatedBy,
-			},
-		}
-		//creating new entry
-		if tx != nil {
-			_, err = impl.deploymentTemplateHistoryRepository.CreateHistoryWithTxn(historyModel, tx)
-		} else {
-			_, err = impl.deploymentTemplateHistoryRepository.CreateHistory(historyModel)
-		}
-		if err != nil {
-			impl.logger.Errorw("err in creating history entry for deployment template", "err", err, "history", historyModel)
-			return err
-		}
-	}
 	return err
 }
 

pkg/pipeline/CiMaterialConfigService.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"github.com/devtron-labs/devtron/internal/sql/constants"
 	"github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig"
+	util2 "github.com/devtron-labs/devtron/internal/util"
 	"github.com/devtron-labs/devtron/pkg/bean"
 	"github.com/devtron-labs/devtron/pkg/build/git/gitMaterial/read"
 	"github.com/devtron-labs/devtron/pkg/build/git/gitMaterial/repository"
@@ -29,6 +30,7 @@ import (
 	"github.com/go-pg/pg"
 	"github.com/juju/errors"
 	"go.uber.org/zap"
+	"net/http"
 	"strings"
 	"time"
 )
@@ -123,6 +125,20 @@ func (impl *CiMaterialConfigServiceImpl) DeleteMaterial(request *bean.UpdateMate
 				return fmt.Errorf("cannot delete git material, is being used in docker config")
 			}
 		}
+		overriddenPipelineIds := make([]int, 0, len(pipelines))
+		for _, dbPipeline := range pipelines {
+			if dbPipeline.IsDockerConfigOverridden {
+				overriddenPipelineIds = append(overriddenPipelineIds, dbPipeline.Id)
+			}
+		}
+		exist, err := impl.ciTemplateService.CheckIfTemplateOverrideExists(overriddenPipelineIds, request.Material.Id)
+		if err != nil {
+			impl.logger.Errorw("error in checking if template override exists", "pipelineIds", overriddenPipelineIds, "gitMaterialId", request.Material.Id, "err", err)
+			return err
+		}
+		if exist {
+			return util2.GetApiErrorAdapter(http.StatusBadRequest, "400", "cannot delete git material, is being used in overridden ci template", "cannot delete git material, is being used in overridden ci template")
+		}
 	}
 	existingMaterial, err := impl.gitMaterialReadService.FindById(request.Material.Id)
 	if err != nil {

pkg/pipeline/history/repository/DeploymentTemplateHistoryRepository.go

@@ -57,6 +57,7 @@ type DeploymentTemplateHistory struct {
 	DeployedOn              time.Time `sql:"deployed_on"`
 	DeployedBy              int32     `sql:"deployed_by"`
 	MergeStrategy           string    `sql:"merge_strategy"`
+	PipelineIds             []int     `sql:"pipeline_ids,array"`
 	sql.AuditLog
 	//getting below data from cd_workflow_runner and users join
 	DeploymentStatus  string `sql:"-"`

scripts/sql/33403601_alter_deployment_template_history.down.sql

@@ -0,0 +1 @@
+-- not dropping the added column as it is used for audit and data loss can be impactful. The old code can run with the column added and should not cause any issue.

scripts/sql/33403601_alter_deployment_template_history.up.sql

@@ -0,0 +1,2 @@
+ALTER TABLE deployment_template_history
+    ADD COLUMN IF NOT EXISTS pipeline_ids integer[];