Copilot commented on code in PR #10681:
URL: https://github.com/apache/gravitino/pull/10681#discussion_r3044926067
##########
dev/charts/gravitino/templates/deployment.yaml:
##########
@@ -60,8 +64,8 @@ spec:
- /bin/bash
- -c
- |
- cp -r /root/gravitino/scripts/* /tmp/scripts/
- VERSION=$(ls /root/gravitino/libs/gravitino-server-* | grep -oP
'[0-9]+\.[0-9]+\.[0-9]+'|head -1)
+ cp -r /opt/gravitino/scripts/* /tmp/scripts/
+ VERSION=$(ls /opt/gravitino/libs/gravitino-server-* | grep -oP
'[0-9]+\.[0-9]+\.[0-9]+'|head -1)
echo $VERSION > /tmp/scripts/version.txt
resources:
{{- toYaml .Values.initResources | nindent 12 }}
Review Comment:
The Gravitino chart sets `runAsNonRoot/runAsUser` only on the main container
(`.Values.containerSecurityContext`), but the `initContainers` in this
Deployment don’t set a securityContext. On clusters enforcing the Kubernetes
Pod Security Standard `restricted` policy, init containers must also explicitly
set `runAsNonRoot: true` (and/or inherit it from a pod-level securityContext),
otherwise the Pod will be rejected even if the image’s Dockerfile uses a
non-root `USER`. Consider adding a securityContext to each initContainer (at
least `runAsNonRoot: true`; for `sqlfile` you can align it with UID 1000).
##########
dev/charts/gravitino/values.yaml:
##########
@@ -504,12 +504,18 @@ livenessProbe:
initialDelaySeconds: 20
timeoutSeconds: 5
+## Pod-level security context configuration
+## ref:
https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
+##
+podSecurityContext: {}
+ # fsGroup: 1000
+
## Container-specific security context configuration
## ref:
https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
##
containerSecurityContext:
- runAsNonRoot: false
- runAsUser: 0
+ runAsNonRoot: true
+ runAsUser: 1000
Review Comment:
With the container now running as UID 1000 by default, writes to mounted
volumes can fail if the volume is owned by root (common for PVCs). In this
chart the storage volume is mounted directly at `.Values.entity.storagePath`
(and persistence can be enabled), so it would be safer to either (a) set a
default `podSecurityContext.fsGroup: 1000` (or document that users must set it
/ pre-chown the volume) or (b) add an init container to fix ownership when
persistence is enabled.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
