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.

Sign up for GitHub

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

fix(test): add securityContext for Helm test #930

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+108 −29
Open

fix(test): add securityContext for Helm test #930

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e48fd91
fix(test): add securityContext for Helm test
fty4 Jul 27, 2023
8bd7d07
chore: use nindent for server.statefulSet.securityContext elements
fty4 Aug 12, 2023
43ca102
chore: remove nindent for securityContext in helperfile
fty4 Aug 16, 2023
63e233d
chore(test): add securityContext for test pod
fty4 Aug 18, 2023
4fea6a3
fix: ensure linebreak for securityContext is present
fty4 Aug 18, 2023
41d19d5
chore(test): add securityContext test for nonRoot
fty4 Aug 18, 2023
8af300b
chore: add missing whitespace
fty4 Aug 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,44 @@ securityContext for the statefulset vault container
{{- end }}
{{- end -}}

{{/*
securityContext for the test pod template.
*/}}
{{- define "server.test.securityContext.pod" -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Hello @fty4 , why do we need to define new template , as its supposed to use the securityContext of the server statefulSet. why not use "server.statefulSet.securityContext.pod" for server-test pod?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @KhizerJaan
I did that because of the indentation of the template.
But now I just added a new commit to use nindent to fix the indentation in the template files.

I hope this is the desired solution.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fty4 Thank you , but can you please check indentation , it does not look right for both server-statefulset and and server-test .

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I run a diff between the templated yaml from main and after the fix I get this change:

381a382
>       
414a416
>           
557a560,565
>   
>   securityContext:
>     runAsNonRoot: true
>     runAsGroup: 1000
>     runAsUser: 100
>     fsGroup: 1000
585a594,596
>       
>       securityContext:
>         allowPrivilegeEscalation: false

This means the indentation should be fine.
The only issue is see here is that two new lines are beeing added...
But this is a issue in the whole chart and I am not sure how to fix this.

I guess you mean the indentation in the helpers file - I was required to remove the indentation here to later use different indentation on the statefulset and the test.
This is because both use different indentation length (6 and 10 for the statefulset and 2 and 6 for the server-test).

Do you agree, @KhizerJaan?

Copy link
Contributor

Choose a reason for hiding this comment

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

this line
&
this
would add to indent 14 and same is with others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually this does not result in a diff in the templated resources - but for sure it is better to remove them when the indentation is set in another place.

I've added commit 43ca102 for this.

{{- if .Values.server.statefulSet.securityContext.pod }}
securityContext:
{{- $tp := typeOf .Values.server.statefulSet.securityContext.pod }}
{{- if eq $tp "string" }}
{{- tpl .Values.server.statefulSet.securityContext.pod . | nindent 4 }}
{{- else }}
{{- toYaml .Values.server.statefulSet.securityContext.pod | nindent 4 }}
{{- end }}
{{- else if not .Values.global.openshift }}
securityContext:
runAsNonRoot: true
runAsGroup: {{ .Values.server.gid | default 1000 }}
runAsUser: {{ .Values.server.uid | default 100 }}
fsGroup: {{ .Values.server.gid | default 1000 }}
{{- end }}
{{- end -}}

{{/*
securityContext for the test vault container
*/}}
{{- define "server.test.securityContext.container" -}}
{{- if .Values.server.statefulSet.securityContext.container }}
securityContext:
{{- $tp := typeOf .Values.server.statefulSet.securityContext.container }}
{{- if eq $tp "string" }}
{{- tpl .Values.server.statefulSet.securityContext.container . | nindent 8 }}
{{- else }}
{{- toYaml .Values.server.statefulSet.securityContext.container | nindent 8 }}
{{- end }}
{{- else if not .Values.global.openshift }}
securityContext:
allowPrivilegeEscalation: false
{{- end }}
{{- end -}}

{{/*
Sets extra injector service account annotations
Expand Down
2 changes: 2 additions & 0 deletions templates/tests/server-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ metadata:
"helm.sh/hook": test
spec:
{{- include "imagePullSecrets" . | nindent 2 }}
{{- template "server.test.securityContext.pod" . }}
containers:
- name: {{ .Release.Name }}-server-test
image: {{ .Values.server.image.repository }}:{{ .Values.server.image.tag | default "latest" }}
Expand Down Expand Up @@ -43,6 +44,7 @@ spec:
fi

exit 0
{{- template "server.test.securityContext.container" . }}
volumeMounts:
{{- if .Values.server.volumeMounts }}
{{- toYaml .Values.server.volumeMounts | nindent 8}}
Expand Down