From ac6249037c6629bbea3eb869cef3aa879d962c8c Mon Sep 17 00:00:00 2001 From: Schnitz <12687466+CptSchnitz@users.noreply.github.com> Date: Mon, 7 Sep 2026 10:44:52 +0300 Subject: [PATCH] feat(helm): detect image references anywhere in a values file Detection was previously narrow, matching only a mapping with both a sibling repository and tag key. Generalise it to be structural: any mapping carrying a repository key is a candidate, corroborated by a sibling tag, pullPolicy or registry key, or by a parent key of image or one ending in Image. This catches references at any depth and parent-key naming without requiring a chart to follow a convention. A tag sibling is now optional rather than required. A reference with no tag key is reported as tagless (tag: undefined) instead of being dropped, since resolving it through the chart's appVersion is a later ticket's job. extension.ts filters tagless references out before existence checks accordingly, since there is nothing to check yet. A repository or tag whose raw text carries unresolved Helm template syntax is skipped: a templated repository drops the whole candidate, a templated tag is treated as tagless. Refs #20 --- apps/vscode/src/extension.ts | 8 +- .../helm/src/extract-image-references.test.ts | 107 ++++++++++++++++-- packages/helm/src/extract-image-references.ts | 96 +++++++++++----- 3 files changed, 176 insertions(+), 35 deletions(-) diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index c30447a..4ebb40d 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -78,8 +78,14 @@ async function checkImageReferencesInDocument( return; } + // A tagless reference resolves through `appVersion`, a later ticket's job — + // nothing to check yet. + const taggedReferences = references.filter( + (reference): reference is ImageReference & { tag: NonNullable } => reference.tag !== undefined + ); + const checks = await Promise.all( - references.map(async (reference) => ({ + taggedReferences.map(async (reference) => ({ reference, verdict: await checkImageExistence({ repository: reference.repository.text, diff --git a/packages/helm/src/extract-image-references.test.ts b/packages/helm/src/extract-image-references.test.ts index 2bff157..17eafa1 100644 --- a/packages/helm/src/extract-image-references.test.ts +++ b/packages/helm/src/extract-image-references.test.ts @@ -8,7 +8,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); expect(reference?.repository.text).toBe('docker.io/library/nginx'); - expect(reference?.tag.text).toBe('1.19'); + expect(reference?.tag?.text).toBe('1.19'); }); it('should read the tag from raw source text, not the value YAML parsed it into', () => { @@ -18,7 +18,7 @@ describe('extractImageReferences', () => { // YAML's core schema coerces the plain scalar 1.10 to the float 1.1. // Reading raw source text must keep the trailing zero. - expect(reference?.tag.text).toBe('1.10'); + expect(reference?.tag?.text).toBe('1.10'); }); it('should read a tag written as an integer as its literal text', () => { @@ -26,7 +26,7 @@ describe('extractImageReferences', () => { const [reference] = extractImageReferences(source); - expect(reference?.tag.text).toBe('12'); + expect(reference?.tag?.text).toBe('12'); }); it('should report source ranges that point at the exact scalar text', () => { @@ -53,7 +53,7 @@ describe('extractImageReferences', () => { expect(source.slice(tag!.range.start, tag!.range.end)).toBe('1.10'); }); - it('should ignore a mapping with only a repository key', () => { + it('should ignore a repository key with no corroborating sibling and no corroborating parent key', () => { const source = ['source:', ' repository: https://github.com/example/example.git', ''].join('\n'); expect(extractImageReferences(source)).toHaveLength(0); @@ -65,13 +65,16 @@ describe('extractImageReferences', () => { expect(extractImageReferences(source)).toHaveLength(0); }); - it('should ignore a repository key whose sibling tag is a nested mapping, not a scalar', () => { + it('should report a reference as tagless when the sibling tag key holds a non-scalar value', () => { const source = ['image:', ' repository: docker.io/library/nginx', ' tag:', ' channel: stable', ''].join('\n'); - expect(extractImageReferences(source)).toHaveLength(0); + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('docker.io/library/nginx'); + expect(reference?.tag).toBeUndefined(); }); - it('should find every matching mapping regardless of nesting depth or parent key name', () => { + it('should find every matching mapping regardless of nesting depth', () => { const source = [ 'app:', ' image:', @@ -88,6 +91,12 @@ describe('extractImageReferences', () => { expect(references).toHaveLength(2); expect(references.map((reference) => reference.repository.text)).toEqual(['docker.io/library/nginx', 'ghcr.io/example/sidecar']); + + const [first, second] = references; + + expect(source.slice(first!.repository.range.start, first!.repository.range.end)).toBe('docker.io/library/nginx'); + expect(source.slice(second!.repository.range.start, second!.repository.range.end)).toBe('ghcr.io/example/sidecar'); + expect(first!.repository.range).not.toEqual(second!.repository.range); }); it('should return an empty array for a document with no matching mapping', () => { @@ -95,4 +104,88 @@ describe('extractImageReferences', () => { expect(extractImageReferences(source)).toHaveLength(0); }); + + describe('corroborating signals', () => { + it('should treat a sibling pullPolicy key as corroborating on its own', () => { + const source = ['container:', ' repository: docker.io/library/nginx', ' pullPolicy: IfNotPresent', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('docker.io/library/nginx'); + }); + + it('should treat a sibling registry key as corroborating on its own', () => { + const source = ['container:', ' repository: my-service', ' registry: docker.io', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('my-service'); + }); + + it('should treat an exact parent key of image as corroborating with no sibling at all', () => { + const source = ['image:', ' repository: docker.io/library/nginx', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('docker.io/library/nginx'); + expect(reference?.tag).toBeUndefined(); + }); + + it('should treat a parent key ending in Image as corroborating with no sibling at all', () => { + const source = ['sidecarImage:', ' repository: ghcr.io/example/sidecar', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('ghcr.io/example/sidecar'); + }); + + it('should not treat a parent key merely containing "image" as corroborating', () => { + const source = ['imageBuilder:', ' repository: https://github.com/example/example.git', ''].join('\n'); + + expect(extractImageReferences(source)).toHaveLength(0); + }); + }); + + describe('tagless references', () => { + it('should report a reference with no tag key as tagless rather than dropping it', () => { + const source = ['image:', ' repository: docker.io/library/nginx', ' pullPolicy: Always', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('docker.io/library/nginx'); + expect(reference?.tag).toBeUndefined(); + }); + + it('should report a reference as tagless when the tag key has nothing after the colon', () => { + const source = ['image:', ' repository: docker.io/library/nginx', ' tag:', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('docker.io/library/nginx'); + expect(reference?.tag).toBeUndefined(); + }); + + it('should ignore a candidate when the repository key has nothing after the colon', () => { + const source = ['image:', ' repository:', ' tag: 1.0', ''].join('\n'); + + expect(extractImageReferences(source)).toHaveLength(0); + }); + }); + + describe('Helm template syntax', () => { + it('should produce no reference when the repository contains template syntax', () => { + const source = ['image:', ' repository: "{{ .Values.global.registry }}/nginx"', ' tag: 1.19', ''].join('\n'); + + expect(extractImageReferences(source)).toHaveLength(0); + }); + + it('should report a reference as tagless when the tag contains template syntax', () => { + const source = ['image:', ' repository: docker.io/library/nginx', ' tag: "{{ .Chart.AppVersion }}"', ''].join('\n'); + + const [reference] = extractImageReferences(source); + + expect(reference?.repository.text).toBe('docker.io/library/nginx'); + expect(reference?.tag).toBeUndefined(); + }); + }); }); diff --git a/packages/helm/src/extract-image-references.ts b/packages/helm/src/extract-image-references.ts index 899aecb..c5515c0 100644 --- a/packages/helm/src/extract-image-references.ts +++ b/packages/helm/src/extract-image-references.ts @@ -1,4 +1,4 @@ -import { isScalar, parseDocument, visit } from 'yaml'; +import { isPair, isScalar, parseDocument, visit, type Pair } from 'yaml'; /** A half-open character range into the original source text. */ interface SourceRange { @@ -13,34 +13,37 @@ interface RawScalar { } /** - * A candidate `repository`/`tag` pair found in a Helm values file. - * - * Both fields carry the scalar's raw source text and its range, not the - * value YAML parsed it into — see {@link extractImageReferences}. + * A candidate image reference found in a Helm values file. `tag` is + * `undefined` for a tagless reference — resolving it via `appVersion` is a + * later ticket's job. */ interface ImageReference { readonly repository: RawScalar; - readonly tag: RawScalar; + readonly tag: RawScalar | undefined; } +/** A sibling key that corroborates `repository` as an image reference. */ +const CORROBORATING_SIBLING_KEYS: ReadonlySet = new Set(['tag', 'pullPolicy', 'registry']); + +/** The exact parent key that corroborates a `repository` mapping on its own. */ +const CORROBORATING_PARENT_KEY = 'image'; + /** * Finds image references in Helm values file source text. * - * Detection is deliberately narrow, matching only the conventional path: a - * candidate is a YAML mapping that carries both a `repository` key and a - * `tag` key as direct siblings, with both values as plain scalars (not - * nested mappings or sequences). Generalising detection to corroborating - * signals other than a sibling `tag` — a sibling `pullPolicy`/`registry`, - * or a parent key of `image` or one ending in `Image` — is deliberately - * deferred to a later ticket, as is resolving a tagless image through a - * chart's `appVersion`. + * Detection is structural: any mapping, at any depth, carrying a + * `repository` key is a candidate. `repository` alone over-matches (e.g. a + * source-control URL), so a candidate also needs a corroborating signal — a + * sibling `tag`/`pullPolicy`/`registry` key, or a parent key of `image` or + * ending in `Image`. A missing `tag` doesn't disqualify a candidate; it + * makes the reference tagless. * - * `repository` and `tag` are read from the raw source text of their scalar - * nodes, never from the value YAML parsed them into: YAML coerces `1.10` to - * the float `1.1` and `12` to an integer, so reading the parsed value would - * put a confident, wrong diagnostic on a correct file. Reading source text - * requires node ranges, which is the same information diagnostics need to - * know where to point. + * `repository` and `tag` are read from raw source text, not the parsed + * value: YAML coerces `1.10` to the float `1.1`, which would put a wrong + * diagnostic on a correct file. + * + * A value containing Helm template syntax is skipped: a templated + * `repository` drops the candidate, a templated `tag` is treated as absent. */ function extractImageReferences(source: string): ImageReference[] { const document = parseDocument(source); @@ -50,28 +53,60 @@ function extractImageReferences(source: string): ImageReference[] { // `Map` is the yaml package's own visitor method name, not a naming // choice made here — it dispatches by AST node kind. // eslint-disable-next-line @typescript-eslint/naming-convention -- required by the `yaml` package's visitor contract - Map(_key, node) { - const repositoryPair = node.items.find((pair) => isScalar(pair.key) && pair.key.value === 'repository'); - const tagPair = node.items.find((pair) => isScalar(pair.key) && pair.key.value === 'tag'); + Map(_key, node, path) { + const repositoryPair = node.items.find((pair) => keyNameOf(pair) === 'repository'); + + if (repositoryPair === undefined) { + return; + } + + const hasCorroboratingSignal = + node.items.some((pair) => { + const name = keyNameOf(pair); + return name !== undefined && CORROBORATING_SIBLING_KEYS.has(name); + }) || hasCorroboratingParentKey(path); - if (repositoryPair === undefined || tagPair === undefined) { + if (!hasCorroboratingSignal) { return; } const repository = readRawScalar(source, repositoryPair.value); - const tag = readRawScalar(source, tagPair.value); - if (repository === undefined || tag === undefined) { + if (repository === undefined || containsHelmTemplateSyntax(repository.text)) { return; } - references.push({ repository, tag }); + const tagPair = node.items.find((pair) => keyNameOf(pair) === 'tag'); + const tag = tagPair === undefined ? undefined : readRawScalar(source, tagPair.value); + + references.push({ + repository, + tag: tag === undefined || containsHelmTemplateSyntax(tag.text) ? undefined : tag, + }); }, }); return references; } +/** Whether a mapping's parent key is `image` or ends in `Image`. */ +function hasCorroboratingParentKey(path: readonly unknown[]): boolean { + const parent = path[path.length - 1]; + const parentKey = isPair(parent) ? keyNameOf(parent) : undefined; + + return parentKey !== undefined && (parentKey === CORROBORATING_PARENT_KEY || parentKey.endsWith('Image')); +} + +/** A pair's key name, or `undefined` when the key isn't a plain scalar. */ +function keyNameOf(pair: Pair): string | undefined { + return isScalar(pair.key) ? String(pair.key.value) : undefined; +} + +/** Whether a scalar's raw text contains unresolved Helm template syntax. */ +function containsHelmTemplateSyntax(text: string): boolean { + return text.includes('{{') && text.includes('}}'); +} + /** * Slices a scalar node's exact source text out of the document, stripping a * single layer of matching quotes when the scalar was written quoted. Only @@ -91,6 +126,13 @@ function readRawScalar(source: string, node: unknown): RawScalar | undefined { } const [start, end] = range; + + // Nothing after the colon (`repository:`) parses as a zero-length scalar, + // not `null` — treat it as absent rather than an empty-string value. + if (start === end) { + return undefined; + } + const raw = source.slice(start, end); const { text, offset } = stripQuotes(raw);