Add configurable securityContext and dnsConfig for the operator Deployment - #2843
Add configurable securityContext and dnsConfig for the operator Deployment#2843dundysm wants to merge 3 commits into
Conversation
…yment The gpu-operator Deployment currently leaves pod and container securityContext empty. Default to a restricted profile matching the image USER 1000, mount an emptyDir at /tmp for a read-only root filesystem, and allow dnsConfig via values. Fixes NVIDIA#2533 Signed-off-by: Dundy Pasupuleti <pasupuletidundy@gmail.com>
8509025 to
e29dd96
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review. 📝 WalkthroughWalkthroughThe Helm chart adds restricted security defaults for the GPU Operator Deployment. The Deployment and hook Job templates conditionally apply pod-level and container-level security settings. The templates also support optional DNS configuration and mount an Merge Risk: 🔵 Low · up to The PR adds restricted security defaults and writable temporary storage to the operator Deployment and lifecycle Jobs, improving isolation without changing operand DaemonSets. Administrators can still weaken these settings, and OpenShift policy may not independently preserve every restriction, so the change is mergeable with explicit owner awareness of that configuration boundary. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
deployments/gpu-operator/templates/operator.yaml-40-43 (1)
40-43: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winKeep the
dnsConfigkey and template reference in sync.This branch reads
.Values.operator.dnsConfig, butdeployments/gpu-operator/values.yamlcontains only the commented# dnsConfig: {}. Add an activednsConfig: {}entry invalues.yaml, or remove this branch, so the supported chart values match the template.As per path instructions, a template reference must have a matching values.yaml key.
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: b7992171-3df3-4684-ba35-243b9ab70b8c
📒 Files selected for processing (2)
deployments/gpu-operator/templates/operator.yamldeployments/gpu-operator/values.yaml
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.
restricted-readonly uses MustRunAsRange. A hardcoded UID 1000 fails namespaces whose allocated range does not include 1000. The image USER 1000:1000 still applies on vanilla Kubernetes. Signed-off-by: Dundy Pasupuleti <pasupuletidundy@gmail.com>
Add discoverable dnsPolicy and dnsConfig values keys, and make dnsPolicy configurable so ClusterFirst remains the default. Apply the same restricted pod and container securityContext used by the operator Deployment to the Helm hook Jobs that run the operator image. Operand DaemonSets are unchanged; they still need host privileges. Signed-off-by: Dundy Pasupuleti <pasupuletidundy@gmail.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@rajathagasthya picking this up from #2533, including Restricted defaults for the operator Deployment and the Helm hook Jobs that run the same image. No pinned UID, so OpenShift This is my first PR here, so copy-pr-bot is blocking CI. If the approach looks right, would you mind an |
|
Thanks @dundysm! Will take a look at this PR soon. |
|
/ok-to-test 0046331 |
2 participants
Description
The
gpu-operatorDeployment currently has no pod or containersecurityContext, so it runs with the cluster default (often unrestricted). This PR makessecurityContext,containerSecurityContext,dnsConfig, anddnsPolicyconfigurable through Helm values and sets restricted defaults that still work on vanilla Kubernetes and OpenShift.This also covers the Helm hook Jobs that run the same operator image (
upgrade-crd,cleanup-crd,cleanup-gpucluster). Those were previously empty like the Deployment.Not in this PR: operand DaemonSets (driver, device plugin, toolkit, MIG, and so on). Those are created later by the operator, not by this chart, and they need host privileges. ClusterPolicy already has
spec.daemonsets.podSecurityContextfor that path.Defaults:
runAsNonRoot: true,seccompProfile: RuntimeDefaultallowPrivilegeEscalation: false,readOnlyRootFilesystem: true,capabilities.drop: [ALL]/tmpemptyDir so a read-only root filesystem still worksdnsPolicy: ""(cluster default ClusterFirst) anddnsConfig: {}UID/GID/
fsGroupare not pinned. The operator image already usesUSER 1000:1000, so vanilla Kubernetes still runs non-root. OpenShiftrestricted-readonlySCC usesMustRunAsRange; a hardcoded UID fails namespaces whose allocated range does not include that ID. Pin a UID withoperator.securityContext.runAsUserif you need one.Fixes #2533
Checklist
make lint)make validate-generated-assets)make validate-modules)Testing
Helm-only change (no Go, CRD, or generated-asset edits). Ran against this branch:
helm lint deployments/gpu-operator/— passhelm templatewith chart defaults — operator Deployment and hook Jobs get the restricted pod/containersecurityContextand/tmpemptyDir;dnsPolicy/dnsConfigare omittedhelm templatewithoperator.dnsPolicy=Noneandoperator.dnsConfig.nameservers=[8.8.8.8]— both render on the Deployment and hook Jobshelm templatewithoperator.securityContext=nullandoperator.containerSecurityContext=null— those fields are omittedhelm templatewithoperator.securityContext.runAsUser=1000— explicit UID still overridesdocker/Dockerfileends withUSER 1000:1000Did not apply this to a live GPU cluster.
make lint/ generated-assets / modules are unchanged because this PR does not touch Go code or generated files.