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.

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

Clarification: nil vs empty string #53

Closed
scottrigby opened this issue Sep 7, 2017 · 7 comments
Closed

Clarification: nil vs empty string #53

scottrigby opened this issue Sep 7, 2017 · 7 comments

Comments

@scottrigby
Copy link

Hello!

While reviewing recent charts PRs (example: helm/charts#2004), I see this interesting pattern:

{{- if .Values.persistence.storageClass }}
{{- if (eq "-" .Values.persistence.storageClass) }}
  storageClassName: ""
{{- else }}
  storageClassName: "{{ .Values.persistence.storageClass }}"
{{- end }}
{{- end }}

The explanation is:

  ## mariadb data Persistent Volume Storage Class
  ## If defined, storageClassName: <storageClass>
  ## If set to "-", storageClassName: "", which disables dynamic provisioning
  ## If undefined (the default) or set to null, no storageClassName spec is
  ##   set, choosing the default provisioner.  (gp2 on AWS, standard on
  ##   GKE, AWS & OpenStack)
  ##
  # storageClass: "-"

My question was, since the template-rendered YAML will either be a storage class name, an empty string (to disable dynamic provisioning), or not set at all, can't we just do this instead?

  ## mariadb data Persistent Volume Storage Class
  ## If defined, storageClassName: <storageClass>
  ## If set to empty string "", this disables dynamic provisioning
  ## If undefined (the default) or set to null, no storageClassName spec is
  ##   set, choosing the default provisioner.  (gp2 on AWS, standard on
  ##   GKE, AWS & OpenStack)
  ##
  # storageClass: ""

Then the template logic could be simplified to:

{{- if .Values.persistence.storageClass }}
  storageClassName: "{{ .Values.persistence.storageClass }}"
{{- end }}

But this would only work if an empty string is different from not set (nil) when checking if .Values.some.value in go template. Can someone help clarify this for me, or point to the relevant part of the docs? I seem to not be able to find this info.

Thanks!

@scottrigby
Copy link
Author

OK, I found the relevant helm issue: helm/helm#2600

But just checking here, is there really no way for sprig to differentiate the difference between nil and "" (empty string) in a template? If not, would that be a desirable PR?

@technosophos
Copy link
Member

Wow. That issue actually helped me nail down an issue I was seeing in a local PVC! Ha!

Okay, so an {{ if $value }} will check for both "" and nil, considering either one to be an "empty value". There is also the empty function, which essentially behaves the same way.

It might be nice to be able to test whether something is nil and possibly empty ({{ if isNil $foo }}). I don't think we have a way of checking that. Maybe eq $foo nil would work... I've never tried.

Does that answer your question, or am I misunderstanding.

@mattfarina
Copy link
Member

@technosophos If something is a string, isn't the initialized value on a struct "" instead of nil? How would you get the nil state without the template engine complaining about the missing field?

I imagine I'm missing something which is why I ask.

@jkroepke
Copy link

Found a solution {{ kindIs "invalid" $value }}

@mattfarina
Copy link
Member

@scottrigby @technosophos Here's an example of the previous comment... https://play.golang.org/p/9yJW5iGT4qa

When I looked at this in the past I'd not thought of this. Thanks @jkroepke

@scottrigby
Copy link
Author

Love it 😄

vincentmli added a commit to vincentmli/cilium that referenced this issue Oct 12, 2022
add missing bpf.hostLegacyRouting, bpf.tproxy, bpf.vlanBypass in helm
reference and helm documents. remove bpf.lbBypassFIBLookup, hostRouting

when original commented helm option is uncommented, the helm template
generated manifest will include the new uncommented helm option
even helm template does not specify the new uncommented option, this
behavior could affect unknown effect, see
cilium#21195 (comment)

For bool type helm option, gandro suggested (not (eq nil $value))
check, but it didn't work.
see cilium#21195 (comment)

Eventually gandro suggested the {{- if (not (kindIs "invalid" .Values.bpf.tproxy)) }}
trick found in Masterminds/sprig#53 (comment)
see:
cilium#21195 (comment)
cilium#21195 (comment)

Signed-off-by: Vincent Li <v.li@f5.com>
qmonnet pushed a commit to cilium/cilium that referenced this issue Oct 18, 2022
add missing bpf.hostLegacyRouting, bpf.tproxy, bpf.vlanBypass in helm
reference and helm documents. remove bpf.lbBypassFIBLookup, hostRouting

when original commented helm option is uncommented, the helm template
generated manifest will include the new uncommented helm option
even helm template does not specify the new uncommented option, this
behavior could affect unknown effect, see
#21195 (comment)

For bool type helm option, gandro suggested (not (eq nil $value))
check, but it didn't work.
see #21195 (comment)

Eventually gandro suggested the {{- if (not (kindIs "invalid" .Values.bpf.tproxy)) }}
trick found in Masterminds/sprig#53 (comment)
see:
#21195 (comment)
#21195 (comment)

Signed-off-by: Vincent Li <v.li@f5.com>
@thesuperzapper
Copy link

Found a solution {{ kindIs "invalid" $value }}

@jkroepke While this solution might work, I imagine there are other cases where the "kind" of something might be "invalid" without it being nil.

I think better option is to use a direct equality check against nil:

{{ eq $value nil }}

PS: the reason that the kind of nil equals "invalid" is actually because nil does not have a type in Go, because it is a "predeclared identifier".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants