feat(helm): detect image references anywhere in a values file

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<ImageReference['tag']> } => reference.tag !== undefined
+  );
+
   const checks = await Promise.all(
-    references.map(async (reference) => ({
+    taggedReferences.map(async (reference) => ({
       reference,
       verdict: await checkImageExistence({
         repository: reference.repository.text,

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,15 +18,15 @@ 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', () => {
     const source = ['image:', '  repository: docker.io/library/nginx', '  tag: 12', ''].join('\n');
 
     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,11 +91,101 @@ 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', () => {
     const source = ['replicaCount: 3', 'service:', '  type: ClusterIP', ''].join('\n');
 
     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();
+    });
+  });
 });

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<string> = 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);