chore: runtime params variable refactorings (#6183)

* chore: runtime variable refactoring

* chore: deprecated api /orchestrator/plugin/global POST

* updated migration number

* fix: mark runner failed

* minor refactoring

* version dep updated

* chore: review feedback incorporated

* fix: special handling for externalCiArtifact

* rbac updated for GetAllGlobalVariables API

* updated migration number

* fix: updated validatePluginVariable

* fix: added default fallback for VariableType

* fix: sync cd validation issue

* updated vendor files

Signed-off-by: Ash-exp <[email protected]>

* updated migration script number

---------

Signed-off-by: Ash-exp <[email protected]>

Author: Ash-exp

688

@@ -154,7 +154,6 @@ import (
 	repository5 "github.com/devtron-labs/devtron/pkg/pipeline/repository"
 	"github.com/devtron-labs/devtron/pkg/pipeline/types"
 	"github.com/devtron-labs/devtron/pkg/plugin"
-	repository6 "github.com/devtron-labs/devtron/pkg/plugin/repository"
 	"github.com/devtron-labs/devtron/pkg/policyGovernance"
 	resourceGroup2 "github.com/devtron-labs/devtron/pkg/resourceGroup"
 	"github.com/devtron-labs/devtron/pkg/resourceQualifiers"
@@ -783,12 +782,7 @@ func InitializeApp() (*App, error) {
 		// history ends
 
 		// plugin starts
-		repository6.NewGlobalPluginRepository,
-		wire.Bind(new(repository6.GlobalPluginRepository), new(*repository6.GlobalPluginRepositoryImpl)),
-
-		plugin.NewGlobalPluginService,
-		wire.Bind(new(plugin.GlobalPluginService), new(*plugin.GlobalPluginServiceImpl)),
-
+		plugin.WireSet,
 		restHandler.NewGlobalPluginRestHandler,
 		wire.Bind(new(restHandler.GlobalPluginRestHandler), new(*restHandler.GlobalPluginRestHandlerImpl)),
 

Wire.go

@@ -19,6 +19,7 @@ package bean
 import (
 	"encoding/json"
 	"github.com/devtron-labs/devtron/util"
+	"github.com/devtron-labs/devtron/util/sliceUtil"
 )
 
 type ConfigMapRootJson struct {
@@ -64,11 +65,11 @@ func (configSecret ConfigSecretMap) GetDataMap() (map[string]string, error) {
 	return datamap, err
 }
 func (configSecretJson ConfigSecretJson) GetDereferencedSecrets() []ConfigSecretMap {
-	return util.GetDeReferencedArray(configSecretJson.Secrets)
+	return sliceUtil.GetDeReferencedSlice(configSecretJson.Secrets)
 }
 
 func (configSecretJson *ConfigSecretJson) SetReferencedSecrets(secrets []ConfigSecretMap) {
-	configSecretJson.Secrets = util.GetReferencedArray(secrets)
+	configSecretJson.Secrets = sliceUtil.GetReferencedSlice(secrets)
 }
 
 func GetTransformedDataForSecretRootJsonData(data string, mode util.SecretTransformMode) (string, error) {

api/bean/ConfigMapAndSecret.go

@@ -23,6 +23,15 @@ import (
 )
 
 type WorkflowType string
+
+func (workflowType WorkflowType) String() string {
+	return string(workflowType)
+}
+
+func NewWorkflowType(workflowType string) WorkflowType {
+	return WorkflowType(workflowType)
+}
+
 type DeploymentConfigurationType string
 
 const (

api/bean/ValuesOverrideRequest.go

@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"github.com/devtron-labs/devtron/api/restHandler/common"
+	"github.com/devtron-labs/devtron/internal/sql/repository/helper"
 	"github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin"
 	"github.com/devtron-labs/devtron/pkg/auth/user"
 	"github.com/devtron-labs/devtron/pkg/pipeline"
@@ -37,7 +38,6 @@ import (
 )
 
 type GlobalPluginRestHandler interface {
-	PatchPlugin(w http.ResponseWriter, r *http.Request)
 	CreatePlugin(w http.ResponseWriter, r *http.Request)
 
 	GetAllGlobalVariables(w http.ResponseWriter, r *http.Request)
@@ -75,7 +75,12 @@ type GlobalPluginRestHandlerImpl struct {
 	userService         user.UserService
 }
 
-func (handler *GlobalPluginRestHandlerImpl) PatchPlugin(w http.ResponseWriter, r *http.Request) {
+// Deprecated: method patchPlugin
+// The below API was initially designed to handle the older design of global plugins.
+// The API is not yet used in UI.
+// The CODE is not yet tested for all the cases.
+// TODO: remove this dead code and all the related handling.
+func (handler *GlobalPluginRestHandlerImpl) patchPlugin(w http.ResponseWriter, r *http.Request) {
 	decoder := json.NewDecoder(r.Body)
 	userId, err := handler.userService.GetLoggedInUser(r)
 	if userId == 0 || err != nil {
@@ -85,7 +90,7 @@ func (handler *GlobalPluginRestHandlerImpl) PatchPlugin(w http.ResponseWriter, r
 	var pluginDataDto bean.PluginMetadataDto
 	err = decoder.Decode(&pluginDataDto)
 	if err != nil {
-		handler.logger.Errorw("request err, PatchPlugin", "error", err, "payload", pluginDataDto)
+		handler.logger.Errorw("request err, patchPlugin", "error", err, "payload", pluginDataDto)
 		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
 		return
 	}
@@ -105,8 +110,8 @@ func (handler *GlobalPluginRestHandlerImpl) PatchPlugin(w http.ResponseWriter, r
 		return
 	}
 	common.WriteJsonResp(w, nil, pluginData, http.StatusOK)
-
 }
+
 func (handler *GlobalPluginRestHandlerImpl) GetDetailedPluginInfoByPluginId(w http.ResponseWriter, r *http.Request) {
 	userId, err := handler.userService.GetLoggedInUser(r)
 	if userId == 0 || err != nil {
@@ -159,7 +164,6 @@ func (handler *GlobalPluginRestHandlerImpl) GetAllDetailedPluginInfo(w http.Resp
 }
 
 func (handler *GlobalPluginRestHandlerImpl) GetAllGlobalVariables(w http.ResponseWriter, r *http.Request) {
-	token := r.Header.Get("token")
 	appIdQueryParam := r.URL.Query().Get("appId")
 	appId, err := strconv.Atoi(appIdQueryParam)
 	if appIdQueryParam == "" || err != nil {
@@ -172,15 +176,24 @@ func (handler *GlobalPluginRestHandlerImpl) GetAllGlobalVariables(w http.Respons
 		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
 		return
 	}
-	//using appId for rbac in plugin(global resource), because this data must be visible to person having create permission
-	//on atleast one app & we can't check this without iterating through every app
-	//TODO: update plugin as a resource in casbin and make rbac independent of appId
-	resourceName := handler.enforcerUtil.GetAppRBACName(app.AppName)
-	ok := handler.enforcerUtil.CheckAppRbacForAppOrJob(token, resourceName, casbin.ActionCreate)
-	if !ok {
-		common.WriteJsonResp(w, fmt.Errorf("unauthorized user"), "Unauthorized User", http.StatusForbidden)
+
+	//RBAC START
+	// TODO: RBAC fix; Mature CheckAppRbacForAppOrJob method to handle based on AppType
+	// Should be implemented as below
+	token := r.Header.Get(common.TokenHeaderKey)
+	object := handler.enforcerUtil.GetAppRBACNameByAppId(appId)
+	authorised := false
+	if app.AppType == helper.Job {
+		authorised = handler.enforcer.Enforce(token, casbin.ResourceApplications, casbin.ActionGet, object)
+	} else if app.AppType == helper.CustomApp {
+		authorised = handler.enforcer.Enforce(token, casbin.ResourceJobs, casbin.ActionGet, object)
+	}
+	if !authorised {
+		common.WriteJsonResp(w, common.ErrUnAuthorized, nil, http.StatusForbidden)
 		return
 	}
+	//RBAC END
+
 	globalVariables, err := handler.globalPluginService.GetAllGlobalVariables(app.AppType)
 	if err != nil {
 		handler.logger.Errorw("error in getting global variable list", "err", err)

api/restHandler/GlobalPluginRestHandler.go

@@ -21,9 +21,11 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	apiBean "github.com/devtron-labs/devtron/api/restHandler/app/pipeline/configure/bean"
 	"github.com/devtron-labs/devtron/internal/sql/constants"
 	"github.com/devtron-labs/devtron/pkg/build/artifacts/imageTagging"
 	bean2 "github.com/devtron-labs/devtron/pkg/build/pipeline/bean"
+	"github.com/devtron-labs/devtron/util/stringsUtil"
 	"golang.org/x/exp/maps"
 	"io"
 	"net/http"
@@ -47,21 +49,12 @@ import (
 	bean1 "github.com/devtron-labs/devtron/pkg/pipeline/bean"
 	"github.com/devtron-labs/devtron/pkg/pipeline/types"
 	resourceGroup "github.com/devtron-labs/devtron/pkg/resourceGroup"
-	util2 "github.com/devtron-labs/devtron/util"
 	"github.com/devtron-labs/devtron/util/response"
 	"github.com/go-pg/pg"
 	"github.com/gorilla/mux"
 	"go.opentelemetry.io/otel"
 )
 
-const GIT_MATERIAL_DELETE_SUCCESS_RESP = "Git material deleted successfully."
-
-type BuildHistoryResponse struct {
-	HideImageTaggingHardDelete bool                     `json:"hideImageTaggingHardDelete"`
-	TagsEditable               bool                     `json:"tagsEditable"`
-	AppReleaseTagNames         []string                 `json:"appReleaseTagNames"` //unique list of tags exists in the app
-	CiWorkflows                []types.WorkflowResponse `json:"ciWorkflows"`
-}
 type DevtronAppBuildRestHandler interface {
 	CreateCiConfig(w http.ResponseWriter, r *http.Request)
 	UpdateCiTemplate(w http.ResponseWriter, r *http.Request)
@@ -648,72 +641,48 @@ func (handler *PipelineConfigRestHandlerImpl) GetExternalCiById(w http.ResponseW
 	common.WriteJsonResp(w, err, ciConf, http.StatusOK)
 }
 
-func (handler *PipelineConfigRestHandlerImpl) TriggerCiPipeline(w http.ResponseWriter, r *http.Request) {
-	userId, err := handler.userAuthService.GetLoggedInUser(r)
-	if userId == 0 || err != nil {
-		common.WriteJsonResp(w, err, "Unauthorized User", http.StatusUnauthorized)
-		return
-	}
-	decoder := json.NewDecoder(r.Body)
-	var ciTriggerRequest bean.CiTriggerRequest
-	err = decoder.Decode(&ciTriggerRequest)
+func (handler *PipelineConfigRestHandlerImpl) validateCiTriggerRBAC(token string, ciPipelineId, triggerEnvironmentId int) error {
+	// RBAC STARTS
+	// checking if user has trigger access on app, if not will be forbidden to trigger independent of number of cd cdPipelines
+	ciPipeline, err := handler.ciPipelineRepository.FindById(ciPipelineId)
 	if err != nil {
-		handler.Logger.Errorw("request err, TriggerCiPipeline", "err", err, "payload", ciTriggerRequest)
-		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
-		return
+		handler.Logger.Errorw("err in finding ci pipeline, TriggerCiPipeline", "err", err, "ciPipelineId", ciPipelineId)
+		errMsg := fmt.Sprintf("error in finding ci pipeline for id '%d'", ciPipelineId)
+		return util.NewApiError(http.StatusBadRequest, errMsg, errMsg)
 	}
-	if !handler.validForMultiMaterial(ciTriggerRequest) {
-		handler.Logger.Errorw("invalid req, commit hash not present for multi-git", "payload", ciTriggerRequest)
-		common.WriteJsonResp(w, errors.New("invalid req, commit hash not present for multi-git"),
-			nil, http.StatusBadRequest)
-	}
-	ciTriggerRequest.TriggeredBy = userId
-	token := r.Header.Get("token")
-
-	handler.Logger.Infow("request payload, TriggerCiPipeline", "payload", ciTriggerRequest)
-
-	//RBAC STARTS
-	//checking if user has trigger access on app, if not will be forbidden to trigger independent of number of cd cdPipelines
-	ciPipeline, err := handler.ciPipelineRepository.FindById(ciTriggerRequest.PipelineId)
-	if err != nil {
-		handler.Logger.Errorw("err in finding ci pipeline, TriggerCiPipeline", "err", err, "ciPipelineId", ciTriggerRequest.PipelineId)
-		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
-		return
-	}
-	appWorkflowMapping, err := handler.appWorkflowService.FindAppWorkflowByCiPipelineId(ciTriggerRequest.PipelineId)
+	appWorkflowMapping, err := handler.appWorkflowService.FindAppWorkflowByCiPipelineId(ciPipelineId)
 	if err != nil {
-		handler.Logger.Errorw("err in finding appWorkflowMapping, TriggerCiPipeline", "err", err, "ciPipelineId", ciTriggerRequest.PipelineId)
-		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
-		return
+		handler.Logger.Errorw("err in finding appWorkflowMapping, TriggerCiPipeline", "err", err, "ciPipelineId", ciPipelineId)
+		errMsg := fmt.Sprintf("error in finding appWorkflowMapping for ciPipelineId '%d'", ciPipelineId)
+		return util.NewApiError(http.StatusBadRequest, errMsg, errMsg)
 	}
 	workflowName := ""
 	if len(appWorkflowMapping) > 0 {
 		workflowName = appWorkflowMapping[0].AppWorkflow.Name
 	}
 	// This is being done for jobs, jobs execute in default-env (devtron-ci) namespace by default. so considering DefaultCiNamespace as env for rbac enforcement
 	envName := ""
-	if ciTriggerRequest.EnvironmentId == 0 {
+	if triggerEnvironmentId == 0 {
 		envName = pipeline.DefaultCiWorkflowNamespace
 	}
 	appObject := handler.enforcerUtil.GetAppRBACNameByAppId(ciPipeline.AppId)
-	workflowObject := handler.enforcerUtil.GetWorkflowRBACByCiPipelineId(ciTriggerRequest.PipelineId, workflowName)
-	triggerObject := handler.enforcerUtil.GetTeamEnvRBACNameByCiPipelineIdAndEnvIdOrName(ciTriggerRequest.PipelineId, ciTriggerRequest.EnvironmentId, envName)
+	workflowObject := handler.enforcerUtil.GetWorkflowRBACByCiPipelineId(ciPipelineId, workflowName)
+	triggerObject := handler.enforcerUtil.GetTeamEnvRBACNameByCiPipelineIdAndEnvIdOrName(ciPipelineId, triggerEnvironmentId, envName)
 	appRbacOk := handler.enforcer.Enforce(token, casbin.ResourceApplications, casbin.ActionTrigger, appObject)
 	if !appRbacOk {
 		appRbacOk = handler.enforcer.Enforce(token, casbin.ResourceJobs, casbin.ActionTrigger, appObject) && handler.enforcer.Enforce(token, casbin.ResourceWorkflow, casbin.ActionTrigger, workflowObject) && handler.enforcer.Enforce(token, casbin.ResourceJobsEnv, casbin.ActionTrigger, triggerObject)
 	}
 
 	if !appRbacOk {
 		handler.Logger.Debug(fmt.Errorf("unauthorized user"), "Unauthorized User", http.StatusForbidden)
-		common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden)
-		return
+		return util.NewApiError(http.StatusForbidden, common.UnAuthorisedUser, common.UnAuthorisedUser)
 	}
-	//checking rbac for cd cdPipelines
-	cdPipelines, err := handler.pipelineRepository.FindByCiPipelineId(ciTriggerRequest.PipelineId)
+	// checking rbac for cd cdPipelines
+	cdPipelines, err := handler.pipelineRepository.FindByCiPipelineId(ciPipelineId)
 	if err != nil {
-		handler.Logger.Errorw("error in finding ccd cdPipelines by ciPipelineId", "err", err, "ciPipelineId", ciTriggerRequest.PipelineId)
-		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
-		return
+		handler.Logger.Errorw("error in finding ccd cdPipelines by ciPipelineId", "err", err, "ciPipelineId", ciPipelineId)
+		errMsg := fmt.Sprintf("error in finding cd cdPipelines for ciPipelineId '%d'", ciPipelineId)
+		return util.NewApiError(http.StatusBadRequest, errMsg, errMsg)
 	}
 	cdPipelineRbacObjects := make([]string, len(cdPipelines))
 	rbacObjectCdTriggerTypeMap := make(map[string]pipelineConfig.TriggerType, len(cdPipelines))
@@ -729,21 +698,50 @@ func (handler *PipelineConfigRestHandlerImpl) TriggerCiPipeline(w http.ResponseW
 		envRbacResultMap := handler.enforcer.EnforceInBatch(token, casbin.ResourceEnvironment, casbin.ActionTrigger, cdPipelineRbacObjects)
 		for rbacObject, rbacResultOk := range envRbacResultMap {
 			if rbacObjectCdTriggerTypeMap[rbacObject] == pipelineConfig.TRIGGER_TYPE_AUTOMATIC && !rbacResultOk {
-				common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden)
-				return
+				return util.NewApiError(http.StatusForbidden, common.UnAuthorisedUser, common.UnAuthorisedUser)
 			}
 			if rbacResultOk { //this flow will come if pipeline is automatic and has access or if pipeline is manual,
 				// by which we can ensure if there are no automatic pipelines then atleast access on one manual is present
 				hasAnyEnvTriggerAccess = true
 			}
 		}
 		if !hasAnyEnvTriggerAccess {
-			common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden)
-			return
+			return util.NewApiError(http.StatusForbidden, common.UnAuthorisedUser, common.UnAuthorisedUser)
 		}
 	}
+	// RBAC ENDS
+	return nil
+}
 
-	//RBAC ENDS
+func (handler *PipelineConfigRestHandlerImpl) TriggerCiPipeline(w http.ResponseWriter, r *http.Request) {
+	userId, err := handler.userAuthService.GetLoggedInUser(r)
+	if userId == 0 || err != nil {
+		common.WriteJsonResp(w, err, "Unauthorized User", http.StatusUnauthorized)
+		return
+	}
+	decoder := json.NewDecoder(r.Body)
+	var ciTriggerRequest bean.CiTriggerRequest
+	err = decoder.Decode(&ciTriggerRequest)
+	if err != nil {
+		handler.Logger.Errorw("request err, TriggerCiPipeline", "err", err, "payload", ciTriggerRequest)
+		common.WriteJsonResp(w, err, nil, http.StatusBadRequest)
+		return
+	}
+	token := r.Header.Get("token")
+	// RBAC block starts
+	err = handler.validateCiTriggerRBAC(token, ciTriggerRequest.PipelineId, ciTriggerRequest.EnvironmentId)
+	if err != nil {
+		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
+		return
+	}
+	// RBAC block ends
+	if !handler.validForMultiMaterial(ciTriggerRequest) {
+		handler.Logger.Errorw("invalid req, commit hash not present for multi-git", "payload", ciTriggerRequest)
+		common.WriteJsonResp(w, errors.New("invalid req, commit hash not present for multi-git"),
+			nil, http.StatusBadRequest)
+	}
+	ciTriggerRequest.TriggeredBy = userId
+	handler.Logger.Infow("request payload, TriggerCiPipeline", "payload", ciTriggerRequest)
 	response := make(map[string]string)
 	resp, err := handler.ciHandler.HandleCIManual(ciTriggerRequest)
 	if errors.Is(err, bean1.ErrImagePathInUse) {
@@ -916,7 +914,7 @@ func (handler *PipelineConfigRestHandlerImpl) GetCiPipelineMin(w http.ResponseWr
 	envIdsString := v.Get("envIds")
 	envIds := make([]int, 0)
 	if len(envIdsString) > 0 {
-		envIds, err = util2.SplitCommaSeparatedIntValues(envIdsString)
+		envIds, err = stringsUtil.SplitCommaSeparatedIntValues(envIdsString)
 		if err != nil {
 			common.WriteJsonResp(w, err, "please provide valid envIds", http.StatusBadRequest)
 			return
@@ -1103,7 +1101,7 @@ func (handler *PipelineConfigRestHandlerImpl) GetBuildHistory(w http.ResponseWri
 	//RBAC for edit tag access , user should have build permission in current ci-pipeline
 	triggerAccess := handler.enforcer.Enforce(token, casbin.ResourceApplications, casbin.ActionTrigger, object) || handler.enforcer.Enforce(token, casbin.ResourceJobs, casbin.ActionTrigger, object)
 	//RBAC
-	resp := BuildHistoryResponse{}
+	resp := apiBean.BuildHistoryResponse{}
 	workflowsResp, err := handler.ciHandler.GetBuildHistory(pipelineId, ciPipeline.AppId, offset, limit)
 	resp.CiWorkflows = workflowsResp
 	if err != nil {
@@ -1501,7 +1499,7 @@ func (handler *PipelineConfigRestHandlerImpl) DeleteMaterial(w http.ResponseWrit
 		common.WriteJsonResp(w, err, nil, http.StatusInternalServerError)
 		return
 	}
-	common.WriteJsonResp(w, err, GIT_MATERIAL_DELETE_SUCCESS_RESP, http.StatusOK)
+	common.WriteJsonResp(w, err, apiBean.GIT_MATERIAL_DELETE_SUCCESS_RESP, http.StatusOK)
 }
 
 func (handler *PipelineConfigRestHandlerImpl) HandleWorkflowWebhook(w http.ResponseWriter, r *http.Request) {

api/restHandler/app/pipeline/configure/BuildPipelineRestHandler.go

@@ -0,0 +1,12 @@
+package bean
+
+import "github.com/devtron-labs/devtron/pkg/pipeline/types"
+
+const GIT_MATERIAL_DELETE_SUCCESS_RESP = "Git material deleted successfully."
+
+type BuildHistoryResponse struct {
+	HideImageTaggingHardDelete bool                     `json:"hideImageTaggingHardDelete"`
+	TagsEditable               bool                     `json:"tagsEditable"`
+	AppReleaseTagNames         []string                 `json:"appReleaseTagNames"` //unique list of tags exists in the app
+	CiWorkflows                []types.WorkflowResponse `json:"ciWorkflows"`
+}