fix: add securityContext support to ephemeral debug containers for restricted PodSecurity namespaces#5050
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: harshitrwt The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @harshitrwt! |
There was a problem hiding this comment.
Pull request overview
This PR addresses #4969 by making the “Debug Pod” ephemeral container compatible with restricted PodSecurity namespaces, while still allowing users to opt out per-cluster via settings.
Changes:
- Add a per-cluster “Restricted Security Context” toggle (default on) in Pod Debug Settings and persist it in
ClusterSettings. - Pass the toggle value through the debug terminal flow to
Pod.addEphemeralContainer(). - Apply a restricted-compatible
securityContextto the created ephemeral container when the toggle is enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/helpers/clusterSettings.ts | Extends ClusterSettings.podDebugTerminal with useRestrictedSecurityContext. |
| frontend/src/lib/k8s/pod.ts | Adds a boolean flag to addEphemeralContainer() and conditionally injects a restricted securityContext. |
| frontend/src/components/pod/PodDebugTerminal.tsx | Reads the new setting and passes it to the ephemeral container creation path. |
| frontend/src/components/App/Settings/PodDebugSettings.tsx | Adds a new settings row + state/update logic for the restricted security context toggle. |
| imagePullPolicy: 'IfNotPresent', | ||
| ...(useRestrictedSecurityContext && { | ||
| securityContext: { | ||
| allowPrivilegeEscalation: false, | ||
| capabilities: { drop: ['ALL'] }, | ||
| runAsNonRoot: true, | ||
| seccompProfile: { type: 'RuntimeDefault' }, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
securityContext is being added to an object typed as KubeContainer, but KubeContainer currently does not declare a securityContext field (see frontend/src/lib/k8s/cluster.ts where it's still marked as a TODO). This will fail TypeScript excess-property checks during build. Add an appropriate securityContext type to KubeContainer (or use a dedicated ephemeral-container type) so this compiles cleanly.
| { | ||
| name: ( | ||
| <HoverInfoLabel | ||
| label={<span>Restricted Security Context</span>} | ||
| hoverInfo={t( | ||
| 'translation|Apply a restricted-compatible securityContext to the debug container. Required for namespaces enforcing the restricted PodSecurity policy. Sets: allowPrivilegeEscalation=false, capabilities.drop=ALL, runAsNonRoot=true, seccompProfile=RuntimeDefault.' | ||
| )} | ||
| /> | ||
| ), | ||
| value: ( | ||
| <Switch | ||
| checked={useRestrictedSecurityContext} | ||
| onChange={e => storeNewRestrictedSecurityContext(e.target.checked)} | ||
| /> | ||
| ), |
There was a problem hiding this comment.
The new "Restricted Security Context" switch is missing an accessible label association. Other switches in Settings pass inputProps={{ 'aria-labelledby': ... }} and render a label element with a matching id; add the same pattern here so screen readers announce what this toggle controls.
| <HoverInfoLabel | ||
| label={<span>Restricted Security Context</span>} | ||
| hoverInfo={t( | ||
| 'translation|Apply a restricted-compatible securityContext to the debug container. Required for namespaces enforcing the restricted PodSecurity policy. Sets: allowPrivilegeEscalation=false, capabilities.drop=ALL, runAsNonRoot=true, seccompProfile=RuntimeDefault.' | ||
| )} | ||
| /> | ||
| ), | ||
| value: ( | ||
| <Switch | ||
| checked={useRestrictedSecurityContext} | ||
| onChange={e => storeNewRestrictedSecurityContext(e.target.checked)} | ||
| /> |
There was a problem hiding this comment.
The new "Restricted Security Context" toggle Switch is missing an accessible label association. The "Enable Pod Debug" switch wires aria-labelledby, but this one doesn’t provide inputProps (or an aria-label), so screen readers may announce it without context.
Add an id to the label element and pass inputProps={{ 'aria-labelledby': <id> }} (or an equivalent aria-label) on the Switch.
| /** | ||
| * Add an ephemeral container to the pod. | ||
| * @param containerName - The name of the ephemeral container to add | ||
| * @param image - The container image to use | ||
| * @param command - Optional command to run in the container (defaults to ['sh']) | ||
| * @returns Promise that resolves when the ephemeral container is added | ||
| */ | ||
| async addEphemeralContainer( | ||
| containerName: string, | ||
| image: string, | ||
| command: string[] = ['sh'] | ||
| command: string[] = ['sh'], | ||
| useRestrictedSecurityContext: boolean = true | ||
| ): Promise<void> { |
There was a problem hiding this comment.
The JSDoc for addEphemeralContainer hasn’t been updated for the new useRestrictedSecurityContext parameter. Please document the new argument (what it does, default behavior, and why/when to disable it) so future callers understand the security implications.
| /** | ||
| * Creates an ephemeral debug container and waits for it to be ready. | ||
| * Polls pod status up to 30 times (30 seconds) for container to reach running state. | ||
| * | ||
| * @param item - Pod to add ephemeral container to | ||
| * @param containerName - Name for the new container | ||
| * @param debugImage - Container image for debugging | ||
| * @param onError - Error handler callback | ||
| * @returns Object with containerName if successful, empty object on error | ||
| */ | ||
| async function debugPod( | ||
| item: Pod, | ||
| containerName: string, | ||
| debugImage: string, | ||
| onError: (message: string) => void | ||
| onError: (message: string) => void, | ||
| useRestrictedSecurityContext: boolean = true | ||
| ) { |
There was a problem hiding this comment.
The JSDoc for debugPod lists parameters but doesn’t include the new useRestrictedSecurityContext argument. Please update the comment to document this parameter and its default, since it changes behavior and is security-relevant.
|
Thanks for the review, working on it. |
3ca1e8c to
29c606a
Compare
|
Hi @illume , Any reviews on new changes? |
This PR fixes #4969 by introducing two complementary changes:
Safe default:
addEphemeralContainer()now applies a restricted-compatiblesecurityContextby default:allowPrivilegeEscalation: falsecapabilities.drop: ["ALL"]runAsNonRoot: trueseccompProfile.type: RuntimeDefaultUser control: A new "Restricted Security Context" toggle is added to Pod Debug Settings (alongside the existing image and enable/disable settings). It defaults to
on, but users can turn it off for clusters where they need elevated permissions in their debug container.Files Changed
frontend/src/helpers/clusterSettings.ts: addeduseRestrictedSecurityContextfield toClusterSettingstypefrontend/src/lib/k8s/pod.ts:addEphemeralContainer()accepts and appliessecurityContextfrontend/src/components/pod/PodDebugTerminal.tsx: reads setting from cluster config and passes it throughfrontend/src/components/App/Settings/PodDebugSettings.tsx: new toggle row in the Pod Debug Settings UITesting
To reproduce the original bug and verify the fix:
Notes
securityContextdefault does not break debugging in non-restricted namespaces, a restricted context is still fully functional for shell access, log inspection, env var checks, etc.