feat:validation for pipeline Type (#4670)

* Bug(linked ci) : validation added * Bug(linked ci) : sql script added * Bug(linked ci) : condition check * Bug(linked ci) : error handling added * Bug(linked ci) : sql file name updated * Bug(linked ci) : validation added * Bug(linked ci) : message changed * story (version up) : sql file updated * story (version up) : comment added in sql file * Bug(linked ci) : validation changed * Bug(linked ci) : validation changed * Bug(same image scan detail) : Normal job flag added * Bug(linked ci) : normal job check removed * Bug(linked ci) : assets updated * Bug(linked ci) : normal job added * Bug(linked ci) : comment removed * Bug(linked ci) : sql file no. updated * code commented * bug(linked ci): sql file updated * bug(linked ci): validation added * all comments removed related to bug * comment added * logger updated * code review comment resolved * error message updated * main merge * sql file name changed * sql file no. updated
devtron-labs · May 15, 2024 · f87d7fd · f87d7fd
1 parent 211a1e4
commit f87d7fd
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 15 deletions.
diff --git a/internal/util/ErrorUtil.go b/internal/util/ErrorUtil.go
@@ -37,6 +37,33 @@ type ApiError struct {
 	UserDetailMessage string      `json:"userDetailMessage,omitempty"`
 }
 
+func NewApiError() *ApiError {
+	return &ApiError{}
+}
+
+func (e *ApiError) WithHttpStatusCode(httpStatusCode int) *ApiError {
+	e.HttpStatusCode = httpStatusCode
+	return e
+}
+
+func (e *ApiError) WithCode(code string) *ApiError {
+	e.Code = code
+	return e
+}
+func (e *ApiError) WithInternalMessage(InternalMessage string) *ApiError {
+	e.InternalMessage = InternalMessage
+	return e
+}
+func (e *ApiError) WithUserMessage(userMessage interface{}) *ApiError {
+	e.UserMessage = userMessage
+	return e
+}
+
+func (e *ApiError) WithUserDetailMessage(UserDetailMessage string) *ApiError {
+	e.UserDetailMessage = UserDetailMessage
+	return e
+}
+
 func (e *ApiError) Error() string {
 	return e.InternalMessage
 }

diff --git a/pkg/bean/app.go b/pkg/bean/app.go
@@ -191,7 +191,6 @@ type ExternalCiConfigRole struct {
 
 // -------------------
 type PatchAction int
-type PipelineType string
 
 const (
 	CREATE        PatchAction = iota

diff --git a/pkg/pipeline/BuildPipelineConfigService.go b/pkg/pipeline/BuildPipelineConfigService.go
@@ -280,7 +280,11 @@ func (impl *CiPipelineConfigServiceImpl) patchCiPipelineUpdateSource(baseCiConfi
 		impl.logger.Errorw("error in fetching pipeline", "id", modifiedCiPipeline.Id, "err", err)
 		return nil, err
 	}
-
+	if !modifiedCiPipeline.PipelineType.IsValidPipelineType() {
+		impl.logger.Debugw(" Invalid PipelineType", "PipelineType", modifiedCiPipeline.PipelineType)
+		errorMessage := fmt.Sprintf(CiPipeline.PIPELINE_TYPE_IS_NOT_VALID, modifiedCiPipeline.Name)
+		return nil, util.NewApiError().WithHttpStatusCode(http.StatusBadRequest).WithInternalMessage(errorMessage).WithUserMessage(errorMessage)
+	}
 	cannotUpdate := false
 	for _, material := range pipeline.CiPipelineMaterials {
 		if material.ScmId != "" {
@@ -1455,7 +1459,7 @@ func (impl *CiPipelineConfigServiceImpl) GetCiPipelineMin(appId int, envIds []in
 	var ciPipelineResp []*bean.CiPipelineMin
 	for _, pipeline := range pipelines {
 		parentCiPipeline := pipelineConfig.CiPipeline{}
-		pipelineType := CiPipeline.NORMAL
+		pipelineType := CiPipeline.CI_BUILD
 
 		if pipelineParentCiMap[pipeline.Id] != nil {
 			parentCiPipeline = *pipelineParentCiMap[pipeline.Id]

diff --git a/pkg/pipeline/CiCdPipelineOrchestrator.go b/pkg/pipeline/CiCdPipelineOrchestrator.go
@@ -28,6 +28,7 @@ import (
 	"fmt"
 	attributesBean "github.com/devtron-labs/devtron/pkg/attributes/bean"
 	"golang.org/x/exp/slices"
+	"net/http"
 	"path"
 	"regexp"
 	"strconv"
@@ -816,7 +817,6 @@ func (impl CiCdPipelineOrchestratorImpl) CreateCiConf(createRequest *bean.CiConf
 		}
 		// Rollback tx on error.
 		defer tx.Rollback()
-
 		ciPipelineObject := &pipelineConfig.CiPipeline{
 			AppId:                    createRequest.AppId,
 			IsManual:                 ciPipeline.IsManual,
@@ -2087,7 +2087,13 @@ func (impl CiCdPipelineOrchestratorImpl) CreateEcrRepo(dockerRepository, AWSRegi
 }
 
 func (impl CiCdPipelineOrchestratorImpl) AddPipelineToTemplate(createRequest *bean.CiConfigRequest, isSwitchCiPipelineRequest bool) (resp *bean.CiConfigRequest, err error) {
-
+	for _, ciPipeline := range createRequest.CiPipelines {
+		if !ciPipeline.PipelineType.IsValidPipelineType() {
+			impl.logger.Debugw(" Invalid PipelineType", "ciPipeline.PipelineType", ciPipeline.PipelineType)
+			errorMessage := fmt.Sprintf(CiPipeline.PIPELINE_TYPE_IS_NOT_VALID, ciPipeline.PipelineType)
+			return nil, util.NewApiError().WithHttpStatusCode(http.StatusBadRequest).WithInternalMessage(errorMessage).WithUserMessage(errorMessage)
+		}
+	}
 	if createRequest.AppWorkflowId == 0 {
 		// create workflow
 		wf := &appWorkflow.AppWorkflow{

diff --git a/pkg/pipeline/adapter/adapter.go b/pkg/pipeline/adapter/adapter.go
@@ -174,11 +174,9 @@ func IsLinkedCD(ci pipelineConfig.CiPipeline) bool {
 }
 
 // IsLinkedCI will return if the pipelineConfig.CiPipeline is a Linked CI
-// Currently there are inconsistent values present in PipelineType ("CI_EXTERNAL", "LINKED") 207_ci_external.up
-// TODO migrate the deprecated values and maintain a consistent PipelineType
 func IsLinkedCI(ci pipelineConfig.CiPipeline) bool {
 	return ci.ParentCiPipeline != 0 &&
-		(ci.PipelineType == string(CiPipeline.CI_EXTERNAL) || ci.PipelineType == string(CiPipeline.LINKED))
+		ci.PipelineType == string(CiPipeline.LINKED)
 }
 
 // IsCIJob will return if the pipelineConfig.CiPipeline is a CI JOB

diff --git a/pkg/pipeline/bean/CiPipeline/CiBuildConfig.go b/pkg/pipeline/bean/CiPipeline/CiBuildConfig.go
@@ -12,17 +12,16 @@ const Main = "main"
 const UniquePlaceHolderForAppName = "$etron"
 
 const PIPELINE_NAME_ALREADY_EXISTS_ERROR = "pipeline name already exist"
+const PIPELINE_TYPE_IS_NOT_VALID = "PipelineType is not valid  for pipeline %s"
 
 type PipelineType string
 
 const (
-	NORMAL PipelineType = "NORMAL"
-	LINKED PipelineType = "LINKED"
-	// CI_EXTERNAL field is been sent from the dashboard in CreateLinkedCI request and directly gets saved to Database without any validations
-	CI_EXTERNAL PipelineType = "CI_EXTERNAL" // Deprecated Enum: TODO fix the PipelineTypes in code and database
-	EXTERNAL    PipelineType = "EXTERNAL"
-	CI_JOB      PipelineType = "CI_JOB"
-	LINKED_CD   PipelineType = "LINKED_CD"
+	CI_BUILD  PipelineType = "CI_BUILD"
+	LINKED    PipelineType = "LINKED"
+	EXTERNAL  PipelineType = "EXTERNAL"
+	CI_JOB    PipelineType = "CI_JOB"
+	LINKED_CD PipelineType = "LINKED_CD"
 )
 
 type CiBuildConfigBean struct {
@@ -58,3 +57,12 @@ type BuildPackConfig struct {
 	Args            map[string]string `json:"args"`
 	ProjectPath     string            `json:"projectPath,omitempty"`
 }
+
+func (pType PipelineType) IsValidPipelineType() bool {
+	switch pType {
+	case CI_BUILD, LINKED, EXTERNAL, CI_JOB, LINKED_CD:
+		return true
+	default:
+		return false
+	}
+}
diff --git a/scripts/sql/246_pipeline_type.down.sql b/scripts/sql/246_pipeline_type.down.sql
@@ -0,0 +1 @@
+---- Not Required
diff --git a/scripts/sql/246_pipeline_type.up.sql b/scripts/sql/246_pipeline_type.up.sql
@@ -0,0 +1,6 @@
+UPDATE   "public"."ci_pipeline"  SET ci_pipeline_type='LINKED' WHERE ci_pipeline_type='CI_EXTERNAL';
+
+UPDATE "public"."ci_pipeline"
+SET ci_pipeline_type = 'CI_BUILD'
+    FROM app
+WHERE ci_pipeline.app_id = app.id AND ci_pipeline.ci_pipeline_type IS NULL AND app.app_type in( 0,2);