Permission guards #8
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive permission guard system for Kubernetes resources, adding authentication and authorization (authz) checks to all frontend components and adding Istio VirtualService and Gateway support to the backend capabilities registry.
Key Changes:
- Added Istio VirtualServices and Gateways capabilities to the backend authorization registry
- Implemented permission guards for all Kubernetes resource operations (view, edit, delete, scale, etc.)
- Wrapped all page containers with
RouteGuardcomponents that enforce access control
Reviewed Changes
Copilot reviewed 53 out of 54 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/authz/capabilities.go | Added Istio VirtualServices and Gateways permission capabilities |
| frontend/src/lib/k8s-workloads.ts | Fixed API path duplication in ingresses endpoint |
| frontend/src/contexts/capabilities-context.tsx | Enhanced capabilities context with deduplication and error handling |
| frontend/src/components/viewers/*.tsx | Added IfAllowed permission guards to all resource detail drawers |
| frontend/src/components/data_tables/*.tsx | Added permission guards to table actions and bulk operations |
| frontend/src/components/containers/*.tsx | Wrapped page containers with RouteGuard and added capability pre-fetching |
| frontend/src/components/authz/RouteGuard.tsx | Improved route guard with unknown capability handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| const totalAvailable = replicaSets.reduce((sum, rs) => sum + rs.available, 0) | ||
| const totalCurrent = replicaSets.reduce((sum, rs) => sum + rs.current, 0) | ||
| const totalDesired = replicaSets.reduce((sum, rs) => rs.desired, 0) | ||
| const totalDesired = replicaSets.reduce((sum, rs) => sum + rs.desired, 0) |
There was a problem hiding this comment.
The reduce function is missing the accumulator parameter in the callback. It should be (sum, rs) => sum + rs.desired but the current code has rs.desired without adding to sum.
Copilot uses AI. Check for mistakes.
| <DropdownMenuContent align="end" className="w-48"> | ||
| {bulkActions.map((action, index) => { | ||
| const isDisabled = action.disabled || (action.requiresSelection !== false && selectedCount === 0) | ||
| <DropdownMenuContent align="end" className="w-auto min-w-max whitespace-nowrap"> |
There was a problem hiding this comment.
[nitpick] The className combines multiple layout properties that could conflict. Consider separating these into distinct, well-named classes or using a more specific width value instead of w-auto min-w-max.
| <DropdownMenuContent align="end" className="w-auto min-w-max whitespace-nowrap"> | |
| <DropdownMenuContent align="end" className="min-w-max whitespace-nowrap"> |
Copilot uses AI. Check for mistakes.
| </IfAllowed> | ||
| <IfAllowed feature="virtualservices.patch" cluster={clusterId} namespace={row.original.namespace} resourceName={row.original.name} fallback={<DropdownMenuItem disabled><IconEdit className="size-4 mr-2" />Edit YAML</DropdownMenuItem>}> | ||
| <ResourceYamlEditor resourceName={row.original.name} namespace={row.original.namespace} resourceKind="VirtualService"> | ||
| <button className="flex w-full items-center gap-2 px-2 py-1.5 text-sm hover:bg-accent rounded-sm cursor-pointer" style={{ background: 'transparent', border: 'none', textAlign: 'left' }}> |
There was a problem hiding this comment.
[nitpick] Inline styles should be avoided in favor of CSS classes. The style object with background: 'transparent', border: 'none', textAlign: 'left' should be moved to a CSS class for better maintainability.
| <button className="flex w-full items-center gap-2 px-2 py-1.5 text-sm hover:bg-accent rounded-sm cursor-pointer" style={{ background: 'transparent', border: 'none', textAlign: 'left' }}> | |
| <button className="flex w-full items-center gap-2 px-2 py-1.5 text-sm hover:bg-accent rounded-sm cursor-pointer bg-transparent border-none text-left"> |
Copilot uses AI. Check for mistakes.
| <DropdownMenuItem onClick={() => { | ||
| const rs = row.original; | ||
| console.log('Export YAML for ReplicaSet:', `${rs.name} in ${rs.namespace}`); |
There was a problem hiding this comment.
Console.log statements should be removed from production code. Consider using a proper logging mechanism or removing debug statements.
| <DropdownMenuItem onClick={() => { | |
| const rs = row.original; | |
| console.log('Export YAML for ReplicaSet:', `${rs.name} in ${rs.namespace}`); |
Copilot uses AI. Check for mistakes.
1 participant
No description provided.