Skip to content

Add opt-in masking of template expressions before parsing - #1321

Open
UgaTheDev wants to merge 2 commits into
redhat-developer:mainfrom
UgaTheDev:template-masking
Open

Add opt-in masking of template expressions before parsing#1321
UgaTheDev wants to merge 2 commits into
redhat-developer:mainfrom
UgaTheDev:template-masking

Conversation

@UgaTheDev

Copy link
Copy Markdown

What does this PR do?

Adds an opt-in yaml.template setting. When set to helm, Go-template expressions are masked with inert text of exactly the same length before the document is parsed, so templated files such as Helm charts parse as plain YAML instead of reporting a cascade of syntax errors.

This implements the design I proposed in Issue #220. It deliberately does not do either of the two things previous attempts asked for and maintainers declined: it teaches the server no Helm semantics, and it does not touch or swap the YAML parser.

How masking works

  • A line whose content is only a template expression ({{- if .Values.enabled }}, {{- end }}) becomes a comment of identical length, so control flow drops out of the document structure.
  • An inline expression (replicas: {{ .Values.replicaCount }}) becomes a plain scalar of identical length, so the value parses as a string.
  • A span crossing line boundaries is masked on every line it covers, each line classified by the rules above. An unclosed span is masked to the end of its line.

Because every replacement is length-preserving, all offsets survive. Diagnostics, hover, completion, and symbol ranges map back to the real document with no position-translation layer anywhere.

Where it hooks in

Masking happens in YamlDocuments.ensureCache, the single choke point that validation, completion, hover, folding, symbols, code lens, links, and definition all read through. The cache-invalidation check gained the template mode so that changing the setting re-parses open documents.

One design note worth flagging for review: the mode is stored on YamlDocuments rather than added to ParserOptions. Most callers of getYamlDocument pass no options at all and get defaultOptions; only validation and completion pass real ones. Since the cache is keyed by URI alone, threading the mode through ParserOptions would have made masking depend on which feature happened to fill the cache first. Happy to change this if you would rather it live somewhere else.

The formatter is the one feature that bypasses the cache, so it returns no edits for documents containing template expressions when the setting is on. Prettier has no notion of template syntax and would rewrite or reject it.

Honest limits

  • No completion, hover, or validation inside {{ }}. That stays helm-ls territory. This only stops template syntax from breaking the document around it.
  • A fully templated value validates as a string, so a schema expecting a number or boolean at that position still reports a type error. This could later be handled by suppressing type diagnostics whose range overlaps a masked span, but I left that out to keep this change small.
  • {{ include "chart.labels" . | nindent 4 }} expanding to block content, and if/else branches that each define the same key, can still produce false positives. Masking cannot know what a template expands to.
  • Multi-line expressions are masked line by line. Positions stay correct; the intermediate text is just uglier.

I did not add a CHANGELOG entry, since there is no 1.25.0 section yet and I did not want to create the heading. Happy to add one wherever you prefer.

What issues does this PR fix or reference?

Implements the proposal in Issue #220. Also covers the parsing half of Issue #766. Related closed issues: Issue #53, Issue #449.

Is it tested? How?

Yes, test/templateMasking.test.ts adds 11 tests. The full suite is green at 1319 passing, 0 failing, and npm run build (clean, lint, compile, UMD, ESM) exits 0.

The tests include the before/after pair directly, so the fix cannot silently regress into a no-op. On this chart:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: {{ include "mychart.fullname" . }}
  labels:
    {{- include "mychart.labels" . | nindent 4 }}
spec:
  {{- if not .Values.autoscaling.enabled }}
  replicas: {{ .Values.replicaCount }}
  {{- end }}
  selector:
    matchLabels:
      app: {{ .Chart.Name }}

with yaml.template: none, the parser reports 10 errors:

Block collections are not allowed within flow collections
Missing , or : between flow map items
Block collections are not allowed within flow collections
Missing , or : between flow map items
All mapping items must start at the same column
All mapping items must start at the same column
Implicit keys need to be on a single line
Block collections are not allowed within flow collections
Missing , or : between flow map items
Implicit map keys need to be followed by map values

with yaml.template: helm, it reports 0.

