Issues with helm templates

[inssein]

Might be out of scope for this project, but I was trying to use yamlfmt on our charts repository and it ran into all sorts of errors with the templates.

Example of error:

configuration-service/templates/postgres.yaml: yaml: did not find expected node content

[braydonk]

Hi @inssein thanks for opening an issue. Admittedly I'm very outside of the Kubernetes world so I can't say confidently why this wouldn't work. I don't know much about helm charting.

I think it's in scope for the project, I would really love it to work because helm charts are such a heavy spot where people use lots of yaml.

It would be a big help if you could post an example of a helm chart that failed (with any info redacted of course) so I could look into it and see if it's something that's fixable. Perhaps helm charts use a different yaml syntax or something that I don't know about.

[inssein]

@braydonk for sure, here is an example I took from our repository:

apiVersion: v1
kind: ConfigMap
metadata:
  name: ratelimiter-config
  labels:
    {{- include "chart.labels" . | nindent 4 }}
data:
  config.yaml: |
    ---
    domain: test
    descriptors:
      - key: foo
        rate_limit:
          unit: minute
          requests_per_unit: 1000000000

When I run this file through yamlfmt, I get:

2023/05/15 10:03:03 encountered the following formatting errors:
[redacted]/configmap-ratelimiter.yaml: yaml: line 5: did not find expected node content

[braydonk]

Ah I see. It's the templating language. Those aren't valid yaml, so the yaml parser is complaining about that. I think it would be very challenging to support this, except for potentially making a whole new formatter that instead parses helm chart templates. If that exists it potentially could be done, but as it is those template files won't work with yamlfmt today.

Are there any helm files that are not templates that also fail in some way?

[inssein]

I haven't come across any other failures yet.

I guess one improvement I can see right away is that when there is an error, no formatting is done to any of the other files. Would be good to skip over the templating files that error out and still get the rest of the files in the interim.

[braydonk]

Yeah I think that's a simple change that I should have done a while ago, and I'll put it on my list. I always put it off because one of the original usages of the tool when I first made it was to fail if there was any invalid YAML by design. The way I handle it right now is not very elegant though, I can make it much nicer.

In the meantime, if you exclude the templates directory, would the tool work on everything else? Just so you might be unblocked and able to use the tool until I get it working a bit nicer.

[braydonk]

I took a look at it, and I think it would be at the moment infeasible for me to implement support for Helm chart templates. Most of the functionality of this tool relies on being able to parse something as valid YAML and output the same valid YAML. It might be possible for this to be implemented, but it would unfortunately require basically reimplementing the YAML parser to also recognize the Go templating language that Helm uses mixed in with YAML. It's not impossible, but I am probably not going to be able to add support for it any time soon.

I definitely will include the continuing on error functionality, so that the whole program doesn't fail upon a single file being incorrect.

[braydonk]

I added the functionality you suggested where yamlfmt can continue and format the rest of the files even if some are invalid. I don't have an ETA just yet for the next release since I have a couple other small bugs I want to get in before I make it.

I think I'm going to close this for now due to helm templates being very difficult to include at this time, although I would like to support them some day I probably won't be able to address it any time soon. Thank you for your help with examples and feedback!

[inssein]

Thank you @braydonk. Best open source interaction in a while :)

[braydonk]

Thank you @inssein that means a lot!