-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(chart): add automountServiceAccountToken support, more commonAnnotations #1754
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,14 @@ metadata: | |
| namespace: {{ .Release.Namespace }} | ||
| labels: | ||
| {{- include "metrics-server.labels" . | nindent 4 }} | ||
| {{- with .Values.serviceAccount.annotations }} | ||
| {{- $annotations := mustMergeOverwrite (dict) (.Values.commonAnnotations | default dict) (.Values.serviceAccount.annotations | default dict) }} | ||
| {{- if $annotations }} | ||
| annotations: | ||
| {{- toYaml . | nindent 4 }} | ||
| {{- toYaml $annotations | nindent 4 }} | ||
| {{- end }} | ||
| {{- if hasKey .Values.serviceAccount "automountServiceAccountToken" }} | ||
| automountServiceAccountToken: {{ .Values.serviceAccount.automountServiceAccountToken }} | ||
| {{- end }} | ||
|
Comment on lines
+14
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're making this change for security hardening you'll likely also need to modify the deployment. |
||
| {{- with .Values.serviceAccount.secrets }} | ||
| secrets: | ||
| {{- toYaml . | nindent 2 }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,9 @@ serviceAccount: | |||||
| # The list of secrets mountable by this service account. | ||||||
| # See https://kubernetes.io/docs/reference/labels-annotations-taints/#enforce-mountable-secrets | ||||||
| secrets: [] | ||||||
| # Specifies whether to automount the service account token | ||||||
| # When not set, Kubernetes default (true) is used | ||||||
| # automountServiceAccountToken: true | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This is already in the service account context, and all values need to be declared in the values file. You can use the following pattern to set the field. |
||||||
|
|
||||||
| rbac: | ||||||
| # Specifies whether RBAC resources should be created | ||||||
|
|
@@ -47,6 +50,8 @@ apiService: | |||||
| caBundle: "" | ||||||
|
|
||||||
| commonLabels: {} | ||||||
| # Annotations to add to all resources | ||||||
| commonAnnotations: {} | ||||||
| podLabels: {} | ||||||
| podAnnotations: {} | ||||||
|
|
||||||
|
|
||||||
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.
I don't agree with this change. FYI this chart is designed to work with the official image or a hardened image built from the upstream source.
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.
No worries. The issue for our usage is for our hardened environment we are required to use a custom entry point in a hardened image forked from upstream and call the application binary as the first arg.
e.g.