Other cases covered: text without templates is returned unchanged (identity, not a rebuild); length and line count are preserved; indentation is preserved on commented-out lines; spans crossing line boundaries and unclosed spans; quoted expressions; offsets still resolve to the correct source text (kind: Deployment is located by node offset against the original, unmasked document); and changing the setting re-parses a document whose version has not changed.

To try it manually, set yaml.template to helm and open any chart under templates/.

Templating languages that wrap YAML, most commonly Helm's Go templates,
produce files that are not valid YAML until they are rendered. A
control-flow line such as `{{- if .Values.autoscaling.enabled }}` breaks
the document structure, and the parser then reports a cascade of syntax
errors on every following line. An inline expression composes as a nested
flow map, so schema validation reports a type error instead.

Add a `yaml.template` setting. When set to `helm`, `{{ ... }}` spans are
replaced with inert text of exactly the same length before the text
reaches the parser:

- A line containing only a template expression becomes a comment, so
  control flow drops out of the document structure.
- An inline expression becomes a plain scalar, so the value parses as a
  string.

Masking is length-preserving, so every offset stays valid and no position
translation is needed downstream. It happens in `YamlDocuments.ensureCache`,
the single choke point every feature reads through, which is also why the
mode is stored on `YamlDocuments` rather than in `ParserOptions`: most
callers of `getYamlDocument` pass no options, so keying it off the options
would make masking depend on which feature filled the cache first. The
formatter bypasses that cache and returns no edits for templated documents,
since prettier would rewrite the template syntax.

The setting defaults to `none`, so behaviour is unchanged unless it is
enabled. There is still no completion, hover, or validation inside
`{{ }}`; this only stops template syntax from breaking the rest of the
document.

Signed-off-by: Kush Zingade <kush.zingade@gmail.com>
@datho7561

Copy link
Copy Markdown
Contributor

This seems like a promising approach for validating simple Helm charts, but I think there's some pretty hard limitations to what we can do based on what you've described. Complex charts (anything with if/else) might require a different approach to get working, and providing Helm-specific features like completion doesn't seem possible without a node in the AST to represent the Helm template segment.

I found this case that doesn't validate properly:

apiVersion: v1
kind: ConfigMap
metadata:
  name: {{ .Release.Name }}-configmap
data:
  myvalue: "Hello World"
  drink: {{ .Values.favorite.drink | default "tea" | quote }}
  food: {{ .Values.favorite.food | upper | quote }}
  {{ if eq .Values.favorite.drink "coffee" }}mug: "true"{{ end }}

My guess is for this and similar cases, we'll need to be a bit more aware of how Helm works and apply that logic when masking/stripping the template. Do you mind seeing if you can get that case working?

Something good about this approach is that we can likely expand it to cover Jinja, a templating language used in Python, too.

A line like '{{ if eq .Values.favorite.drink "coffee" }}mug: "true"{{ end }}'
previously took the inline-filler path, producing trailing filler after a
closed quote — a parse error. The wrapped content is conditional and cannot
be validated as always-present, so the whole line is commented out, same as
standalone control-flow lines. Pass 1 now marks control-flow spans with a
distinct sentinel so pass 2 can classify them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WpgboEXhCRG5bScRWM1YjQ
@UgaTheDev

Copy link
Copy Markdown
Author

Good catch — the inline {{ if }}…{{ end }} line was taking the inline-filler path, which left filler after the closing quote and broke the parse. Fixed in 4508ed8: lines that start with a control-flow expression are now commented out like standalone control-flow lines, since their content is conditional and can't be validated as always-present anyway. Added your ConfigMap as a test case.

Agreed on Jinja — the masking pass is dialect-agnostic apart from the span regex, so it should slot in as another TemplateMode.

One known limitation worth stating: diagnostics inside a conditional line disappear (a typo in mug: won't be flagged) — validating conditional content properly would need the AST-level Helm awareness you mentioned.

@UgaTheDev

Copy link
Copy Markdown
Author

Checking in on this after a few quiet weeks. CI is green and the masking remains opt-in (default off), so current behavior is unchanged unless the setting is enabled. Happy to rebase, rename the setting, or adjust scope if the approach needs discussion.

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

Successfully merging this pull request may close these issues.

2 participants