-
Notifications
You must be signed in to change notification settings - Fork 347
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
[jaeger-operator] Enforce default container security context #263
base: main
Are you sure you want to change the base?
Conversation
lint-test stage has failed without a clear error message:
|
I saw similar failures on the job for jaeger helm chart PR's. Pod is failing to be scheduled because node has a taint.
|
Signed-off-by: Majid Azimi <[email protected]>
Signed-off-by: Majid Azimi <[email protected]>
I fixed the version conflict by bumping chart version. Would you please take a look into this? |
@@ -36,6 +36,11 @@ spec: | |||
- name: {{ include "jaeger-operator.fullname" . }} | |||
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" | |||
imagePullPolicy: {{ .Values.image.pullPolicy }} | |||
securityContext: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you don't mind a small comment: I personally prefer the flexibility of setting the entire securityContext via values.yaml, e.g.:
# values.yaml:
securityContext:
allowPrivilegeEscalation: false
capabilities:
drop:
- ALL
readOnlyRootFilesystem: true
runAsNonRoot: true
privileged: false
runAsUser: 65532
with deployment.yaml being:
securityContext:
{{- toYaml .securityContext | nindent 12 }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. However, since there is already securityContext
at pod level, it would be better to add containerSecurityContext
in values.yml
Since upstream docker image is already using non root user, I thought it might be good to enforce security context at container level in the chart to prevent accidental problems.
Does it make sense to make this configurable? jaeger-operator will always run under non root and it will not modify local file system anyways. So I hardcoded all the parameters.