ikatyang / tree-sitter-yaml

YAML grammar for tree-sitter
https://ikatyang.github.io/tree-sitter-yaml/
MIT License
94 stars 38 forks source link

[Feature Suggestion/Request] Helm YAML Templates #36

Open hellerbarde opened 2 years ago

hellerbarde commented 2 years ago

Hi!

Helm Templates mostly consist of YAML, due to kubernetes resources commonly being specified in YAML. But due to the double curly bracket templating signifier, it trips this tree-sitter up (understandably, of course).

Is there something I could do to help tree-sitter in a direction to support this situation in some way?

And just to be clear: I absolutely understand that syntactically, Helm templates are not valid YAML. And I absolutely do expect an answer along the lines of "this is not tree-sitter-yaml's problem", I just would like a hint in which direction I should go in search for a solution, because I don't know the tree-sitter ecosystem yet.

Helm Reference: https://helm.sh/docs/chart_template_guide/getting_started/#adding-a-simple-template-call

Example case:

before.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: {{ .Values.app.label }}-deployment
spec:
  replicas: {{ .Values.app.replicas | default 2 }}
  selector:
    matchLabels:
      app: {{ .Values.app.label }}
  template:
    metadata:
      labels:
        app: {{ .Values.app.label }}
    spec:
      imagePullSecrets:
        - name: secrets
      containers:
        - name: example
          image: 'example/example:{{ .Values.image.tag }}'

after.yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: {{ .Values.app.label }}-deployment
spec:
  replicas: {{ .Values.app.replicas | default 2 }}
  selector:
    matchLabels:
      app: {{ .Values.app.label }}
  template:
    metadata:
      labels:
        foo: bar
        app: {{ .Values.app.label }}
    spec:
      imagePullSecrets:
        - name: new-secrets
      containers:
        - name: example
          image: 'example/example:{{ .Values.image.tag }}'

Tree-Sitter result is the same for both:

ERROR (0, 0) - (3, 31)
  block_mapping_pair (0, 0) - (0, 19)
    flow_node (0, 0) - (0, 10)
      plain_scalar (0, 0) - (0, 10)
        string_scalar (0, 0) - (0, 10) "apiVersion"
    : (0, 10) - (0, 11) ":"
    flow_node (0, 12) - (0, 19)
      plain_scalar (0, 12) - (0, 19)
        string_scalar (0, 12) - (0, 19) "apps/v1"
  block_mapping_pair (1, 0) - (1, 16)
    flow_node (1, 0) - (1, 4)
      plain_scalar (1, 0) - (1, 4)
        string_scalar (1, 0) - (1, 4) "kind"
    : (1, 4) - (1, 5) ":"
    flow_node (1, 6) - (1, 16)
      plain_scalar (1, 6) - (1, 16)
        string_scalar (1, 6) - (1, 16) "Deployment"
  flow_node (2, 0) - (2, 8)
    plain_scalar (2, 0) - (2, 8)
      string_scalar (2, 0) - (2, 8) "metadata"
  : (2, 8) - (2, 9) ":"
  block_mapping_pair (3, 2) - (3, 31)
    flow_node (3, 2) - (3, 6)
      plain_scalar (3, 2) - (3, 6)
        string_scalar (3, 2) - (3, 6) "name"
    : (3, 6) - (3, 7) ":"
    flow_node (3, 8) - (3, 31)
      flow_mapping (3, 8) - (3, 31)
        { (3, 8) - (3, 9) "{"
        flow_node (3, 9) - (3, 30)
          flow_mapping (3, 9) - (3, 30)
            { (3, 9) - (3, 10) "{"
            flow_node (3, 11) - (3, 28)
              plain_scalar (3, 11) - (3, 28)
                string_scalar (3, 11) - (3, 28) ".Values.app.label"
            } (3, 29) - (3, 30) "}"
        } (3, 30) - (3, 31) "}"
char0n commented 2 years ago

HELM should be valid YAML. I couldn't find a specific version of YAML, but I'd assume HELM is valid YAML 1.2. The problem you describing is not specific to this library, it will crash any YAML parser. In order to fix this ambiguity you have to do the following:

1. Enclose in templates in quotes

You can either use single or double quotes. For your specific usecase, if shouldn't really matter.

