From 33f57640b9c47cfcc7d3d71e03d808d13939edcc Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 11:18:07 +0000 Subject: [PATCH 01/14] fix(security): enforce caller RBAC in API token webhook endpoint GetAllApiTokensForWebhook checked whether each STORED api token had trigger permission on the requested project/env, but never checked whether the CALLING user was permitted to query that project/env at all. As a result any authenticated user could request an arbitrary projectName/environmentName/appName and receive the plaintext token values of every stored api token (including super-admin tokens, whose Casbin policy is "*" and therefore always passes the per-token check). Add the same enforcement pattern used by the sibling handlers in this file, but scoped to the resource being queried: verify the requesting user's own token has trigger permission on the requested project and every requested environment before the service call is made, so no token data is looked up or returned for a project/env the caller cannot access. Fixes #7013 --- api/apiToken/ApiTokenRestHandler.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/api/apiToken/ApiTokenRestHandler.go b/api/apiToken/ApiTokenRestHandler.go index d003385d98..a7b02073a1 100644 --- a/api/apiToken/ApiTokenRestHandler.go +++ b/api/apiToken/ApiTokenRestHandler.go @@ -18,6 +18,7 @@ package apiToken import ( "encoding/json" + "fmt" openapi "github.com/devtron-labs/devtron/api/openapi/openapiClient" "github.com/devtron-labs/devtron/api/restHandler/common" "github.com/devtron-labs/devtron/pkg/apiToken" @@ -29,6 +30,7 @@ import ( "gopkg.in/go-playground/validator.v9" "net/http" "strconv" + "strings" ) type ApiTokenRestHandler interface { @@ -220,12 +222,24 @@ func (impl ApiTokenRestHandlerImpl) GetAllApiTokensForWebhook(w http.ResponseWri return } - // handle super-admin RBAC v := r.URL.Query() projectName := v.Get("projectName") environmentName := v.Get("environmentName") appName := v.Get("appName") + // handle RBAC - verify that the REQUESTING user (not the stored api-tokens + // being evaluated below) has trigger permission on the requested + // project/environment/app before any token data is looked up and returned. + token := r.Header.Get("token") + projectObject := fmt.Sprintf("%s/%s", projectName, appName) + for _, environment := range strings.Split(environmentName, ",") { + envObject := fmt.Sprintf("%s/%s", environment, appName) + if ok := impl.CheckAuthorizationForWebhook(token, projectObject, envObject); !ok { + common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) + return + } + } + // service call res, err := impl.apiTokenService.GetAllApiTokensForWebhook(projectName, environmentName, appName, impl.CheckAuthorizationForWebhook) if err != nil { From 4657d97bf557f132e4debef228d02125b6e7b26f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 11:26:30 +0000 Subject: [PATCH 02/14] chore: address SonarCloud finding in GetAllApiTokensForWebhook Inline the unnecessary "ok" variable in the caller-RBAC check added in the previous commit, per SonarCloud's "remove this unnecessary variable declaration and use the expression directly" finding on PR #7016. --- api/apiToken/ApiTokenRestHandler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/apiToken/ApiTokenRestHandler.go b/api/apiToken/ApiTokenRestHandler.go index a7b02073a1..c605604099 100644 --- a/api/apiToken/ApiTokenRestHandler.go +++ b/api/apiToken/ApiTokenRestHandler.go @@ -234,7 +234,7 @@ func (impl ApiTokenRestHandlerImpl) GetAllApiTokensForWebhook(w http.ResponseWri projectObject := fmt.Sprintf("%s/%s", projectName, appName) for _, environment := range strings.Split(environmentName, ",") { envObject := fmt.Sprintf("%s/%s", environment, appName) - if ok := impl.CheckAuthorizationForWebhook(token, projectObject, envObject); !ok { + if !impl.CheckAuthorizationForWebhook(token, projectObject, envObject) { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } From 5566030a7b236262a26b6c01b2949b0673445578 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 11:50:39 +0000 Subject: [PATCH 03/14] fix(security): don't leak more-privileged tokens to less-privileged callers Checking that a caller has trigger permission on the requested project/env was not sufficient: a super-admin api-token's Casbin policy is "*", so it always satisfies that same per-object check regardless of how narrow the actual project/env being queried is. That meant any caller with legitimate, narrow trigger access to some app/env could still have super-admin (or other more-privileged) tokens returned alongside their own in the same response. Add callerDominatesToken, which excludes a stored token from the response whenever it is a super-admin token and the caller is not, reusing the same super-admin check already used by the other handlers in this file. The per-object check now runs against each stored token before this dominance check, matching the existing per-token-per-object evaluation the service already performs. The Enforcer interface here only exposes single-tuple Enforce/ EnforceInBatch calls, not policy or role enumeration, so a fully generic "is token's permission a strict subset of the caller's" comparison across every possible object isn't available without extending that interface. Super-admin dominance is the well-defined, checkable case that matches this issue (leaking admin token values), so it's what this fix targets; a more general scope comparison would need broader RBAC changes outside this endpoint's blast radius. --- api/apiToken/ApiTokenRestHandler.go | 35 ++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/api/apiToken/ApiTokenRestHandler.go b/api/apiToken/ApiTokenRestHandler.go index c605604099..0f911d6fc5 100644 --- a/api/apiToken/ApiTokenRestHandler.go +++ b/api/apiToken/ApiTokenRestHandler.go @@ -230,18 +230,31 @@ func (impl ApiTokenRestHandlerImpl) GetAllApiTokensForWebhook(w http.ResponseWri // handle RBAC - verify that the REQUESTING user (not the stored api-tokens // being evaluated below) has trigger permission on the requested // project/environment/app before any token data is looked up and returned. - token := r.Header.Get("token") + callerToken := r.Header.Get("token") projectObject := fmt.Sprintf("%s/%s", projectName, appName) for _, environment := range strings.Split(environmentName, ",") { envObject := fmt.Sprintf("%s/%s", environment, appName) - if !impl.CheckAuthorizationForWebhook(token, projectObject, envObject) { + if !impl.CheckAuthorizationForWebhook(callerToken, projectObject, envObject) { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } } + // per-token authorization: a stored api-token's own Casbin policy always + // satisfies the project/env check above if that token is a super-admin + // ("*" matches any object), regardless of how narrow the caller's own + // access is. So a stored token is only eligible to be returned if, on top + // of its own project/env check, it is not more privileged than the caller + // - i.e. its scope must be no broader than the requesting user's own. + authForToken := func(storedToken string, projObj string, envObj string) bool { + if !impl.CheckAuthorizationForWebhook(storedToken, projObj, envObj) { + return false + } + return impl.callerDominatesToken(callerToken, storedToken) + } + // service call - res, err := impl.apiTokenService.GetAllApiTokensForWebhook(projectName, environmentName, appName, impl.CheckAuthorizationForWebhook) + res, err := impl.apiTokenService.GetAllApiTokensForWebhook(projectName, environmentName, appName, authForToken) if err != nil { impl.logger.Errorw("service err, GetAllApiTokensForWebhook", "err", err) common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) @@ -259,3 +272,19 @@ func (handler ApiTokenRestHandlerImpl) CheckAuthorizationForWebhook(token string } return true } + +// callerDominatesToken returns false when storedToken has strictly broader +// access than callerToken, so that a caller can never see an api-token more +// privileged than themselves through this endpoint. A super-admin token's +// policy is "*", so it always passes the per-object checks in +// CheckAuthorizationForWebhook irrespective of the project/env queried; the +// only way to tell it apart from a token that is genuinely scoped to the +// requested object is to check super-admin status directly, using the same +// check the other handlers in this file use to gate super-admin-only actions. +func (handler ApiTokenRestHandlerImpl) callerDominatesToken(callerToken string, storedToken string) bool { + if !handler.enforcer.Enforce(storedToken, casbin.ResourceGlobal, casbin.ActionUpdate, "*") { + // stored token is not a super-admin token, nothing further to check + return true + } + return handler.enforcer.Enforce(callerToken, casbin.ResourceGlobal, casbin.ActionUpdate, "*") +} From 73b7d2ae0cdef8a456b5c78a98c27a760e88cfe0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 11:56:26 +0000 Subject: [PATCH 04/14] chore: address SonarCloud param-grouping findings in ApiTokenRestHandler Group consecutive same-type parameters (storedToken/projObj/envObj and callerToken/storedToken) per SonarCloud's finding on the code added in the previous two commits. --- api/apiToken/ApiTokenRestHandler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/apiToken/ApiTokenRestHandler.go b/api/apiToken/ApiTokenRestHandler.go index 0f911d6fc5..77c7495368 100644 --- a/api/apiToken/ApiTokenRestHandler.go +++ b/api/apiToken/ApiTokenRestHandler.go @@ -246,7 +246,7 @@ func (impl ApiTokenRestHandlerImpl) GetAllApiTokensForWebhook(w http.ResponseWri // access is. So a stored token is only eligible to be returned if, on top // of its own project/env check, it is not more privileged than the caller // - i.e. its scope must be no broader than the requesting user's own. - authForToken := func(storedToken string, projObj string, envObj string) bool { + authForToken := func(storedToken, projObj, envObj string) bool { if !impl.CheckAuthorizationForWebhook(storedToken, projObj, envObj) { return false } @@ -281,7 +281,7 @@ func (handler ApiTokenRestHandlerImpl) CheckAuthorizationForWebhook(token string // only way to tell it apart from a token that is genuinely scoped to the // requested object is to check super-admin status directly, using the same // check the other handlers in this file use to gate super-admin-only actions. -func (handler ApiTokenRestHandlerImpl) callerDominatesToken(callerToken string, storedToken string) bool { +func (handler ApiTokenRestHandlerImpl) callerDominatesToken(callerToken, storedToken string) bool { if !handler.enforcer.Enforce(storedToken, casbin.ResourceGlobal, casbin.ActionUpdate, "*") { // stored token is not a super-admin token, nothing further to check return true From ba2f3b9f18eae1f15fe35c7df4d4209997d4d536 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 12:05:17 +0000 Subject: [PATCH 05/14] fix(security): simplify to a straightforward super-admin gate Per thread decision: GetAllApiTokensForWebhook's only real caller is the dashboard's webhook config modal, which is already super-admin-only in the UI. Rather than layering a per-project/env caller check plus a token-dominance check on top of the existing per-stored-token filter, just gate the whole endpoint on the caller being a super-admin - the same Enforce(token, ResourceGlobal, ActionUpdate, "*") check already used by every other handler in this file (GetAllApiTokens, CreateApiToken, UpdateApiToken, DeleteApiToken). This removes the per-object caller check and the callerDominatesToken logic added in the previous two commits; the existing per-stored-token CheckAuthorizationForWebhook callback into the service is left as-is since it still does useful filtering (only tokens with trigger permission on the requested project/env are returned), it's just no longer load-bearing for access control now that the endpoint itself requires a super-admin caller. Net effect vs main: an 8-line super-admin check added right after GetLoggedInUser, matching the pattern already used by every sibling handler in this file. --- api/apiToken/ApiTokenRestHandler.go | 55 ++++++----------------------- 1 file changed, 10 insertions(+), 45 deletions(-) diff --git a/api/apiToken/ApiTokenRestHandler.go b/api/apiToken/ApiTokenRestHandler.go index 77c7495368..d1c2b9cb3b 100644 --- a/api/apiToken/ApiTokenRestHandler.go +++ b/api/apiToken/ApiTokenRestHandler.go @@ -18,7 +18,6 @@ package apiToken import ( "encoding/json" - "fmt" openapi "github.com/devtron-labs/devtron/api/openapi/openapiClient" "github.com/devtron-labs/devtron/api/restHandler/common" "github.com/devtron-labs/devtron/pkg/apiToken" @@ -30,7 +29,6 @@ import ( "gopkg.in/go-playground/validator.v9" "net/http" "strconv" - "strings" ) type ApiTokenRestHandler interface { @@ -222,39 +220,22 @@ func (impl ApiTokenRestHandlerImpl) GetAllApiTokensForWebhook(w http.ResponseWri return } + // handle super-admin RBAC - this endpoint's only caller is the dashboard's + // webhook config modal, which is itself super-admin-only, so gate the + // whole endpoint the same way the other handlers in this file do. + token := r.Header.Get("token") + if ok := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*"); !ok { + common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) + return + } + v := r.URL.Query() projectName := v.Get("projectName") environmentName := v.Get("environmentName") appName := v.Get("appName") - // handle RBAC - verify that the REQUESTING user (not the stored api-tokens - // being evaluated below) has trigger permission on the requested - // project/environment/app before any token data is looked up and returned. - callerToken := r.Header.Get("token") - projectObject := fmt.Sprintf("%s/%s", projectName, appName) - for _, environment := range strings.Split(environmentName, ",") { - envObject := fmt.Sprintf("%s/%s", environment, appName) - if !impl.CheckAuthorizationForWebhook(callerToken, projectObject, envObject) { - common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) - return - } - } - - // per-token authorization: a stored api-token's own Casbin policy always - // satisfies the project/env check above if that token is a super-admin - // ("*" matches any object), regardless of how narrow the caller's own - // access is. So a stored token is only eligible to be returned if, on top - // of its own project/env check, it is not more privileged than the caller - // - i.e. its scope must be no broader than the requesting user's own. - authForToken := func(storedToken, projObj, envObj string) bool { - if !impl.CheckAuthorizationForWebhook(storedToken, projObj, envObj) { - return false - } - return impl.callerDominatesToken(callerToken, storedToken) - } - // service call - res, err := impl.apiTokenService.GetAllApiTokensForWebhook(projectName, environmentName, appName, authForToken) + res, err := impl.apiTokenService.GetAllApiTokensForWebhook(projectName, environmentName, appName, impl.CheckAuthorizationForWebhook) if err != nil { impl.logger.Errorw("service err, GetAllApiTokensForWebhook", "err", err) common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) @@ -272,19 +253,3 @@ func (handler ApiTokenRestHandlerImpl) CheckAuthorizationForWebhook(token string } return true } - -// callerDominatesToken returns false when storedToken has strictly broader -// access than callerToken, so that a caller can never see an api-token more -// privileged than themselves through this endpoint. A super-admin token's -// policy is "*", so it always passes the per-object checks in -// CheckAuthorizationForWebhook irrespective of the project/env queried; the -// only way to tell it apart from a token that is genuinely scoped to the -// requested object is to check super-admin status directly, using the same -// check the other handlers in this file use to gate super-admin-only actions. -func (handler ApiTokenRestHandlerImpl) callerDominatesToken(callerToken, storedToken string) bool { - if !handler.enforcer.Enforce(storedToken, casbin.ResourceGlobal, casbin.ActionUpdate, "*") { - // stored token is not a super-admin token, nothing further to check - return true - } - return handler.enforcer.Enforce(callerToken, casbin.ResourceGlobal, casbin.ActionUpdate, "*") -} From 5d2c90578c645861aae796fc44f47bea505f987d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 1 Sep 2026 12:09:40 +0000 Subject: [PATCH 06/14] chore: address SonarCloud finding in super-admin gate check Inline the unnecessary "ok" variable in the super-admin check added to GetAllApiTokensForWebhook, per SonarCloud's "remove this unnecessary variable declaration" finding on PR #7016. --- api/apiToken/ApiTokenRestHandler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/apiToken/ApiTokenRestHandler.go b/api/apiToken/ApiTokenRestHandler.go index d1c2b9cb3b..b4918900e5 100644 --- a/api/apiToken/ApiTokenRestHandler.go +++ b/api/apiToken/ApiTokenRestHandler.go @@ -224,7 +224,7 @@ func (impl ApiTokenRestHandlerImpl) GetAllApiTokensForWebhook(w http.ResponseWri // webhook config modal, which is itself super-admin-only, so gate the // whole endpoint the same way the other handlers in this file do. token := r.Header.Get("token") - if ok := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*"); !ok { + if !impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*") { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } From 5e5ebdc9f65ec61944966e369b0187777ebea587 Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Tue, 1 Sep 2026 23:02:26 +0530 Subject: [PATCH 07/14] feat: decouple app lifecycle actions; add indexes to improve CI query performance - Introduce `createApp`/`deleteApp` actions to segregate application lifecycle control from pipeline/workflow/config operations. - Update RBAC enforcement to reflect the new `createApp`/`deleteApp` actions for better granularity. - Add `idx_ci_artifact`, `idx_ci_workflow`, and other indexes to improve CI query execution and resolve PG_READ_TIMEOUT issues. - Introduce GIN index for `git_triggers` to optimize material/commit lookup in `validateBuildSequence`. --- api/restHandler/CoreAppRestHandler.go | 3 +- .../configure/PipelineConfigRestHandler.go | 29 ++++++- pkg/auth/authorisation/casbin/rbacpolicy.go | 5 ++ ...600_add_ci_artifact_query_indexes.down.sql | 8 ++ ...04600_add_ci_artifact_query_indexes.up.sql | 77 +++++++++++++++++++ ...dd_ci_workflow_git_triggers_index.down.sql | 1 + ..._add_ci_workflow_git_triggers_index.up.sql | 23 ++++++ .../36504600_dev_admin_app_lifecycle.down.sql | 5 ++ .../36504600_dev_admin_app_lifecycle.up.sql | 13 ++++ 9 files changed, 161 insertions(+), 3 deletions(-) create mode 100644 scripts/sql/36304600_add_ci_artifact_query_indexes.down.sql create mode 100644 scripts/sql/36304600_add_ci_artifact_query_indexes.up.sql create mode 100644 scripts/sql/36404600_add_ci_workflow_git_triggers_index.down.sql create mode 100644 scripts/sql/36404600_add_ci_workflow_git_triggers_index.up.sql create mode 100644 scripts/sql/36504600_dev_admin_app_lifecycle.down.sql create mode 100644 scripts/sql/36504600_dev_admin_app_lifecycle.up.sql diff --git a/api/restHandler/CoreAppRestHandler.go b/api/restHandler/CoreAppRestHandler.go index 2f0e6286c5..bbe991a5b3 100644 --- a/api/restHandler/CoreAppRestHandler.go +++ b/api/restHandler/CoreAppRestHandler.go @@ -295,7 +295,8 @@ func (handler CoreAppRestHandlerImpl) CreateApp(w http.ResponseWriter, r *http.R return } // with admin roles, you have to access for all the apps of the project to create new app. (admin or manager with specific app permission can't create app.) - if ok := handler.enforcer.Enforce(token, casbin.ResourceApplications, casbin.ActionCreate, fmt.Sprintf("%s/%s", team.Name, "*")); !ok { + // v1beta1 application API creates devtron apps only, so it uses the dedicated app-lifecycle action. + if ok := handler.enforcer.Enforce(token, casbin.ResourceApplications, casbin.ActionCreateApp, fmt.Sprintf("%s/%s", team.Name, "*")); !ok { common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden) return } diff --git a/api/restHandler/app/pipeline/configure/PipelineConfigRestHandler.go b/api/restHandler/app/pipeline/configure/PipelineConfigRestHandler.go index ab8a097f6f..e683c2cfcb 100644 --- a/api/restHandler/app/pipeline/configure/PipelineConfigRestHandler.go +++ b/api/restHandler/app/pipeline/configure/PipelineConfigRestHandler.go @@ -57,6 +57,7 @@ import ( "go.opentelemetry.io/otel" "github.com/devtron-labs/devtron/internal/sql/repository" + "github.com/devtron-labs/devtron/internal/sql/repository/helper" "github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig" "github.com/devtron-labs/devtron/internal/util" "github.com/devtron-labs/devtron/pkg/appClone" @@ -265,7 +266,22 @@ func (handler *PipelineConfigRestHandlerImpl) DeleteApp(w http.ResponseWriter, r return } resourceObject := handler.enforcerUtil.GetAppRBACNameByAppId(appId) - ok := handler.enforcerUtil.CheckAppRbacForAppOrJob(token, resourceObject, casbin.ActionDelete) + app, err := handler.pipelineBuilder.GetApp(appId) + if err != nil { + handler.Logger.Errorw("service err, GetApp", "err", err, "appId", appId) + common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) + return + } + var ok bool + if app.AppType == helper.Job { + // app type is known to be a job, so enforce the jobs resource ONLY (not the applications OR), + // otherwise an applications:delete holder could delete the job entity, which is itself an app record + ok = handler.enforcer.Enforce(token, casbin.ResourceJobs, casbin.ActionDelete, resourceObject) + } else { + // devtron apps use the dedicated app-lifecycle action so the app-entity delete can be + // restricted independently of pipeline/workflow delete (which stay on ActionDelete) + ok = handler.enforcer.Enforce(token, casbin.ResourceApplications, casbin.ActionDeleteApp, resourceObject) + } if !ok { common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden) return @@ -385,7 +401,16 @@ func (handler *PipelineConfigRestHandlerImpl) CreateApp(w http.ResponseWriter, r // with admin roles, you have to access for all the apps of the project to create new app. (admin or manager with specific app permission can't create app.) object := fmt.Sprintf("%s/%s", project.Name, "*") - isAuthorised := handler.enforcerUtil.CheckAppRbacForAppOrJob(token, object, casbin.ActionCreate) + var isAuthorised bool + if createRequest.AppType == helper.Job { + // app type is known to be a job, so enforce the jobs resource ONLY (not the applications OR), + // otherwise an applications:create holder could create the job entity, which is itself an app record + isAuthorised = handler.enforcer.Enforce(token, casbin.ResourceJobs, casbin.ActionCreate, object) + } else { + // devtron apps use the dedicated app-lifecycle action so the app-entity create can be + // restricted independently of pipeline/workflow/config creation (which stay on ActionCreate) + isAuthorised = handler.enforcer.Enforce(token, casbin.ResourceApplications, casbin.ActionCreateApp, object) + } if !isAuthorised { common.WriteJsonResp(w, err, "Unauthorized User", http.StatusForbidden) return diff --git a/pkg/auth/authorisation/casbin/rbacpolicy.go b/pkg/auth/authorisation/casbin/rbacpolicy.go index d61234b868..09423de63a 100644 --- a/pkg/auth/authorisation/casbin/rbacpolicy.go +++ b/pkg/auth/authorisation/casbin/rbacpolicy.go @@ -53,6 +53,11 @@ const ( ActionTrigger = "trigger" ActionNotify = "notify" ActionExec = "exec" + // ActionCreateApp and ActionDeleteApp gate the application-entity lifecycle (create/delete an app), + // decoupled from ActionCreate/ActionDelete which also gate pipeline/workflow/config operations. + // admin/manager/super-admin keep app lifecycle via their wildcard (`*`) action policies. + ActionCreateApp = "createApp" + ActionDeleteApp = "deleteApp" ClusterResourceRegex = "%s/%s" // {cluster}/{namespace} ClusterObjectRegex = "%s/%s/%s" // {groupName}/{kindName}/{objectName} diff --git a/scripts/sql/36304600_add_ci_artifact_query_indexes.down.sql b/scripts/sql/36304600_add_ci_artifact_query_indexes.down.sql new file mode 100644 index 0000000000..8a610b15ab --- /dev/null +++ b/scripts/sql/36304600_add_ci_artifact_query_indexes.down.sql @@ -0,0 +1,8 @@ +-- Rollback: drop indexes added by 36204600_add_ci_artifact_query_indexes.up.sql + +DROP INDEX IF EXISTS idx_ci_artifact_external_ci_pipeline_id; +DROP INDEX IF EXISTS idx_ci_artifact_pipeline_id; +DROP INDEX IF EXISTS idx_ci_artifact_component_id_data_source; +DROP INDEX IF EXISTS idx_ci_workflow_ci_pipeline_id; +DROP INDEX IF EXISTS idx_dar_pipeline_id_active; +DROP INDEX IF EXISTS idx_wes_ci_execution_lookup; diff --git a/scripts/sql/36304600_add_ci_artifact_query_indexes.up.sql b/scripts/sql/36304600_add_ci_artifact_query_indexes.up.sql new file mode 100644 index 0000000000..88df30f843 --- /dev/null +++ b/scripts/sql/36304600_add_ci_artifact_query_indexes.up.sql @@ -0,0 +1,77 @@ +-- Hotfix: add indexes for ci_artifact / ci_workflow / workflow_execution_stage / +-- deployment_approval_request to eliminate sequential scans that were causing +-- 60s PG_READ_TIMEOUT errors on the following hot queries: +-- * CiArtifactRepositoryImpl.GetArtifactsByCDPipelineV3 (external CI + UNION variants) +-- * CiArtifactRepositoryImpl.FindArtifactByListFilter +-- * CiWorkflowRepositoryImpl.FindByPipelineId +-- * CiWorkflowRepositoryImpl.FindByStatusesIn +-- * WorkflowStageRepositoryImpl.GetSuccessfulCIExecutionStages +-- +-- Note: CREATE INDEX CONCURRENTLY cannot run inside a transaction (golang-migrate +-- wraps each migration in a transaction). We use plain CREATE INDEX IF NOT EXISTS +-- here so the migration is idempotent. For large existing tables, operators +-- SHOULD create these indexes manually with CONCURRENTLY BEFORE deploying this +-- release to avoid blocking writes during pod startup: +-- +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_ci_artifact_external_ci_pipeline_id +-- ON ci_artifact (external_ci_pipeline_id) WHERE external_ci_pipeline_id IS NOT NULL; +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_ci_artifact_pipeline_id +-- ON ci_artifact (pipeline_id) WHERE pipeline_id IS NOT NULL; +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_ci_artifact_component_id_data_source +-- ON ci_artifact (component_id, data_source) WHERE component_id IS NOT NULL; +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_ci_workflow_ci_pipeline_id +-- ON ci_workflow (ci_pipeline_id); +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_dar_pipeline_id_active +-- ON deployment_approval_request (pipeline_id) WHERE active = true; +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_wes_ci_execution_lookup +-- ON workflow_execution_stage (created_on) +-- WHERE workflow_type = 'CI' AND stage_name = 'Execution' +-- AND status = 'SUCCEEDED' AND status_for = 'workflow'; +-- +-- Once the indexes exist, this migration is a no-op (IF NOT EXISTS). + +-- 1. ci_artifact.external_ci_pipeline_id +-- Filter used by GetArtifactsByCDPipelineV3 (external webhook CI variant). +-- Partial index: most rows are internal CI (external_ci_pipeline_id IS NULL), +-- so a partial index is much smaller and covers the actual query. +CREATE INDEX IF NOT EXISTS idx_ci_artifact_external_ci_pipeline_id + ON ci_artifact (external_ci_pipeline_id) + WHERE external_ci_pipeline_id IS NOT NULL; + +-- 2. ci_artifact.pipeline_id +-- Filter used by GetArtifactsByCDPipelineV3 (UNION variant) and several other +-- repository methods (Where pipeline_id = ?). +CREATE INDEX IF NOT EXISTS idx_ci_artifact_pipeline_id + ON ci_artifact (pipeline_id) + WHERE pipeline_id IS NOT NULL; + +-- 3. ci_artifact.(component_id, data_source) +-- Composite filter used by GetArtifactsByCDPipelineV3 (UNION variant) and +-- FindArtifactByListFilter: (component_id = ? AND data_source = 'pre_cd'|'post_ci'). +CREATE INDEX IF NOT EXISTS idx_ci_artifact_component_id_data_source + ON ci_artifact (component_id, data_source) + WHERE component_id IS NOT NULL; + +-- 4. ci_workflow.ci_pipeline_id +-- Filter used by FindByPipelineId (WHERE ci_pipeline_id = ? ORDER BY started_on DESC). +CREATE INDEX IF NOT EXISTS idx_ci_workflow_ci_pipeline_id + ON ci_workflow (ci_pipeline_id); + +-- 5. deployment_approval_request.pipeline_id (partial on active = true) +-- Filter used by FindArtifactByListFilter NOT IN subquery +-- (WHERE pipeline_id = ? AND active = true AND artifact_deployment_triggered = false). +-- Partial index on active = true keeps it small. +CREATE INDEX IF NOT EXISTS idx_dar_pipeline_id_active + ON deployment_approval_request (pipeline_id) + WHERE active = true; + +-- 6. workflow_execution_stage covering the exact GetSuccessfulCIExecutionStages filter +-- (workflow_type='CI' AND stage_name='Execution' AND status='SUCCEEDED' +-- AND status_for='workflow' AND created_on BETWEEN ? AND ?). +-- Highly selective partial index shaped to that query. +CREATE INDEX IF NOT EXISTS idx_wes_ci_execution_lookup + ON workflow_execution_stage (created_on) + WHERE workflow_type = 'CI' + AND stage_name = 'Execution' + AND status = 'SUCCEEDED' + AND status_for = 'workflow'; diff --git a/scripts/sql/36404600_add_ci_workflow_git_triggers_index.down.sql b/scripts/sql/36404600_add_ci_workflow_git_triggers_index.down.sql new file mode 100644 index 0000000000..3b18e67b0f --- /dev/null +++ b/scripts/sql/36404600_add_ci_workflow_git_triggers_index.down.sql @@ -0,0 +1 @@ +DROP INDEX IF EXISTS idx_ci_workflow_git_triggers_gin; diff --git a/scripts/sql/36404600_add_ci_workflow_git_triggers_index.up.sql b/scripts/sql/36404600_add_ci_workflow_git_triggers_index.up.sql new file mode 100644 index 0000000000..b97e14d842 --- /dev/null +++ b/scripts/sql/36404600_add_ci_workflow_git_triggers_index.up.sql @@ -0,0 +1,23 @@ +-- Supports CiWorkflowRepositoryImpl.FindWorkflowByMaterialAndCommit, used by +-- validateBuildSequence to check a pipeline's full build history (not just the +-- last build) for whether a given material/commit combination has already been +-- built, before rejecting a stale/redelivered automatic CI trigger. +-- +-- git_triggers is stored as `json`, not `jsonb`, so a plain GIN index can't be +-- created directly on the column. We index the jsonb-cast expression instead, +-- using jsonb_path_ops (smaller and faster than the default jsonb_ops for +-- containment-only (@>) queries, which is the only operator this lookup uses). +-- +-- Note: CREATE INDEX CONCURRENTLY cannot run inside a transaction (golang-migrate +-- wraps each migration in a transaction). We use plain CREATE INDEX IF NOT EXISTS +-- here so the migration is idempotent. For large existing ci_workflow tables, +-- operators SHOULD create this index manually with CONCURRENTLY BEFORE deploying +-- this release, to avoid blocking writes during pod startup: +-- +-- CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_ci_workflow_git_triggers_gin +-- ON ci_workflow USING gin ((git_triggers::jsonb) jsonb_path_ops); +-- +-- Once the index exists, this migration is a no-op (IF NOT EXISTS). + +CREATE INDEX IF NOT EXISTS idx_ci_workflow_git_triggers_gin + ON ci_workflow USING gin ((git_triggers::jsonb) jsonb_path_ops); diff --git a/scripts/sql/36504600_dev_admin_app_lifecycle.down.sql b/scripts/sql/36504600_dev_admin_app_lifecycle.down.sql new file mode 100644 index 0000000000..36b659520e --- /dev/null +++ b/scripts/sql/36504600_dev_admin_app_lifecycle.down.sql @@ -0,0 +1,5 @@ +-- Revert: remove `createApp` / `deleteApp` from the `applications` resource whitelist. +UPDATE rbac_policy_resource_detail +SET allowed_actions = array_remove(array_remove(allowed_actions, 'createApp'), 'deleteApp'), + updated_on = now() +WHERE resource = 'applications'; \ No newline at end of file diff --git a/scripts/sql/36504600_dev_admin_app_lifecycle.up.sql b/scripts/sql/36504600_dev_admin_app_lifecycle.up.sql new file mode 100644 index 0000000000..74064d84d1 --- /dev/null +++ b/scripts/sql/36504600_dev_admin_app_lifecycle.up.sql @@ -0,0 +1,13 @@ +-- Dev-Admin (Release 1): decouple application-entity lifecycle from the overloaded create/delete actions. +-- Adds `createApp` / `deleteApp` to the `applications` resource whitelist so custom roles can grant or +-- withhold application create & delete independently of pipeline/workflow/config operations +-- (which remain on `create`/`delete`). +-- Note: jobs are intentionally NOT touched here (job lifecycle is enforced against the jobs resource). +-- Built-in admin/manager/super-admin keep application lifecycle via their wildcard (`*`) action policies +-- and require no change. +UPDATE rbac_policy_resource_detail +SET allowed_actions = array_cat(allowed_actions, ARRAY ['createApp','deleteApp']), + updated_on = now() +WHERE resource = 'applications' + AND allowed_actions IS NOT NULL + AND NOT ('createApp' = ANY (allowed_actions)); \ No newline at end of file From 4b2bf385d81c45f3b9ca00eb76cfbebcbfb33c32 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 03:34:04 +0000 Subject: [PATCH 08/14] fix(user): scope filter-based bulk delete to match listing filters DELETE /orchestrator/user/bulk with a listingRequest filter (instead of explicit ids) resolved the delete target list via the same query builder the listing endpoint uses, but skipped the normalization the listing endpoint applies first (SetDefaultValuesIfNotPresent, setStatusFilterType). With no default page size and no status filtering applied, a filter such as {"searchKey":"","status":["inactive"]} resolved to every active user instead of the intended filtered subset, causing a bulk delete of far more users than intended. getUserIdsHonoringFilters now applies the same normalization the listing path (GetAllWithFilters) applies before building the query, and rejects a filter-based bulk delete that carries no concrete filter criteria instead of letting it silently resolve to "every user". The explicit ids-based delete path is unchanged. Fixes #7020 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01J5DesTG5XvPdmPCDuL6izq --- pkg/auth/user/UserService.go | 9 ++ .../UserService_bulkdelete_filter_test.go | 100 ++++++++++++++++++ pkg/auth/user/UserService_ent.go | 7 ++ 3 files changed, 116 insertions(+) create mode 100644 pkg/auth/user/UserService_bulkdelete_filter_test.go diff --git a/pkg/auth/user/UserService.go b/pkg/auth/user/UserService.go index 21320edd23..a7724ecc69 100644 --- a/pkg/auth/user/UserService.go +++ b/pkg/auth/user/UserService.go @@ -1480,6 +1480,15 @@ func (impl *UserServiceImpl) BulkDeleteUsers(request *userBean.BulkDeleteRequest // getUserIdsHonoringFilters get the filtered user ids according to the request filters and returns userIds and error(not nil) if any exception is caught. func (impl *UserServiceImpl) getUserIdsHonoringFilters(request *userBean.ListingRequest) ([]int32, error) { + // a filter based bulk delete must always be scoped by at least one concrete filter; an empty/default + // filter must never be allowed to silently widen into "delete every user" (no filter => no filter based bulk delete). + if len(strings.TrimSpace(request.SearchKey)) == 0 && !isStatusFilterApplied(request) { + return nil, &util.ApiError{Code: "400", HttpStatusCode: http.StatusBadRequest, UserMessage: "at least one filter criteria (searchKey or status) is required for a filter based bulk delete"} + } + // applying the same normalization the listing path (GetAllWithFilters) applies before resolving the + // query, so that filter resolution for delete matches filter resolution for listing exactly. + impl.userCommonService.SetDefaultValuesIfNotPresent(request, false) + setStatusFilterType(request) //query to get particular models respecting filters query, queryParams := helper.GetQueryForUserListingWithFilters(request) models, err := impl.userRepository.GetAllExecutingQuery(query, queryParams) diff --git a/pkg/auth/user/UserService_bulkdelete_filter_test.go b/pkg/auth/user/UserService_bulkdelete_filter_test.go new file mode 100644 index 0000000000..85aecd4654 --- /dev/null +++ b/pkg/auth/user/UserService_bulkdelete_filter_test.go @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2024. Devtron Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package user + +import ( + "testing" + + "github.com/devtron-labs/devtron/pkg/auth/user/bean" + "github.com/devtron-labs/devtron/pkg/auth/user/repository" + "github.com/devtron-labs/devtron/pkg/auth/user/repository/helper" + "github.com/stretchr/testify/assert" +) + +// fakeUserRepoForFilterTest is a minimal repository.UserRepository fake that only answers +// GetAllExecutingQuery - the single repository call getUserIdsHonoringFilters makes. Every other +// method is left to panic on the embedded nil interface, since these tests never expect them to run. +type fakeUserRepoForFilterTest struct { + repository.UserRepository + called bool + gotQuery string + gotParams []interface{} + modelsToRet []repository.UserModel +} + +func (f *fakeUserRepoForFilterTest) GetAllExecutingQuery(query string, queryParams []interface{}) ([]repository.UserModel, error) { + f.called = true + f.gotQuery = query + f.gotParams = queryParams + return f.modelsToRet, nil +} + +func newTestUserServiceForFilterTest(repo *fakeUserRepoForFilterTest) *UserServiceImpl { + return &UserServiceImpl{ + userRepository: repo, + userCommonService: &UserCommonServiceImpl{}, + } +} + +// TestGetUserIdsHonoringFilters_RejectsUnscopedFilter is a regression test for the production incident +// where `DELETE /orchestrator/user/bulk` with body {"listingRequest":{"searchKey":"","status":["inactive"]}} +// (an empty search key, intended to be narrowed down by status) deleted every active user on the platform +// instead of only the intended inactive ones. The delete path resolved the filter directly through the +// query builder without the normalization/bound the listing endpoint applies, so an effectively empty +// filter silently widened into "every user". With the fix, a listingRequest carrying no concrete filter +// (status isn't a filterable field in this OSS build) is rejected outright instead of being resolved at all. +func TestGetUserIdsHonoringFilters_RejectsUnscopedFilter(t *testing.T) { + repo := &fakeUserRepoForFilterTest{} + impl := newTestUserServiceForFilterTest(repo) + + // mirrors the exact incident payload once decoded: SearchKey is empty and there is no other + // concrete filter field available on this build's ListingRequest. + request := &bean.ListingRequest{SearchKey: ""} + + ids, err := impl.getUserIdsHonoringFilters(request) + + assert.Error(t, err) + assert.Nil(t, ids) + assert.False(t, repo.called, "no query should ever be executed for an unscoped filter based bulk delete") +} + +// TestGetUserIdsHonoringFilters_MatchesListingQuery asserts that once a concrete filter is supplied, the +// query getUserIdsHonoringFilters resolves is identical - query text and params - to the query the listing +// endpoint (GetAllWithFilters) would build for the exact same input. This is the core of the fix: delete-side +// filter resolution can no longer diverge from listing-side filter resolution (e.g. via a missing default +// page size), so the delete path's resolved id set only ever contains records actually matching the filter. +func TestGetUserIdsHonoringFilters_MatchesListingQuery(t *testing.T) { + matchingUser := repository.UserModel{Id: 42, EmailId: "inactive-user@example.com"} + repo := &fakeUserRepoForFilterTest{modelsToRet: []repository.UserModel{matchingUser}} + impl := newTestUserServiceForFilterTest(repo) + + deleteRequest := &bean.ListingRequest{SearchKey: "inactive-user"} + ids, err := impl.getUserIdsHonoringFilters(deleteRequest) + assert.NoError(t, err) + assert.Equal(t, []int32{42}, ids) + assert.True(t, repo.called) + + // Build the query the listing endpoint would run for the exact same input, applying the exact + // same normalization GetAllWithFilters applies before calling the shared query builder. + listingRequest := &bean.ListingRequest{SearchKey: "inactive-user"} + (&UserCommonServiceImpl{}).SetDefaultValuesIfNotPresent(listingRequest, false) + setStatusFilterType(listingRequest) + listingQuery, listingParams := helper.GetQueryForUserListingWithFilters(listingRequest) + + assert.Equal(t, listingQuery, repo.gotQuery, "delete path query must match listing path query for the same filter") + assert.Equal(t, listingParams, repo.gotParams, "delete path query params must match listing path query params for the same filter") +} diff --git a/pkg/auth/user/UserService_ent.go b/pkg/auth/user/UserService_ent.go index f576f49da3..ab1dc4e6b8 100644 --- a/pkg/auth/user/UserService_ent.go +++ b/pkg/auth/user/UserService_ent.go @@ -119,6 +119,13 @@ func setStatusFilterType(request *userBean.ListingRequest) { return } +// isStatusFilterApplied reports whether the request carries a concrete status filter. +// the OSS ListingRequest has no status filtering concept, so this is always false here; +// an enterprise build that adds status filtering should override this accordingly. +func isStatusFilterApplied(request *userBean.ListingRequest) bool { + return false +} + func setCurrentTimeInUserInfo(request *userBean.ListingRequest) { return } From 1679300d2e101c321c328f393bf8d23dc4376f5b Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Fri, 11 Sep 2026 08:31:15 +0530 Subject: [PATCH 09/14] feat: resolve RBAC inconsistencies for helm apps with unique identifiers When an external helm app is linked to the chart store, app_name holds the unique identifier (--) while display_name holds the release name. RBAC policies are always created against the release name, since that is what the listing APIs expose to the user. EnforcerUtil did not account for this: - GetHelmObjectByAppNameAndEnvId looked the app up by app_name only, so a release-name lookup returned ErrNoRows and the object collapsed to "//", making the app invisible to any non super-admin user. - Both it and GetHelmObject built the object from application.AppName, i.e. the unique identifier, which never matched the granted policy. - Neither substituted the "unassigned" project when team_id is 0, so an app linked without a project produced an empty project segment. EnforcerUtilHelm.GetAppRBACNameByInstalledAppId and GetAppRBACNameByInstalledAppIdAndTeamId had the same raw AppName problem. Resolve the app the same way getAppObject already does: reconstruct the unique identifier from the environment and try that first, falling back to the plain name for regular chart store apps, legacy rows and callers that already pass the identifier. Emit the display name via the new App.GetAppNameForRbac(), and fall back to the unassigned project when no project is assigned. Fixing the primitives covers all downstream call sites without touching the handlers. Co-Authored-By: Claude Opus 5 (1M context) --- internal/sql/repository/app/AppRepository.go | 12 ++ util/rbac/EnforcerUtil.go | 72 +++++-- util/rbac/EnforcerUtilHelm.go | 8 +- util/rbac/EnforcerUtilHelmObject_test.go | 212 +++++++++++++++++++ 4 files changed, 285 insertions(+), 19 deletions(-) create mode 100644 util/rbac/EnforcerUtilHelmObject_test.go diff --git a/internal/sql/repository/app/AppRepository.go b/internal/sql/repository/app/AppRepository.go index d237ec8f7a..538490fbc9 100644 --- a/internal/sql/repository/app/AppRepository.go +++ b/internal/sql/repository/app/AppRepository.go @@ -53,6 +53,18 @@ func (app *App) IsAppJobOrExternalType() bool { return len(app.DisplayName) > 0 } +// GetAppNameForRbac returns the app name that rbac objects are built with. +// For an external helm app linked to chart store, app_name holds the unique identifier +// (--) while display_name holds the release name. Rbac policies are +// always created against the release name, as that is what is shown to the user, so display_name must +// win whenever it is set. +func (app *App) GetAppNameForRbac() string { + if len(app.DisplayName) > 0 { + return app.DisplayName + } + return app.AppName +} + type AppRepository interface { SaveWithTxn(pipelineGroup *App, tx *pg.Tx) error Update(app *App) error diff --git a/util/rbac/EnforcerUtil.go b/util/rbac/EnforcerUtil.go index 51dcd1aaaf..b619a9d2e0 100644 --- a/util/rbac/EnforcerUtil.go +++ b/util/rbac/EnforcerUtil.go @@ -18,6 +18,7 @@ package rbac import ( "fmt" + bean3 "github.com/devtron-labs/devtron/api/helm-app/service/bean" "github.com/devtron-labs/devtron/pkg/app/dbMigration" repository2 "github.com/devtron-labs/devtron/pkg/cluster/environment/repository" bean2 "github.com/devtron-labs/devtron/pkg/k8s/application/bean" @@ -464,6 +465,8 @@ func (impl EnforcerUtilImpl) GetHelmObject(appId int, envId int) (string, string impl.logger.Errorw("error on fetching data for rbac object", "err", err) return fmt.Sprintf("%s/%s/%s", "", "", ""), "" } + appNameForRbac := application.GetAppNameForRbac() + teamName := getTeamNameForHelmRbac(application) clusterName := env.Cluster.ClusterName namespace := env.Namespace @@ -486,31 +489,26 @@ func (impl EnforcerUtilImpl) GetHelmObject(appId int, envId int) (string, string }*/ if environmentIdentifier2 == "" { - return fmt.Sprintf("%s/%s/%s", application.Team.Name, environmentIdentifier, application.AppName), "" + return fmt.Sprintf("%s/%s/%s", teamName, environmentIdentifier, appNameForRbac), "" } - return fmt.Sprintf("%s/%s/%s", application.Team.Name, environmentIdentifier, application.AppName), - fmt.Sprintf("%s/%s/%s", application.Team.Name, environmentIdentifier2, application.AppName) + return fmt.Sprintf("%s/%s/%s", teamName, environmentIdentifier, appNameForRbac), + fmt.Sprintf("%s/%s/%s", teamName, environmentIdentifier2, appNameForRbac) } func (impl EnforcerUtilImpl) GetHelmObjectByAppNameAndEnvId(appName string, envId int) (string, string) { - application, err := impl.appRepo.FindAppAndProjectByAppName(appName) - if err == pg.ErrMultiRows { - application, err = impl.dbMigration.FixMultipleAppsForInstalledApp(appName) - if err != nil { - impl.logger.Errorw("error on fetching data for rbac object", "appName", appName, "err", err) - return fmt.Sprintf("%s/%s/%s", "", "", ""), "" - } - } + env, err := impl.environmentRepository.FindById(envId) if err != nil { impl.logger.Errorw("error on fetching data for rbac object", "err", err) return fmt.Sprintf("%s/%s/%s", "", "", ""), "" } - env, err := impl.environmentRepository.FindById(envId) + application, err := impl.getAppForHelmRbac(appName, env.ClusterId, env.Namespace) if err != nil { - impl.logger.Errorw("error on fetching data for rbac object", "err", err) + impl.logger.Errorw("error on fetching data for rbac object", "appName", appName, "err", err) return fmt.Sprintf("%s/%s/%s", "", "", ""), "" } + appNameForRbac := application.GetAppNameForRbac() + teamName := getTeamNameForHelmRbac(application) clusterName := env.Cluster.ClusterName namespace := env.Namespace environmentIdentifier := env.EnvironmentIdentifier @@ -524,7 +522,7 @@ func (impl EnforcerUtilImpl) GetHelmObjectByAppNameAndEnvId(appName string, envI } } if environmentIdentifier2 == "" { - return strings.ToLower(fmt.Sprintf("%s/%s/%s", application.Team.Name, environmentIdentifier, application.AppName)), "" + return strings.ToLower(fmt.Sprintf("%s/%s/%s", teamName, environmentIdentifier, appNameForRbac)), "" } //TODO - FIX required for futuristic permission for cluster__* all environment for migrated environment identifier only @@ -532,8 +530,50 @@ func (impl EnforcerUtilImpl) GetHelmObjectByAppNameAndEnvId(appName string, envI if !strings.HasPrefix(env.EnvironmentIdentifier, fmt.Sprintf("%s__", env.Cluster.ClusterName)) { environmentIdentifier = fmt.Sprintf("%s__%s", env.Cluster.ClusterName, env.EnvironmentIdentifier) }*/ - return strings.ToLower(fmt.Sprintf("%s/%s/%s", application.Team.Name, environmentIdentifier, application.AppName)), - strings.ToLower(fmt.Sprintf("%s/%s/%s", application.Team.Name, environmentIdentifier2, application.AppName)) + return strings.ToLower(fmt.Sprintf("%s/%s/%s", teamName, environmentIdentifier, appNameForRbac)), + strings.ToLower(fmt.Sprintf("%s/%s/%s", teamName, environmentIdentifier2, appNameForRbac)) +} + +// getTeamNameForHelmRbac returns the project name to be used while building a helm rbac object. +// An external helm app can be linked to chart store without a project being assigned to it, in which case +// team_id stays 0 and the joined Team row is empty. Permissions for such apps are granted under the +// "unassigned" project, so that literal has to be used instead of an empty project name. +func getTeamNameForHelmRbac(application *app.App) string { + if application.TeamId == 0 { + return repository3.UNASSIGNED_PROJECT + } + return application.Team.Name +} + +// getAppForHelmRbac resolves the app row for a helm rbac object, given a name that may either be the +// app_name stored in db or the display_name shown to the user. +// For an external helm app linked to chart store, app_name is the unique identifier +// (--) while callers here generally only have the release name, so the +// identifier is reconstructed from the environment and tried first. The plain-name lookup is kept as a +// fallback for regular chart store apps, for legacy rows created before the unique identifier existed, and +// for callers that already pass the identifier. +func (impl EnforcerUtilImpl) getAppForHelmRbac(appName string, clusterId int, namespace string) (*app.App, error) { + appIdentifier := &bean3.AppIdentifier{ + ClusterId: clusterId, + Namespace: namespace, + ReleaseName: appName, + } + uniqueAppNameIdentifier := appIdentifier.GetUniqueAppNameIdentifier() + application, err := impl.appRepo.FindAppAndProjectByAppName(uniqueAppNameIdentifier) + if err == pg.ErrMultiRows { + application, err = impl.dbMigration.FixMultipleAppsForInstalledApp(uniqueAppNameIdentifier) + } + if application == nil || err == pg.ErrNoRows { + impl.logger.Debugw("app not found by unique identifier, falling back to app name", "appIdentifier", uniqueAppNameIdentifier, "appName", appName) + application, err = impl.appRepo.FindAppAndProjectByAppName(appName) + if err == pg.ErrMultiRows { + application, err = impl.dbMigration.FixMultipleAppsForInstalledApp(appName) + } + } + if err != nil { + return nil, err + } + return application, nil } func (impl EnforcerUtilImpl) GetHelmObjectByProjectIdAndEnvId(teamId int, envId int) (string, string) { diff --git a/util/rbac/EnforcerUtilHelm.go b/util/rbac/EnforcerUtilHelm.go index 268092a6e9..1631db5223 100644 --- a/util/rbac/EnforcerUtilHelm.go +++ b/util/rbac/EnforcerUtilHelm.go @@ -177,7 +177,9 @@ func (impl EnforcerUtilHelmImpl) GetAppRBACNameByInstalledAppId(installedAppVers impl.logger.Errorw("error in fetching installed app version data", "err", err) return fmt.Sprintf("%s/%s/%s", "", "", ""), fmt.Sprintf("%s/%s/%s", "", "", "") } - rbacOne := fmt.Sprintf("%s/%s/%s", InstalledApp.App.Team.Name, InstalledApp.Environment.EnvironmentIdentifier, InstalledApp.App.AppName) + appNameForRbac := InstalledApp.App.GetAppNameForRbac() + teamName := getTeamNameForHelmRbac(&InstalledApp.App) + rbacOne := fmt.Sprintf("%s/%s/%s", teamName, InstalledApp.Environment.EnvironmentIdentifier, appNameForRbac) if InstalledApp.Environment.IsVirtualEnvironment { return rbacOne, "" @@ -186,7 +188,7 @@ func (impl EnforcerUtilHelmImpl) GetAppRBACNameByInstalledAppId(installedAppVers var rbacTwo string if !InstalledApp.Environment.IsVirtualEnvironment { if InstalledApp.Environment.EnvironmentIdentifier != InstalledApp.Environment.Cluster.ClusterName+"__"+InstalledApp.Environment.Namespace { - rbacTwo = fmt.Sprintf("%s/%s/%s", InstalledApp.App.Team.Name, InstalledApp.Environment.Cluster.ClusterName+"__"+InstalledApp.Environment.Namespace, InstalledApp.App.AppName) + rbacTwo = fmt.Sprintf("%s/%s/%s", teamName, InstalledApp.Environment.Cluster.ClusterName+"__"+InstalledApp.Environment.Namespace, appNameForRbac) return rbacOne, rbacTwo } } @@ -205,6 +207,6 @@ func (impl EnforcerUtilHelmImpl) GetAppRBACNameByInstalledAppIdAndTeamId(install impl.logger.Errorw("error in fetching project by teamID", "err", err) return fmt.Sprintf("%s/%s/%s", "", "", "") } - rbac := fmt.Sprintf("%s/%s/%s", project.Name, installedApp.Environment.EnvironmentIdentifier, installedApp.App.AppName) + rbac := fmt.Sprintf("%s/%s/%s", project.Name, installedApp.Environment.EnvironmentIdentifier, installedApp.App.GetAppNameForRbac()) return rbac } diff --git a/util/rbac/EnforcerUtilHelmObject_test.go b/util/rbac/EnforcerUtilHelmObject_test.go new file mode 100644 index 0000000000..b0416f5042 --- /dev/null +++ b/util/rbac/EnforcerUtilHelmObject_test.go @@ -0,0 +1,212 @@ +/* + * Copyright (c) 2020-2024. Devtron Inc. + */ + +package rbac + +import ( + "testing" + + "github.com/devtron-labs/devtron/internal/sql/repository/app" + environmentRepository "github.com/devtron-labs/devtron/pkg/cluster/environment/repository" + clusterRepository "github.com/devtron-labs/devtron/pkg/cluster/repository" + teamRepo "github.com/devtron-labs/devtron/pkg/team/repository" + "github.com/go-pg/pg" + "go.uber.org/zap" +) + +// stubAppRepo overrides only the lookups used while building helm rbac objects; every other method of the +// interface is left nil and will panic if a code path unexpectedly reaches it. +type stubAppRepo struct { + app.AppRepository + byName map[string]*app.App + byId map[int]*app.App +} + +func (s *stubAppRepo) FindAppAndProjectByAppName(appName string) (*app.App, error) { + if a, ok := s.byName[appName]; ok { + return a, nil + } + // mirrors the real repository, which returns an empty (non nil) app alongside ErrNoRows + return &app.App{}, pg.ErrNoRows +} + +func (s *stubAppRepo) FindAppAndProjectByAppId(appId int) (*app.App, error) { + if a, ok := s.byId[appId]; ok { + return a, nil + } + return &app.App{}, pg.ErrNoRows +} + +type stubEnvRepo struct { + environmentRepository.EnvironmentRepository + env *environmentRepository.Environment +} + +func (s *stubEnvRepo) FindById(id int) (*environmentRepository.Environment, error) { + if s.env == nil || s.env.Id != id { + return nil, pg.ErrNoRows + } + return s.env, nil +} + +const ( + testClusterName = "default_cluster" + testNamespace = "devtron-demo" + testEnvId = 1 + testClusterId = 1 +) + +// linkedExternalApp is how an external helm app looks in the app table once it is linked to chart store: +// app_name carries the unique identifier while display_name carries the release name. +func linkedExternalApp(id int, releaseName string, teamId int, teamName string) *app.App { + return &app.App{ + Id: id, + AppName: releaseName + "-" + testNamespace + "-1", + DisplayName: releaseName, + TeamId: teamId, + Team: teamRepo.Team{Name: teamName}, + } +} + +func newTestEnforcerUtil(envIdentifier string) *EnforcerUtilImpl { + nginx := linkedExternalApp(1, "nginx", 2, "myproject") + // linked with no project assigned -> team_id stays 0 and the joined team row is empty + redis := linkedExternalApp(2, "redis", 0, "") + // a regular chart store app, which has no display name at all + grafana := &app.App{Id: 3, AppName: "grafana", TeamId: 2, Team: teamRepo.Team{Name: "myproject"}} + + appRepo := &stubAppRepo{ + byName: map[string]*app.App{ + nginx.AppName: nginx, + redis.AppName: redis, + grafana.AppName: grafana, + }, + byId: map[int]*app.App{1: nginx, 2: redis, 3: grafana}, + } + envRepo := &stubEnvRepo{env: &environmentRepository.Environment{ + Id: testEnvId, + Name: testNamespace, + ClusterId: testClusterId, + Namespace: testNamespace, + EnvironmentIdentifier: envIdentifier, + Cluster: &clusterRepository.Cluster{Id: testClusterId, ClusterName: testClusterName}, + }} + return &EnforcerUtilImpl{logger: zap.NewNop().Sugar(), appRepo: appRepo, environmentRepository: envRepo} +} + +func TestGetHelmObjectByAppNameAndEnvId(t *testing.T) { + envIdentifier := testClusterName + "__" + testNamespace + tests := []struct { + name string + appName string + wantObject string + wantObject2 string + }{ + { + name: "external app linked to chart store is resolved by its display name", + // this is the regression: the listing APIs hand over the release name, but app_name in db holds + // the unique identifier, so the lookup used to miss and the object came back as "//" + appName: "nginx", + wantObject: "myproject/" + envIdentifier + "/nginx", + }, + { + name: "external app linked without a project falls under the unassigned project", + appName: "redis", + wantObject: teamRepo.UNASSIGNED_PROJECT + "/" + envIdentifier + "/redis", + }, + { + name: "regular chart store app keeps working via the plain app name", + appName: "grafana", + wantObject: "myproject/" + envIdentifier + "/grafana", + }, + { + name: "caller passing the unique identifier still gets the display name based object", + // some handlers hand over installedApp.App.AppName directly + appName: "nginx-" + testNamespace + "-1", + wantObject: "myproject/" + envIdentifier + "/nginx", + }, + { + name: "unknown app yields an empty object", + appName: "does-not-exist", + wantObject: "//", + }, + } + impl := newTestEnforcerUtil(envIdentifier) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + object, object2 := impl.GetHelmObjectByAppNameAndEnvId(tt.appName, testEnvId) + if object != tt.wantObject { + t.Errorf("object = %q, want %q", object, tt.wantObject) + } + if object2 != tt.wantObject2 { + t.Errorf("object2 = %q, want %q", object2, tt.wantObject2) + } + }) + } +} + +// When the environment identifier is not clusterName__namespace, both the migrated and the futuristic +// permission objects are returned, and each has to carry the display name. +func TestGetHelmObjectByAppNameAndEnvId_FuturisticPermissionObject(t *testing.T) { + impl := newTestEnforcerUtil("devtron-demo") + object, object2 := impl.GetHelmObjectByAppNameAndEnvId("nginx", testEnvId) + if want := "myproject/devtron-demo/nginx"; object != want { + t.Errorf("object = %q, want %q", object, want) + } + if want := "myproject/" + testClusterName + "__" + testNamespace + "/nginx"; object2 != want { + t.Errorf("object2 = %q, want %q", object2, want) + } +} + +// GetHelmObjectByAppNameAndEnvId lowercases its objects, so a mixed case release name still has to line up +// with the policy that was stored for it. +func TestGetHelmObjectByAppNameAndEnvId_LowerCased(t *testing.T) { + envIdentifier := testClusterName + "__" + testNamespace + mixed := &app.App{ + Id: 4, + AppName: "MyRelease-" + testNamespace + "-1", + DisplayName: "MyRelease", + TeamId: 2, + Team: teamRepo.Team{Name: "MyProject"}, + } + impl := newTestEnforcerUtil(envIdentifier) + impl.appRepo.(*stubAppRepo).byName[mixed.AppName] = mixed + + object, _ := impl.GetHelmObjectByAppNameAndEnvId("MyRelease", testEnvId) + if want := "myproject/" + envIdentifier + "/myrelease"; object != want { + t.Errorf("object = %q, want %q", object, want) + } +} + +func TestGetHelmObject(t *testing.T) { + envIdentifier := testClusterName + "__" + testNamespace + impl := newTestEnforcerUtil(envIdentifier) + + // app id lookup always succeeded, but the object used to be built from app_name (the unique identifier) + object, _ := impl.GetHelmObject(1, testEnvId) + if want := "myproject/" + envIdentifier + "/nginx"; object != want { + t.Errorf("object = %q, want %q", object, want) + } + + object, _ = impl.GetHelmObject(2, testEnvId) + if want := teamRepo.UNASSIGNED_PROJECT + "/" + envIdentifier + "/redis"; object != want { + t.Errorf("object = %q, want %q", object, want) + } + + object, _ = impl.GetHelmObject(3, testEnvId) + if want := "myproject/" + envIdentifier + "/grafana"; object != want { + t.Errorf("object = %q, want %q", object, want) + } +} + +func TestGetAppNameForRbac(t *testing.T) { + external := &app.App{AppName: "nginx-devtron-demo-1", DisplayName: "nginx"} + if got := external.GetAppNameForRbac(); got != "nginx" { + t.Errorf("GetAppNameForRbac() = %q, want %q", got, "nginx") + } + regular := &app.App{AppName: "grafana"} + if got := regular.GetAppNameForRbac(); got != "grafana" { + t.Errorf("GetAppNameForRbac() = %q, want %q", got, "grafana") + } +} From 81c8bcf9a9f5e780d7fa38230d5867befb802fd8 Mon Sep 17 00:00:00 2001 From: Prakash Kumar Date: Mon, 14 Sep 2026 16:26:58 +0530 Subject: [PATCH 10/14] Conditions saved with the plugin definition (plugin_step_condition) are intentionally not sent for the plugin's internal steps. --- .../PipelineStagePipelineConditions_test.go | 75 ++++++ .../PipelineStagePluginConditions_test.go | 230 ++++++++++++++++++ pkg/pipeline/PipelineStageService.go | 47 ++-- 3 files changed, 319 insertions(+), 33 deletions(-) create mode 100644 pkg/pipeline/PipelineStagePipelineConditions_test.go create mode 100644 pkg/pipeline/PipelineStagePluginConditions_test.go diff --git a/pkg/pipeline/PipelineStagePipelineConditions_test.go b/pkg/pipeline/PipelineStagePipelineConditions_test.go new file mode 100644 index 0000000000..396175f225 --- /dev/null +++ b/pkg/pipeline/PipelineStagePipelineConditions_test.go @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2024. Devtron Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package pipeline + +import ( + "testing" + + "github.com/devtron-labs/devtron/pkg/pipeline/repository" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +type fakePipelineStageRepoForWfRequest struct { + repository.PipelineStageRepository + variables []*repository.PipelineStageStepVariable + conditions []*repository.PipelineStageStepCondition +} + +func (f *fakePipelineStageRepoForWfRequest) GetVariablesByStepId(stepId int) ([]*repository.PipelineStageStepVariable, error) { + return f.variables, nil +} + +func (f *fakePipelineStageRepoForWfRequest) GetConditionsByStepId(stepId int) ([]*repository.PipelineStageStepCondition, error) { + return f.conditions, nil +} + +// Regression guard for the plugin-condition change: the conditions saved on the pipeline step +// (pipeline_stage_step_condition) are the ones that must keep reaching ci-runner. +func TestBuildVariableAndConditionDataForWfRequest_PipelineLevelConditionsAreSent(t *testing.T) { + repo := &fakePipelineStageRepoForWfRequest{ + variables: []*repository.PipelineStageStepVariable{ + {Id: 198, PipelineStageStepId: 225, Name: "FILE_PATH", Format: repository.PIPELINE_STAGE_STEP_VARIABLE_FORMAT_TYPE_STRING, VariableType: repository.PIPELINE_STAGE_STEP_VARIABLE_TYPE_INPUT, ValueType: repository.PIPELINE_STAGE_STEP_VARIABLE_VALUE_TYPE_NEW, Value: "/tmp/other", AllowEmptyValue: true, IsExposed: true, VariableStepIndexInPlugin: 1}, + {Id: 199, PipelineStageStepId: 225, Name: "CONTENT", Format: repository.PIPELINE_STAGE_STEP_VARIABLE_FORMAT_TYPE_STRING, VariableType: repository.PIPELINE_STAGE_STEP_VARIABLE_TYPE_OUTPUT, ValueType: repository.PIPELINE_STAGE_STEP_VARIABLE_VALUE_TYPE_NEW, IsExposed: true, VariableStepIndexInPlugin: 1}, + }, + conditions: []*repository.PipelineStageStepCondition{ + {Id: 15, PipelineStageStepId: 225, ConditionVariableId: 198, ConditionType: repository.PIPELINE_STAGE_STEP_CONDITION_TYPE_TRIGGER, ConditionalOperator: "==", ConditionalValue: "/tmp/other"}, + {Id: 16, PipelineStageStepId: 225, ConditionVariableId: 199, ConditionType: repository.PIPELINE_STAGE_STEP_CONDITION_TYPE_SUCCESS, ConditionalOperator: "==", ConditionalValue: "ok"}, + {Id: 17, PipelineStageStepId: 225, ConditionVariableId: 0, ConditionType: repository.PIPELINE_STAGE_STEP_CONDITION_TYPE_SKIP, ConditionalOperator: "!=", ConditionalValue: "x"}, + }, + } + impl := &PipelineStageServiceImpl{logger: zap.NewNop().Sugar(), pipelineStageRepository: repo} + data, err := impl.buildVariableAndConditionDataForWfRequest(225) + assert.NoError(t, err) + if assert.Len(t, data.GetInputVariables(), 1) { + assert.Equal(t, "/tmp/other", data.GetInputVariables()[0].Value) + assert.Equal(t, 1, data.GetInputVariables()[0].VariableStepIndexInPlugin) + } + assert.Len(t, data.GetOutputVariables(), 1) + triggerSkip := data.GetTriggerSkipConditions() + if assert.Len(t, triggerSkip, 2) { + assert.Equal(t, "FILE_PATH", triggerSkip[0].ConditionOnVariable) + assert.Equal(t, "TRIGGER", triggerSkip[0].ConditionType) + assert.Equal(t, "==", triggerSkip[0].ConditionalOperator) + assert.Equal(t, "/tmp/other", triggerSkip[0].ConditionalValue) + assert.Equal(t, "", triggerSkip[1].ConditionOnVariable, "condition on an unknown variable id is sent without a variable name (pre-existing behaviour)") + } + if assert.Len(t, data.GetSuccessFailureConditions(), 1) { + assert.Equal(t, "CONTENT", data.GetSuccessFailureConditions()[0].ConditionOnVariable) + assert.Equal(t, "PASS", data.GetSuccessFailureConditions()[0].ConditionType) + } +} diff --git a/pkg/pipeline/PipelineStagePluginConditions_test.go b/pkg/pipeline/PipelineStagePluginConditions_test.go new file mode 100644 index 0000000000..5169fd2c51 --- /dev/null +++ b/pkg/pipeline/PipelineStagePluginConditions_test.go @@ -0,0 +1,230 @@ +/* + * Copyright (c) 2024. Devtron Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package pipeline + +import ( + "errors" + "testing" + + commonBean "github.com/devtron-labs/common-lib/workflow" + repository2 "github.com/devtron-labs/devtron/pkg/plugin/repository" + "github.com/go-pg/pg" + "github.com/stretchr/testify/assert" + "go.uber.org/zap" +) + +// fakePluginRepoForWfRequest serves a plugin definition (steps, script, variables, conditions) to the workflow request +// builders. Any repository method not overridden here panics if called, which is intended. +type fakePluginRepoForWfRequest struct { + repository2.GlobalPluginRepository + steps []*repository2.PluginStep + scripts map[int]*repository2.PluginPipelineScript + variables map[int][]*repository2.PluginStepVariable + variablesErr error + conditions map[int][]*repository2.PluginStepCondition + conditionCalls int +} + +func (f *fakePluginRepoForWfRequest) GetStepsByPluginIds(pluginIds []int) ([]*repository2.PluginStep, error) { + return f.steps, nil +} + +func (f *fakePluginRepoForWfRequest) GetScriptDetailById(id int) (*repository2.PluginPipelineScript, error) { + if script, ok := f.scripts[id]; ok { + return script, nil + } + return nil, pg.ErrNoRows +} + +func (f *fakePluginRepoForWfRequest) GetScriptMappingDetailByScriptId(scriptId int) ([]*repository2.ScriptPathArgPortMapping, error) { + return nil, nil +} + +func (f *fakePluginRepoForWfRequest) GetVariablesByStepId(stepId int) ([]*repository2.PluginStepVariable, error) { + if f.variablesErr != nil { + return nil, f.variablesErr + } + return f.variables[stepId], nil +} + +func (f *fakePluginRepoForWfRequest) GetConditionsByStepId(stepId int) ([]*repository2.PluginStepCondition, error) { + f.conditionCalls++ + return f.conditions[stepId], nil +} + +func newPluginConditionsTestService(repo *fakePluginRepoForWfRequest) *PipelineStageServiceImpl { + return &PipelineStageServiceImpl{logger: zap.NewNop().Sugar(), globalPluginRepository: repo} +} + +func pluginStepVariable(name string, variableType repository2.PluginStepVariableType, exposed bool, value, defaultValue string) *repository2.PluginStepVariable { + return &repository2.PluginStepVariable{ + Id: 1, + Name: name, + Format: repository2.PLUGIN_VARIABLE_FORMAT_TYPE_STRING, + VariableType: variableType, + ValueType: repository2.PLUGIN_VARIABLE_VALUE_TYPE_NEW, + IsExposed: exposed, + Value: value, + DefaultValue: defaultValue, + VariableStepIndex: 1, + } +} + +// pluginWithConditions models a custom plugin saved from the dashboard: one shell step, an exposed input variable +// FILE_PATH with a TRIGGER condition on it and an exposed output variable CONTENT with a PASS condition on it. +func pluginWithConditions() *fakePluginRepoForWfRequest { + input := pluginStepVariable("FILE_PATH", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "/tmp/shivam", "") + input.Id = 386 + output := pluginStepVariable("CONTENT", repository2.PLUGIN_VARIABLE_TYPE_OUTPUT, true, "", "") + output.Id = 387 + return &fakePluginRepoForWfRequest{ + steps: []*repository2.PluginStep{ + {Id: 69, PluginId: 71, Name: "Task 1", Index: 1, StepType: repository2.PLUGIN_STEP_TYPE_INLINE, ScriptId: 5}, + }, + scripts: map[int]*repository2.PluginPipelineScript{5: {Type: repository2.SCRIPT_TYPE_SHELL, Script: "echo hi > $FILE_PATH"}}, + variables: map[int][]*repository2.PluginStepVariable{69: {input, output}}, + conditions: map[int][]*repository2.PluginStepCondition{69: { + {Id: 23, PluginStepId: 69, ConditionVariableId: 386, ConditionType: repository2.PLUGIN_CONDITION_TYPE_TRIGGER, ConditionalOperator: "==", ConditionalValue: "/tmp/shivam"}, + {Id: 24, PluginStepId: 69, ConditionVariableId: 387, ConditionType: repository2.PLUGIN_CONDITION_TYPE_SUCCESS, ConditionalOperator: "==", ConditionalValue: "ok"}, + }}, + } +} + +func TestBuildPluginStepDataForWfRequest_PluginConditionsAreNotSentToCiRunner(t *testing.T) { + t.Run("inline plugin step: variables and script are sent, plugin-defined conditions are not", func(t *testing.T) { + repo := pluginWithConditions() + stepData, err := newPluginConditionsTestService(repo).BuildPluginStepDataForWfRequest(repo.steps[0]) + assert.NoError(t, err) + assert.Equal(t, "Task 1", stepData.Name) + assert.Equal(t, 1, stepData.Index) + assert.Equal(t, string(repository2.PLUGIN_STEP_TYPE_INLINE), stepData.StepType) + assert.Equal(t, string(repository2.SCRIPT_TYPE_SHELL), stepData.ExecutorType) + assert.Equal(t, "echo hi > $FILE_PATH", stepData.Script) + if assert.Len(t, stepData.InputVars, 1) { + assert.Equal(t, "FILE_PATH", stepData.InputVars[0].Name) + assert.Equal(t, "/tmp/shivam", stepData.InputVars[0].Value) + } + if assert.Len(t, stepData.OutputVars, 1) { + assert.Equal(t, "CONTENT", stepData.OutputVars[0].Name) + } + assert.Nil(t, stepData.TriggerSkipConditions, "plugin-level trigger/skip conditions must not reach ci-runner") + assert.Nil(t, stepData.SuccessFailureConditions, "plugin-level pass/fail conditions must not reach ci-runner") + assert.Equal(t, 0, repo.conditionCalls, "plugin_step_condition must not even be read for the workflow request") + }) + t.Run("plugin step referencing another plugin: no conditions either", func(t *testing.T) { + repo := pluginWithConditions() + nested := &repository2.PluginStep{Id: 69, PluginId: 71, Name: "nested", Index: 1, StepType: repository2.PLUGIN_STEP_TYPE_REF_PLUGIN, RefPluginId: 42} + stepData, err := newPluginConditionsTestService(repo).BuildPluginStepDataForWfRequest(nested) + assert.NoError(t, err) + assert.Equal(t, "PLUGIN", stepData.ExecutorType) + assert.Equal(t, 42, stepData.RefPluginId) + assert.Len(t, stepData.InputVars, 1) + assert.Nil(t, stepData.TriggerSkipConditions) + assert.Nil(t, stepData.SuccessFailureConditions) + assert.Equal(t, 0, repo.conditionCalls) + }) + t.Run("variable lookup error is propagated", func(t *testing.T) { + repo := pluginWithConditions() + repo.variablesErr = errors.New("db down") + stepData, err := newPluginConditionsTestService(repo).BuildPluginStepDataForWfRequest(repo.steps[0]) + assert.EqualError(t, err, "db down") + assert.Nil(t, stepData) + }) +} + +func TestGetRefPluginStepsByIds_NoPluginConditionOnAnyStep(t *testing.T) { + repo := pluginWithConditions() + pluginIdStepsMap, err := newPluginConditionsTestService(repo).GetRefPluginStepsByIds([]int{71}, map[int]bool{71: true}) + assert.NoError(t, err) + if assert.Len(t, pluginIdStepsMap[71], 1) { + assert.Nil(t, pluginIdStepsMap[71][0].TriggerSkipConditions) + assert.Nil(t, pluginIdStepsMap[71][0].SuccessFailureConditions) + assert.Len(t, pluginIdStepsMap[71][0].InputVars, 1) + } + assert.Equal(t, 0, repo.conditionCalls) +} + +func TestBuildPluginVariableDataForWfRequest(t *testing.T) { + newRepo := func(vars ...*repository2.PluginStepVariable) *fakePluginRepoForWfRequest { + return &fakePluginRepoForWfRequest{variables: map[int][]*repository2.PluginStepVariable{69: vars}} + } + t.Run("exposed input without default uses the stored value, non-exposed input uses its default", func(t *testing.T) { + exposed := pluginStepVariable("A", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "user", "") + exposed.VariableStepIndexInPlugin = 3 // only meaningful for plugins nested in plugins; must be passed through untouched + internal := pluginStepVariable("B", repository2.PLUGIN_VARIABLE_TYPE_INPUT, false, "ignored", "default") + exposedWithDefault := pluginStepVariable("C", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "", "default") + inputs, outputs, err := newPluginConditionsTestService(newRepo(exposed, internal, exposedWithDefault)).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + assert.Empty(t, outputs) + if assert.Len(t, inputs, 3) { + assert.Equal(t, "user", inputs[0].Value) + assert.Equal(t, "default", inputs[1].Value) + assert.Equal(t, "", inputs[2].Value, "exposed variable value is filled from the pipeline step by ci-runner") + assert.Equal(t, commonBean.VariableTypeValue, inputs[0].VariableType) + assert.Equal(t, commonBean.FormatTypeString, inputs[0].Format) + assert.Equal(t, 3, inputs[0].VariableStepIndexInPlugin, "nested-plugin step index is passed through") + assert.Equal(t, 0, inputs[1].VariableStepIndexInPlugin) + } + }) + t.Run("value types map to ci-runner variable types", func(t *testing.T) { + global := pluginStepVariable("G", repository2.PLUGIN_VARIABLE_TYPE_INPUT, false, "", "") + global.ValueType = repository2.PLUGIN_VARIABLE_VALUE_TYPE_GLOBAL + global.ReferenceVariableName = "DOCKER_IMAGE" + previous := pluginStepVariable("P", repository2.PLUGIN_VARIABLE_TYPE_INPUT, false, "", "") + previous.ValueType = repository2.PLUGIN_VARIABLE_VALUE_TYPE_PREVIOUS + previous.PreviousStepIndex = 1 + previous.ReferenceVariableName = "OUT" + exposedPrevious := pluginStepVariable("E", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "", "") + exposedPrevious.ValueType = repository2.PLUGIN_VARIABLE_VALUE_TYPE_PREVIOUS + inputs, _, err := newPluginConditionsTestService(newRepo(global, previous, exposedPrevious)).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + if assert.Len(t, inputs, 3) { + assert.Equal(t, commonBean.VariableTypeRefGlobal, inputs[0].VariableType) + assert.Equal(t, "DOCKER_IMAGE", inputs[0].ReferenceVariableName) + assert.Equal(t, commonBean.VariableTypeRefPlugin, inputs[1].VariableType) + assert.Equal(t, 1, inputs[1].ReferenceVariableStepIndex) + assert.Equal(t, commonBean.VariableTypeValue, inputs[2].VariableType, "exposed previous-step variables are resolved at pipeline level") + } + }) + t.Run("output variables are returned separately", func(t *testing.T) { + out := pluginStepVariable("OUT", repository2.PLUGIN_VARIABLE_TYPE_OUTPUT, true, "", "") + inputs, outputs, err := newPluginConditionsTestService(newRepo(out)).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + assert.Empty(t, inputs) + if assert.Len(t, outputs, 1) { + assert.Equal(t, "OUT", outputs[0].Name) + } + }) + t.Run("no variables (pg.ErrNoRows) yields empty lists", func(t *testing.T) { + repo := &fakePluginRepoForWfRequest{variablesErr: pg.ErrNoRows} + inputs, outputs, err := newPluginConditionsTestService(repo).BuildPluginVariableDataForWfRequest(69) + assert.NoError(t, err) + assert.Empty(t, inputs) + assert.Empty(t, outputs) + }) + t.Run("repository error is propagated", func(t *testing.T) { + repo := &fakePluginRepoForWfRequest{variablesErr: errors.New("db down")} + _, _, err := newPluginConditionsTestService(repo).BuildPluginVariableDataForWfRequest(69) + assert.EqualError(t, err, "db down") + }) + t.Run("invalid variable format is an error", func(t *testing.T) { + bad := pluginStepVariable("X", repository2.PLUGIN_VARIABLE_TYPE_INPUT, true, "", "") + bad.Format = "BOGUS" + _, _, err := newPluginConditionsTestService(newRepo(bad)).BuildPluginVariableDataForWfRequest(69) + assert.Error(t, err) + }) +} diff --git a/pkg/pipeline/PipelineStageService.go b/pkg/pipeline/PipelineStageService.go index a3a2812c1d..c3e0449a37 100644 --- a/pkg/pipeline/PipelineStageService.go +++ b/pkg/pipeline/PipelineStageService.go @@ -2118,34 +2118,38 @@ func (impl *PipelineStageServiceImpl) BuildPluginStepDataForWfRequest(step *repo stepData.ExecutorType = "PLUGIN" //added only to avoid un-marshaling issues at ci-runner side, will not be used stepData.RefPluginId = step.RefPluginId } - inputVars, outputVars, triggerSkipConditions, successFailureConditions, err := impl.BuildPluginVariableAndConditionDataForWfRequest(step.Id) + inputVars, outputVars, err := impl.BuildPluginVariableDataForWfRequest(step.Id) if err != nil { - impl.logger.Errorw("error in getting variable and conditions data for wf request", "err", err, "stepId", step.Id) + impl.logger.Errorw("error in getting variable data for wf request", "err", err, "stepId", step.Id) return nil, err } stepData.InputVars = inputVars stepData.OutputVars = outputVars - stepData.TriggerSkipConditions = triggerSkipConditions - stepData.SuccessFailureConditions = successFailureConditions + // Conditions saved with the plugin definition (plugin_step_condition) are intentionally not sent for the + // plugin's internal steps. They are only defaults that pre-fill the task when the plugin is attached to a + // pipeline; the conditions to evaluate at trigger time are the ones saved on the pipeline step + // (pipeline_stage_step_condition), which are sent with the pre/post stage step referencing this plugin. return stepData, nil } -func (impl *PipelineStageServiceImpl) BuildPluginVariableAndConditionDataForWfRequest(stepId int) ([]*commonBean.VariableObject, []*commonBean.VariableObject, []*bean.ConditionObject, []*bean.ConditionObject, error) { +// BuildPluginVariableDataForWfRequest builds the input and output variables of a plugin's internal step for the +// workflow request. Plugin step conditions (plugin_step_condition) are deliberately not part of the workflow request: +// they are defaults used to pre-fill a task when the plugin is attached to a pipeline, and the effective conditions +// are the ones saved on the pipeline step (pipeline_stage_step_condition). +func (impl *PipelineStageServiceImpl) BuildPluginVariableDataForWfRequest(stepId int) ([]*commonBean.VariableObject, []*commonBean.VariableObject, error) { var inputVariables []*commonBean.VariableObject var outputVariables []*commonBean.VariableObject //getting all variables in the step variables, err := impl.globalPluginRepository.GetVariablesByStepId(stepId) if err != nil && err != pg.ErrNoRows { impl.logger.Errorw("error in getting variables by stepId", "err", err, "stepId", stepId) - return nil, nil, nil, nil, err + return nil, nil, err } - variableNameIdMap := make(map[int]string) for _, variable := range variables { - variableNameIdMap[variable.Id] = variable.Name format, err := commonBean.NewFormat(variable.Format.String()) if err != nil { impl.logger.Errorw("error in creating format object", "err", err, "format", variable.Format) - return nil, nil, nil, nil, err + return nil, nil, err } variableData := &commonBean.VariableObject{ Name: variable.Name, @@ -2181,30 +2185,7 @@ func (impl *PipelineStageServiceImpl) BuildPluginVariableAndConditionDataForWfRe outputVariables = append(outputVariables, variableData) } } - conditions, err := impl.globalPluginRepository.GetConditionsByStepId(stepId) - if err != nil && err != pg.ErrNoRows { - impl.logger.Errorw("error in getting conditions by stepId", "err", err, "stepId", stepId) - return nil, nil, nil, nil, err - } - var triggerSkipConditions []*bean.ConditionObject - var successFailureConditions []*bean.ConditionObject - for _, condition := range conditions { - conditionData := &bean.ConditionObject{ - ConditionalOperator: condition.ConditionalOperator, - ConditionalValue: condition.ConditionalValue, - ConditionType: string(condition.ConditionType), - } - varName, ok := variableNameIdMap[condition.ConditionVariableId] - if ok { - conditionData.ConditionOnVariable = varName - } - if condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_TRIGGER || condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_SKIP { - triggerSkipConditions = append(triggerSkipConditions, conditionData) - } else if condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_SUCCESS || condition.ConditionType == repository2.PLUGIN_CONDITION_TYPE_FAIL { - successFailureConditions = append(successFailureConditions, conditionData) - } - } - return inputVariables, outputVariables, triggerSkipConditions, successFailureConditions, nil + return inputVariables, outputVariables, nil } //BuildPrePostAndRefPluginStepsDataForWfRequest and related methods ends From bbb3ec7aa7d12620adb4f3c390f59aa2af98fdbf Mon Sep 17 00:00:00 2001 From: vivekanandan-devtron Date: Wed, 16 Sep 2026 12:19:24 +0530 Subject: [PATCH 11/14] feat: RBAC support for external argo/flux apps (#7014) * chore: create a makefile to setup and clean resources for development * feat: add flag for frontend to show argo/flux tabs - add migration files - append flags in the /orchestrator/user/check/roles API to show/hide argo/flux tabs * feat: changes for dropdown API in user permissions page * fix: missing dependencies in constructor method * fix: improper filtering logic - earlier filter was only applied with the cluster ID - add changes to filter based on cluster ID's and namespaces * fix: prevent connecting to kubelink for failed clusters * wip: policy enforcements * wip: policy enforcements * chore: remove test folder * fix: code review fixes * fix: code review fixes * fix: code review fixes * chore: rename migration scripts to maintain sync with enterprise * chore: change migration script number * fix: roles missing while fetching user roles * fix: deleted roles being listed * fix: deleted roles being listed * fix: review fixes --- .../ArgoApplicationRestHandler.go | 50 ++++-- api/auth/user/UserRestHandler.go | 33 +++- .../FluxApplicationRestHandler.go | 29 ++-- api/helm-app/wire_helmApp.go | 3 + .../application/k8sApplicationRestHandler.go | 77 ++++++--- api/userResource/bean/bean.go | 2 + cmd/external-app/wire_gen.go | 9 +- pkg/auth/authorisation/casbin/rbacpolicy.go | 6 + pkg/auth/user/UserCommonService.go | 29 +++- pkg/auth/user/bean/bean.go | 2 + .../user/repository/UserAuthRepository.go | 52 +++++- pkg/fluxApplication/FluxApplicationService.go | 152 +++++++++++++----- pkg/k8s/application/k8sApplicationService.go | 10 +- .../UserResourceExtendedService.go | 11 +- .../UserResourceRbacExtendedService.go | 18 +++ pkg/userResource/UserResourceRbacService.go | 7 +- pkg/userResource/UserResourceService.go | 88 +++++++--- pkg/userResource/adapter/adapter.go | 32 ++++ pkg/userResource/bean/bean.go | 39 +++-- pkg/userResource/bean/messages.go | 5 +- pkg/userResource/helper/helper.go | 52 +++++- pkg/userResource/logicRouteService.go | 25 ++- scripts/casbin/12_argo_flux_rbac.down.sql | 2 + scripts/casbin/12_argo_flux_rbac.up.sql | 3 + scripts/sql/36604600_argo_flux_rbac.down.sql | 2 + scripts/sql/36604600_argo_flux_rbac.up.sql | 111 +++++++++++++ util/rbac/EnforcerUtilGitOps.go | 74 +++++++++ wire_gen.go | 9 +- 28 files changed, 784 insertions(+), 148 deletions(-) create mode 100644 scripts/casbin/12_argo_flux_rbac.down.sql create mode 100644 scripts/casbin/12_argo_flux_rbac.up.sql create mode 100644 scripts/sql/36604600_argo_flux_rbac.down.sql create mode 100644 scripts/sql/36604600_argo_flux_rbac.up.sql create mode 100644 util/rbac/EnforcerUtilGitOps.go diff --git a/api/argoApplication/ArgoApplicationRestHandler.go b/api/argoApplication/ArgoApplicationRestHandler.go index edcbabdca9..5b816db2cd 100644 --- a/api/argoApplication/ArgoApplicationRestHandler.go +++ b/api/argoApplication/ArgoApplicationRestHandler.go @@ -21,8 +21,10 @@ import ( "errors" "github.com/devtron-labs/devtron/api/restHandler/common" "github.com/devtron-labs/devtron/pkg/argoApplication" + "github.com/devtron-labs/devtron/pkg/argoApplication/bean" "github.com/devtron-labs/devtron/pkg/argoApplication/read" "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin" + "github.com/devtron-labs/devtron/util/rbac" "go.uber.org/zap" "net/http" "strconv" @@ -39,26 +41,24 @@ type ArgoApplicationRestHandlerImpl struct { readService read.ArgoApplicationReadService logger *zap.SugaredLogger enforcer casbin.Enforcer + enforcerUtilGitOps rbac.EnforcerUtilGitOps } func NewArgoApplicationRestHandlerImpl(argoApplicationService argoApplication.ArgoApplicationService, - readService read.ArgoApplicationReadService, logger *zap.SugaredLogger, enforcer casbin.Enforcer) *ArgoApplicationRestHandlerImpl { + readService read.ArgoApplicationReadService, logger *zap.SugaredLogger, enforcer casbin.Enforcer, + enforcerUtilGitOps rbac.EnforcerUtilGitOps) *ArgoApplicationRestHandlerImpl { return &ArgoApplicationRestHandlerImpl{ argoApplicationService: argoApplicationService, readService: readService, logger: logger, enforcer: enforcer, + enforcerUtilGitOps: enforcerUtilGitOps, } } func (handler *ArgoApplicationRestHandlerImpl) ListApplications(w http.ResponseWriter, r *http.Request) { - // handle super-admin RBAC token := r.Header.Get("token") - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { - common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) - return - } v := r.URL.Query() clusterIdString := v.Get("clusterIds") var clusterIds []int @@ -80,16 +80,34 @@ func (handler *ArgoApplicationRestHandlerImpl) ListApplications(w http.ResponseW common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) return } - common.WriteJsonResp(w, nil, resp, http.StatusOK) + // RBAC enforcer applying: filter the listing to the applications the caller may see. + // Batched rather than a per-app Enforce loop; an app whose object cannot be built is dropped. + objects := make([]string, 0, len(resp)) + objectByApp := make(map[*bean.ArgoApplicationListDto]string, len(resp)) + for _, app := range resp { + object := handler.enforcerUtilGitOps.GetExternalGitOpsAppObjectByClusterName(app.ClusterName, app.Namespace, app.Name) + if len(object) == 0 { + continue + } + objectByApp[app] = object + objects = append(objects, object) + } + authorisedObjects := make(map[string]bool) + if len(objects) > 0 { + authorisedObjects = handler.enforcer.EnforceInBatch(token, casbin.ResourceArgoApp, casbin.ActionGet, objects) + } + authorisedApps := make([]*bean.ArgoApplicationListDto, 0, len(resp)) + for _, app := range resp { + if object, ok := objectByApp[app]; ok && authorisedObjects[strings.ToLower(object)] { + authorisedApps = append(authorisedApps, app) + } + } + //RBAC enforcer Ends + common.WriteJsonResp(w, nil, authorisedApps, http.StatusOK) } func (handler *ArgoApplicationRestHandlerImpl) GetApplicationDetail(w http.ResponseWriter, r *http.Request) { - // handle super-admin RBAC token := r.Header.Get("token") - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { - common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) - return - } ctx := r.Context() ctx = context.WithValue(ctx, "token", token) @@ -108,6 +126,14 @@ func (handler *ArgoApplicationRestHandlerImpl) GetApplicationDetail(w http.Respo return } } + // RBAC enforcer applying + object := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(clusterId, namespace, resourceName) + if len(object) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceArgoApp, casbin.ActionGet, object) { + common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) + return + } + //RBAC enforcer Ends + resp, err := handler.readService.GetAppDetailEA(ctx, resourceName, namespace, clusterId) if err != nil { handler.logger.Errorw("error in getting argo application app detail", "err", err, "resourceName", resourceName, "clusterId", clusterId) diff --git a/api/auth/user/UserRestHandler.go b/api/auth/user/UserRestHandler.go index 37188c14bd..8e617250b3 100644 --- a/api/auth/user/UserRestHandler.go +++ b/api/auth/user/UserRestHandler.go @@ -19,13 +19,14 @@ package user import ( "encoding/json" "errors" + "net/http" + "strconv" + "strings" + util2 "github.com/devtron-labs/devtron/api/auth/user/util" "github.com/devtron-labs/devtron/pkg/auth/user/helper" "github.com/devtron-labs/devtron/util/commonEnforcementFunctionsUtil" "github.com/gorilla/schema" - "net/http" - "strconv" - "strings" "github.com/devtron-labs/devtron/api/restHandler/common" "github.com/devtron-labs/devtron/internal/util" @@ -38,6 +39,16 @@ import ( "gopkg.in/go-playground/validator.v9" ) +const ( + resultKeyArgoAppAccess = "hasArgoAppAccess" + resultKeyFluxAppAccess = "hasFluxAppAccess" +) + +var gitOpsAccessResultKeyByResource = map[string]string{ + casbin.ResourceArgoApp: resultKeyArgoAppAccess, + casbin.ResourceFluxApp: resultKeyFluxAppAccess, +} + type UserRestHandler interface { CreateUser(w http.ResponseWriter, r *http.Request) UpdateUser(w http.ResponseWriter, r *http.Request) @@ -795,9 +806,25 @@ func (handler UserRestHandlerImpl) CheckUserRoles(w http.ResponseWriter, r *http result := make(map[string]interface{}) result["roles"] = roles result["superAdmin"] = false + result[resultKeyArgoAppAccess] = false + result[resultKeyFluxAppAccess] = false for _, item := range roles { if item == bean2.SUPERADMIN { result["superAdmin"] = true + result[resultKeyArgoAppAccess] = true + result[resultKeyFluxAppAccess] = true + continue + } + + roleFragments := strings.Split(item, "_") + resourceActionFragment := strings.Split(roleFragments[0], ":") + + if len(resourceActionFragment) < 2 { + continue + } + + if resultKey, ok := gitOpsAccessResultKeyByResource[resourceActionFragment[0]]; ok { + result[resultKey] = true } } common.WriteJsonResp(w, err, result, http.StatusOK) diff --git a/api/fluxApplication/FluxApplicationRestHandler.go b/api/fluxApplication/FluxApplicationRestHandler.go index f1ef3d616c..bc1b9c3c02 100644 --- a/api/fluxApplication/FluxApplicationRestHandler.go +++ b/api/fluxApplication/FluxApplicationRestHandler.go @@ -5,6 +5,7 @@ import ( "github.com/devtron-labs/devtron/api/restHandler/common" "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin" clientErrors "github.com/devtron-labs/devtron/pkg/errors" + "github.com/devtron-labs/devtron/util/rbac" "github.com/devtron-labs/devtron/pkg/fluxApplication" "github.com/gorilla/mux" "go.uber.org/zap" @@ -20,26 +21,34 @@ type FluxApplicationRestHandlerImpl struct { fluxApplicationService fluxApplication.FluxApplicationService logger *zap.SugaredLogger enforcer casbin.Enforcer + enforcerUtilGitOps rbac.EnforcerUtilGitOps } func NewFluxApplicationRestHandlerImpl(fluxApplicationService fluxApplication.FluxApplicationService, - logger *zap.SugaredLogger, enforcer casbin.Enforcer) *FluxApplicationRestHandlerImpl { + logger *zap.SugaredLogger, enforcer casbin.Enforcer, + enforcerUtilGitOps rbac.EnforcerUtilGitOps) *FluxApplicationRestHandlerImpl { return &FluxApplicationRestHandlerImpl{ fluxApplicationService: fluxApplicationService, logger: logger, enforcer: enforcer, + enforcerUtilGitOps: enforcerUtilGitOps, } } +// checkFluxAppAuth builds the RBAC object from the app identity and enforces on it. Passed into +// the service because the app list is streamed and cannot be filtered after the fact. +func (handler *FluxApplicationRestHandlerImpl) checkFluxAppAuth(token string, clusterName string, namespace string, appName string) bool { + object := handler.enforcerUtilGitOps.GetExternalGitOpsAppObjectByClusterName(clusterName, namespace, appName) + if len(object) == 0 { + return false + } + return handler.enforcer.Enforce(token, casbin.ResourceFluxApp, casbin.ActionGet, object) +} + func (handler *FluxApplicationRestHandlerImpl) ListFluxApplications(w http.ResponseWriter, r *http.Request) { - //handle super-admin RBAC token := r.Header.Get("token") - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { - common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) - return - } v := r.URL.Query() clusterIdString := v.Get("clusterIds") var clusterIds []int @@ -59,7 +68,7 @@ func (handler *FluxApplicationRestHandlerImpl) ListFluxApplications(w http.Respo return } handler.logger.Debugw("extracted ClusterIds successfully ", "clusterIds", clusterIds) - handler.fluxApplicationService.ListFluxApplications(r.Context(), clusterIds, noStream, w) + handler.fluxApplicationService.ListFluxApplications(r.Context(), clusterIds, noStream, w, token, handler.checkFluxAppAuth) } func (handler *FluxApplicationRestHandlerImpl) GetApplicationDetail(w http.ResponseWriter, r *http.Request) { @@ -76,12 +85,14 @@ func (handler *FluxApplicationRestHandlerImpl) GetApplicationDetail(w http.Respo return } - // handle super-admin RBAC + // RBAC enforcer applying token := r.Header.Get("token") - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { + object := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.Name) + if len(object) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceFluxApp, casbin.ActionGet, object) { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } + //RBAC enforcer Ends res, err := handler.fluxApplicationService.GetFluxAppDetail(r.Context(), appIdentifier) if err != nil { diff --git a/api/helm-app/wire_helmApp.go b/api/helm-app/wire_helmApp.go index 86f45e672c..af668990bb 100644 --- a/api/helm-app/wire_helmApp.go +++ b/api/helm-app/wire_helmApp.go @@ -41,4 +41,7 @@ var HelmAppWireSet = wire.NewSet( gRPC.GetConfig, rbac.NewEnforcerUtilHelmImpl, wire.Bind(new(rbac.EnforcerUtilHelm), new(*rbac.EnforcerUtilHelmImpl)), + + rbac.NewEnforcerUtilGitOpsImpl, + wire.Bind(new(rbac.EnforcerUtilGitOps), new(*rbac.EnforcerUtilGitOpsImpl)), ) diff --git a/api/k8s/application/k8sApplicationRestHandler.go b/api/k8s/application/k8sApplicationRestHandler.go index a3d64a7ff5..8417225c8e 100644 --- a/api/k8s/application/k8sApplicationRestHandler.go +++ b/api/k8s/application/k8sApplicationRestHandler.go @@ -23,6 +23,13 @@ import ( "encoding/json" "errors" "fmt" + "io" + "net/http" + "regexp" + "strconv" + "strings" + "time" + "github.com/devtron-labs/common-lib/utils" util3 "github.com/devtron-labs/common-lib/utils/k8s" k8sCommonBean "github.com/devtron-labs/common-lib/utils/k8s/commonBean" @@ -52,14 +59,8 @@ import ( errors2 "github.com/juju/errors" "go.uber.org/zap" "gopkg.in/go-playground/validator.v9" - "io" errors3 "k8s.io/apimachinery/pkg/api/errors" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "net/http" - "regexp" - "strconv" - "strings" - "time" ) type K8sApplicationRestHandler interface { @@ -91,6 +92,7 @@ type K8sApplicationRestHandlerImpl struct { validator *validator.Validate enforcerUtil rbac.EnforcerUtil enforcerUtilHelm rbac.EnforcerUtilHelm + enforcerUtilGitOps rbac.EnforcerUtilGitOps helmAppService client.HelmAppService userService user.UserService k8sCommonService k8s.K8sCommonService @@ -99,7 +101,7 @@ type K8sApplicationRestHandlerImpl struct { argoApplicationReadService read.ArgoApplicationReadService } -func NewK8sApplicationRestHandlerImpl(logger *zap.SugaredLogger, k8sApplicationService application2.K8sApplicationService, pump connector.Pump, terminalSessionHandler terminal.TerminalSessionHandler, enforcer casbin.Enforcer, enforcerUtilHelm rbac.EnforcerUtilHelm, enforcerUtil rbac.EnforcerUtil, helmAppService client.HelmAppService, userService user.UserService, k8sCommonService k8s.K8sCommonService, validator *validator.Validate, envVariables *util.EnvironmentVariables, fluxAppService fluxApplication.FluxApplicationService, argoApplicationReadService read.ArgoApplicationReadService, +func NewK8sApplicationRestHandlerImpl(logger *zap.SugaredLogger, k8sApplicationService application2.K8sApplicationService, pump connector.Pump, terminalSessionHandler terminal.TerminalSessionHandler, enforcer casbin.Enforcer, enforcerUtilHelm rbac.EnforcerUtilHelm, enforcerUtilGitOps rbac.EnforcerUtilGitOps, enforcerUtil rbac.EnforcerUtil, helmAppService client.HelmAppService, userService user.UserService, k8sCommonService k8s.K8sCommonService, validator *validator.Validate, envVariables *util.EnvironmentVariables, fluxAppService fluxApplication.FluxApplicationService, argoApplicationReadService read.ArgoApplicationReadService, ) *K8sApplicationRestHandlerImpl { return &K8sApplicationRestHandlerImpl{ logger: logger, @@ -109,6 +111,7 @@ func NewK8sApplicationRestHandlerImpl(logger *zap.SugaredLogger, k8sApplicationS enforcer: enforcer, validator: validator, enforcerUtilHelm: enforcerUtilHelm, + enforcerUtilGitOps: enforcerUtilGitOps, enforcerUtil: enforcerUtil, helmAppService: helmAppService, userService: userService, @@ -207,7 +210,20 @@ func (handler *K8sApplicationRestHandlerImpl) GetResource(w http.ResponseWriter, canUpdate := false // Obfuscate secret if user does not have edit access - if request.AppIdentifier == nil && request.DevtronAppIdentifier == nil && request.AppType != bean2.ArgoAppType && request.ClusterId > 0 { // if the appType is not argoAppType,then verify logic w.r.t resource browser, when rbac for argoApp is introduced, handle rbac accordingly + if request.AppType == bean2.ArgoAppType && request.ExternalArgoAppIdentifier != nil { + // External Argo app: edit access is decided by the app-level permission, not by the + // resource-browser cluster entity. Without this an Argo admin would never see Secret + // values, because canUpdate would stay false and the manifest would be masked below. + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(request.ExternalArgoAppIdentifier.ClusterId, + request.ExternalArgoAppIdentifier.Namespace, request.ExternalArgoAppIdentifier.AppName) + canUpdate = len(rbacObject) > 0 && + handler.enforcer.Enforce(token, casbin.ResourceArgoApp, casbin.ActionUpdate, rbacObject) + } else if request.AppType == bean2.FluxAppType && request.ExternalFluxAppIdentifier != nil { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(request.ExternalFluxAppIdentifier.ClusterId, + request.ExternalFluxAppIdentifier.Namespace, request.ExternalFluxAppIdentifier.Name) + canUpdate = len(rbacObject) > 0 && + handler.enforcer.Enforce(token, casbin.ResourceFluxApp, casbin.ActionUpdate, rbacObject) + } else if request.AppIdentifier == nil && request.DevtronAppIdentifier == nil && request.AppType != bean2.ArgoAppType && request.ClusterId > 0 { // if the appType is not argoAppType,then verify logic w.r.t resource browser // Verify update access for Resource Browser canUpdate = handler.k8sApplicationService.ValidateClusterResourceBean(r.Context(), request.ClusterId, resource.ManifestResponse.Manifest, request.K8sRequest.ResourceIdentifier.GroupVersionKind, handler.getRbacCallbackForResource(token, casbin.ActionUpdate)) if !canUpdate { @@ -299,7 +315,8 @@ func (handler *K8sApplicationRestHandlerImpl) GetHostUrlsByBatch(w http.Response return } // RBAC enforcer applying - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.AppName) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceArgoApp, casbin.ActionGet, rbacObject) { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } @@ -326,7 +343,8 @@ func (handler *K8sApplicationRestHandlerImpl) GetHostUrlsByBatch(w http.Response return } // RBAC enforcer applying - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.Name) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceFluxApp, casbin.ActionGet, rbacObject) { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } @@ -718,7 +736,9 @@ func (handler *K8sApplicationRestHandlerImpl) requestValidationAndRBAC(w http.Re return } //RBAC enforcer starts here - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(request.ExternalFluxAppIdentifier.ClusterId, + request.ExternalFluxAppIdentifier.Namespace, request.ExternalFluxAppIdentifier.Name) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceFluxApp, casbin.ActionGet, rbacObject) { common.WriteJsonResp(w, errors2.New("unauthorized"), nil, http.StatusForbidden) return } @@ -738,7 +758,8 @@ func (handler *K8sApplicationRestHandlerImpl) requestValidationAndRBAC(w http.Re } //RBAC enforcer starts here - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.AppName) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceArgoApp, casbin.ActionGet, rbacObject) { common.WriteJsonResp(w, errors2.New("unauthorized"), nil, http.StatusForbidden) return } @@ -803,7 +824,9 @@ func (handler *K8sApplicationRestHandlerImpl) GetTerminalSession(w http.Response //RBAC enforcer Ends } else if resourceRequestBean.ExternalFluxAppIdentifier != nil { // RBAC enforcer applying For external flux app - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*"); !ok { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(resourceRequestBean.ExternalFluxAppIdentifier.ClusterId, + resourceRequestBean.ExternalFluxAppIdentifier.Namespace, resourceRequestBean.ExternalFluxAppIdentifier.Name) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceFluxApp, casbin.ActionUpdate, rbacObject) { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } @@ -811,7 +834,13 @@ func (handler *K8sApplicationRestHandlerImpl) GetTerminalSession(w http.Response } else if resourceRequestBean.ExternalArgoApplicationName != "" { // RBAC enforcer applying For external Argo app - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*"); !ok { + if resourceRequestBean.ExternalArgoAppIdentifier == nil { + common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) + return + } + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(resourceRequestBean.ExternalArgoAppIdentifier.ClusterId, + resourceRequestBean.ExternalArgoAppIdentifier.Namespace, resourceRequestBean.ExternalArgoAppIdentifier.AppName) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceArgoApp, casbin.ActionUpdate, rbacObject) { common.WriteJsonResp(w, errors.New("unauthorized"), nil, http.StatusForbidden) return } @@ -1125,14 +1154,18 @@ func (handler *K8sApplicationRestHandlerImpl) handleEphemeralRBAC(podName, names //RBAC enforcer Ends } else if resourceRequestBean.ExternalFluxAppIdentifier != nil { //RBAC enforcer starts here - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(resourceRequestBean.ExternalFluxAppIdentifier.ClusterId, + resourceRequestBean.ExternalFluxAppIdentifier.Namespace, resourceRequestBean.ExternalFluxAppIdentifier.Name) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceFluxApp, casbin.ActionUpdate, rbacObject) { common.WriteJsonResp(w, errors2.New("unauthorized"), nil, http.StatusForbidden) return resourceRequestBean } //RBAC enforcer ends here } else if resourceRequestBean.ExternalArgoApplicationName != "" { //RBAC enforcer starts here - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionGet, "*"); !ok { + rbacObject := handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(resourceRequestBean.ExternalArgoAppIdentifier.ClusterId, + resourceRequestBean.ExternalArgoAppIdentifier.Namespace, resourceRequestBean.ExternalArgoAppIdentifier.AppName) + if len(rbacObject) == 0 || !handler.enforcer.Enforce(token, casbin.ResourceArgoApp, casbin.ActionUpdate, rbacObject) { common.WriteJsonResp(w, errors2.New("unauthorized"), nil, http.StatusForbidden) return resourceRequestBean } @@ -1179,7 +1212,11 @@ func (handler *K8sApplicationRestHandlerImpl) verifyRbacForAppRequests(token str return false, err } //RBAC enforcer starts here - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, actionType, "*"); !ok { + rbacObject = handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(argoAppIdentifier.ClusterId, argoAppIdentifier.Namespace, argoAppIdentifier.AppName) + if len(rbacObject) == 0 { + return false, nil + } + if ok := handler.enforcer.Enforce(token, casbin.ResourceArgoApp, actionType, rbacObject); !ok { return false, nil } return true, nil @@ -1247,7 +1284,11 @@ func (handler *K8sApplicationRestHandlerImpl) verifyRbacForAppRequests(token str return false, err } //RBAC enforcer starts here - if ok := handler.enforcer.Enforce(token, casbin.ResourceGlobal, actionType, "*"); !ok { + rbacObject = handler.enforcerUtilGitOps.GetExternalGitOpsAppObject(appIdentifier.ClusterId, appIdentifier.Namespace, appIdentifier.Name) + if len(rbacObject) == 0 { + return false, nil + } + if ok := handler.enforcer.Enforce(token, casbin.ResourceFluxApp, actionType, rbacObject); !ok { return false, nil } return true, nil diff --git a/api/userResource/bean/bean.go b/api/userResource/bean/bean.go index b6f5c9ea2f..375b6de21a 100644 --- a/api/userResource/bean/bean.go +++ b/api/userResource/bean/bean.go @@ -31,6 +31,8 @@ type AppAndJobReqDto struct { } type ClusterReqDto struct { *bean3.ResourceRequestBean + ClusterIds []int `json:"clusterIds,omitempty"` + EnvironmentIdentifiers []string `json:"environmentIdentifiers,omitempty"` } type JobWorkflowReqDto struct { *bean2.WorkflowNamesRequest diff --git a/cmd/external-app/wire_gen.go b/cmd/external-app/wire_gen.go index 2edf485c1b..658246eeaa 100644 --- a/cmd/external-app/wire_gen.go +++ b/cmd/external-app/wire_gen.go @@ -384,8 +384,9 @@ func InitializeApp() (*App, error) { environmentReadServiceImpl := read8.NewEnvironmentReadServiceImpl(sugaredLogger, environmentRepositoryImpl) environmentRestHandlerImpl := cluster2.NewEnvironmentRestHandlerImpl(environmentServiceImpl, environmentReadServiceImpl, sugaredLogger, userServiceImpl, validate, enforcerImpl, deleteServiceImpl, k8sServiceImpl, k8sCommonServiceImpl, commonEnforcementUtilImpl) environmentRouterImpl := cluster2.NewEnvironmentRouterImpl(environmentRestHandlerImpl) + enforcerUtilGitOpsImpl := rbac.NewEnforcerUtilGitOpsImpl(sugaredLogger, clusterReadServiceImpl) argoApplicationReadServiceImpl := read9.NewArgoApplicationReadServiceImpl(sugaredLogger, clusterRepositoryImpl, k8sServiceImpl, helmAppClientImpl, helmAppServiceImpl) - k8sApplicationRestHandlerImpl := application2.NewK8sApplicationRestHandlerImpl(sugaredLogger, k8sApplicationServiceImpl, pumpImpl, terminalSessionHandlerImpl, enforcerImpl, enforcerUtilHelmImpl, enforcerUtilImpl, helmAppServiceImpl, userServiceImpl, k8sCommonServiceImpl, validate, environmentVariables, fluxApplicationServiceImpl, argoApplicationReadServiceImpl) + k8sApplicationRestHandlerImpl := application2.NewK8sApplicationRestHandlerImpl(sugaredLogger, k8sApplicationServiceImpl, pumpImpl, terminalSessionHandlerImpl, enforcerImpl, enforcerUtilHelmImpl, enforcerUtilGitOpsImpl, enforcerUtilImpl, helmAppServiceImpl, userServiceImpl, k8sCommonServiceImpl, validate, environmentVariables, fluxApplicationServiceImpl, argoApplicationReadServiceImpl) k8sApplicationRouterImpl := application2.NewK8sApplicationRouterImpl(k8sApplicationRestHandlerImpl) chartRepositoryRestHandlerImpl := chartRepo2.NewChartRepositoryRestHandlerImpl(sugaredLogger, userServiceImpl, chartRepositoryServiceImpl, enforcerImpl, validate, deleteServiceImpl, attributesServiceImpl) chartRepositoryRouterImpl := chartRepo2.NewChartRepositoryRouterImpl(chartRepositoryRestHandlerImpl) @@ -505,11 +506,11 @@ func InitializeApp() (*App, error) { rbacRoleServiceImpl := user.NewRbacRoleServiceImpl(sugaredLogger, rbacRoleDataRepositoryImpl) rbacRoleRestHandlerImpl := user2.NewRbacRoleHandlerImpl(sugaredLogger, validate, rbacRoleServiceImpl, userServiceImpl, enforcerImpl, enforcerUtilImpl) rbacRoleRouterImpl := user2.NewRbacRoleRouterImpl(sugaredLogger, validate, rbacRoleRestHandlerImpl) - argoApplicationRestHandlerImpl := argoApplication2.NewArgoApplicationRestHandlerImpl(argoApplicationServiceImpl, argoApplicationReadServiceImpl, sugaredLogger, enforcerImpl) + argoApplicationRestHandlerImpl := argoApplication2.NewArgoApplicationRestHandlerImpl(argoApplicationServiceImpl, argoApplicationReadServiceImpl, sugaredLogger, enforcerImpl, enforcerUtilGitOpsImpl) argoApplicationRouterImpl := argoApplication2.NewArgoApplicationRouterImpl(argoApplicationRestHandlerImpl) - fluxApplicationRestHandlerImpl := fluxApplication2.NewFluxApplicationRestHandlerImpl(fluxApplicationServiceImpl, sugaredLogger, enforcerImpl) + fluxApplicationRestHandlerImpl := fluxApplication2.NewFluxApplicationRestHandlerImpl(fluxApplicationServiceImpl, sugaredLogger, enforcerImpl, enforcerUtilGitOpsImpl) fluxApplicationRouterImpl := fluxApplication2.NewFluxApplicationRouterImpl(fluxApplicationRestHandlerImpl) - userResourceServiceImpl := userResource.NewUserResourceServiceImpl(sugaredLogger, teamServiceImpl, environmentServiceImpl, clusterServiceImpl, k8sApplicationServiceImpl, enforcerUtilImpl, commonEnforcementUtilImpl, enforcerImpl, appCrudOperationServiceImpl) + userResourceServiceImpl := userResource.NewUserResourceServiceImpl(sugaredLogger, teamServiceImpl, environmentServiceImpl, clusterServiceImpl, k8sApplicationServiceImpl, enforcerUtilImpl, commonEnforcementUtilImpl, enforcerImpl, appCrudOperationServiceImpl, argoApplicationServiceImpl, fluxApplicationServiceImpl) restHandlerImpl := userResource2.NewUserResourceRestHandler(sugaredLogger, userServiceImpl, userResourceServiceImpl) routerImpl := userResource2.NewUserResourceRouterImpl(restHandlerImpl) clusterOverviewConfig, err := config4.GetClusterOverviewConfig() diff --git a/pkg/auth/authorisation/casbin/rbacpolicy.go b/pkg/auth/authorisation/casbin/rbacpolicy.go index 09423de63a..1bca4a6418 100644 --- a/pkg/auth/authorisation/casbin/rbacpolicy.go +++ b/pkg/auth/authorisation/casbin/rbacpolicy.go @@ -45,6 +45,12 @@ const ( ResourceAdmin = "admin" ResourceGlobal = "global-resource" ResourceHelmApp = "helm-app" + + // ResourceArgoApp, ResourceFluxApp are used for app-level RBAC on external + // Argo CD / Flux CD applications. Object shape is __/. + ResourceArgoApp = "argo-app" + ResourceFluxApp = "flux-app" + ActionGet = "get" ActionCreate = "create" ActionUpdate = "update" diff --git a/pkg/auth/user/UserCommonService.go b/pkg/auth/user/UserCommonService.go index 9f107d898b..17878833e5 100644 --- a/pkg/auth/user/UserCommonService.go +++ b/pkg/auth/user/UserCommonService.go @@ -18,13 +18,14 @@ package user import ( "fmt" + "math" + "strings" + "time" + bean3 "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin/bean" "github.com/devtron-labs/devtron/pkg/auth/user/adapter" "github.com/devtron-labs/devtron/pkg/auth/user/repository/bean" "golang.org/x/exp/maps" - "math" - "strings" - "time" "github.com/caarlos0/env/v6" "github.com/devtron-labs/authenticator/middleware" @@ -372,6 +373,8 @@ func (impl UserCommonServiceImpl) RemoveRolesAndReturnEliminatedPolicies(userInf if role, ok := roleIdVsRoleMap[userRoleModel.RoleId]; ok { isValidAuth := impl.checkRbacForARole(role, token, managerAuth) if !isValidAuth { + impl.logger.Warnw("not authorised to delete role, skipping", "roleId", role.Id, + "entity", role.Entity, "accessType", role.AccessType, "action", role.Action) continue } toBeDeletedUserRolesIds = append(toBeDeletedUserRolesIds, userRoleModel.Id) @@ -527,6 +530,8 @@ func (impl UserCommonServiceImpl) RemoveRolesAndReturnEliminatedPoliciesForGroup if role, ok := roleIdVsRoleMap[model.RoleId]; ok { isValidAuth := impl.checkRbacForARole(role, token, managerAuth) if !isValidAuth { + impl.logger.Warnw("not authorised to delete role, skipping", "roleId", role.Id, + "entity", role.Entity, "accessType", role.AccessType, "action", role.Action) continue } toBeDeletedRoleGroupRoleMappingsIds = append(toBeDeletedRoleGroupRoleMappingsIds, model.Id) @@ -547,7 +552,9 @@ func (impl UserCommonServiceImpl) RemoveRolesAndReturnEliminatedPoliciesForGroup func (impl UserCommonServiceImpl) checkRbacForARole(role *repository.RoleModel, token string, managerAuth func(resource string, token string, object string) bool) bool { isAuthorised := true switch { - case role.Action == bean2.SUPER_ADMIN || role.AccessType == bean2.APP_ACCESS_TYPE_HELM || role.Entity == bean2.EntityJobs: + case role.Action == bean2.SUPER_ADMIN || role.AccessType == bean2.APP_ACCESS_TYPE_HELM || + role.AccessType == bean2.APP_ACCESS_TYPE_ARGO || role.AccessType == bean2.APP_ACCESS_TYPE_FLUX || + role.Entity == bean2.EntityJobs: isValidAuth := managerAuth(casbin.ResourceGlobal, token, "*") if !isValidAuth { isAuthorised = false @@ -780,6 +787,20 @@ func (impl UserCommonServiceImpl) GetUniqueKeyForAllEntity(entityProcessor Entit default: key = fmt.Sprintf("%s_%s_%s_%s", entityProcessor.GetTeam(), entityProcessor.GetAction(), entityProcessor.GetAccessType(), entityProcessor.GetEntity()) } + } else if entityProcessor.GetEntity() == bean2.ENTITY_APPS && + (entityProcessor.GetAccessType() == bean2.APP_ACCESS_TYPE_ARGO || + entityProcessor.GetAccessType() == bean2.APP_ACCESS_TYPE_FLUX) { + switch baseToConsider { + case bean2.EnvironmentBasedKey: + key = fmt.Sprintf("%s_%s_%s_%s", entityProcessor.GetEntity(), entityProcessor.GetEnvironment(), + entityProcessor.GetAction(), entityProcessor.GetAccessType()) + case bean2.ApplicationBasedKey: + key = fmt.Sprintf("%s_%s_%s_%s", entityProcessor.GetEntity(), entityProcessor.GetEntityName(), + entityProcessor.GetAction(), entityProcessor.GetAccessType()) + default: + key = fmt.Sprintf("%s_%s_%s", entityProcessor.GetEntity(), entityProcessor.GetAction(), + entityProcessor.GetAccessType()) + } } else if len(entityProcessor.GetEntity()) > 0 { if entityProcessor.GetEntity() == bean2.CLUSTER_ENTITIY { key = fmt.Sprintf("%s_%s_%s_%s_%s", entityProcessor.GetEntity(), entityProcessor.GetAction(), entityProcessor.GetCluster(), diff --git a/pkg/auth/user/bean/bean.go b/pkg/auth/user/bean/bean.go index 2b03b0ad49..5447775239 100644 --- a/pkg/auth/user/bean/bean.go +++ b/pkg/auth/user/bean/bean.go @@ -66,6 +66,8 @@ const ( const ( DEVTRON_APP = "devtron-app" APP_ACCESS_TYPE_HELM = "helm-app" + APP_ACCESS_TYPE_ARGO = "argo-app" + APP_ACCESS_TYPE_FLUX = "flux-app" EmptyAccessType = "" ) diff --git a/pkg/auth/user/repository/UserAuthRepository.go b/pkg/auth/user/repository/UserAuthRepository.go index ce8fe2940f..7f37ab0b08 100644 --- a/pkg/auth/user/repository/UserAuthRepository.go +++ b/pkg/auth/user/repository/UserAuthRepository.go @@ -22,11 +22,12 @@ package repository import ( "encoding/json" "fmt" + "strings" + "time" + bean3 "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin/bean" "github.com/devtron-labs/devtron/pkg/auth/user/adapter" bean4 "github.com/devtron-labs/devtron/pkg/auth/user/repository/bean" - "strings" - "time" "github.com/devtron-labs/devtron/api/bean" casbin2 "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin" @@ -73,6 +74,7 @@ type UserAuthRepository interface { GetRoleForClusterEntity(cluster, namespace, group, kind, resource, action string) (RoleModel, error) GetRoleForJobsEntity(entity, team, app, env, act string, workflow string) (RoleModel, error) GetRoleForOtherEntity(team, app, env, act, accessType string, oldValues bool) (RoleModel, error) + GetRoleForExternalGitOpsEntity(app, env, act, accessType string, oldValues bool) (RoleModel, error) GetRoleForChartGroupEntity(entity, app, act, accessType string) (RoleModel, error) } @@ -294,6 +296,13 @@ func (impl UserAuthRepositoryImpl) GetRoleByFilterForAllTypes(roleFieldDto *bean default: { team, app, env, accessType, oldValues := roleFieldDto.Team, roleFieldDto.App, roleFieldDto.Env, roleFieldDto.AccessType, roleFieldDto.OldValues + // External Argo/Flux applications share entity "apps" with devtron-app and + // helm-app but have no project, so they get their own project-free lookup + // instead of being clubbed into the team-keyed queries below. + switch accessType { + case bean2.APP_ACCESS_TYPE_ARGO, bean2.APP_ACCESS_TYPE_FLUX: + return impl.GetRoleForExternalGitOpsEntity(app, env, action, accessType, oldValues) + } return impl.GetRoleForOtherEntity(team, app, env, action, accessType, oldValues) } } @@ -1152,6 +1161,8 @@ func (impl UserAuthRepositoryImpl) GetRoleForOtherEntity(team, app, env, act, ac } else if team == "" && app == "" && env == "" && act == "" { return model, nil } else { + impl.Logger.Warnw("no query branch for the given role filter combination, returning empty role", + "team", team, "app", app, "env", env, "action", act, "accessType", accessType) return model, nil } if err != nil { @@ -1159,3 +1170,40 @@ func (impl UserAuthRepositoryImpl) GetRoleForOtherEntity(team, app, env, act, ac } return model, err } + +func (impl UserAuthRepositoryImpl) GetRoleForExternalGitOpsEntity(app, env, act, accessType string, oldValues bool) (RoleModel, error) { + var model RoleModel + if oldValues { + return model, nil + } + if len(act) == 0 || len(accessType) == 0 { + impl.Logger.Warnw("incomplete filter for external gitops role, returning empty role", + "app", app, "env", env, "action", act, "accessType", accessType) + return model, nil + } + query := "SELECT role.* FROM roles role WHERE coalesce(role.team,'') = ? AND role.action = ? AND role.access_type = ?" + queryParams := []interface{}{EMPTY_PLACEHOLDER_FOR_QUERY, act, accessType} + + if len(app) > 0 { + query += " AND role.entity_name = ?" + queryParams = append(queryParams, app) + } else { + query += " AND coalesce(role.entity_name,'') = ?" + queryParams = append(queryParams, EMPTY_PLACEHOLDER_FOR_QUERY) + } + + if len(env) > 0 { + query += " AND role.environment = ?" + queryParams = append(queryParams, env) + } else { + query += " AND coalesce(role.environment,'') = ?" + queryParams = append(queryParams, EMPTY_PLACEHOLDER_FOR_QUERY) + } + + _, err := impl.dbConnection.Query(&model, query, queryParams...) + if err != nil { + impl.Logger.Errorw("error in getting role for external gitops entity", "err", err, + "app", app, "env", env, "action", act, "accessType", accessType) + } + return model, err +} diff --git a/pkg/fluxApplication/FluxApplicationService.go b/pkg/fluxApplication/FluxApplicationService.go index 779b5b2c63..12a9ec121b 100644 --- a/pkg/fluxApplication/FluxApplicationService.go +++ b/pkg/fluxApplication/FluxApplicationService.go @@ -3,6 +3,9 @@ package fluxApplication import ( "context" "fmt" + "io" + "net/http" + "github.com/devtron-labs/common-lib/utils/k8s/commonBean" "github.com/devtron-labs/devtron/api/connector" "github.com/devtron-labs/devtron/api/helm-app/gRPC" @@ -19,15 +22,15 @@ import ( "github.com/gogo/protobuf/proto" "go.opentelemetry.io/otel" "go.uber.org/zap" - "io" - "net/http" ) type FluxApplicationService interface { - ListFluxApplications(ctx context.Context, clusterIds []int, noStream bool, w http.ResponseWriter) + ListFluxApplications(ctx context.Context, clusterIds []int, noStream bool, w http.ResponseWriter, + token string, fluxAuth func(token string, clusterName string, namespace string, appName string) bool) GetFluxAppDetail(ctx context.Context, app *bean.FluxAppIdentifier) (*bean.FluxApplicationDetailDto, error) HibernateFluxApplication(ctx context.Context, app *bean.FluxAppIdentifier, hibernateRequest *openapi.HibernateRequest) ([]*openapi.HibernateStatus, error) UnHibernateFluxApplication(ctx context.Context, app *bean.FluxAppIdentifier, hibernateRequest *openapi.HibernateRequest) ([]*openapi.HibernateStatus, error) + GetFluxApplicationList(ctx context.Context, clusterIds []int) ([]bean.FluxApplication, error) } type FluxApplicationServiceImpl struct { @@ -92,23 +95,55 @@ func (impl *FluxApplicationServiceImpl) UnHibernateFluxApplication(ctx context.C return response, nil } -func (impl *FluxApplicationServiceImpl) ListFluxApplications(ctx context.Context, clusterIds []int, noStream bool, w http.ResponseWriter) { +func (impl *FluxApplicationServiceImpl) GetFluxApplicationList(ctx context.Context, clusterIds []int) ([]bean.FluxApplication, error) { appStream, err := impl.listApplications(ctx, clusterIds) if err != nil { - impl.logger.Errorw("error in listing flux applications", "clusterIds", clusterIds, "err", err) - return + return nil, err + } + cdPipelineMap, installedAppMap, err := impl.getDevtronManagedMaps(clusterIds) + if err != nil { + return nil, err } + apps := make([]bean.FluxApplication, 0) + for { + appDetail, err := appStream.Recv() + if err == io.EOF { + break + } + if err != nil { + return nil, err + } + if appDetail.Errored { + impl.logger.Errorw("error in listing flux applications for cluster, skipping it", + "clusterId", appDetail.ClusterId, "errorMsg", appDetail.ErrorMsg) + continue + } + for _, d := range appDetail.FluxApplication { + key := fmt.Sprintf("%v-%s", d.EnvironmentDetail.ClusterId, d.EnvironmentDetail.Namespace) + if _, ok := cdPipelineMap[key][d.Name]; ok { + continue + } + if _, ok := installedAppMap[key][d.Name]; ok { + continue + } + apps = append(apps, toFluxApplication(d)) + } + } + return apps, nil +} + +func (impl *FluxApplicationServiceImpl) getDevtronManagedMaps(clusterIds []int) (map[string]map[string]bool, map[string]map[string]bool, error) { fluxCdPipelines, err := impl.pipelineRepository.GetAppAndEnvDetailsForDeploymentAppTypePipeline(util.PIPELINE_DEPLOYMENT_TYPE_FLUX, clusterIds) if err != nil { impl.logger.Errorw("error in fetching helm app list from DB created using cd_pipelines", "clusters", clusterIds, "err", err) - return + return nil, nil, err } installedHelmApps, err := impl.installedAppRepository.GetAppAndEnvDetailsForDeploymentAppTypeInstalledApps(util.PIPELINE_DEPLOYMENT_TYPE_FLUX, clusterIds) if err != nil { impl.logger.Errorw("error in fetching helm app list from DB created from app store", "clusters", clusterIds, "err", err) - return + return nil, nil, err } cdPipelineMap := make(map[string]map[string]bool) // map of clusterId-namespace, deploymentAppName @@ -129,52 +164,70 @@ func (impl *FluxApplicationServiceImpl) ListFluxApplications(ctx context.Context deploymentAppName := fmt.Sprintf("%s-%s", i.App.AppName, i.Environment.Namespace) installedAppMap[key][deploymentAppName] = true } + + return cdPipelineMap, installedAppMap, nil +} + +func toFluxApplication(app *gRPC.FluxApplication) bean.FluxApplication { + fluxApp := bean.FluxApplication{ + Name: app.Name, + HealthStatus: app.HealthStatus, + SyncStatus: app.SyncStatus, + ClusterId: int(app.EnvironmentDetail.ClusterId), + ClusterName: app.EnvironmentDetail.ClusterName, + Namespace: app.EnvironmentDetail.Namespace, + FluxAppDeploymentType: app.FluxAppDeploymentType, + } + + return fluxApp +} + +func (impl *FluxApplicationServiceImpl) ListFluxApplications(ctx context.Context, clusterIds []int, noStream bool, w http.ResponseWriter, + token string, fluxAuth func(token string, clusterName string, namespace string, appName string) bool) { + if !noStream { + appStream, err := impl.listApplications(ctx, clusterIds) + if err != nil { + impl.logger.Errorw("error in listing flux applications", "clusterIds", clusterIds, "err", err) + common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) + return + } + cdPipelineMap, installedAppMap, err := impl.getDevtronManagedMaps(clusterIds) + if err != nil { + impl.logger.Errorw("error in getting devtron managed flux apps", "clusterIds", clusterIds, "err", err) + common.WriteJsonResp(w, err, nil, http.StatusInternalServerError) + return + } impl.pump.StartStreamWithTransformer(w, func() (proto.Message, error) { return appStream.Recv() }, err, func(message interface{}) interface{} { - return impl.appListRespProtoTransformer(message.(*gRPC.FluxApplicationList), cdPipelineMap, installedAppMap) + return impl.appListRespProtoTransformer(message.(*gRPC.FluxApplicationList), cdPipelineMap, installedAppMap, token, fluxAuth) }) } else { - fluxApps := make([]bean.FluxApplication, 0) - for { - appDetail, err := appStream.Recv() - if err == io.EOF { - break - } - if err != nil { - return + fluxApps, err := impl.GetFluxApplicationList(ctx, clusterIds) + if err != nil { + impl.logger.Errorw("error in getting flux application list", "clusterIds", clusterIds, "err", err) + errored := true + errMsg := err.Error() + appList := bean.FluxAppList{ + Errored: &errored, + ErrorMsg: &errMsg, } - if appDetail.Errored { - appList := bean.FluxAppList{ - Errored: &appDetail.Errored, - ErrorMsg: &appDetail.ErrorMsg, - } - common.WriteJsonResp(w, nil, appList, http.StatusOK) - return - } else { - for _, deployedApp := range appDetail.FluxApplication { - key := fmt.Sprintf("%v-%s", deployedApp.EnvironmentDetail.ClusterId, deployedApp.EnvironmentDetail.Namespace) - if _, ok := cdPipelineMap[key][deployedApp.Name]; ok { - continue - } - if _, ok := installedAppMap[key][deployedApp.Name]; ok { - continue - } - fluxApp := bean.FluxApplication{ - Name: deployedApp.Name, - HealthStatus: deployedApp.HealthStatus, - SyncStatus: deployedApp.SyncStatus, - ClusterId: int(deployedApp.EnvironmentDetail.ClusterId), - ClusterName: deployedApp.EnvironmentDetail.ClusterName, - Namespace: deployedApp.EnvironmentDetail.Namespace, - FluxAppDeploymentType: deployedApp.FluxAppDeploymentType, - } - fluxApps = append(fluxApps, fluxApp) + common.WriteJsonResp(w, nil, appList, http.StatusOK) + return + } + + if fluxAuth != nil { + authorised := make([]bean.FluxApplication, 0, len(fluxApps)) + for _, app := range fluxApps { + if fluxAuth(token, app.ClusterName, app.Namespace, app.Name) { + authorised = append(authorised, app) } } + fluxApps = authorised } + //RBAC enforcer Ends clusterIdsInt32 := sliceUtil.NewSliceFromFuncExec(clusterIds, func(clusterId int) int32 { return int32(clusterId) }) @@ -234,6 +287,11 @@ func (impl *FluxApplicationServiceImpl) listApplications(ctx context.Context, cl } for _, clusterDetail := range clusters { + if clusterDetail.IsVirtualCluster || len(clusterDetail.ErrorInConnecting) != 0 { + impl.logger.Debugw("skipping cluster for flux app listing", "clusterId", clusterDetail.Id, + "isVirtualCluster", clusterDetail.IsVirtualCluster, "errorInConnecting", clusterDetail.ErrorInConnecting) + continue + } config := &gRPC.ClusterConfig{ ApiServerUrl: clusterDetail.ServerUrl, Token: clusterDetail.Config[commonBean.BearerToken], @@ -252,7 +310,8 @@ func (impl *FluxApplicationServiceImpl) listApplications(ctx context.Context, cl return applicationStream, err } -func (impl *FluxApplicationServiceImpl) appListRespProtoTransformer(deployedApps *gRPC.FluxApplicationList, fluxCdPipelines map[string]map[string]bool, fluxInstalledApps map[string]map[string]bool) bean.FluxAppList { +func (impl *FluxApplicationServiceImpl) appListRespProtoTransformer(deployedApps *gRPC.FluxApplicationList, fluxCdPipelines map[string]map[string]bool, fluxInstalledApps map[string]map[string]bool, + token string, fluxAuth func(token string, clusterName string, namespace string, appName string) bool) bean.FluxAppList { appList := bean.FluxAppList{ClusterId: &[]int32{deployedApps.ClusterId}} if deployedApps.Errored { @@ -268,6 +327,11 @@ func (impl *FluxApplicationServiceImpl) appListRespProtoTransformer(deployedApps if _, ok := fluxInstalledApps[key][deployedApp.Name]; ok { continue } + if fluxAuth != nil && !fluxAuth(token, deployedApp.EnvironmentDetail.ClusterName, + deployedApp.EnvironmentDetail.Namespace, deployedApp.Name) { + continue + } + //RBAC enforcer Ends fluxApp := bean.FluxApplication{ Name: deployedApp.Name, HealthStatus: deployedApp.HealthStatus, diff --git a/pkg/k8s/application/k8sApplicationService.go b/pkg/k8s/application/k8sApplicationService.go index 550de045bb..8cbaeb88b9 100644 --- a/pkg/k8s/application/k8sApplicationService.go +++ b/pkg/k8s/application/k8sApplicationService.go @@ -21,6 +21,11 @@ import ( "encoding/json" "errors" "fmt" + "io" + "net/http" + "strconv" + "strings" + "github.com/devtron-labs/common-lib/utils" "github.com/devtron-labs/devtron/api/helm-app/gRPC" client "github.com/devtron-labs/devtron/api/helm-app/service" @@ -33,11 +38,7 @@ import ( "github.com/devtron-labs/devtron/pkg/fluxApplication" bean2 "github.com/devtron-labs/devtron/pkg/fluxApplication/bean" bean4 "github.com/devtron-labs/devtron/pkg/k8s/bean" - "io" v1 "k8s.io/client-go/kubernetes/typed/core/v1" - "net/http" - "strconv" - "strings" "github.com/caarlos0/env/v6" k8s2 "github.com/devtron-labs/common-lib/utils/k8s" @@ -369,6 +370,7 @@ func (impl *K8sApplicationServiceImpl) ValidateTerminalRequestQuery(r *http.Requ } resourceRequestBean.ExternalArgoApplicationName = appIdentifier.AppName resourceRequestBean.ClusterId = appIdentifier.ClusterId + resourceRequestBean.ExternalArgoAppIdentifier = appIdentifier request.ClusterId = appIdentifier.ClusterId request.ExternalArgoApplicationName = appIdentifier.AppName request.ExternalArgoApplicationNamespace = appIdentifier.Namespace diff --git a/pkg/userResource/UserResourceExtendedService.go b/pkg/userResource/UserResourceExtendedService.go index 358a5000e3..8d78cbb741 100644 --- a/pkg/userResource/UserResourceExtendedService.go +++ b/pkg/userResource/UserResourceExtendedService.go @@ -2,14 +2,18 @@ package userResource import ( "context" + "net/http" + apiBean "github.com/devtron-labs/devtron/api/userResource/bean" "github.com/devtron-labs/devtron/internal/util" "github.com/devtron-labs/devtron/pkg/app" "github.com/devtron-labs/devtron/pkg/appStore/chartGroup" "github.com/devtron-labs/devtron/pkg/appWorkflow" + argoApplication2 "github.com/devtron-labs/devtron/pkg/argoApplication" "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin" "github.com/devtron-labs/devtron/pkg/cluster" "github.com/devtron-labs/devtron/pkg/cluster/environment" + "github.com/devtron-labs/devtron/pkg/fluxApplication" application2 "github.com/devtron-labs/devtron/pkg/k8s/application" "github.com/devtron-labs/devtron/pkg/team" "github.com/devtron-labs/devtron/pkg/userResource/adapter" @@ -18,7 +22,6 @@ import ( "github.com/devtron-labs/devtron/util/commonEnforcementFunctionsUtil" "github.com/devtron-labs/devtron/util/rbac" "go.uber.org/zap" - "net/http" ) type UserResourceExtendedServiceImpl struct { @@ -39,13 +42,15 @@ func NewUserResourceExtendedServiceImpl(logger *zap.SugaredLogger, teamService t clusterService cluster.ClusterService, rbacEnforcementUtil commonEnforcementFunctionsUtil.CommonEnforcementUtil, enforcerUtil rbac.EnforcerUtil, - enforcer casbin.Enforcer) *UserResourceExtendedServiceImpl { + enforcer casbin.Enforcer, + argoService argoApplication2.ArgoApplicationService, + fluxService fluxApplication.FluxApplicationService) *UserResourceExtendedServiceImpl { return &UserResourceExtendedServiceImpl{ logger: logger, chartGroupService: chartGroupService, appListingService: appListingService, appWorkflowService: appWorkflowService, - UserResourceServiceImpl: NewUserResourceServiceImpl(logger, teamService, envService, clusterService, k8sApplicationService, enforcerUtil, rbacEnforcementUtil, enforcer, appService), + UserResourceServiceImpl: NewUserResourceServiceImpl(logger, teamService, envService, clusterService, k8sApplicationService, enforcerUtil, rbacEnforcementUtil, enforcer, appService, argoService, fluxService), } } diff --git a/pkg/userResource/UserResourceRbacExtendedService.go b/pkg/userResource/UserResourceRbacExtendedService.go index 0f6c30205d..eb146d5c3e 100644 --- a/pkg/userResource/UserResourceRbacExtendedService.go +++ b/pkg/userResource/UserResourceRbacExtendedService.go @@ -65,6 +65,24 @@ func (impl *UserResourceServiceImpl) enforceRbacForHelmAppsListing(token string, return adapter.BuildUserResourceResponseDto(resourceOptions.TeamAppResp), nil } +func (impl *UserResourceServiceImpl) enforceRbacForArgoAppsListing(token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error) { + isAuthorised := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*") + if !isAuthorised { + impl.logger.Errorw("user is unauthorized enforceRbacForArgoAppsListing") + return adapter.BuildNullDataUserResourceResponseDto(), nil + } + return adapter.BuildUserResourceResponseDto(resourceOptions.ExternalGitOpsAppResp), nil +} + +func (impl *UserResourceServiceImpl) enforceRbacForFluxAppsListing(token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error) { + isAuthorised := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*") + if !isAuthorised { + impl.logger.Errorw("user is unauthorized enforceRbacForFluxAppsListing") + return adapter.BuildNullDataUserResourceResponseDto(), nil + } + return adapter.BuildUserResourceResponseDto(resourceOptions.ExternalGitOpsAppResp), nil +} + func (impl *UserResourceServiceImpl) enforceRbacForJobs(token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error) { isAuthorised := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*") if !isAuthorised { diff --git a/pkg/userResource/UserResourceRbacService.go b/pkg/userResource/UserResourceRbacService.go index 6e0e75e891..2ee5ef8ae8 100644 --- a/pkg/userResource/UserResourceRbacService.go +++ b/pkg/userResource/UserResourceRbacService.go @@ -16,10 +16,13 @@ func (impl *UserResourceServiceImpl) enforceRbacForTeamForHelmApp(token string, return adapter.BuildUserResourceResponseDto(resourceOptions.TeamsResp), nil } -func (impl *UserResourceServiceImpl) enforceRbacForEnvForHelmApp(token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error) { +// enforceRbacForEnvOptions gates the environment options dropdown. It serves all three +// external application types — helm-app, argo-app and flux-app — because the options are +// cluster/namespace pairs regardless of who deployed the application. +func (impl *UserResourceServiceImpl) enforceRbacForEnvOptions(token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error) { isAuthorised := impl.enforcer.Enforce(token, casbin.ResourceGlobal, casbin.ActionUpdate, "*") if !isAuthorised { - impl.logger.Errorw("user is unauthorized to enforceRbacForEnvForHelmApp") + impl.logger.Errorw("user is unauthorized to enforceRbacForEnvOptions") return adapter.BuildNullDataUserResourceResponseDto(), nil } return adapter.BuildUserResourceResponseDto(resourceOptions.HelmEnvResp), nil diff --git a/pkg/userResource/UserResourceService.go b/pkg/userResource/UserResourceService.go index 48274c5fe8..49d44e273d 100644 --- a/pkg/userResource/UserResourceService.go +++ b/pkg/userResource/UserResourceService.go @@ -2,23 +2,27 @@ package userResource import ( "context" + "net/http" + apiBean "github.com/devtron-labs/devtron/api/userResource/bean" app2 "github.com/devtron-labs/devtron/internal/sql/repository/app" "github.com/devtron-labs/devtron/internal/util" "github.com/devtron-labs/devtron/pkg/app" + argoApplication2 "github.com/devtron-labs/devtron/pkg/argoApplication" "github.com/devtron-labs/devtron/pkg/auth/authorisation/casbin" "github.com/devtron-labs/devtron/pkg/auth/user/bean" "github.com/devtron-labs/devtron/pkg/cluster" "github.com/devtron-labs/devtron/pkg/cluster/environment" + "github.com/devtron-labs/devtron/pkg/fluxApplication" application2 "github.com/devtron-labs/devtron/pkg/k8s/application" bean4 "github.com/devtron-labs/devtron/pkg/k8s/bean" "github.com/devtron-labs/devtron/pkg/team" + "github.com/devtron-labs/devtron/pkg/userResource/adapter" bean5 "github.com/devtron-labs/devtron/pkg/userResource/bean" "github.com/devtron-labs/devtron/pkg/userResource/helper" "github.com/devtron-labs/devtron/util/commonEnforcementFunctionsUtil" "github.com/devtron-labs/devtron/util/rbac" "go.uber.org/zap" - "net/http" ) type UserResourceService interface { @@ -26,15 +30,17 @@ type UserResourceService interface { params *apiBean.PathParams) (*bean5.UserResourceResponseDto, error) } type UserResourceServiceImpl struct { - logger *zap.SugaredLogger - teamService team.TeamService - envService environment.EnvironmentService - clusterService cluster.ClusterService - k8sApplicationService application2.K8sApplicationService - enforcerUtil rbac.EnforcerUtil - rbacEnforcementUtil commonEnforcementFunctionsUtil.CommonEnforcementUtil - enforcer casbin.Enforcer - appService app.AppCrudOperationService + logger *zap.SugaredLogger + teamService team.TeamService + envService environment.EnvironmentService + clusterService cluster.ClusterService + k8sApplicationService application2.K8sApplicationService + enforcerUtil rbac.EnforcerUtil + rbacEnforcementUtil commonEnforcementFunctionsUtil.CommonEnforcementUtil + enforcer casbin.Enforcer + appService app.AppCrudOperationService + argoApplicationService argoApplication2.ArgoApplicationService + fluxApplicationService fluxApplication.FluxApplicationService } func NewUserResourceServiceImpl(logger *zap.SugaredLogger, @@ -45,17 +51,21 @@ func NewUserResourceServiceImpl(logger *zap.SugaredLogger, enforcerUtil rbac.EnforcerUtil, rbacEnforcementUtil commonEnforcementFunctionsUtil.CommonEnforcementUtil, enforcer casbin.Enforcer, - appService app.AppCrudOperationService) *UserResourceServiceImpl { + appService app.AppCrudOperationService, + argoApplicationService argoApplication2.ArgoApplicationService, + fluxApplicationService fluxApplication.FluxApplicationService) *UserResourceServiceImpl { return &UserResourceServiceImpl{ - logger: logger, - teamService: teamService, - envService: envService, - clusterService: clusterService, - k8sApplicationService: k8sApplicationService, - enforcerUtil: enforcerUtil, - rbacEnforcementUtil: rbacEnforcementUtil, - enforcer: enforcer, - appService: appService, + logger: logger, + teamService: teamService, + envService: envService, + clusterService: clusterService, + k8sApplicationService: k8sApplicationService, + enforcerUtil: enforcerUtil, + rbacEnforcementUtil: rbacEnforcementUtil, + enforcer: enforcer, + appService: appService, + argoApplicationService: argoApplicationService, + fluxApplicationService: fluxApplicationService, } } @@ -121,7 +131,43 @@ func (impl *UserResourceServiceImpl) getHelmAppResourceOptions(context context.C return bean5.NewResourceOptionsDto().WithTeamAppResp(apps), nil } -func (impl *UserResourceServiceImpl) getHelmEnvResourceOptions(context context.Context, token string, +func (impl *UserResourceServiceImpl) getArgoAppResourceOptions(context context.Context, token string, + reqBean *apiBean.ResourceOptionsReqDto, params *apiBean.PathParams) (*bean5.ResourceOptionsDto, error) { + clusterIds, err := helper.GetValidatedClusterIds(reqBean) + if err != nil { + impl.logger.Errorw("error encountered in getArgoAppResourceOptions", "err", err) + return nil, err + } + apps, err := impl.argoApplicationService.ListApplications(clusterIds) + if err != nil { + impl.logger.Errorw("error encountered in getArgoAppResourceOptions", "err", err) + return nil, err + } + appDtos := helper.FilterExternalGitOpsAppsByEnvIdentifier( + adapter.ArgoAppToExternalGitOpsApp(apps), reqBean.EnvironmentIdentifiers) + + return bean5.NewResourceOptionsDto().WithExternalGitOpsAppResp(appDtos), nil +} + +func (impl *UserResourceServiceImpl) getFluxAppResourceOptions(context context.Context, token string, + reqBean *apiBean.ResourceOptionsReqDto, params *apiBean.PathParams) (*bean5.ResourceOptionsDto, error) { + clusterIds, err := helper.GetValidatedClusterIds(reqBean) + if err != nil { + impl.logger.Errorw("error encountered in getFluxAppResourceOptions", "err", err) + return nil, err + } + apps, err := impl.fluxApplicationService.GetFluxApplicationList(context, clusterIds) + if err != nil { + impl.logger.Errorw("error encountered in getFluxAppResourceOptions", "err", err) + return nil, err + } + appDtos := helper.FilterExternalGitOpsAppsByEnvIdentifier( + adapter.FluxAppToExternalGitOpsApp(apps), reqBean.EnvironmentIdentifiers) + + return bean5.NewResourceOptionsDto().WithExternalGitOpsAppResp(appDtos), nil +} + +func (impl *UserResourceServiceImpl) getCombinedEnvResourceOptions(context context.Context, token string, reqBean *apiBean.ResourceOptionsReqDto, params *apiBean.PathParams) (*bean5.ResourceOptionsDto, error) { // get helm env resource options diff --git a/pkg/userResource/adapter/adapter.go b/pkg/userResource/adapter/adapter.go index 874b61b8f2..73b30b9a3c 100644 --- a/pkg/userResource/adapter/adapter.go +++ b/pkg/userResource/adapter/adapter.go @@ -4,6 +4,8 @@ import ( bean2 "github.com/devtron-labs/devtron/api/userResource/bean" "github.com/devtron-labs/devtron/internal/sql/repository/helper" "github.com/devtron-labs/devtron/pkg/app" + bean3 "github.com/devtron-labs/devtron/pkg/argoApplication/bean" + bean4 "github.com/devtron-labs/devtron/pkg/fluxApplication/bean" "github.com/devtron-labs/devtron/pkg/userResource/bean" ) @@ -26,3 +28,33 @@ func BuildFetchAppListingReqForJobFromDto(reqBean *bean2.ResourceOptionsReqDto) SortOrder: helper.Asc, // default values set } } + +func ArgoAppToExternalGitOpsApp(applications []*bean3.ArgoApplicationListDto) []*bean.ExternalGitOpsAppDto { + result := make([]*bean.ExternalGitOpsAppDto, 0, len(applications)) + for _, application := range applications { + appDto := bean.ExternalGitOpsAppDto{ + AppName: application.Name, + Namespace: application.Namespace, + ClusterId: application.ClusterId, + ClusterName: application.ClusterName, + } + result = append(result, &appDto) + } + + return result +} + +func FluxAppToExternalGitOpsApp(applications []bean4.FluxApplication) []*bean.ExternalGitOpsAppDto { + result := make([]*bean.ExternalGitOpsAppDto, 0, len(applications)) + for _, application := range applications { + appDto := bean.ExternalGitOpsAppDto{ + AppName: application.Name, + Namespace: application.Namespace, + ClusterId: application.ClusterId, + ClusterName: application.ClusterName, + } + result = append(result, &appDto) + } + + return result +} diff --git a/pkg/userResource/bean/bean.go b/pkg/userResource/bean/bean.go index 03f07bd071..7d085bb8dd 100644 --- a/pkg/userResource/bean/bean.go +++ b/pkg/userResource/bean/bean.go @@ -15,17 +15,25 @@ type UserResourceResponseDto struct { Data interface{} `json:"data"` } type ResourceOptionsDto struct { - TeamsResp []bean2.TeamRequest - HelmEnvResp []*bean.ClusterEnvDto - ClusterResp []bean3.ClusterBean - NameSpaces []string - ApiResourcesResp *k8s.GetAllApiResourcesResponse - ClusterResourcesResp *k8s.ClusterResourceListMap - TeamAppResp []*app.TeamAppBean - EnvResp []bean.EnvironmentBean - ChartGroupResp *chartGroup.ChartGroupList - JobsResp []*AppView.JobContainer - AppWfsResp *bean4.WorkflowNamesResponse + TeamsResp []bean2.TeamRequest + HelmEnvResp []*bean.ClusterEnvDto + ClusterResp []bean3.ClusterBean + NameSpaces []string + ApiResourcesResp *k8s.GetAllApiResourcesResponse + ClusterResourcesResp *k8s.ClusterResourceListMap + TeamAppResp []*app.TeamAppBean + EnvResp []bean.EnvironmentBean + ChartGroupResp *chartGroup.ChartGroupList + JobsResp []*AppView.JobContainer + AppWfsResp *bean4.WorkflowNamesResponse + ExternalGitOpsAppResp []*ExternalGitOpsAppDto +} + +type ExternalGitOpsAppDto struct { + AppName string `json:"appName"` + Namespace string `json:"namespace"` + ClusterId int `json:"clusterId"` + ClusterName string `json:"clusterName"` } func NewResourceOptionsDto() *ResourceOptionsDto { @@ -78,6 +86,11 @@ func (r *ResourceOptionsDto) WithAppWfsResp(appWfsResp *bean4.WorkflowNamesRespo return r } +func (r *ResourceOptionsDto) WithExternalGitOpsAppResp(externalGitOpsAppResp []*ExternalGitOpsAppDto) *ResourceOptionsDto { + r.ExternalGitOpsAppResp = externalGitOpsAppResp + return r +} + type Version string type UserResourceKind string @@ -95,6 +108,10 @@ const ( ClusterNamespaces UserResourceKind = "cluster/namespaces" ClusterApiResources UserResourceKind = "cluster/apiResources" ClusterResources UserResourceKind = "cluster/resources" + KindArgoEnvironment UserResourceKind = "environment/argo" + KindFluxEnvironment UserResourceKind = "environment/flux" + KindArgoApplication UserResourceKind = Application + "/argo" + KindFluxApplication UserResourceKind = Application + "/flux" ) const ( diff --git a/pkg/userResource/bean/messages.go b/pkg/userResource/bean/messages.go index b71c8c199e..27571ddf96 100644 --- a/pkg/userResource/bean/messages.go +++ b/pkg/userResource/bean/messages.go @@ -1,8 +1,9 @@ package bean const ( - InvalidPayloadMessage = "Invalid Payload" - InvalidEntityMessage = "Invalid Entity" + InvalidPayloadMessage = "Invalid Payload" + InvalidEntityMessage = "Invalid Entity" + InvalidClusterIdMessage = "Invalid clusterId" ) // messages diff --git a/pkg/userResource/helper/helper.go b/pkg/userResource/helper/helper.go index 58251e3c91..4fe89415b2 100644 --- a/pkg/userResource/helper/helper.go +++ b/pkg/userResource/helper/helper.go @@ -1,10 +1,13 @@ package helper import ( + "fmt" + "net/http" + "strings" + apiBean "github.com/devtron-labs/devtron/api/userResource/bean" "github.com/devtron-labs/devtron/internal/util" bean5 "github.com/devtron-labs/devtron/pkg/userResource/bean" - "net/http" ) func ValidateResourceOptionReqBean(reqBean *apiBean.ResourceOptionsReqDto) error { @@ -16,3 +19,50 @@ func ValidateResourceOptionReqBean(reqBean *apiBean.ResourceOptionsReqDto) error } return nil } + +func GetValidatedClusterIds(reqBean *apiBean.ResourceOptionsReqDto) ([]int, error) { + invalid := util.GetApiErrorAdapter(http.StatusBadRequest, "400", + bean5.InvalidClusterIdMessage, bean5.InvalidClusterIdMessage) + if reqBean == nil { + return nil, invalid + } + seen := make(map[int]bool, len(reqBean.ClusterIds)+1) + clusterIds := make([]int, 0, len(reqBean.ClusterIds)+1) + appendIfValid := func(clusterId int) { + if clusterId > 0 && !seen[clusterId] { + seen[clusterId] = true + clusterIds = append(clusterIds, clusterId) + } + } + for _, clusterId := range reqBean.ClusterIds { + appendIfValid(clusterId) + } + if reqBean.ResourceRequestBean != nil { + appendIfValid(reqBean.ClusterId) + } + if len(clusterIds) == 0 { + return nil, invalid + } + return clusterIds, nil +} + +func FilterExternalGitOpsAppsByEnvIdentifier(apps []*bean5.ExternalGitOpsAppDto, envIdentifiers []string) []*bean5.ExternalGitOpsAppDto { + if len(envIdentifiers) == 0 { + return apps + } + selected := make(map[string]bool, len(envIdentifiers)) + for _, identifier := range envIdentifiers { + selected[strings.ToLower(identifier)] = true + } + filtered := make([]*bean5.ExternalGitOpsAppDto, 0, len(apps)) + for _, app := range apps { + if app == nil { + continue + } + identifier := fmt.Sprintf("%s__%s", app.ClusterName, app.Namespace) + if selected[strings.ToLower(identifier)] { + filtered = append(filtered, app) + } + } + return filtered +} diff --git a/pkg/userResource/logicRouteService.go b/pkg/userResource/logicRouteService.go index 9b9b41ab2c..5447b6db22 100644 --- a/pkg/userResource/logicRouteService.go +++ b/pkg/userResource/logicRouteService.go @@ -3,6 +3,7 @@ package userResource import ( "context" "fmt" + apiBean "github.com/devtron-labs/devtron/api/userResource/bean" bean2 "github.com/devtron-labs/devtron/pkg/auth/user/bean" "github.com/devtron-labs/devtron/pkg/userResource/bean" @@ -17,12 +18,16 @@ func getUserResourceKindWithVersionKey(kind bean.UserResourceKind, version bean. var mapOfUserResourceKindToAllResourceOptionsFunc = map[string]func(impl *UserResourceServiceImpl, context context.Context, token string, reqBean *apiBean.ResourceOptionsReqDto, params *apiBean.PathParams) (*bean.ResourceOptionsDto, error){ getUserResourceKindWithVersionKey(bean.KindTeam, bean.Alpha1Version): (*UserResourceServiceImpl).getTeamResourceOptions, - getUserResourceKindWithVersionKey(bean.KindHelmEnvironment, bean.Alpha1Version): (*UserResourceServiceImpl).getHelmEnvResourceOptions, + getUserResourceKindWithVersionKey(bean.KindHelmEnvironment, bean.Alpha1Version): (*UserResourceServiceImpl).getCombinedEnvResourceOptions, getUserResourceKindWithVersionKey(bean.KindHelmApplication, bean.Alpha1Version): (*UserResourceServiceImpl).getHelmAppResourceOptions, getUserResourceKindWithVersionKey(bean.KindCluster, bean.Alpha1Version): (*UserResourceServiceImpl).getClusterResourceOptions, getUserResourceKindWithVersionKey(bean.ClusterApiResources, bean.Alpha1Version): (*UserResourceServiceImpl).getClusterApiResourceOptions, getUserResourceKindWithVersionKey(bean.ClusterNamespaces, bean.Alpha1Version): (*UserResourceServiceImpl).getClusterNamespacesResourceOptions, getUserResourceKindWithVersionKey(bean.ClusterResources, bean.Alpha1Version): (*UserResourceServiceImpl).getClusterResourceListOptions, + getUserResourceKindWithVersionKey(bean.KindArgoEnvironment, bean.Alpha1Version): (*UserResourceServiceImpl).getCombinedEnvResourceOptions, + getUserResourceKindWithVersionKey(bean.KindFluxEnvironment, bean.Alpha1Version): (*UserResourceServiceImpl).getCombinedEnvResourceOptions, + getUserResourceKindWithVersionKey(bean.KindArgoApplication, bean.Alpha1Version): (*UserResourceServiceImpl).getArgoAppResourceOptions, + getUserResourceKindWithVersionKey(bean.KindFluxApplication, bean.Alpha1Version): (*UserResourceServiceImpl).getFluxAppResourceOptions, } func getAllResourceOptionsFunc(kind bean.UserResourceKind, version bean.Version) func(impl *UserResourceServiceImpl, context context.Context, token string, reqBean *apiBean.ResourceOptionsReqDto, params *apiBean.PathParams) (*bean.ResourceOptionsDto, error) { @@ -34,7 +39,7 @@ func getAllResourceOptionsFunc(kind bean.UserResourceKind, version bean.Version) var mapOfUserResourceKindToAllResourceOptionsExtendedFunc = map[string]func(impl *UserResourceExtendedServiceImpl, context context.Context, token string, reqBean *apiBean.ResourceOptionsReqDto, params *apiBean.PathParams) (*bean.ResourceOptionsDto, error){ getUserResourceKindWithVersionKey(bean.KindTeam, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getTeamResourceOptions, - getUserResourceKindWithVersionKey(bean.KindHelmEnvironment, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getHelmEnvResourceOptions, + getUserResourceKindWithVersionKey(bean.KindHelmEnvironment, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getCombinedEnvResourceOptions, getUserResourceKindWithVersionKey(bean.KindHelmApplication, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getHelmAppResourceOptions, getUserResourceKindWithVersionKey(bean.KindCluster, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getClusterResourceOptions, getUserResourceKindWithVersionKey(bean.ClusterApiResources, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getClusterApiResourceOptions, @@ -45,6 +50,10 @@ var mapOfUserResourceKindToAllResourceOptionsExtendedFunc = map[string]func(impl getUserResourceKindWithVersionKey(bean.KindChartGroup, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getChartGroupResourceOptions, getUserResourceKindWithVersionKey(bean.KindJobs, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getJobsResourceOptions, getUserResourceKindWithVersionKey(bean.KindWorkflow, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getAppWfsResourceOptions, + getUserResourceKindWithVersionKey(bean.KindArgoEnvironment, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getCombinedEnvResourceOptions, + getUserResourceKindWithVersionKey(bean.KindFluxEnvironment, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getCombinedEnvResourceOptions, + getUserResourceKindWithVersionKey(bean.KindArgoApplication, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getArgoAppResourceOptions, + getUserResourceKindWithVersionKey(bean.KindFluxApplication, bean.Alpha1Version): (*UserResourceExtendedServiceImpl).getFluxAppResourceOptions, } func getAllResourceOptionsExtendedFunc(kind bean.UserResourceKind, version bean.Version) func(impl *UserResourceExtendedServiceImpl, context context.Context, token string, reqBean *apiBean.ResourceOptionsReqDto, params *apiBean.PathParams) (*bean.ResourceOptionsDto, error) { @@ -57,11 +66,15 @@ func getAllResourceOptionsExtendedFunc(kind bean.UserResourceKind, version bean. var mapOfKindWithEntityAccessTypeKeyToResourceOptionRbacFunc = map[string]func(impl *UserResourceServiceImpl, token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error){ getUserResourceKindWithEntityAccessKey(bean.KindTeam, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceServiceImpl).enforceRbacForTeamForHelmApp, getUserResourceKindWithEntityAccessKey(bean.KindHelmApplication, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceServiceImpl).enforceRbacForHelmAppsListing, - getUserResourceKindWithEntityAccessKey(bean.KindHelmEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceServiceImpl).enforceRbacForEnvForHelmApp, + getUserResourceKindWithEntityAccessKey(bean.KindHelmEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceServiceImpl).enforceRbacForEnvOptions, getUserResourceKindWithEntityAccessKey(bean.KindCluster, bean.Alpha1Version, bean2.CLUSTER_ENTITIY, bean2.EmptyAccessType): (*UserResourceServiceImpl).enforceRbacForClusterList, getUserResourceKindWithEntityAccessKey(bean.ClusterApiResources, bean.Alpha1Version, bean2.CLUSTER_ENTITIY, bean2.EmptyAccessType): (*UserResourceServiceImpl).enforceRbacForClusterApiResource, getUserResourceKindWithEntityAccessKey(bean.ClusterNamespaces, bean.Alpha1Version, bean2.CLUSTER_ENTITIY, bean2.EmptyAccessType): (*UserResourceServiceImpl).enforceRbacForClusterNamespaces, getUserResourceKindWithEntityAccessKey(bean.ClusterResources, bean.Alpha1Version, bean2.CLUSTER_ENTITIY, bean2.EmptyAccessType): (*UserResourceServiceImpl).enforceRbacForClusterResourceList, + getUserResourceKindWithEntityAccessKey(bean.KindArgoEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_ARGO): (*UserResourceServiceImpl).enforceRbacForEnvOptions, + getUserResourceKindWithEntityAccessKey(bean.KindFluxEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_FLUX): (*UserResourceServiceImpl).enforceRbacForEnvOptions, + getUserResourceKindWithEntityAccessKey(bean.KindArgoApplication, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_ARGO): (*UserResourceServiceImpl).enforceRbacForArgoAppsListing, + getUserResourceKindWithEntityAccessKey(bean.KindFluxApplication, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_FLUX): (*UserResourceServiceImpl).enforceRbacForFluxAppsListing, } func getResourceOptionRbacFunc(kind bean.UserResourceKind, version bean.Version, entity string, accessType string) func(impl *UserResourceServiceImpl, token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error) { @@ -74,7 +87,7 @@ func getResourceOptionRbacFunc(kind bean.UserResourceKind, version bean.Version, var mapOfKindWithEntityAccessTypeKeyToResourceOptionRbacExtendedFunc = map[string]func(impl *UserResourceExtendedServiceImpl, token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error){ getUserResourceKindWithEntityAccessKey(bean.KindTeam, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceExtendedServiceImpl).enforceRbacForTeamForHelmApp, getUserResourceKindWithEntityAccessKey(bean.KindHelmApplication, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceExtendedServiceImpl).enforceRbacForHelmAppsListing, - getUserResourceKindWithEntityAccessKey(bean.KindHelmEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceExtendedServiceImpl).enforceRbacForEnvForHelmApp, + getUserResourceKindWithEntityAccessKey(bean.KindHelmEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_HELM): (*UserResourceExtendedServiceImpl).enforceRbacForEnvOptions, getUserResourceKindWithEntityAccessKey(bean.KindCluster, bean.Alpha1Version, bean2.CLUSTER_ENTITIY, bean2.EmptyAccessType): (*UserResourceExtendedServiceImpl).enforceRbacForClusterList, getUserResourceKindWithEntityAccessKey(bean.ClusterApiResources, bean.Alpha1Version, bean2.CLUSTER_ENTITIY, bean2.EmptyAccessType): (*UserResourceExtendedServiceImpl).enforceRbacForClusterApiResource, getUserResourceKindWithEntityAccessKey(bean.ClusterNamespaces, bean.Alpha1Version, bean2.CLUSTER_ENTITIY, bean2.EmptyAccessType): (*UserResourceExtendedServiceImpl).enforceRbacForClusterNamespaces, @@ -87,6 +100,10 @@ var mapOfKindWithEntityAccessTypeKeyToResourceOptionRbacExtendedFunc = map[strin getUserResourceKindWithEntityAccessKey(bean.KindChartGroup, bean.Alpha1Version, bean2.CHART_GROUP_ENTITY, bean2.EmptyAccessType): (*UserResourceExtendedServiceImpl).enforceRbacForChartGroup, getUserResourceKindWithEntityAccessKey(bean.KindJobs, bean.Alpha1Version, bean2.EntityJobs, bean2.EmptyAccessType): (*UserResourceExtendedServiceImpl).enforceRbacForJobs, getUserResourceKindWithEntityAccessKey(bean.KindWorkflow, bean.Alpha1Version, bean2.EntityJobs, bean2.EmptyAccessType): (*UserResourceExtendedServiceImpl).enforceRbacForJobsWfs, + getUserResourceKindWithEntityAccessKey(bean.KindArgoEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_ARGO): (*UserResourceExtendedServiceImpl).enforceRbacForEnvOptions, + getUserResourceKindWithEntityAccessKey(bean.KindFluxEnvironment, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_FLUX): (*UserResourceExtendedServiceImpl).enforceRbacForEnvOptions, + getUserResourceKindWithEntityAccessKey(bean.KindArgoApplication, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_ARGO): (*UserResourceExtendedServiceImpl).enforceRbacForArgoAppsListing, + getUserResourceKindWithEntityAccessKey(bean.KindFluxApplication, bean.Alpha1Version, bean2.ENTITY_APPS, bean2.APP_ACCESS_TYPE_FLUX): (*UserResourceExtendedServiceImpl).enforceRbacForFluxAppsListing, } func getResourceOptionRbacExtendedFunc(kind bean.UserResourceKind, version bean.Version, entity string, accessType string) func(impl *UserResourceExtendedServiceImpl, token string, params *apiBean.PathParams, resourceOptions *bean.ResourceOptionsDto) (*bean.UserResourceResponseDto, error) { diff --git a/scripts/casbin/12_argo_flux_rbac.down.sql b/scripts/casbin/12_argo_flux_rbac.down.sql new file mode 100644 index 0000000000..59b12b5f86 --- /dev/null +++ b/scripts/casbin/12_argo_flux_rbac.down.sql @@ -0,0 +1,2 @@ +DELETE FROM casbin_rule WHERE v0='role:super-admin___' AND v1='argo-app'; +DELETE FROM casbin_rule WHERE v0='role:super-admin___' AND v1='flux-app'; \ No newline at end of file diff --git a/scripts/casbin/12_argo_flux_rbac.up.sql b/scripts/casbin/12_argo_flux_rbac.up.sql new file mode 100644 index 0000000000..17e4b72057 --- /dev/null +++ b/scripts/casbin/12_argo_flux_rbac.up.sql @@ -0,0 +1,3 @@ +INSERT INTO "public"."casbin_rule" ("p_type","v0","v1","v2","v3","v4","v5") VALUES + ('p','role:super-admin___','argo-app','*','*','allow',''), + ('p','role:super-admin___','flux-app','*','*','allow',''); \ No newline at end of file diff --git a/scripts/sql/36604600_argo_flux_rbac.down.sql b/scripts/sql/36604600_argo_flux_rbac.down.sql new file mode 100644 index 0000000000..57d8b04829 --- /dev/null +++ b/scripts/sql/36604600_argo_flux_rbac.down.sql @@ -0,0 +1,2 @@ +DELETE FROM "public"."rbac_policy_data" WHERE entity='apps' AND access_type IN ('argo-app','flux-app'); +DELETE FROM "public"."rbac_role_data" WHERE entity='apps' AND access_type IN ('argo-app','flux-app'); \ No newline at end of file diff --git a/scripts/sql/36604600_argo_flux_rbac.up.sql b/scripts/sql/36604600_argo_flux_rbac.up.sql new file mode 100644 index 0000000000..c7ce5e8a0c --- /dev/null +++ b/scripts/sql/36604600_argo_flux_rbac.up.sql @@ -0,0 +1,111 @@ +INSERT INTO "public"."rbac_policy_data" +("entity","access_type","role","policy_data", + "created_on","created_by","updated_on","updated_by","is_preset_role","deleted") +VALUES + ('apps','argo-app','view','{ + "type": { "value": "p", "indexKeyMap": {} }, + "sub": { "value": "argo-app:view_%_%", + "indexKeyMap": { "14": "Env", "16": "App" } }, + "resActObjSet": [ + { "res": { "value": "argo-app", "indexKeyMap": {} }, + "act": { "value": "get", "indexKeyMap": {} }, + "obj": { "value": "%/%", + "indexKeyMap": { "0": "EnvObj", "2": "AppObj" } } }, + { "res": { "value": "global-environment", "indexKeyMap": {} }, + "act": { "value": "get", "indexKeyMap": {} }, + "obj": { "value": "%", "indexKeyMap": { "0": "EnvObj" } } } + ] +}','now()','1','now()','1',true,false), + + ('apps','argo-app','admin','{ + "type": { "value": "p", "indexKeyMap": {} }, + "sub": { "value": "argo-app:admin_%_%", + "indexKeyMap": { "15": "Env", "17": "App" } }, + "resActObjSet": [ + { "res": { "value": "argo-app", "indexKeyMap": {} }, + "act": { "value": "*", "indexKeyMap": {} }, + "obj": { "value": "%/%", + "indexKeyMap": { "0": "EnvObj", "2": "AppObj" } } }, + { "res": { "value": "global-environment", "indexKeyMap": {} }, + "act": { "value": "get", "indexKeyMap": {} }, + "obj": { "value": "%", "indexKeyMap": { "0": "EnvObj" } } } + ] +}','now()','1','now()','1',true,false), + + ('apps','flux-app','view','{ + "type": { "value": "p", "indexKeyMap": {} }, + "sub": { "value": "flux-app:view_%_%", + "indexKeyMap": { "14": "Env", "16": "App" } }, + "resActObjSet": [ + { "res": { "value": "flux-app", "indexKeyMap": {} }, + "act": { "value": "get", "indexKeyMap": {} }, + "obj": { "value": "%/%", + "indexKeyMap": { "0": "EnvObj", "2": "AppObj" } } }, + { "res": { "value": "global-environment", "indexKeyMap": {} }, + "act": { "value": "get", "indexKeyMap": {} }, + "obj": { "value": "%", "indexKeyMap": { "0": "EnvObj" } } } + ] +}','now()','1','now()','1',true,false), + + ('apps','flux-app','admin','{ + "type": { "value": "p", "indexKeyMap": {} }, + "sub": { "value": "flux-app:admin_%_%", + "indexKeyMap": { "15": "Env", "17": "App" } }, + "resActObjSet": [ + { "res": { "value": "flux-app", "indexKeyMap": {} }, + "act": { "value": "*", "indexKeyMap": {} }, + "obj": { "value": "%/%", + "indexKeyMap": { "0": "EnvObj", "2": "AppObj" } } }, + { "res": { "value": "global-environment", "indexKeyMap": {} }, + "act": { "value": "get", "indexKeyMap": {} }, + "obj": { "value": "%", "indexKeyMap": { "0": "EnvObj" } } } + ] +}','now()','1','now()','1',true,false); + +INSERT INTO "public"."rbac_role_data" +("entity","access_type","role","role_display_name","role_description","role_data", + "created_on","created_by","updated_on","updated_by","is_preset_role","deleted") +VALUES + ('apps','argo-app','view','View only', + 'Can view selected Argo CD application(s) and resource manifests of selected application(s)','{ + "role": { "value": "argo-app:view_%_%", + "indexKeyMap": { "14": "Env", "16": "App" } }, + "entityName": { "value": "%", "indexKeyMap": { "0": "App" } }, + "environment":{ "value": "%", "indexKeyMap": { "0": "Env" } }, + "action": { "value": "view", "indexKeyMap": {} }, + "entity": { "value": "%", "indexKeyMap": { "0": "Entity" } }, + "accessType": { "value": "argo-app", "indexKeyMap": {} } +}','now()','1','now()','1',true,false), + + ('apps','argo-app','admin','Admin', + 'Complete access on selected Argo CD application(s)','{ + "role": { "value": "argo-app:admin_%_%", + "indexKeyMap": { "15": "Env", "17": "App" } }, + "entityName": { "value": "%", "indexKeyMap": { "0": "App" } }, + "environment":{ "value": "%", "indexKeyMap": { "0": "Env" } }, + "action": { "value": "admin", "indexKeyMap": {} }, + "entity": { "value": "%", "indexKeyMap": { "0": "Entity" } }, + "accessType": { "value": "argo-app", "indexKeyMap": {} } +}','now()','1','now()','1',true,false), + + ('apps','flux-app','view','View only', + 'Can view selected Flux CD application(s) and resource manifests of selected application(s)','{ + "role": { "value": "flux-app:view_%_%", + "indexKeyMap": { "14": "Env", "16": "App" } }, + "entityName": { "value": "%", "indexKeyMap": { "0": "App" } }, + "environment":{ "value": "%", "indexKeyMap": { "0": "Env" } }, + "action": { "value": "view", "indexKeyMap": {} }, + "entity": { "value": "%", "indexKeyMap": { "0": "Entity" } }, + "accessType": { "value": "flux-app", "indexKeyMap": {} } +}','now()','1','now()','1',true,false), + + ('apps','flux-app','admin','Admin', + 'Complete access on selected Flux CD application(s)','{ + "role": { "value": "flux-app:admin_%_%", + "indexKeyMap": { "15": "Env", "17": "App" } }, + "entityName": { "value": "%", "indexKeyMap": { "0": "App" } }, + "environment":{ "value": "%", "indexKeyMap": { "0": "Env" } }, + "action": { "value": "admin", "indexKeyMap": {} }, + "entity": { "value": "%", "indexKeyMap": { "0": "Entity" } }, + "accessType": { "value": "flux-app", "indexKeyMap": {} } +}','now()','1','now()','1',true,false); \ No newline at end of file diff --git a/util/rbac/EnforcerUtilGitOps.go b/util/rbac/EnforcerUtilGitOps.go new file mode 100644 index 0000000000..546dcf2fab --- /dev/null +++ b/util/rbac/EnforcerUtilGitOps.go @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2024. Devtron Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package rbac + +import ( + "fmt" + + "github.com/devtron-labs/devtron/pkg/cluster/read" + "go.uber.org/zap" +) + +// EnforcerUtilGitOps builds RBAC objects for external Argo CD and Flux CD applications. +// +// The object is two segments — __/ — where the first segment +// is the environment_identifier convention used throughout Devtron. There is no project +// segment: external GitOps applications have no Devtron project, so unlike external Helm apps +// there is no team name and no "unassigned" placeholder to fill. +// +// Both segments are always populated, which matters because matchKeyByPart rejects an empty +// segment on either side unconditionally. +type EnforcerUtilGitOps interface { + // GetExternalGitOpsAppObject returns the RBAC object for an external Argo/Flux application. + // Returns an empty string if the cluster cannot be resolved, which callers must treat as + // a denial rather than as a wildcard. + GetExternalGitOpsAppObject(clusterId int, namespace string, appName string) string + // GetExternalGitOpsAppObjectByClusterName is the same, for callers that already hold the + // cluster name and can avoid the lookup. + GetExternalGitOpsAppObjectByClusterName(clusterName string, namespace string, appName string) string +} + +type EnforcerUtilGitOpsImpl struct { + logger *zap.SugaredLogger + clusterReadService read.ClusterReadService +} + +func NewEnforcerUtilGitOpsImpl(logger *zap.SugaredLogger, + clusterReadService read.ClusterReadService) *EnforcerUtilGitOpsImpl { + return &EnforcerUtilGitOpsImpl{ + logger: logger, + clusterReadService: clusterReadService, + } +} + +func (impl EnforcerUtilGitOpsImpl) GetExternalGitOpsAppObject(clusterId int, namespace string, appName string) string { + cluster, err := impl.clusterReadService.FindById(clusterId) + if err != nil { + impl.logger.Errorw("error on fetching cluster for rbac object", "err", err, "clusterId", clusterId) + return "" + } + return impl.GetExternalGitOpsAppObjectByClusterName(cluster.ClusterName, namespace, appName) +} + +func (impl EnforcerUtilGitOpsImpl) GetExternalGitOpsAppObjectByClusterName(clusterName string, namespace string, appName string) string { + if len(clusterName) == 0 || len(namespace) == 0 || len(appName) == 0 { + impl.logger.Errorw("incomplete identifier for rbac object, denying", + "clusterName", clusterName, "namespace", namespace, "appName", appName) + return "" + } + return fmt.Sprintf("%s__%s/%s", clusterName, namespace, appName) +} diff --git a/wire_gen.go b/wire_gen.go index e3518a51bb..12fafa8ce5 100644 --- a/wire_gen.go +++ b/wire_gen.go @@ -1009,7 +1009,8 @@ func InitializeApp() (*App, error) { coreAppRouterImpl := router.NewCoreAppRouterImpl(coreAppRestHandlerImpl) helmAppRestHandlerImpl := client3.NewHelmAppRestHandlerImpl(sugaredLogger, helmAppServiceImpl, enforcerImpl, clusterServiceImplExtended, enforcerUtilHelmImpl, appStoreDeploymentServiceImpl, installedAppDBServiceImpl, userServiceImpl, attributesServiceImpl, serverEnvConfigServerEnvConfig, fluxApplicationServiceImpl, argoApplicationServiceExtendedImpl) helmAppRouterImpl := client3.NewHelmAppRouterImpl(helmAppRestHandlerImpl) - k8sApplicationRestHandlerImpl := application3.NewK8sApplicationRestHandlerImpl(sugaredLogger, k8sApplicationServiceImpl, pumpImpl, terminalSessionHandlerImpl, enforcerImpl, enforcerUtilHelmImpl, enforcerUtilImpl, helmAppServiceImpl, userServiceImpl, k8sCommonServiceImpl, validate, environmentVariables, fluxApplicationServiceImpl, argoApplicationReadServiceImpl) + enforcerUtilGitOpsImpl := rbac.NewEnforcerUtilGitOpsImpl(sugaredLogger, clusterReadServiceImpl) + k8sApplicationRestHandlerImpl := application3.NewK8sApplicationRestHandlerImpl(sugaredLogger, k8sApplicationServiceImpl, pumpImpl, terminalSessionHandlerImpl, enforcerImpl, enforcerUtilHelmImpl, enforcerUtilGitOpsImpl, enforcerUtilImpl, helmAppServiceImpl, userServiceImpl, k8sCommonServiceImpl, validate, environmentVariables, fluxApplicationServiceImpl, argoApplicationReadServiceImpl) k8sApplicationRouterImpl := application3.NewK8sApplicationRouterImpl(k8sApplicationRestHandlerImpl) pProfRestHandlerImpl := restHandler.NewPProfRestHandler(userServiceImpl, enforcerImpl) pProfRouterImpl := router.NewPProfRouter(sugaredLogger, pProfRestHandlerImpl) @@ -1098,18 +1099,18 @@ func InitializeApp() (*App, error) { deploymentConfigurationRouterImpl := configDiff3.NewDeploymentConfigurationRouter(deploymentConfigurationRestHandlerImpl) infraConfigRestHandlerImpl := infraConfig.NewInfraConfigRestHandlerImpl(sugaredLogger, infraConfigServiceImpl, userServiceImpl, enforcerImpl, enforcerUtilImpl, validate) infraConfigRouterImpl := infraConfig.NewInfraProfileRouterImpl(infraConfigRestHandlerImpl) - argoApplicationRestHandlerImpl := argoApplication2.NewArgoApplicationRestHandlerImpl(argoApplicationServiceExtendedImpl, argoApplicationReadServiceImpl, sugaredLogger, enforcerImpl) + argoApplicationRestHandlerImpl := argoApplication2.NewArgoApplicationRestHandlerImpl(argoApplicationServiceExtendedImpl, argoApplicationReadServiceImpl, sugaredLogger, enforcerImpl, enforcerUtilGitOpsImpl) argoApplicationRouterImpl := argoApplication2.NewArgoApplicationRouterImpl(argoApplicationRestHandlerImpl) deploymentHistoryServiceImpl := cdPipeline.NewDeploymentHistoryServiceImpl(sugaredLogger, cdHandlerImpl, imageTaggingReadServiceImpl, imageTaggingServiceImpl, pipelineRepositoryImpl, deployedConfigurationHistoryServiceImpl) apiReqDecoderServiceImpl := devtronResource.NewAPIReqDecoderServiceImpl(sugaredLogger, pipelineRepositoryImpl) historyRestHandlerImpl := devtronResource2.NewHistoryRestHandlerImpl(sugaredLogger, enforcerImpl, deploymentHistoryServiceImpl, apiReqDecoderServiceImpl, enforcerUtilImpl) historyRouterImpl := devtronResource2.NewHistoryRouterImpl(historyRestHandlerImpl) devtronResourceRouterImpl := devtronResource2.NewDevtronResourceRouterImpl(historyRouterImpl) - fluxApplicationRestHandlerImpl := fluxApplication2.NewFluxApplicationRestHandlerImpl(fluxApplicationServiceImpl, sugaredLogger, enforcerImpl) + fluxApplicationRestHandlerImpl := fluxApplication2.NewFluxApplicationRestHandlerImpl(fluxApplicationServiceImpl, sugaredLogger, enforcerImpl, enforcerUtilGitOpsImpl) fluxApplicationRouterImpl := fluxApplication2.NewFluxApplicationRouterImpl(fluxApplicationRestHandlerImpl) scanningResultRestHandlerImpl := resourceScan.NewScanningResultRestHandlerImpl(sugaredLogger, userServiceImpl, imageScanServiceImpl, enforcerImpl, enforcerUtilImpl, validate) scanningResultRouterImpl := resourceScan.NewScanningResultRouterImpl(scanningResultRestHandlerImpl) - userResourceExtendedServiceImpl := userResource.NewUserResourceExtendedServiceImpl(sugaredLogger, teamServiceImpl, environmentServiceImpl, appCrudOperationServiceImpl, chartGroupServiceImpl, appListingServiceImpl, appWorkflowServiceImpl, k8sApplicationServiceImpl, clusterServiceImplExtended, commonEnforcementUtilImpl, enforcerUtilImpl, enforcerImpl) + userResourceExtendedServiceImpl := userResource.NewUserResourceExtendedServiceImpl(sugaredLogger, teamServiceImpl, environmentServiceImpl, appCrudOperationServiceImpl, chartGroupServiceImpl, appListingServiceImpl, appWorkflowServiceImpl, k8sApplicationServiceImpl, clusterServiceImplExtended, commonEnforcementUtilImpl, enforcerUtilImpl, enforcerImpl, argoApplicationServiceExtendedImpl, fluxApplicationServiceImpl) restHandlerImpl := userResource2.NewUserResourceRestHandler(sugaredLogger, userServiceImpl, userResourceExtendedServiceImpl) routerImpl := userResource2.NewUserResourceRouterImpl(restHandlerImpl) appManagementServiceImpl := overview.NewAppManagementServiceImpl(sugaredLogger, appRepositoryImpl, pipelineRepositoryImpl, ciPipelineRepositoryImpl, ciWorkflowRepositoryImpl, cdWorkflowRepositoryImpl, environmentRepositoryImpl, teamRepositoryImpl, workflowStageRepositoryImpl, repositoryImpl) From 5e957a5e34301372de1b0db6e886a303371cabd5 Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Mon, 21 Sep 2026 17:07:37 +0530 Subject: [PATCH 12/14] chore: update devtron-services deps to main for v0.47.0 Points common-lib and authenticator at devtron-services main (f0b3fba15081b8d40a795928aaad5df55473c681, the v0.47.0 RC merge). Co-Authored-By: Claude Opus 5 (1M context) --- go.mod | 4 ++-- go.sum | 8 ++++---- vendor/modules.txt | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index e2cc19dca3..500ac628bb 100644 --- a/go.mod +++ b/go.mod @@ -341,7 +341,7 @@ replace ( // Keep the left side unversioned: a versioned LHS stops matching whenever the // require line is bumped, which silently un-pins this and breaks the linux build. github.com/cyphar/filepath-securejoin => github.com/cyphar/filepath-securejoin v0.3.6 // indirect - github.com/devtron-labs/authenticator => github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260803100101-66fcb35e4b0e - github.com/devtron-labs/common-lib => github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260803100101-66fcb35e4b0e + github.com/devtron-labs/authenticator => github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260921111017-f0b3fba15081 + github.com/devtron-labs/common-lib => github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260921111017-f0b3fba15081 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.61.0 => go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.46.1 ) diff --git a/go.sum b/go.sum index f22eaecf9a..12d371f020 100644 --- a/go.sum +++ b/go.sum @@ -237,10 +237,10 @@ github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc h1:VRRKCwnzq github.com/denisenkom/go-mssqldb v0.0.0-20200428022330-06a60b6afbbc/go.mod h1:xbL0rPBG9cCiLr28tMa8zpbdarY27NDyej4t/EjAShU= github.com/devtron-labs/argo-workflows/v3 v3.5.13 h1:3pINq0gXOSeTw2z/vYe+j80lRpSN5Rp/8mfQORh8SmU= github.com/devtron-labs/argo-workflows/v3 v3.5.13/go.mod h1:/vqxcovDPT4zqr4DjR5v7CF8ggpY1l3TSa2CIG3jmjA= -github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260803100101-66fcb35e4b0e h1:mfAbqKv+CswkCah0JCSzeTJThvjaNBt51TunM8lBsPU= -github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260803100101-66fcb35e4b0e/go.mod h1:m/dMed6iiZQ/4iOJ6kIVWXQ+T71rJjU7DnCr9RcYh1w= -github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260803100101-66fcb35e4b0e h1:gh/iIVC9FNpPE8C72duWZGNgwgE2jrYonpYDvty2cFM= -github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260803100101-66fcb35e4b0e/go.mod h1:QZfnuGy0xqrYWhq3jCRe6fDDwIsrpbjAR2PXNrh0su8= +github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260921111017-f0b3fba15081 h1:BPkaewVaxMTnIAHcxwsWgebl5GV7uc6R/a0tCBoTZn0= +github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260921111017-f0b3fba15081/go.mod h1:m/dMed6iiZQ/4iOJ6kIVWXQ+T71rJjU7DnCr9RcYh1w= +github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260921111017-f0b3fba15081 h1:niegDwFaPhH5b+7UP2QuGdR6rB43hFYqNY1pJ6ZbXQw= +github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260921111017-f0b3fba15081/go.mod h1:QZfnuGy0xqrYWhq3jCRe6fDDwIsrpbjAR2PXNrh0su8= github.com/devtron-labs/go-bitbucket v0.9.60-beta h1:VEx1jvDgdtDPS6A1uUFoaEi0l1/oLhbr+90xOwr6sDU= github.com/devtron-labs/go-bitbucket v0.9.60-beta/go.mod h1:GnuiCesvh8xyHeMCb+twm8lBR/kQzJYSKL28ZfObp1Y= github.com/devtron-labs/protos v0.0.3-0.20250323220609-ecf8a0f7305e h1:U6UdYbW8a7xn5IzFPd8cywjVVPfutGJCudjePAfL/Hs= diff --git a/vendor/modules.txt b/vendor/modules.txt index 4ee86c28d3..eae973ecd9 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -523,7 +523,7 @@ github.com/davecgh/go-spew/spew # github.com/deckarep/golang-set v1.8.0 ## explicit; go 1.17 github.com/deckarep/golang-set -# github.com/devtron-labs/authenticator v0.4.35-0.20240809073103-6e11da8083f8 => github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260803100101-66fcb35e4b0e +# github.com/devtron-labs/authenticator v0.4.35-0.20240809073103-6e11da8083f8 => github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260921111017-f0b3fba15081 ## explicit; go 1.25.0 github.com/devtron-labs/authenticator/apiToken github.com/devtron-labs/authenticator/client @@ -531,7 +531,7 @@ github.com/devtron-labs/authenticator/jwt github.com/devtron-labs/authenticator/middleware github.com/devtron-labs/authenticator/oidc github.com/devtron-labs/authenticator/password -# github.com/devtron-labs/common-lib v0.18.1-0.20241001061923-eda545dc839e => github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260803100101-66fcb35e4b0e +# github.com/devtron-labs/common-lib v0.18.1-0.20241001061923-eda545dc839e => github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260921111017-f0b3fba15081 ## explicit; go 1.25.0 github.com/devtron-labs/common-lib/async github.com/devtron-labs/common-lib/blob-storage @@ -2697,5 +2697,5 @@ xorm.io/xorm/names xorm.io/xorm/schemas xorm.io/xorm/tags # github.com/cyphar/filepath-securejoin => github.com/cyphar/filepath-securejoin v0.3.6 -# github.com/devtron-labs/authenticator => github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260803100101-66fcb35e4b0e -# github.com/devtron-labs/common-lib => github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260803100101-66fcb35e4b0e +# github.com/devtron-labs/authenticator => github.com/devtron-labs/devtron-services/authenticator v0.0.0-20260921111017-f0b3fba15081 +# github.com/devtron-labs/common-lib => github.com/devtron-labs/devtron-services/common-lib v0.0.0-20260921111017-f0b3fba15081 From c83900e0c7b76647f3c450c1a2b30dae33465028 Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Mon, 21 Sep 2026 17:30:01 +0530 Subject: [PATCH 13/14] chore(sql): resolve migration 36504600 collision and make argo/flux rbac idempotent Two different migrations shared version 36504600 on the release branch: add_ci_workflow_active_status_index and dev_admin_app_lifecycle. The latter is already released on main at that number, so the CI index migration (RC-only) is renumbered to 36904600, the next free slot. Also adds ON CONFLICT ("entity","access_type","role") DO NOTHING to both INSERTs in 36804600_argo_flux_rbac.up.sql. Both rbac_policy_data and rbac_role_data declare UNIQUE (entity, access_type, role), so a re-run previously failed on the unique constraint instead of being a no-op. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/sql/36804600_argo_flux_rbac.up.sql | 6 ++++-- ...> 36904600_add_ci_workflow_active_status_index.down.sql} | 0 ... => 36904600_add_ci_workflow_active_status_index.up.sql} | 0 3 files changed, 4 insertions(+), 2 deletions(-) rename scripts/sql/{36504600_add_ci_workflow_active_status_index.down.sql => 36904600_add_ci_workflow_active_status_index.down.sql} (100%) rename scripts/sql/{36504600_add_ci_workflow_active_status_index.up.sql => 36904600_add_ci_workflow_active_status_index.up.sql} (100%) diff --git a/scripts/sql/36804600_argo_flux_rbac.up.sql b/scripts/sql/36804600_argo_flux_rbac.up.sql index c7ce5e8a0c..0f10c65062 100644 --- a/scripts/sql/36804600_argo_flux_rbac.up.sql +++ b/scripts/sql/36804600_argo_flux_rbac.up.sql @@ -60,7 +60,8 @@ VALUES "act": { "value": "get", "indexKeyMap": {} }, "obj": { "value": "%", "indexKeyMap": { "0": "EnvObj" } } } ] -}','now()','1','now()','1',true,false); +}','now()','1','now()','1',true,false) +ON CONFLICT ("entity", "access_type", "role") DO NOTHING; INSERT INTO "public"."rbac_role_data" ("entity","access_type","role","role_display_name","role_description","role_data", @@ -108,4 +109,5 @@ VALUES "action": { "value": "admin", "indexKeyMap": {} }, "entity": { "value": "%", "indexKeyMap": { "0": "Entity" } }, "accessType": { "value": "flux-app", "indexKeyMap": {} } -}','now()','1','now()','1',true,false); \ No newline at end of file +}','now()','1','now()','1',true,false) +ON CONFLICT ("entity", "access_type", "role") DO NOTHING; diff --git a/scripts/sql/36504600_add_ci_workflow_active_status_index.down.sql b/scripts/sql/36904600_add_ci_workflow_active_status_index.down.sql similarity index 100% rename from scripts/sql/36504600_add_ci_workflow_active_status_index.down.sql rename to scripts/sql/36904600_add_ci_workflow_active_status_index.down.sql diff --git a/scripts/sql/36504600_add_ci_workflow_active_status_index.up.sql b/scripts/sql/36904600_add_ci_workflow_active_status_index.up.sql similarity index 100% rename from scripts/sql/36504600_add_ci_workflow_active_status_index.up.sql rename to scripts/sql/36904600_add_ci_workflow_active_status_index.up.sql From c24e9d825d7cf927be755dbc6a5056e290e80f7a Mon Sep 17 00:00:00 2001 From: Shivam-nagar23 Date: Tue, 22 Sep 2026 14:32:34 +0530 Subject: [PATCH 14/14] fix: apply Hibernating app-status filter in SQL instead of after pagination The Devtron Apps list dropped the entire AppStatuses filter whenever HIBERNATING was among the selected statuses, ran LIMIT/OFFSET and count(distinct a.id) over the *unfiltered* app set, and only then re-applied the status filter in memory -- to the single page that came back -- while overwriting the total count with the size of that surviving slice. So the filter returned just the hibernating apps that happened to land on the requested page of the unfiltered listing, and reported that handful as the grand total, leaving the UI with no further pages. Verified against a customer database: 592 hibernating apps exist, 2 were shown. HIBERNATING is an ordinary value persisted in app_status.status (appStatus.UpdateStatusWithAppIdEnvId normalises Suspended/Hibernated to it), and the in-memory pass compared against that very same column -- so the predicate was always expressible in SQL. buildAppListingWhereCondition already emits `aps.status IN (?)` for any status list, including the NOT DEPLOYED combination, so the workaround is simply removed rather than replaced. This also fixes combination filters that were collateral damage: selecting Healthy + Hibernating blanked both predicates, and Hibernating + Not Deployed additionally killed the NOT DEPLOYED SQL branch. The workaround was originally added for Helm-type pipelines, whose status is not persisted in app_status. That cohort is unaffected either way -- the in-memory pass read the same empty column -- and is now documented as a TODO in place of the removed code. Pager: devtron-labs/sprint-tasks#2974 Co-Authored-By: Claude Opus 5 (1M context) --- pkg/app/AppListingService.go | 42 ++++++++++-------------------------- 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/pkg/app/AppListingService.go b/pkg/app/AppListingService.go index 553443d97e..2a264e433f 100644 --- a/pkg/app/AppListingService.go +++ b/pkg/app/AppListingService.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "github.com/devtron-labs/devtron/api/bean/AppView" - bean2 "github.com/devtron-labs/devtron/client/argocdServer/bean" "github.com/devtron-labs/devtron/internal/middleware" "github.com/devtron-labs/devtron/internal/sql/repository/app" "github.com/devtron-labs/devtron/internal/sql/repository/pipelineConfig/bean/workflow/cdWorkflow" @@ -438,16 +437,17 @@ func (impl AppListingServiceImpl) FetchAppsByEnvironmentV2(fetchAppListingReques return []*AppView.AppEnvironmentContainer{}, 0, nil } - // Currently AppStatus is available in Db for only ArgoApps - // We fetch AppStatus on the fly for Helm Apps from scoop, So AppStatus filter will be applied in last - // fun to check if "HIBERNATING" exists in fetchAppListingRequest.AppStatuses - isFilteredOnHibernatingStatus := impl.isFilteredOnHibernatingStatus(fetchAppListingRequest) - // remove ""HIBERNATING" from fetchAppListingRequest.AppStatuses - appStatusesFilter := make([]string, 0) - if isFilteredOnHibernatingStatus { - appStatusesFilter = fetchAppListingRequest.AppStatuses - fetchAppListingRequest.AppStatuses = []string{} - } + // NOTE on "HIBERNATING": it is an ordinary value persisted in app_status.status + // (appStatus.UpdateStatusWithAppIdEnvId normalises Suspended/Hibernated to it), so it is + // filterable in SQL exactly like every other app status and is applied below, before + // LIMIT/OFFSET, by buildAppListingWhereCondition. + // + // TODO (helm pipelines): Helm-type (non-GitOps) CD pipelines never get an app_status row -- + // their status is only known by querying the running release. So a hibernated helm-pipeline + // app cannot be matched by this filter today. The fix is to persist their status into + // app_status from the status-sync worker (after which the predicate below covers them for + // free), NOT to fan out live status calls during a list request -- that is what + // updateAppStatusForHelmTypePipelines did and it was removed from this path for slowness. appListingFilter := helper.AppListingFilter{ Environments: fetchAppListingRequest.Environments, @@ -506,29 +506,9 @@ func (impl AppListingServiceImpl) FetchAppsByEnvironmentV2(fetchAppListingReques impl.Logger.Errorw("error, UpdateAppStatusForHelmTypePipelines", "envIds", envIds, "err", err) } - // apply filter for "HIBERNATING" status - if isFilteredOnHibernatingStatus { - filteredContainers := make([]*AppView.AppEnvironmentContainer, 0) - for _, container := range envContainers { - if slices.Contains(appStatusesFilter, container.AppStatus) { - filteredContainers = append(filteredContainers, container) - } - } - envContainers = filteredContainers - appSize = len(filteredContainers) - } return envContainers, appSize, nil } -func (impl AppListingServiceImpl) isFilteredOnHibernatingStatus(fetchAppListingRequest FetchAppListingRequest) bool { - if fetchAppListingRequest.AppStatuses != nil && len(fetchAppListingRequest.AppStatuses) > 0 { - if slices.Contains(fetchAppListingRequest.AppStatuses, bean2.HIBERNATING) { - return true - } - } - return false -} - func (impl AppListingServiceImpl) ISLastReleaseStopType(appId, envId int) (bool, error) { override, err := impl.pipelineOverrideRepository.GetLatestRelease(appId, envId) if err != nil && !util.IsErrNoRows(err) {