Empty list in hull.util.transformation.include

khmarochos commented 9 months ago

Hello again,

May I bother you with another newbie question? If so - here it goes.

I have a named template that I "call" with hull.util.transformation.include from the values dictionary:

hull:
  objects:
    deployment:
      some-nice-pod:
        pod:
          imagePullSecrets: _HT/secrets/some.nice.utility

The template usually renders a YAML containing a list of entities:

secrets:
- alpha
- bravo
- charlie

It works fine, but there are cases when some.nice.utility must return an empty list (secrets: []). In this case, Helm fails with the following error message: Error: INSTALLATION FAILED: unable to build Kubernetes objects from release manifest: error validating "": error validating data: [apiVersion not set, kind not set]. Perhaps it happens because the manifest structure is maimed, but I can't see exactly how it looks. The --debug parameter doesn't help me see the resulting manifest before it's being validated, so I can't even see what's broken.

Would you be so kind as to provide me with any glues on how to return an empty list to hull.util.transformation.include and prevent Helm from failing?

I really appreciate any help you can provide.

khmarochos commented 9 months ago

...of course, I can make some.nice.utility return something like that:

secrets:
- ""

But this is not a solution either. :-(

I'd like the deployment to contain imagePullSecrets: [] or don't contain imagePullSecrets parameter at all in the case when some.nice.utility returns an empty list or any other sign that imagePullSecrets is empty.

gre9ory commented 9 months ago

Hi @khmarochos,

I did try to reproduce but could not directly.

Wrote a test function:

{{- define "hull.include.test.array.empty" -}}
result: []
{{- end -}}

and called it with someKey: _HT/result/hull.include.test.array.empty and it gave me:

someKey: []

as expected.

But it looks like you are doing everything correctly so I'd very much like to investigate your concrete problem further if possible, The error you are getting indicates that the whole YAML structure is messed up pretty badly which normally shouldn't happen.

Is it possible you can provide me:

the values.yaml of your chart
any values.yaml's you are applying on top
your tpl file with the function

Happy to do a deeper check on what is going wrong if you can provide me this info. Thanks!

khmarochos commented 9 months ago

Thank you very much for your attention!

Here's my values.yaml:

hull:
  objects:
    deployment:
      my-deployment:
        pod:
          imagePullSecrets: _HT/secrets/myUtilities.getMySecrets
          containers:
            my-pod:
              image:
                repository: hello-world

Here's the templates/_my_utilities.tpl:

{{- define "myUtilities.getMySecrets" -}}
secrets: []
{{- end -}}

Here's what I get when I run helm install --dry-run test .:

install.go:214: [debug] Original chart version: ""
install.go:231: [debug] CHART PATH: /home/volodymyr/src/hull-test-1

Error: INSTALLATION FAILED: unable to build kubernetes objects from release manifest: error validating "": error validating data: [apiVersion not set, kind not set]
helm.go:84: [debug] error validating "": error validating data: [apiVersion not set, kind not set]
helm.sh/helm/v3/pkg/kube.scrubValidationError
        helm.sh/helm/v3/pkg/kube/client.go:815
helm.sh/helm/v3/pkg/kube.(*Client).Build
        helm.sh/helm/v3/pkg/kube/client.go:358
helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
        helm.sh/helm/v3/pkg/action/install.go:320
main.runInstall
        helm.sh/helm/v3/cmd/helm/install.go:306
main.newInstallCmd.func2
        helm.sh/helm/v3/cmd/helm/install.go:152
github.com/spf13/cobra.(*Command).execute
        github.com/spf13/cobra@v1.7.0/command.go:940
github.com/spf13/cobra.(*Command).ExecuteC
        github.com/spf13/cobra@v1.7.0/command.go:1068
github.com/spf13/cobra.(*Command).Execute
        github.com/spf13/cobra@v1.7.0/command.go:992
main.main
        helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
        runtime/proc.go:250
runtime.goexit
        runtime/asm_amd64.s:1598
unable to build kubernetes objects from release manifest
helm.sh/helm/v3/pkg/action.(*Install).RunWithContext
        helm.sh/helm/v3/pkg/action/install.go:322
main.runInstall
        helm.sh/helm/v3/cmd/helm/install.go:306
main.newInstallCmd.func2
        helm.sh/helm/v3/cmd/helm/install.go:152
github.com/spf13/cobra.(*Command).execute
        github.com/spf13/cobra@v1.7.0/command.go:940
github.com/spf13/cobra.(*Command).ExecuteC
        github.com/spf13/cobra@v1.7.0/command.go:1068
github.com/spf13/cobra.(*Command).Execute
        github.com/spf13/cobra@v1.7.0/command.go:992
main.main
        helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
        runtime/proc.go:250
runtime.goexit
        runtime/asm_amd64.s:1598
INSTALLATION FAILED
main.newInstallCmd.func2
        helm.sh/helm/v3/cmd/helm/install.go:154
github.com/spf13/cobra.(*Command).execute
        github.com/spf13/cobra@v1.7.0/command.go:940
github.com/spf13/cobra.(*Command).ExecuteC
        github.com/spf13/cobra@v1.7.0/command.go:1068
github.com/spf13/cobra.(*Command).Execute
        github.com/spf13/cobra@v1.7.0/command.go:992
main.main
        helm.sh/helm/v3/cmd/helm/helm.go:83
runtime.main
        runtime/proc.go:250
runtime.goexit
        runtime/asm_amd64.s:1598

Here's the output of helm version:

version.BuildInfo{Version:"v3.13.2", GitCommit:"2a2fb3b98829f1e0be6fb18af2f6599e0f4e8243", GitTreeState:"clean", GoVersion:"go1.20.10"}

Here's the output of kubectl version:

Client Version: v1.28.2
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.27.7

Do you see the same output with these two files?

gre9ory commented 9 months ago

Thank you, I am now able to reproduce and understand the problem. It is not a general one but an individual problem with the imagePullSecrets property where you apply the _HT/ and I didn't realize this initially.

A number of properties are tied to a feature in HULL for improved container image handling and therefore have a special treatment:

image: for specifying container images there is always a three-way split for registry, repository and tag so you can better handle changing a central registry for all your images, this is documented here. See the options config.general.globalImageRegistryServer and config.general.globalImageRegistryToFirstRegistrySecretServer where you can automaically prefix all images with an explicitly specified registry server or simply take the server name from the first registry object
imagePullSecrets: these are by default set automatically also according to the presence of a registry object. See config.general.createImagePullSecretsFromRegistries where you can control the automatic population of imagePullSecrets.

Naturally there exists some special code around these properties and there is the actual bug which will not show up on setting other properties. In hull/templates/_objects_pod.tpl the following function has the issue:

*/ -}}
{{- define "hull.object.pod.imagePullSecrets" -}}
{{- $parent := (index . "PARENT_CONTEXT") -}}
{{- $spec := default nil (index . "SPEC") -}}
{{- $hullRootKey := default "hull" (index . "HULL_ROOT_KEY") -}}
{{ if hasKey $spec.pod "imagePullSecrets" }}
imagePullSecrets: 
{{ $spec.pod.imagePullSecrets | toYaml }}
{{- else -}}
{{ if (index $parent.Values $hullRootKey).config.general.createImagePullSecretsFromRegistries }}
{{ if (gt (len (keys (default dict (index $parent.Values $hullRootKey).objects.registry))) 1) }}
imagePullSecrets: 
{{- range $name, $specRegistry := (index $parent.Values $hullRootKey).objects.registry }}
{{- if (ne $name "_HULL_OBJECT_TYPE_DEFAULT_") }}
- name: {{ template "hull.metadata.fullname" (dict "PARENT_CONTEXT" $parent "SPEC" (index (index $parent.Values $hullRootKey).objects.registry $name) "COMPONENT" $name "HULL_ROOT_KEY" $hullRootKey) }}
{{- end }}
{{- end }}
{{- else }}
imagePullSecrets: []
{{- end }}
{{- end }}
{{- end }}
{{ end }}