apiVersion: apps/v1
kind: Deployment
metadata:
  name: "{{ .Values.app.label }}-deployment"
spec:
  replicas: "{{ .Values.app.replicas | default 2 }}"
  selector:
    matchLabels:
      app: "{{ .Values.app.label }}"
  template:
    metadata:
      labels:
        app: "{{ .Values.app.label }}"
    spec:
      imagePullSecrets:
        - name: secrets
      containers:
        - name: example
          image: 'example/example:{{ .Values.image.tag }}'

2. Use multiline strings

In this particular example I've used Folded Block Scalar Style with Strip Block Chomping (more info here)

apiVersion: apps/v1
kind: Deployment
metadata:
  name: >-
    {{ .Values.app.label }}-deployment
spec:
  replicas: >-
    {{ .Values.app.replicas | default 2 }}
  selector:
    matchLabels:
      app: >-
        {{ .Values.app.label }}
  template:
    metadata:
      labels:
        app: >-
          {{ .Values.app.label }}
    spec:
      imagePullSecrets:
        - name: secrets
      containers:
        - name: example
          image: 'example/example:{{ .Values.image.tag }}'

Both examples are parsing just fine as demonstrated in https://ikatyang.github.io/tree-sitter-yaml/. Also have a look at HEML | Appendix: YAML Techniques | Indenting and Templates.

canadiannomad commented 2 years ago

To me it seems that it breaks when the helm chart uses control blocks, eg:

{{- if .Values.ingress.enabled -}}
...
{{- end }}

Or for loops are another possibility that isn't covered in normal YAML.

hellerbarde commented 2 years ago

@char0n You're correct, my example was poorly chosen. But @canadiannomad illustrates it better. It's not just possible to template values or items in arrays, or even only strings. But it's also necessary to template random parts of the chart. As far as I can tell, Helm files simply are not valid YAML.

The problem you describing is not specific to this library, it will crash any YAML parser.

Yes, you are correct. And I wasn't trying to imply that. If I miscommunicated, I apologize for that. I simply stumbled over this problem when trying to use difft as the default diff tool for git. Unfortunately, I maintain Helm charts and thus we are here. Now, one can argue, that Helm should not use the suffix .yaml or .yml, if they are not valid YAML. And I would agree. However, this is the hand we are dealt, so I wanted to check in, to see how complicated this would be. But it seems like it would be very challenging, and it's definitely not your responsibility to fix this for me.

char0n commented 2 years ago

@hellerbarde np, the examples could be written as valid YAML but having @canadiannomad provide more examples that prove that HEML is not a yaml disregard my comment ;]

I'm not a maintainer here, so cannot do anything anyway to really help.

mandos commented 2 years ago

According to my (little) knowledge, they can be at least two approaches to this issue:

  1. Thread helm YAML template as go template with embedded YAML. https://github.com/ngalaiko/tree-sitter-go-template parser has a text node that can be used to inject YAML parses. It works pretty well but the YAML part is not threated as a full document so there are some errors.
  2. Add an additional node to YAML parser to cover this embedded part. I see that there is "{{ }}" flow_node, I don't know what is for but maybe it can be extend to "{{- -}}" and then we can use inject to parse it as go template.
hellerbarde commented 10 months ago

Yeah, I think helm templates are go templates first and should be treated as such.

The question I don't quite understand is who is responsible for identifying those go templates among the other yaml files. Is that the editor's job or does tree-sitter help with that?

secteur0 commented 9 months ago

Hi, I have the same problem with Rails YAML Templates. Yes, the yaml file is not valid but is after being processed by erb. the solution would be to get a .yml.erb file and use https://github.com/tree-sitter/tree-sitter-embedded-template but I can't do it. The Rails framework does not use this extension. It may be a mistake but what can we do?

production:
  clients:
    default:
      uri: <%= MongoClientUriBuilder.new(mongo_client_name: 'audits').call %>
      options:
        max_pool_size: <%= MaxPoolSizeComputer.new.call %>
cabrinha commented 6 months ago

Do helm templates need to be included in this tree-sitter or should helm have it's own tree-sitter?

dperetti commented 4 months ago

I need this badly.

cabrinha commented 3 months ago

https://github.com/ngalaiko/tree-sitter-go-template has support for Helm which includes yaml as well.