feat: Argo flux permission - #3145
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved environment selection, loading-state, stale-value, and validation issues block approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds Argo CD and Flux CD permission management and enables authorized users to access corresponding application views.
Changes:
- Adds permission tabs, APIs, validation, and environment-scoped selectors.
- Propagates Argo/Flux access controls through navigation, lists, and details.
- Updates shared UI dependencies and development proxy configuration.
File summaries
| File | Review summary |
|---|---|
yarn.lock |
Locks the updated shared dependency. |
vite.config.mts |
Changes the default proxy to staging; retain the existing default and use VITE_TARGET_URL instead. |
src/Pages/GlobalConfigurations/Authorization/utils.ts |
Adds projectless validation, but “All applications” can bypass mandatory environment validation. |
src/Pages/GlobalConfigurations/Authorization/types.ts |
Adds Argo/Flux permission and resource types. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/utils.ts |
Adds Argo/Flux tabs, helpers, and cluster IDs. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/types.ts |
Extends supported permission types. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/EnvironmentSelector.tsx |
Reuses environment selection, but drops clusterId, preventing application requests. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/DirectPermission.tsx |
Hides project selection for projectless applications. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/constants.ts |
Adds empty Argo/Flux permission rows. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/AppPermissions.component.tsx |
Integrates permission workflows, but retains stale applications when environments change. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/AppPermissionDetail.tsx |
Updates projectless permission headers and layouts. |
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/AppOrJobSelector.tsx |
Loads environment-scoped applications, but clearing environments can leave loading stuck. |
src/Pages/GlobalConfigurations/Authorization/constants.ts |
Adds Argo/Flux validation state. |
src/Pages/GlobalConfigurations/Authorization/authorization.service.ts |
Adds Argo/Flux resource APIs. |
src/Pages/GlobalConfigurations/Authorization/authorization.scss |
Adds projectless permission-row styling. |
src/Pages/App/Details/ExternalFlux/ExternalFluxAppDetails.tsx |
Allows permitted users to view Flux details. |
src/components/common/navigation/NavigationRoutes.tsx |
Publishes Argo/Flux access flags through context. |
src/components/app/list-new/GenericAppList.tsx |
Enforces application-type access. |
src/components/app/list-new/AppList.tsx |
Shows authorized Argo/Flux tabs. |
package.json |
Updates the shared UI library. |
Review details
Suppressed comments (2)
src/Pages/GlobalConfigurations/Authorization/Shared/components/AppPermissions/AppOrJobSelector.tsx:167
- Projectless rows use a truthy placeholder
team, so this condition enables the application picker before any environment is selected. Users can select “All applications” and save, but validation treats the row as untouched because its environment list is empty and silently drops it. Require an environment first for Argo/Flux.
isDisabled={
!permission.team || (projectId && listForAccessType.get(projectId)?.loading) || isLoadingArgoFluxApps
}
vite.config.mts:33
- This feature change also redirects the default local-development proxy from preview to staging, so unrelated local API calls now hit a different shared environment. The PR does not require that operational change; keep the existing default and use
VITE_TARGET_URLwhen staging is needed.
const TARGET_URL = 'https://staging.devtron.info/'
- Files reviewed: 19/20 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
github-actions
Bot
added
the
PR:Issue-verification-failed
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
| export type ProjectsListType = Record<ACCESS_TYPE_MAP, Teams[]> | ||
| export type EnvironmentsListType = Record<ACCESS_TYPE_MAP.DEVTRON_APPS | ACCESS_TYPE_MAP.JOBS, EnvListMinDTO[]> | ||
|
|
||
| type PermissionTabAccessType = |
There was a problem hiding this comment.
Should we export this 1 level up. So that it can be reused in other places as well
| environment?: string | ||
| action: string | ||
| accessType?: ACCESS_TYPE_MAP.DEVTRON_APPS | ACCESS_TYPE_MAP.HELM_APPS | ACCESS_TYPE_MAP.JOBS | ||
| accessType?: |
There was a problem hiding this comment.
We should use 'PermissionTabAccessType'
| export interface DirectPermissionsRoleFilter extends RoleFilter, PermissionStatusAndTimeout { | ||
| entity: EntityTypes.DIRECT | EntityTypes.JOB | ||
| accessType: ACCESS_TYPE_MAP.DEVTRON_APPS | ACCESS_TYPE_MAP.HELM_APPS | ACCESS_TYPE_MAP.JOBS | ||
| accessType: |
There was a problem hiding this comment.
we should use 'PermissionTabAccessType'
| extends Partial<Pick<K8sResourceListPayloadType, 'clusterId' | 'k8sRequest'>> { | ||
| entity: EntityTypes | ||
| accessType?: ACCESS_TYPE_MAP.DEVTRON_APPS | ACCESS_TYPE_MAP.HELM_APPS | ||
| accessType?: |
|
|
||
| const isArgoCDAppList = appType === InfrastructureManagementAppListType.ARGO_CD | ||
| const isFluxCDAppList = appType === InfrastructureManagementAppListType.FLUX_CD | ||
| const hasAppTypeAccess = isSuperAdmin || (isArgoCDAppList ? hasArgoAppAccess : hasFluxAppAccess) |
There was a problem hiding this comment.
Should we also check 'isFluxCDAppList'?
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
Some linked issues are invalid. Please update the issue links:\nIssue # in is not found or invalid (HTTP }404).\n |
|
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Fixes # (issue)
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: