Skip to content

feat: resolve RBAC inconsistencies for helm apps with unique identifiers - #7034

Open
Shivam-nagar23 wants to merge 1 commit into
mainfrom
fix-link-app-rbac
Open

feat: resolve RBAC inconsistencies for helm apps with unique identifiers#7034
Shivam-nagar23 wants to merge 1 commit into
mainfrom
fix-link-app-rbac

Conversation

@Shivam-nagar23

@Shivam-nagar23 Shivam-nagar23 commented Sep 11, 2026

Copy link
Copy Markdown
Member

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.

Description

Fixes https://github.com/devtron-labs/sprint-tasks/issues/2960

Checklist:

  • The title of the PR states what changed and the related issues number (used for the release note).
  • Does this PR requires documentation updates?
  • I've updated documentation as required by this PR.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have tested it for all user roles.
  • I have added all the required unit/api test cases.

Does this PR introduce a user-facing change?


Summary by Bito

  • Added GetAppNameForRbac method to App model to prioritize display name for RBAC policies.
  • Updated EnforcerUtil to use the new app name resolution and handle unassigned projects for Helm RBAC objects.
  • Improved app lookup logic in EnforcerUtil to support unique identifiers for external Helm apps.
  • Refactored EnforcerUtilHelm to consistently use the new RBAC name resolution logic.
  • Added unit tests for Helm RBAC object generation.

When an external helm app is linked to the chart store, app_name holds the
unique identifier (<releaseName>-<namespace>-<clusterId>) 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) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n

@github-actions

Copy link
Copy Markdown

Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n

@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant