Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make the default security policies of kueue compatible with the restricted policy #2105

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions charts/kueue/templates/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ spec:
name: https
protocol: TCP
resources: {}
securityContext:
{{- toYaml .Values.controllerManager.kubeRbacProxy.containerSecurityContext | nindent 10 }}
securityContext:
{{- toYaml .Values.controllerManager.manager.podSecurityContext | nindent 8 }}
serviceAccountName: {{ include "kueue.fullname" . }}-controller-manager
Expand Down
10 changes: 10 additions & 0 deletions charts/kueue/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ controllerManager:
tag: v0.8.0
# This should be set to 'IfNotPresent' for released version
pullPolicy: IfNotPresent
containerSecurityContext:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it can perfectly run in a restricted environment... let's not make it configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, that seems fine. I thought folks would want to make it even more restricted, (e.g. using readOnlyRootFilesystem: true, which I haven't bothered to do here, or a more restrictive seccompProfile).

I'm happy to just change it and leave it unconfigurable too.

Off the top of your head, do you know which directory/ies kueue tries to write to? (so we can make them emptyDirs and restrict read/write of root FS). No worries if it's too complicated to just write, I asked in case it's easy for you.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't write to disk at all and I don't think we intend to. kueue is pretty stateless.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'll look into why readOnlyRootFilesystem: true failed on my cluster then.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhmm... I think I remember seeing that the cert logic writes to disk.

allowPrivilegeEscalation: false
readOnlyRootFilesystem: false
capabilities:
drop: ["ALL"]
manager:
image:
repository: gcr.io/k8s-staging-kueue/kueue
Expand All @@ -34,8 +39,13 @@ controllerManager:
memory: 512Mi
podSecurityContext:
runAsNonRoot: true
seccompProfile:
type: "RuntimeDefault"
containerSecurityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: false
capabilities:
drop: ["ALL"]
replicas: 1
imagePullSecrets: []
readinessProbe:
Expand Down
5 changes: 5 additions & 0 deletions config/components/manager/manager.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ spec:
spec:
securityContext:
runAsNonRoot: true
seccompProfile:
type: "RuntimeDefault"
containers:
- command:
- /manager
Expand All @@ -27,6 +29,9 @@ spec:
name: manager
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: false
capabilities:
drop: ["ALL"]
livenessProbe:
httpGet:
path: /healthz
Expand Down
5 changes: 5 additions & 0 deletions config/default/manager_auth_proxy_patch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,8 @@ spec:
- containerPort: 8443
protocol: TCP
name: https
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: false
capabilities:
drop: ["ALL"]