For imagePullSecrets there was the idea that explicit setting of the property overrrides the special handling. When explicitly overriding imagePullSecrets as in your case, the following code is triggered:

{{ if hasKey $spec.pod "imagePullSecrets" }}
imagePullSecrets: 
{{ $spec.pod.imagePullSecrets | toYaml }}

Interestingly this works ok if $spec.pod.imagePullSecrets is a list with at least one item. With your _HT/ transformation returning something similar to this

secret: 
- name: secret-a
- name: secret-b

It renders out ok like this when getting the value of secret:

imagePullSecrets: 
- name: secret-a
- name secret-b

However, if the list is empty:

secret: []

I assume it currently renders out like this internally:

imagePullSecrets: 
[]

and this totally breaks the YAML because it produces a plain error message instead of the object YAML structure. As a byproduct it of course misses required kind and apiVersion properties as you saw in your error message. Essentially the error is due to a key ([]) missing a : afterwards and that seems understandable since the [] in the second line is apparently not associated with the line above anymore and interpreted as an individual property key.

The fix is easy it seems: add indent 2 to the lower code line:

{{ if hasKey $spec.pod "imagePullSecrets" }}
imagePullSecrets: 
{{ $spec.pod.imagePullSecrets | toYaml | indent 2 }}

and that way it turns out like this for a proper list:

imagePullSecrets: 
  - name: secret-a
  - name secret-b

and like this for an empty list

imagePullSecrets: 
  []

The indented [] apparently is associated with the key above making it an empty array. This is accepted by the YAML/JSON parsers and the result is turning out correct for both emtpy and non-empty lists.

So thanks for the finding, I hadn't written a test for this with an empty array so it slipped through - also in my usage of HULL I use the auto-populate feature and never overwrite the imagePullSecrets so it didn't get noticed yet.

TLDR: Will produce fixed HULL versions in the next couple of days which should have the issue resolved and your use case supported.

khmarochos commented 9 months ago

Ah, now I see what was wrong! Thank you very much. I'll be looking forward for the next release.

gre9ory commented 9 months ago

@khmarochos

These are the new releases which should now allow overwriting the imagePullSecrets:

1.27.9 1.28.7 1.29.1

Let me know if that resolves this issue for you, thanks!

gre9ory commented 8 months ago

@khmarochos Did you have any chance to test this? If the issue is resolved from your side feel free to close it, otherwise I'll leave it open for the time being.

khmarochos commented 8 months ago

Thank you so much, v1.27.9 works just as I expected!

vidispine / hull

Empty list in hull.util.transformation.include #265