Skip to content

Fix bad yaml when only secretKeyRef #267

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

Open
asiegman wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix bad yaml when only secretKeyRef #267

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
13cb36b
Fix bad yaml when only secretKeyRef
asiegman Mar 2, 2021
4d9e934
Less confusing if statement
asiegman Mar 3, 2021
68f0ee9
String safety
asiegman Mar 3, 2021
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
4 changes: 2 additions & 2 deletions incubator/monochart/Chart.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
apiVersion: v1
description: A declarative helm chart for deploying common types of services on Kubernetes
name: monochart
version: 0.25.0
appVersion: 0.25.0
version: 0.25.1
appVersion: 0.25.1
home: https://github.com/cloudposse/charts/tree/master/incubator/monochart
icon: https://raw.githubusercontent.com/cloudposse/charts/master/incubator/monochart/logo.png
maintainers:
Expand Down
4 changes: 3 additions & 1 deletion incubator/monochart/templates/_helpers.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ env:
{{/*
https://kubernetes.io/docs/tasks/inject-data-application/environment-variable-expose-pod-information/#use-pod-fields-as-values-for-environment-variables
*/}}
{{- with $root.Values.envFromFieldRefFieldPath }}
{{- if or (not (empty $root.Values.envFromFieldRefFieldPath)) (not (empty $root.Values.envFromSecretKeyRef)) }}
Copy link
Contributor

@Nuru Nuru Mar 2, 2021

Choose a reason for hiding this comment

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

You need to consider

charts/incubator/monochart/templates/_helpers.tpl

Lines 57 to 63 in 37a41f5

{{- with $root.Values.env }}
env:
{{- range $name, $value := . }}
- name: {{ $name }}
value: {{ default "" $value | quote }}
{{- end }}
{{- end }}

And you can use a more concise syntax like

charts/incubator/monochart/templates/_helpers.tpl

Line 26 in 37a41f5

{{- if $root.Values.configMaps | or $root.Values.secrets | or $root.Values.envFrom }}

  • empty is redundant, conditionals calculate emptiness for you

Copy link
Contributor Author

@asiegman asiegman Mar 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I really didn't like my if statement there, but missed the better syntax. Updated that.

Is the forced quoting of values for safety? I didn't see it done in the fieldRef feature just above the secretKeyRef feature, so I did not do it. This is all part of the following Kubernetes features:
https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-environment-variables

This should all be Kubernetes field paths and secret names and safe for YAML without unnecessary quoting. I'm always a bit fuzzy on this, the rules around quoting in YAML seem inconsistent to me everytime I read them.

Copy link
Contributor Author

@asiegman asiegman Mar 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Added in the string safety after testing it. Seems Kubernetes doesn't mind the explicit strings there, in my limited testing.

env:
{{- with $root.Values.envFromFieldRefFieldPath }}
{{- range $name, $value := . }}
- name: {{ $name }}
valueFrom:
Expand All @@ -85,6 +86,7 @@ https://kubernetes.io/docs/concepts/configuration/secret/#using-secrets-as-envir
key: {{ $data.key }}
{{- end }}
{{- end }}
{{- end }}
{{- end -}}


Expand Down