diff --git a/apps/vscode/src/diagnostics.test.ts b/apps/vscode/src/diagnostics.test.ts index 7dccb1d..1d2f4ea 100644 --- a/apps/vscode/src/diagnostics.test.ts +++ b/apps/vscode/src/diagnostics.test.ts @@ -1,18 +1,19 @@ import * as vscode from 'vscode'; import { describe, expect, it } from 'vitest'; -import type { SourceRange } from 'helm'; +import type { ResolvedTag, SourceRange } from 'helm'; +import type { ImageVerdict, UnverifiableReason } from 'oci-registry'; import { createFakeDocument } from '../test/fake-document'; import { diagnosticsFor } from './diagnostics'; -import type { ReferenceCheck, ReferenceVerdict, UncheckedReason } from './reference-check'; +import type { ReferenceCheck } from './reference-check'; const REPOSITORY = 'registry.example.com/svc'; const TAG = '1.0'; const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: "${TAG}"`, ''].join('\n'); +const CHART_METADATA_PATH = '/repo/chart/Chart.yaml'; // A record rather than a list, so a new reason fails the build here instead // of going untested. -const UNCHECKED_VERDICTS: Record = { - 'no-tag': { kind: 'unverifiable', reason: 'no-tag' }, +const UNCHECKED_VERDICTS: Record = { 'guessed-registry': { kind: 'unverifiable', reason: 'guessed-registry' }, 'needs-login': { kind: 'unverifiable', reason: 'needs-login', registry: REPOSITORY }, 'authentication-failure': { kind: 'unverifiable', reason: 'authentication-failure' }, @@ -21,6 +22,11 @@ const UNCHECKED_VERDICTS: Record = { 'malformed-reference': { kind: 'unverifiable', reason: 'malformed-reference' }, }; +/** Stands in for `vscode.workspace.asRelativePath`, shortening enough that a message can be seen to have used it. */ +function describeChartPath(path: string): string { + return path.replace('/repo/', ''); +} + /** The source range of `text`'s first occurrence in {@link VALUES_YAML}. */ function rangeOfText(text: string): SourceRange { const start = VALUES_YAML.indexOf(text); @@ -36,21 +42,27 @@ function documentRangeOfText(document: vscode.TextDocument, text: string): vscod } /** A check over the single reference in {@link VALUES_YAML}, carrying that file's real offsets. */ -function createCheck(verdict: ReferenceVerdict): ReferenceCheck { +function createCheck(verdict: ImageVerdict, tag: ResolvedTag = { source: 'file', text: TAG, range: rangeOfText(TAG) }): ReferenceCheck { return { reference: { repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, - tag: { text: TAG, range: rangeOfText(TAG) }, + tag: tag.source === 'file' ? { text: tag.text, range: tag.range } : undefined, registry: undefined, }, + tag, verdict, }; } +/** The same reference written without a tag, checked against the chart's `appVersion` instead. */ +function createChartMetadataCheck(verdict: ImageVerdict): ReferenceCheck { + return createCheck(verdict, { source: 'chart-metadata', text: TAG, metadataPath: CHART_METADATA_PATH }); +} + describe('diagnostics', () => { it('should report an error naming the missing repository, positioned on the repository value', () => { const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - const fileDiagnostics = diagnosticsFor(document, [createCheck({ kind: 'repository-not-found', repository: REPOSITORY })]); + const fileDiagnostics = diagnosticsFor(document, [createCheck({ kind: 'repository-not-found', repository: REPOSITORY })], describeChartPath); expect(fileDiagnostics).toHaveLength(1); expect(fileDiagnostics[0]?.severity).toBe(vscode.DiagnosticSeverity.Error); @@ -60,25 +72,36 @@ describe('diagnostics', () => { it('should report an error naming the missing tag, positioned on the tag value', () => { const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - const fileDiagnostics = diagnosticsFor(document, [createCheck({ kind: 'tag-not-found', repository: REPOSITORY, tag: TAG })]); + const fileDiagnostics = diagnosticsFor(document, [createCheck({ kind: 'tag-not-found', repository: REPOSITORY, tag: TAG })], describeChartPath); expect(fileDiagnostics).toHaveLength(1); expect(fileDiagnostics[0]?.severity).toBe(vscode.DiagnosticSeverity.Error); - expect(fileDiagnostics[0]?.message).toContain(TAG); + expect(fileDiagnostics[0]?.message).toBe(`Tag '${TAG}' not found in '${REPOSITORY}'.`); expect(fileDiagnostics[0]?.range).toEqual(documentRangeOfText(document, TAG)); }); + it('should attach a tag taken from chart metadata to the repository value, and name the chart it came from', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const check = createChartMetadataCheck({ kind: 'tag-not-found', repository: REPOSITORY, tag: TAG }); + const fileDiagnostics = diagnosticsFor(document, [check], describeChartPath); + + // Nothing in this file spells the tag out, so there is no text to + // underline and no way for the reader to find it without being told. + expect(fileDiagnostics[0]?.range).toEqual(documentRangeOfText(document, REPOSITORY)); + expect(fileDiagnostics[0]?.message).toBe(`Tag '${TAG}' not found in '${REPOSITORY}'. Tag taken from appVersion in chart/Chart.yaml.`); + }); + it('should report nothing for a reference that exists', () => { const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - expect(diagnosticsFor(document, [createCheck({ kind: 'exists', registry: 'registry.example.com' })])).toEqual([]); + expect(diagnosticsFor(document, [createCheck({ kind: 'exists', registry: 'registry.example.com' })], describeChartPath)).toEqual([]); }); it('should report nothing for any unverifiable reason, since an unreachable registry is not a missing image', () => { const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); for (const verdict of Object.values(UNCHECKED_VERDICTS)) { - expect(diagnosticsFor(document, [createCheck(verdict)])).toEqual([]); + expect(diagnosticsFor(document, [createCheck(verdict)], describeChartPath)).toEqual([]); } }); }); diff --git a/apps/vscode/src/diagnostics.ts b/apps/vscode/src/diagnostics.ts index 43c18fd..6644d9d 100644 --- a/apps/vscode/src/diagnostics.ts +++ b/apps/vscode/src/diagnostics.ts @@ -1,17 +1,25 @@ import * as vscode from 'vscode'; import type { ReferenceCheck } from './reference-check'; import { rangeOf } from './source-range'; +import { chartProvenanceSentence } from './tag-provenance'; /** * The diagnostics a document's checks call for. Only the two not-found * verdicts qualify: that an unverifiable verdict never renders as an error * is the one invariant this feature must not break, because an expired token * or an unreachable registry must never look like a missing image. + * + * `describeChartPath` shortens a chart metadata path for display — an + * absolute path in the Problems panel is noise the reader has to scan past. */ -function diagnosticsFor(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): vscode.Diagnostic[] { +function diagnosticsFor( + document: vscode.TextDocument, + checks: readonly ReferenceCheck[], + describeChartPath: (path: string) => string +): vscode.Diagnostic[] { const fileDiagnostics: vscode.Diagnostic[] = []; - for (const { reference, verdict } of checks) { + for (const { reference, tag, verdict } of checks) { if (verdict.kind === 'repository-not-found') { fileDiagnostics.push( new vscode.Diagnostic( @@ -23,10 +31,12 @@ function diagnosticsFor(document: vscode.TextDocument, checks: readonly Referenc } else if (verdict.kind === 'tag-not-found') { fileDiagnostics.push( new vscode.Diagnostic( - // Unreachable fallback: only a reference that named a tag can come - // back tag-not-found. It keeps the type honest without an assertion. - rangeOf(document, reference.tag?.range ?? reference.repository.range), - `Tag '${verdict.tag}' not found in '${verdict.repository}'.`, + // A tag taken from chart metadata has no text in this file to + // underline, so the squiggle goes on the repository it applies to + // and the message points at the chart instead of leaving the + // reader hunting for a tag that was never written here. + rangeOf(document, tag.source === 'file' ? tag.range : reference.repository.range), + `Tag '${verdict.tag}' not found in '${verdict.repository}'.${chartProvenanceSentence(tag, describeChartPath)}`, vscode.DiagnosticSeverity.Error ) ); diff --git a/apps/vscode/src/extension.test.ts b/apps/vscode/src/extension.test.ts index 78cdd56..04f91f9 100644 --- a/apps/vscode/src/extension.test.ts +++ b/apps/vscode/src/extension.test.ts @@ -3,13 +3,17 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createFakeContext } from '../test/fake-context'; import { noDockerCredentials } from '../test/fake-credentials'; import { bumpVersion, createFakeDocument } from '../test/fake-document'; +import { createFakeFileSystem } from '../test/fake-file-system'; import { fakeFetchResponse } from '../test/fake-fetch'; import { createTextEditorStub, + emitDidChangeChartMetadata, emitDidChangeVisibleTextEditors, emitDidOpenTextDocument, getLastDiagnosticCollection, + getLastFileSystemWatcher, getRegisteredHoverProvider, + setOpenTextDocuments, setVisibleTextEditors, setWarningMessageAnswer, window, @@ -20,6 +24,17 @@ import { activate, deactivate } from './extension'; const REPOSITORY = 'docker.io/library/nginx'; const TAG = '1.19'; const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: ${TAG}`, ''].join('\n'); +const TAGLESS_VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ' pullPolicy: IfNotPresent', ''].join('\n'); + +// No chart metadata anywhere, so the fixtures named `values.yaml` resolve +// through the filename fallback and the older tests keep describing exactly +// what they did before chart context existed. +const NO_FILES = createFakeFileSystem({}); + +/** Chart metadata declaring `appVersion`, for tests about what a bump re-checks. */ +function chartMetadataWithAppVersion(appVersion: string): string { + return ['apiVersion: v2', 'name: my-service', `appVersion: ${appVersion}`, ''].join('\n'); +} /** Stages `document` as the only visible editor, then fires the open event. */ async function openInVisibleEditor(document: vscode.TextDocument): Promise { @@ -79,12 +94,13 @@ describe('extension', () => { } setVisibleTextEditors([]); + setOpenTextDocuments([]); setWarningMessageAnswer(undefined); window.showWarningMessage.mockClear(); }); it('should create an output channel and register it for disposal on activate', () => { - activate(context, { fetch: vi.fn(), credentials: noDockerCredentials }); + activate(context, { fetch: vi.fn(), credentials: noDockerCredentials, readTextFile: NO_FILES }); expect(vscode.window.createOutputChannel).toHaveBeenCalledWith('Infra Tools'); expect(context.subscriptions.length).toBeGreaterThanOrEqual(1); @@ -95,20 +111,20 @@ describe('extension', () => { }); it('should register a yaml hover provider on activate', () => { - activate(context, { fetch: vi.fn(), credentials: noDockerCredentials }); + activate(context, { fetch: vi.fn(), credentials: noDockerCredentials, readTextFile: NO_FILES }); expect(vscode.languages.registerHoverProvider).toHaveBeenCalledWith({ language: 'yaml' }, expect.anything()); }); it('should create one mark decoration type on activate', () => { - activate(context, { fetch: vi.fn(), credentials: noDockerCredentials }); + activate(context, { fetch: vi.fn(), credentials: noDockerCredentials, readTextFile: NO_FILES }); expect(vscode.window.createTextEditorDecorationType).toHaveBeenCalledWith({ after: { margin: '0 0 0 0.5em' } }); }); it('should set both diagnostics and marks when a values file opens', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); - activate(context, { fetch, credentials: noDockerCredentials }); + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); const editor = await openInVisibleEditor(document); @@ -121,7 +137,7 @@ describe('extension', () => { it('should hover a checked reference, and stop once the document has been edited past the check', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch, credentials: noDockerCredentials }); + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); await openInVisibleEditor(document); @@ -135,7 +151,7 @@ describe('extension', () => { it('should re-apply marks to an editor that becomes visible after the document was checked', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch, credentials: noDockerCredentials }); + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); @@ -150,7 +166,7 @@ describe('extension', () => { it('should survive an editor disposed mid-check, since an unhandled rejection kills the extension host', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); - activate(context, { fetch, credentials: noDockerCredentials }); + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); const disposedEditor: TextEditorStub = { @@ -167,14 +183,14 @@ describe('extension', () => { }); it('should create a status bar item on activate', () => { - activate(context, { fetch: vi.fn(), credentials: noDockerCredentials }); + activate(context, { fetch: vi.fn(), credentials: noDockerCredentials, readTextFile: NO_FILES }); expect(vscode.window.createStatusBarItem).toHaveBeenCalled(); }); it('should notify, and raise no diagnostic, for a registry with no local credential', async () => { const fetch = vi.fn(); - activate(context, { fetch, credentials: noDockerCredentials }); + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }); const document = createFakeDocument( '/repo/chart/values.yaml', @@ -194,7 +210,7 @@ describe('extension', () => { }); it('should notify once per registry across files, not once per file', async () => { - activate(context, { fetch: vi.fn(), credentials: noDockerCredentials }); + activate(context, { fetch: vi.fn(), credentials: noDockerCredentials, readTextFile: NO_FILES }); const values = ['image:', ' repository: private.example.com/svc', ' tag: 1.0.0', ''].join('\n'); @@ -206,10 +222,78 @@ describe('extension', () => { it('should issue no request for a document that is not a values file', async () => { const fetch = vi.fn(); - activate(context, { fetch, credentials: noDockerCredentials }); + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }); await emitDidOpenTextDocument(createFakeDocument('/repo/chart/deployment.yaml', VALUES_YAML)); expect(fetch).not.toHaveBeenCalled(); }); + + it('should watch the chart metadata name chart resolution actually reads', () => { + activate(context, { fetch: vi.fn(), credentials: noDockerCredentials, readTextFile: NO_FILES }); + + expect(getLastFileSystemWatcher()?.globPattern).toBe('**/Chart.yaml'); + }); + + it('should re-check the documents a chart governs against its new appVersion, and leave the others alone', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const files: Record = { + '/repo/chart/Chart.yaml': chartMetadataWithAppVersion('1.18'), + '/repo/other/Chart.yaml': ['apiVersion: v2', 'name: other', 'appVersion: 2.0.0', ''].join('\n'), + }; + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: createFakeFileSystem(files) }); + + const governed = createFakeDocument('/repo/chart/values.yaml', TAGLESS_VALUES_YAML); + const unrelated = createFakeDocument( + '/repo/other/values.yaml', + ['image:', ' repository: docker.io/library/redis', ' pullPolicy: Always', ''].join('\n') + ); + + await emitDidOpenTextDocument(governed); + await emitDidOpenTextDocument(unrelated); + setOpenTextDocuments([governed, unrelated]); + fetch.mockClear(); + + files['/repo/chart/Chart.yaml'] = chartMetadataWithAppVersion(TAG); + await emitDidChangeChartMetadata({ path: '/repo/chart/Chart.yaml' }); + + // A bumped `appVersion` is exactly when a stale checkmark costs the most, + // and the chart nobody touched has nothing new to be asked about. + expect(fetch).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledWith(`https://registry-1.docker.io/v2/library/nginx/manifests/${TAG}`, expect.anything()); + }); + + it('should keep the latest re-check when an earlier one for the same document answers after it', async () => { + type FakeResponse = ReturnType; + let answerSlowRequest: (response: FakeResponse) => void = () => undefined; + const slowAnswer = new Promise((resolve) => { + answerSlowRequest = resolve; + }); + const fetch = vi.fn(async (url: string) => (url.endsWith('/manifests/1.18') ? slowAnswer : Promise.resolve(fakeFetchResponse(200)))); + const files: Record = { '/repo/chart/Chart.yaml': chartMetadataWithAppVersion('1.17') }; + activate(context, { fetch, credentials: noDockerCredentials, readTextFile: createFakeFileSystem(files) }); + + const document = createFakeDocument('/repo/chart/values.yaml', TAGLESS_VALUES_YAML); + await emitDidOpenTextDocument(document); + setOpenTextDocuments([document]); + + // Two saves in quick succession, as autosave produces while typing a + // version: the first one's registry answer is still in flight when the + // second one lands. + files['/repo/chart/Chart.yaml'] = chartMetadataWithAppVersion('1.18'); + const firstRecheck = emitDidChangeChartMetadata({ path: '/repo/chart/Chart.yaml' }); + await vi.waitFor(() => { + expect(fetch).toHaveBeenCalledWith(expect.stringMatching(/\/manifests\/1\.18$/), expect.anything()); + }); + + files['/repo/chart/Chart.yaml'] = chartMetadataWithAppVersion(TAG); + await emitDidChangeChartMetadata({ path: '/repo/chart/Chart.yaml' }); + + answerSlowRequest(fakeFetchResponse(404, { errors: [{ code: 'MANIFEST_UNKNOWN' }] })); + await firstRecheck; + + const setCalls = getLastDiagnosticCollection()?.set.mock.calls ?? []; + + expect(setCalls[setCalls.length - 1]?.[1]).toEqual([]); + }); }); diff --git a/apps/vscode/src/extension.ts b/apps/vscode/src/extension.ts index caeceee..4bbcbb1 100644 --- a/apps/vscode/src/extension.ts +++ b/apps/vscode/src/extension.ts @@ -1,4 +1,5 @@ import * as vscode from 'vscode'; +import { CHART_METADATA_FILE_NAME, type ReadTextFile } from 'helm'; import { localDockerCredentials, type CredentialEnvironment, type FetchLike } from 'oci-registry'; import { diagnosticsFor } from './diagnostics'; import { hoverFor } from './hover'; @@ -8,16 +9,43 @@ import { checkImageReferencesInDocument, checksAsOf, registriesNeedingLogin, typ const DIAGNOSTIC_COLLECTION_NAME = 'infra-tools-images'; +// Built from the name chart resolution looks for, rather than spelled out +// again here: a watcher covering a different set of files than the resolver +// reads is a staleness bug that shows up as nothing happening. +const CHART_METADATA_GLOB = `**/${CHART_METADATA_FILE_NAME}`; + interface ActivateDependencies { /** Only tests override this; production activation uses the platform's `fetch`. */ readonly fetch?: FetchLike; /** Only tests override this; production activation reads the developer's real Docker config. */ readonly credentials?: CredentialEnvironment; + /** Only tests override this; production activation reads through the workspace file system. */ + readonly readTextFile?: ReadTextFile; +} + +/** + * Reads a workspace file as text, through VS Code's file system rather than + * Node's, so a remote workspace resolves its charts on the machine the + * extension host actually runs on. `Uri.file` pins the scheme to `file`, so + * a virtual workspace — a repository browsed without being cloned — resolves + * no charts at all and falls back to the values-file naming rule. + * + * A read that throws is reported as nothing readable rather than as a + * failure: chart resolution asks about `Chart.yaml` in every ancestor + * directory of a file, so absent is the ordinary answer, not an error. + */ +async function readWorkspaceTextFile(path: string): Promise { + try { + return new TextDecoder().decode(await vscode.workspace.fs.readFile(vscode.Uri.file(path))); + } catch { + return undefined; + } } /** * Called by the extension host on activation. Wires the three surfaces a - * check is shown on and re-checks a Helm values file whenever one opens. + * check is shown on, re-checks a Helm values file whenever one opens, and + * re-checks the files a chart governs whenever its metadata changes. */ function activate(context: vscode.ExtensionContext, dependencies: ActivateDependencies = {}): void { const channel = vscode.window.createOutputChannel('Infra Tools'); @@ -27,41 +55,81 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend const checkDependencies = { fetch: dependencies.fetch ?? (globalThis as unknown as { fetch: FetchLike }).fetch, credentials: dependencies.credentials ?? localDockerCredentials, + readTextFile: dependencies.readTextFile ?? readWorkspaceTextFile, }; const diagnostics = vscode.languages.createDiagnosticCollection(DIAGNOSTIC_COLLECTION_NAME); context.subscriptions.push(diagnostics); const checksByDocument = new Map(); + // The most recent check started per document. A registry answer can land + // after a later check's, so only the latest check may publish. + const latestCheckByDocument = new Map(); + let checksStarted = 0; const markDecorationType = createMarkDecorationType(); context.subscriptions.push(markDecorationType); const loginPrompts = createLoginPrompts(context.globalState); context.subscriptions.push(loginPrompts); - context.subscriptions.push( - vscode.workspace.onDidOpenTextDocument(async (document) => { - // VS Code never awaits a listener, so anything escaping this callback - // is an unhandled rejection, and that takes the extension host down. - try { - const checked = await checkImageReferencesInDocument(document, checkDependencies); - - if (checked === undefined) { - return; - } - - checksByDocument.set(document.uri.toString(), checked); - diagnostics.set(document.uri, diagnosticsFor(document, checksAsOf(checked, document))); - applyMarks(vscode.window.visibleTextEditors, checksByDocument, markDecorationType); - - // Not awaited: a notification stays up until the developer answers - // it, and holding an open-document listener for that long would tie - // this file's check to a dialog about a registry. - void loginPrompts.report(registriesNeedingLogin(checked.checks)); - } catch (error) { - channel.appendLine(`Checking image references in ${document.uri.toString()} failed: ${String(error)}`); + /** + * Checks one document and republishes everything shown for it. The open + * listener and the chart watcher both go through here, so the two can + * never drift into publishing different things about the same document. + * + * A check superseded while it waited on a registry publishes nothing. Two + * `Chart.yaml` saves in quick succession would otherwise let the slower + * answer win, and that answer is about the `appVersion` already replaced. + */ + async function checkDocument(document: vscode.TextDocument): Promise { + const key = document.uri.toString(); + const checkNumber = ++checksStarted; + latestCheckByDocument.set(key, checkNumber); + + // VS Code never awaits a listener, so anything escaping this callback + // is an unhandled rejection, and that takes the extension host down. + try { + const checked = await checkImageReferencesInDocument(document, checkDependencies); + + if (checked === undefined || latestCheckByDocument.get(key) !== checkNumber) { + return; } - }) - ); + + checksByDocument.set(key, checked); + diagnostics.set(document.uri, diagnosticsFor(document, checksAsOf(checked, document), vscode.workspace.asRelativePath)); + applyMarks(vscode.window.visibleTextEditors, checksByDocument, markDecorationType); + + // Not awaited: a notification stays up until the developer answers + // it, and holding an open-document listener for that long would tie + // this file's check to a dialog about a registry. + void loginPrompts.report(registriesNeedingLogin(checked.checks)); + } catch (error) { + channel.appendLine(`Checking image references in ${document.uri.toString()} failed: ${String(error)}`); + } + } + + /** + * Re-checks every open document whose last check read this metadata file. + * A bumped `appVersion` otherwise leaves a stale checkmark standing at + * exactly the moment the developer is relying on it. + * + * Only a document that already resolved through this chart qualifies. A + * chart appearing above a file checked without one would also change that + * file's answer, but catching it means re-resolving every open document on + * every write, and no ticket has asked for it yet. + */ + async function recheckDocumentsGovernedBy(uri: vscode.Uri): Promise { + for (const document of vscode.workspace.textDocuments) { + if (checksByDocument.get(document.uri.toString())?.chartMetadataPath === uri.path) { + await checkDocument(document); + } + } + } + + context.subscriptions.push(vscode.workspace.onDidOpenTextDocument(checkDocument)); + + const chartMetadataWatcher = vscode.workspace.createFileSystemWatcher(CHART_METADATA_GLOB); + context.subscriptions.push(chartMetadataWatcher); + context.subscriptions.push(chartMetadataWatcher.onDidChange(recheckDocumentsGovernedBy)); context.subscriptions.push( vscode.languages.registerHoverProvider( @@ -70,7 +138,7 @@ function activate(context: vscode.ExtensionContext, dependencies: ActivateDepend provideHover(document: vscode.TextDocument, position: vscode.Position): vscode.Hover | undefined { const checked = checksByDocument.get(document.uri.toString()); - return checked === undefined ? undefined : hoverFor(document, position, checksAsOf(checked, document)); + return checked === undefined ? undefined : hoverFor(document, position, checksAsOf(checked, document), vscode.workspace.asRelativePath); }, } ) diff --git a/apps/vscode/src/hover.test.ts b/apps/vscode/src/hover.test.ts index adbd2f0..2a52c8a 100644 --- a/apps/vscode/src/hover.test.ts +++ b/apps/vscode/src/hover.test.ts @@ -1,20 +1,22 @@ import type * as vscode from 'vscode'; import { describe, expect, it } from 'vitest'; -import type { SourceRange } from 'helm'; +import type { ResolvedTag, SourceRange } from 'helm'; +import type { ImageVerdict, UnverifiableReason } from 'oci-registry'; import { createFakeDocument } from '../test/fake-document'; import { hoverFor } from './hover'; -import type { ReferenceCheck, ReferenceVerdict, UncheckedReason } from './reference-check'; +import type { ReferenceCheck } from './reference-check'; const REPOSITORY = 'registry.example.com/svc'; const TAG = '1.0'; const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: "${TAG}"`, ''].join('\n'); +const CHART_METADATA_PATH = '/repo/chart/Chart.yaml'; +const PROVENANCE_SENTENCE = ' Tag taken from appVersion in chart/Chart.yaml.'; -const VERIFIED_VERDICT: ReferenceVerdict = { kind: 'exists', registry: 'mirror.example.com' }; +const VERIFIED_VERDICT: ImageVerdict = { kind: 'exists', registry: 'mirror.example.com' }; // The sentence each reason owes the reader, as a record so a new reason fails // the build here instead of hovering with someone else's explanation. -const UNCHECKED_REASON_SENTENCES: Record = { - 'no-tag': 'the reference names no tag to check.', +const UNCHECKED_REASON_SENTENCES: Record = { 'guessed-registry': 'nothing here names a registry, and Docker Hub — the only one left to try — does not have it.', 'needs-login': 'no local Docker credential for that registry. Run `docker login` against it.', 'authentication-failure': 'the registry refused the local Docker credential for it.', @@ -23,8 +25,13 @@ const UNCHECKED_REASON_SENTENCES: Record = { 'malformed-reference': 'the reference is not a valid image reference.', }; +/** Stands in for `vscode.workspace.asRelativePath`, shortening enough that a hover can be seen to have used it. */ +function describeChartPath(path: string): string { + return path.replace('/repo/', ''); +} + /** The unverifiable verdict a reason produces. Only `needs-login` carries a registry, so the shape cannot be built generically. */ -function unverifiableVerdict(reason: UncheckedReason): ReferenceVerdict { +function unverifiableVerdict(reason: UnverifiableReason): ImageVerdict { return reason === 'needs-login' ? { kind: 'unverifiable', reason, registry: 'registry.example.com' } : { kind: 'unverifiable', reason }; } @@ -36,32 +43,26 @@ function rangeOfText(text: string): SourceRange { } /** A check over the single reference in {@link VALUES_YAML}, carrying that file's real offsets. */ -function createCheck(verdict: ReferenceVerdict): ReferenceCheck { +function createCheck(verdict: ImageVerdict, tag: ResolvedTag = { source: 'file', text: TAG, range: rangeOfText(TAG) }): ReferenceCheck { return { reference: { repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, - tag: { text: TAG, range: rangeOfText(TAG) }, + tag: tag.source === 'file' ? { text: tag.text, range: tag.range } : undefined, registry: undefined, }, + tag, verdict, }; } -/** A check over the same reference written without a tag, which is what a `no-tag` verdict comes from. */ -function createTaglessCheck(verdict: ReferenceVerdict): ReferenceCheck { - return { - reference: { - repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, - tag: undefined, - registry: undefined, - }, - verdict, - }; +/** The same reference written without a tag, checked against the chart's `appVersion` instead. */ +function createChartMetadataCheck(verdict: ImageVerdict): ReferenceCheck { + return createCheck(verdict, { source: 'chart-metadata', text: TAG, metadataPath: CHART_METADATA_PATH }); } /** The hover at the first character of `text`'s occurrence in {@link VALUES_YAML}. */ function hoverAtText(document: vscode.TextDocument, checks: readonly ReferenceCheck[], text: string): vscode.Hover | undefined { - return hoverFor(document, document.positionAt(VALUES_YAML.indexOf(text)), checks); + return hoverFor(document, document.positionAt(VALUES_YAML.indexOf(text)), checks, describeChartPath); } /** The plain text of a hover's single content entry. */ @@ -87,18 +88,30 @@ describe('hover', () => { it('should render its own sentence for every reason a reference went unverified', () => { const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - for (const [reason, sentence] of Object.entries(UNCHECKED_REASON_SENTENCES) as [UncheckedReason, string][]) { + for (const [reason, sentence] of Object.entries(UNCHECKED_REASON_SENTENCES) as [UnverifiableReason, string][]) { const hoverText = getHoverText(hoverAtText(document, [createCheck(unverifiableVerdict(reason))], REPOSITORY)); expect(hoverText).toBe(`Not verified: ${sentence}`); } }); - it('should explain a tagless reference, which carries no tag to hover in the first place', () => { + it('should name the chart a tag was taken from, on a verified reference and an unverified one alike', () => { + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const verified = createChartMetadataCheck(VERIFIED_VERDICT); + const unverified = createChartMetadataCheck(unverifiableVerdict('network-error')); + + // A checkmark earned by a tag the file never wrote is worth attributing: + // the hover is the only surface that can say where it came from. + expect(getHoverText(hoverAtText(document, [verified], REPOSITORY))).toBe(`Verified on \`mirror.example.com\`.${PROVENANCE_SENTENCE}`); + expect(getHoverText(hoverAtText(document, [unverified], REPOSITORY))).toBe( + `Not verified: ${UNCHECKED_REASON_SENTENCES['network-error']}${PROVENANCE_SENTENCE}` + ); + }); + + it('should say nothing about a chart for a tag the file writes itself', () => { const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); - const check = createTaglessCheck({ kind: 'unverifiable', reason: 'no-tag' }); - expect(getHoverText(hoverAtText(document, [check], REPOSITORY))).toContain('no tag'); + expect(getHoverText(hoverAtText(document, [createCheck(VERIFIED_VERDICT)], REPOSITORY))).not.toContain('appVersion'); }); it('should provide no hover for a reference that does not exist, which already speaks through its diagnostic', () => { diff --git a/apps/vscode/src/hover.ts b/apps/vscode/src/hover.ts index 5094767..422cb26 100644 --- a/apps/vscode/src/hover.ts +++ b/apps/vscode/src/hover.ts @@ -1,11 +1,12 @@ import * as vscode from 'vscode'; -import type { ReferenceCheck, UncheckedReason } from './reference-check'; +import type { UnverifiableReason } from 'oci-registry'; +import type { ReferenceCheck } from './reference-check'; import { containsOffset } from './source-range'; +import { chartProvenanceSentence } from './tag-provenance'; // A table rather than a switch, so adding a reason fails the build here // instead of hovering with no explanation. -const UNCHECKED_REASON_TEXT: Record = { - 'no-tag': 'the reference names no tag to check.', +const UNCHECKED_REASON_TEXT: Record = { 'guessed-registry': 'nothing here names a registry, and Docker Hub — the only one left to try — does not have it.', 'needs-login': 'no local Docker credential for that registry. Run `docker login` against it.', 'authentication-failure': 'the registry refused the local Docker credential for it.', @@ -17,8 +18,18 @@ const UNCHECKED_REASON_TEXT: Record = { /** * What hovering a checked reference reports. A not-found verdict gets no * hover: it already speaks through its diagnostic, and it names no registry. + * + * A reference checked against a tag it never wrote is told so here as well + * as in the diagnostic, because a hover is the only surface a verified + * reference has, and a checkmark earned by someone else's tag is worth + * knowing about. */ -function hoverFor(document: vscode.TextDocument, position: vscode.Position, checks: readonly ReferenceCheck[]): vscode.Hover | undefined { +function hoverFor( + document: vscode.TextDocument, + position: vscode.Position, + checks: readonly ReferenceCheck[], + describeChartPath: (path: string) => string +): vscode.Hover | undefined { const offset = document.offsetAt(position); const check = checks.find( ({ reference }) => @@ -29,14 +40,15 @@ function hoverFor(document: vscode.TextDocument, position: vscode.Position, chec return undefined; } - const { verdict } = check; + const { tag, verdict } = check; + const provenance = chartProvenanceSentence(tag, describeChartPath); if (verdict.kind === 'exists') { - return new vscode.Hover(new vscode.MarkdownString(`Verified on \`${verdict.registry}\`.`)); + return new vscode.Hover(new vscode.MarkdownString(`Verified on \`${verdict.registry}\`.${provenance}`)); } if (verdict.kind === 'unverifiable') { - return new vscode.Hover(new vscode.MarkdownString(`Not verified: ${UNCHECKED_REASON_TEXT[verdict.reason]}`)); + return new vscode.Hover(new vscode.MarkdownString(`Not verified: ${UNCHECKED_REASON_TEXT[verdict.reason]}${provenance}`)); } return undefined; diff --git a/apps/vscode/src/marks.test.ts b/apps/vscode/src/marks.test.ts index 3e3377e..03adae9 100644 --- a/apps/vscode/src/marks.test.ts +++ b/apps/vscode/src/marks.test.ts @@ -1,18 +1,22 @@ import * as vscode from 'vscode'; import { describe, expect, it, vi } from 'vitest'; -import type { SourceRange } from 'helm'; +import type { ResolvedTag, SourceRange } from 'helm'; +import type { ImageVerdict } from 'oci-registry'; import { bumpVersion, createFakeDocument } from '../test/fake-document'; import { createTextEditorStub, type TextEditorStub } from '../test/vscode-stub'; import { applyMarks, createMarkDecorationType, marksFor } from './marks'; -import type { DocumentChecks, ReferenceCheck, ReferenceVerdict } from './reference-check'; +import type { DocumentChecks, ReferenceCheck } from './reference-check'; const REPOSITORY = 'registry.example.com/svc'; const TAG = '1.0'; const VALUES_YAML = ['image:', ` repository: ${REPOSITORY}`, ` tag: "${TAG}"`, ''].join('\n'); -const VERIFIED_VERDICT: ReferenceVerdict = { kind: 'exists', registry: 'registry.example.com' }; -const MISSING_VERDICT: ReferenceVerdict = { kind: 'tag-not-found', repository: REPOSITORY, tag: TAG }; -const UNCHECKED_VERDICT: ReferenceVerdict = { kind: 'unverifiable', reason: 'network-error' }; +const VERIFIED_VERDICT: ImageVerdict = { kind: 'exists', registry: 'registry.example.com' }; +const MISSING_VERDICT: ImageVerdict = { kind: 'tag-not-found', repository: REPOSITORY, tag: TAG }; +const UNCHECKED_VERDICT: ImageVerdict = { kind: 'unverifiable', reason: 'network-error' }; + +/** A tagless reference's tag comes from the governing chart, which is the only way one is checked at all. */ +const CHART_METADATA_TAG: ResolvedTag = { source: 'chart-metadata', text: TAG, metadataPath: '/repo/chart/Chart.yaml' }; /** The source range of `text`'s first occurrence in {@link VALUES_YAML}. */ function rangeOfText(text: string): SourceRange { @@ -22,13 +26,14 @@ function rangeOfText(text: string): SourceRange { } /** A check over the single reference in {@link VALUES_YAML}, carrying that file's real offsets. */ -function createCheck(verdict: ReferenceVerdict): ReferenceCheck { +function createCheck(verdict: ImageVerdict): ReferenceCheck { return { reference: { repository: { text: REPOSITORY, range: rangeOfText(REPOSITORY) }, tag: { text: TAG, range: rangeOfText(TAG) }, registry: undefined, }, + tag: { source: 'file', text: TAG, range: rangeOfText(TAG) }, verdict, }; } @@ -40,7 +45,7 @@ function asEditors(editors: readonly TextEditorStub[]): readonly vscode.TextEdit /** One document's stored checks, tagged with the version it currently has. */ function createChecksByDocument(document: vscode.TextDocument, checks: readonly ReferenceCheck[]): Map { - return new Map([[document.uri.toString(), { version: document.version, checks }]]); + return new Map([[document.uri.toString(), { version: document.version, checks, chartMetadataPath: undefined }]]); } /** An editor that throws the way a disposed one does. */ @@ -92,7 +97,7 @@ describe('marks', () => { registry: 'ghcr.io', }; - const [mark] = marksFor(document, [{ reference, verdict: { kind: 'exists', registry: 'ghcr.io' } }]); + const [mark] = marksFor(document, [{ reference, tag: CHART_METADATA_TAG, verdict: { kind: 'exists', registry: 'ghcr.io' } }]); // The file says `ghcr.io` two lines up. Appending it restates the file. expect(mark?.renderOptions?.after?.contentText).toBe(' ✓'); @@ -107,7 +112,7 @@ describe('marks', () => { registry: undefined, }; - const [mark] = marksFor(document, [{ reference, verdict: { kind: 'exists', registry: 'docker.io' } }]); + const [mark] = marksFor(document, [{ reference, tag: CHART_METADATA_TAG, verdict: { kind: 'exists', registry: 'docker.io' } }]); // A file that spells out no registry cannot have the answer restated to // it, so naming Docker Hub is the only way the mark says where it looked. diff --git a/apps/vscode/src/marks.ts b/apps/vscode/src/marks.ts index 42a2aaf..47222c3 100644 --- a/apps/vscode/src/marks.ts +++ b/apps/vscode/src/marks.ts @@ -1,7 +1,7 @@ import * as vscode from 'vscode'; import type { ImageReference } from 'helm'; -import { resolveExplicitHost } from 'oci-registry'; -import { checksAsOf, type DocumentChecks, type ReferenceCheck, type ReferenceVerdict } from './reference-check'; +import { resolveExplicitHost, type ImageVerdict } from 'oci-registry'; +import { checksAsOf, type DocumentChecks, type ReferenceCheck } from './reference-check'; import { rangeOf } from './source-range'; type MarkKind = 'verified' | 'missing' | 'unchecked'; @@ -16,7 +16,7 @@ const MARKS: Record { - const checked = await checkImageReferencesInDocument(document, { fetch, credentials: noDockerCredentials }); +async function checkValuesFile(document: vscode.TextDocument, fetch: FetchLike, readTextFile: ReadTextFile = NO_FILES): Promise { + const checked = await checkImageReferencesInDocument(document, { fetch, credentials: noDockerCredentials, readTextFile }); if (checked === undefined) { throw new Error('expected the document to be checked'); @@ -30,12 +42,13 @@ async function checkValuesFile(document: vscode.TextDocument, fetch: FetchLike): } describe('reference-check', () => { - it('should ignore a document that is not the conventional values file name', async () => { + it('should ignore a document that is neither named as a values file nor sits under a chart', async () => { const fetch = vi.fn(); const document = createFakeDocument('/repo/chart/deployment.yaml', VALUES_YAML); - expect(isHelmValuesFile(document)).toBe(false); - await expect(checkImageReferencesInDocument(document, { fetch, credentials: noDockerCredentials })).resolves.toBeUndefined(); + await expect( + checkImageReferencesInDocument(document, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }) + ).resolves.toBeUndefined(); expect(fetch).not.toHaveBeenCalled(); }); @@ -43,8 +56,29 @@ describe('reference-check', () => { const fetch = vi.fn(); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML, 'plaintext'); - expect(isHelmValuesFile(document)).toBe(false); - await expect(checkImageReferencesInDocument(document, { fetch, credentials: noDockerCredentials })).resolves.toBeUndefined(); + await expect( + checkImageReferencesInDocument(document, { fetch, credentials: noDockerCredentials, readTextFile: NO_FILES }) + ).resolves.toBeUndefined(); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('should check any yaml file beneath a chart directory, whatever it is named', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/production.yaml', VALUES_YAML); + + const { checks } = await checkValuesFile(document, fetch, ONE_CHART); + + expect(checks).toHaveLength(1); + expect(fetch).toHaveBeenCalledTimes(1); + }); + + it('should ignore a file under the chart templates directory, where nothing renders to a real reference', async () => { + const fetch = vi.fn(); + const document = createFakeDocument('/repo/chart/templates/values.yaml', VALUES_YAML); + + await expect( + checkImageReferencesInDocument(document, { fetch, credentials: noDockerCredentials, readTextFile: ONE_CHART }) + ).resolves.toBeUndefined(); expect(fetch).not.toHaveBeenCalled(); }); @@ -55,21 +89,75 @@ describe('reference-check', () => { // The extractor collects YAML errors instead of throwing, so a malformed // file yields zero references rather than the `undefined` that means this // feature has nothing to say about the document. - await expect(checkValuesFile(document, fetch)).resolves.toEqual({ version: document.version, checks: [] }); + await expect(checkValuesFile(document, fetch)).resolves.toEqual({ version: document.version, checks: [], chartMetadataPath: undefined }); expect(fetch).not.toHaveBeenCalled(); }); - it('should ask no registry about a tagless reference, and report it as unverifiable', async () => { + it('should check a tagless reference against the governing chart appVersion', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', TAGLESS_VALUES_YAML); + + await checkValuesFile(document, fetch, ONE_CHART); + + expect(fetch).toHaveBeenCalledWith('https://ghcr.io/v2/my-org/my-service/manifests/1.2.3', expect.anything()); + }); + + it('should record where a tag taken from chart metadata came from', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', TAGLESS_VALUES_YAML); + + const { checks } = await checkValuesFile(document, fetch, ONE_CHART); + + expect(checks[0]?.tag).toEqual({ source: 'chart-metadata', text: '1.2.3', metadataPath: '/repo/chart/Chart.yaml' }); + }); + + it('should resolve a subchart values file against the subchart own appVersion, not the parent chart', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/charts/sub/values.yaml', TAGLESS_VALUES_YAML); + const readTextFile = createFakeFileSystem({ + '/repo/chart/Chart.yaml': CHART_YAML, + '/repo/chart/charts/sub/Chart.yaml': SUBCHART_YAML, + }); + + await checkValuesFile(document, fetch, readTextFile); + + // The parent's 1.2.3 is what Helm would ignore here, so asking about it + // would report on a release nobody is deploying. + expect(fetch).toHaveBeenCalledWith('https://ghcr.io/v2/my-org/my-service/manifests/4.5.6', expect.anything()); + }); + + it('should produce no check at all for a tagless reference no chart metadata can resolve', async () => { const fetch = vi.fn(); const document = createFakeDocument('/repo/chart/values.yaml', TAGLESS_VALUES_YAML); const { checks } = await checkValuesFile(document, fetch); - expect(checks).toHaveLength(1); - expect(checks[0]?.verdict).toEqual({ kind: 'unverifiable', reason: 'no-tag' }); + // Silence, not an unchecked mark: there is nothing to check it against + // and nothing the developer could do about a mark saying so. + expect(checks).toEqual([]); + expect(fetch).not.toHaveBeenCalled(); + }); + + it('should produce no check for a tagless reference whose governing chart declares no appVersion', async () => { + const fetch = vi.fn(); + const document = createFakeDocument('/repo/chart/values.yaml', TAGLESS_VALUES_YAML); + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': APPVERSIONLESS_CHART_YAML }); + + const { checks } = await checkValuesFile(document, fetch, readTextFile); + + expect(checks).toEqual([]); expect(fetch).not.toHaveBeenCalled(); }); + it('should report the chart metadata file the checks depend on', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const governed = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + const standalone = createFakeDocument('/elsewhere/values.yaml', VALUES_YAML); + + await expect(checkValuesFile(governed, fetch, ONE_CHART)).resolves.toMatchObject({ chartMetadataPath: '/repo/chart/Chart.yaml' }); + await expect(checkValuesFile(standalone, fetch)).resolves.toMatchObject({ chartMetadataPath: undefined }); + }); + it('should return the verdict the registry answer implies for a tagged reference', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); @@ -78,10 +166,24 @@ describe('reference-check', () => { expect(checks).toHaveLength(1); expect(checks[0]?.reference.repository.text).toBe(REPOSITORY); + expect(checks[0]?.tag).toEqual({ + source: 'file', + text: TAG, + range: { start: VALUES_YAML.indexOf(TAG), end: VALUES_YAML.indexOf(TAG) + TAG.length }, + }); expect(checks[0]?.verdict).toEqual({ kind: 'exists', registry: 'docker.io' }); expect(fetch).toHaveBeenCalledTimes(1); }); + it('should prefer the tag the file writes over the governing chart appVersion', async () => { + const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); + const document = createFakeDocument('/repo/chart/values.yaml', VALUES_YAML); + + await checkValuesFile(document, fetch, ONE_CHART); + + expect(fetch).toHaveBeenCalledWith(`https://registry-1.docker.io/v2/library/nginx/manifests/${TAG}`, expect.anything()); + }); + it('should check a bare repository against the registry its document declares', async () => { const fetch = vi.fn().mockResolvedValue(fakeFetchResponse(200)); const document = createFakeDocument('/repo/chart/values.yaml', DECLARED_REGISTRY_VALUES_YAML); diff --git a/apps/vscode/src/reference-check.ts b/apps/vscode/src/reference-check.ts index c657eff..b8f1af3 100644 --- a/apps/vscode/src/reference-check.ts +++ b/apps/vscode/src/reference-check.ts @@ -1,53 +1,33 @@ import type * as vscode from 'vscode'; -import { extractImageReferences, type ImageReference } from 'helm'; -import { checkImageExistence, type CredentialEnvironment, type FetchLike, type ImageVerdict, type UnverifiableReason } from 'oci-registry'; - -// Matching any YAML file beneath a chart directory, and excluding that -// chart's templates directory, is Helm chart-context knowledge this ticket -// doesn't implement yet. -const VALUES_FILE_NAME_PATTERN = /^values\.ya?ml$/i; - -/** Every reason the registry package reports, plus the ones settled before asking it. */ -type UncheckedReason = UnverifiableReason | 'no-tag'; - -/** - * A registry verdict, or the one outcome the extension settles itself. - * - * `ImageVerdict` is taken whole rather than picked apart, so the registry - * package's own split — only `'needs-login'` carries a host — survives the - * trip to the UI instead of being flattened into an optional field the - * notification code would then have to re-check. - */ -type ReferenceVerdict = ImageVerdict | { readonly kind: 'unverifiable'; readonly reason: 'no-tag' }; +import { extractImageReferences, resolveTag, resolveValuesFileContext, type ImageReference, type ReadTextFile, type ResolvedTag } from 'helm'; +import { checkImageExistence, type CredentialEnvironment, type FetchLike, type ImageVerdict } from 'oci-registry'; /** * One checked reference, projected onto all three surfaces, so diagnostics, * marks, and hovers can never disagree about a reference. + * + * The resolved tag rides along because it is not always in the file: a tag + * taken from `appVersion` has a chart to name and no range to underline, and + * both of those are decisions a surface has to make per reference. */ interface ReferenceCheck { readonly reference: ImageReference; - readonly verdict: ReferenceVerdict; + readonly tag: ResolvedTag; + readonly verdict: ImageVerdict; } /** A document's checks, tagged with the document version they describe. */ interface DocumentChecks { readonly version: number; readonly checks: readonly ReferenceCheck[]; -} - -function isHelmValuesFile(document: vscode.TextDocument): boolean { - if (document.languageId !== 'yaml') { - return false; - } - - const fileName = document.uri.path.split('/').pop() ?? ''; - - return VALUES_FILE_NAME_PATTERN.test(fileName); + /** The metadata file whose `appVersion` these checks may depend on. */ + readonly chartMetadataPath: string | undefined; } interface CheckDependencies { readonly fetch: FetchLike; readonly credentials: CredentialEnvironment; + readonly readTextFile: ReadTextFile; } /** @@ -55,13 +35,23 @@ interface CheckDependencies { * nothing to say about the document at all, which is not the same as a * checked document that produced no findings: the caller replaces a * document's diagnostics and marks only when it gets checks back. + * + * Which documents those are is the Helm package's call, not this file's — a + * values file is anything beneath a chart directory, and that is chart + * knowledge a future CLI would otherwise have to reimplement. */ async function checkImageReferencesInDocument(document: vscode.TextDocument, dependencies: CheckDependencies): Promise { - if (!isHelmValuesFile(document)) { + if (document.languageId !== 'yaml') { + return undefined; + } + + const { fetch, credentials, readTextFile } = dependencies; + const context = await resolveValuesFileContext(document.uri.path, readTextFile); + + if (context === undefined) { return undefined; } - const { fetch, credentials } = dependencies; const version = document.version; // No guard around this: the extractor collects YAML syntax errors rather @@ -69,26 +59,31 @@ async function checkImageReferencesInDocument(document: vscode.TextDocument, dep // open listener's own catch covers a genuine fault. const references = extractImageReferences(document.getText()); - // A tagless reference has nothing to ask a registry until `appVersion` - // resolution lands, but it still comes through as a check. Dropping it is - // what made real references render nothing, which reads as a broken tool. + // A reference that resolves to no tag at all is dropped here rather than + // carried as an outcome: there is nothing to ask a registry and nothing + // truthful to render, and a mark that says only "unchecked" on a file the + // developer cannot act on is noise they would switch the feature off over. + const resolved = references.flatMap((reference) => { + const tag = resolveTag(reference, context.chart); + + return tag === undefined ? [] : [{ reference, tag }]; + }); + const checks = await Promise.all( - references.map(async (reference) => ({ + resolved.map(async ({ reference, tag }) => ({ reference, - verdict: - reference.tag === undefined - ? ({ kind: 'unverifiable', reason: 'no-tag' } as const) - : await checkImageExistence({ - repository: reference.repository.text, - tag: reference.tag.text, - declaredRegistry: reference.registry, - fetch, - credentials, - }), + tag, + verdict: await checkImageExistence({ + repository: reference.repository.text, + tag: tag.text, + declaredRegistry: reference.registry, + fetch, + credentials, + }), })) ); - return { version, checks }; + return { version, checks, chartMetadataPath: context.chart?.path }; } /** @@ -120,5 +115,5 @@ function registriesNeedingLogin(checks: readonly ReferenceCheck[]): string[] { return [...registries]; } -export { checkImageReferencesInDocument, checksAsOf, isHelmValuesFile, registriesNeedingLogin }; -export type { DocumentChecks, ReferenceCheck, ReferenceVerdict, UncheckedReason }; +export { checkImageReferencesInDocument, checksAsOf, registriesNeedingLogin }; +export type { DocumentChecks, ReferenceCheck }; diff --git a/apps/vscode/src/tag-provenance.ts b/apps/vscode/src/tag-provenance.ts new file mode 100644 index 0000000..74dfc2d --- /dev/null +++ b/apps/vscode/src/tag-provenance.ts @@ -0,0 +1,16 @@ +import type { ResolvedTag } from 'helm'; + +/** + * The sentence a message owes the reader when the tag it reports was never + * written in the file. + * + * Shared by the diagnostic and the hover so the two cannot word the same + * provenance differently, and empty for a tag the file wrote, which needs no + * explaining. `describeChartPath` shortens the path for display: an absolute + * path in the Problems panel is noise the reader has to scan past. + */ +function chartProvenanceSentence(tag: ResolvedTag, describeChartPath: (path: string) => string): string { + return tag.source === 'file' ? '' : ` Tag taken from appVersion in ${describeChartPath(tag.metadataPath)}.`; +} + +export { chartProvenanceSentence }; diff --git a/apps/vscode/test/fake-file-system.ts b/apps/vscode/test/fake-file-system.ts new file mode 100644 index 0000000..b50637b --- /dev/null +++ b/apps/vscode/test/fake-file-system.ts @@ -0,0 +1,16 @@ +import type { ReadTextFile } from 'helm'; + +/** + * A filesystem described as a path-to-text map. + * + * Chart resolution walks ancestor directories looking for metadata, so a + * test's subject is the shape of a directory tree. Writing that shape as a + * literal keeps it visible in the test that depends on it, where temporary + * files would put it somewhere the reader has to go and find. + */ +function createFakeFileSystem(files: Record): ReadTextFile { + // eslint-disable-next-line @typescript-eslint/promise-function-async -- a canned lookup, nothing to await + return (path: string) => Promise.resolve(files[path]); +} + +export { createFakeFileSystem }; diff --git a/apps/vscode/test/vscode-stub.ts b/apps/vscode/test/vscode-stub.ts index 6195326..2f4decb 100644 --- a/apps/vscode/test/vscode-stub.ts +++ b/apps/vscode/test/vscode-stub.ts @@ -283,8 +283,50 @@ function getLastTerminal(): TerminalStub | undefined { return terminals[terminals.length - 1]; } +/** + * Test-only stand-in for `vscode.FileSystemWatcher`. The glob it was created + * with is recorded, because which files a watcher covers is as much of the + * extension's behaviour as what it does when one changes. Each of the three + * events is a separate emitter, so a test firing a change never drives a + * create listener that would have re-run the same work. + */ +interface FileSystemWatcherStub { + readonly globPattern: string; + readonly onDidChange: (listener: (uri: unknown) => unknown) => { dispose: () => void }; + readonly onDidCreate: (listener: (uri: unknown) => unknown) => { dispose: () => void }; + readonly onDidDelete: (listener: (uri: unknown) => unknown) => { dispose: () => void }; + readonly dispose: ReturnType; + /** Test-only: drives the change listeners and awaits them. Not part of the real `vscode` API. */ + readonly fireDidChange: (uri: unknown) => Promise; +} + +const fileSystemWatchers: FileSystemWatcherStub[] = []; + +function createFileSystemWatcherStub(globPattern: string): FileSystemWatcherStub { + const didChange = createEventEmitterStub(); + const didCreate = createEventEmitterStub(); + const didDelete = createEventEmitterStub(); + const watcher: FileSystemWatcherStub = { + globPattern, + onDidChange: didChange.event, + onDidCreate: didCreate.event, + onDidDelete: didDelete.event, + dispose: vi.fn(), + fireDidChange: didChange.fire, + }; + + fileSystemWatchers.push(watcher); + + return watcher; +} + const workspace = { onDidOpenTextDocument: onDidOpenTextDocumentEmitter.event, + textDocuments: [] as readonly unknown[], + // The real one shortens a path against the open workspace folders. There + // are none here, and a test asserting on a message wants the path it wrote. + asRelativePath: vi.fn((path: string) => path), + createFileSystemWatcher: vi.fn((globPattern: string) => createFileSystemWatcherStub(globPattern)), }; /** @@ -296,6 +338,29 @@ async function emitDidOpenTextDocument(document: unknown): Promise { await onDidOpenTextDocumentEmitter.fire(document); } +/** + * Test-only helper that replaces `workspace.textDocuments`. Not part of the + * real `vscode` API — tests set it to stage which documents the editor holds + * open, and reset it so one test's documents never leak into another. + */ +function setOpenTextDocuments(documents: readonly unknown[]): void { + workspace.textDocuments = documents; +} + +/** Test-only helper: the most recently created file system watcher. */ +function getLastFileSystemWatcher(): FileSystemWatcherStub | undefined { + return fileSystemWatchers[fileSystemWatchers.length - 1]; +} + +/** + * Test-only helper that fires the most recent watcher's `onDidChange` and + * awaits every registered listener, standing in for a developer saving a + * chart's metadata file. + */ +async function emitDidChangeChartMetadata(uri: unknown): Promise { + await getLastFileSystemWatcher()?.fireDidChange(uri); +} + /** * Test-only helper that replaces `window.visibleTextEditors`. Not part of the * real `vscode` API — tests set it to stage which editors the extension can @@ -314,14 +379,16 @@ async function emitDidChangeVisibleTextEditors(editors: readonly TextEditorStub[ await onDidChangeVisibleTextEditorsEmitter.fire(editors); } -export type { StatusBarItemStub, TerminalStub, TextEditorStub }; +export type { FileSystemWatcherStub, StatusBarItemStub, TerminalStub, TextEditorStub }; export { createTextEditorStub, Diagnostic, DiagnosticSeverity, + emitDidChangeChartMetadata, emitDidChangeVisibleTextEditors, emitDidOpenTextDocument, getLastDiagnosticCollection, + getLastFileSystemWatcher, getLastStatusBarItem, getLastTerminal, getRegisteredHoverProvider, @@ -330,6 +397,7 @@ export { MarkdownString, Position, Range, + setOpenTextDocuments, setVisibleTextEditors, setWarningMessageAnswer, StatusBarAlignment, diff --git a/packages/helm/src/chart-context.test.ts b/packages/helm/src/chart-context.test.ts new file mode 100644 index 0000000..650e005 --- /dev/null +++ b/packages/helm/src/chart-context.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, it } from 'vitest'; +import { resolveTag, resolveValuesFileContext, type ChartMetadata, type ReadTextFile } from './chart-context'; +import type { ImageReference } from './extract-image-references'; + +/** + * A file system described as a path-to-contents record, so a test states the + * directory shape it is about instead of writing temporary files. + */ +function createFakeFileSystem(files: Record): ReadTextFile { + return async (path) => Promise.resolve(files[path]); +} + +/** A reference whose only field `resolveTag` reads is the tag. */ +function createReference(tag: string | undefined): ImageReference { + return { + repository: { text: 'my-service', range: { start: 0, end: 10 } }, + tag: tag === undefined ? undefined : { text: tag, range: { start: 20, end: 20 + tag.length } }, + registry: undefined, + }; +} + +const CHART_METADATA = ['name: my-chart', 'appVersion: 1.2.3', ''].join('\n'); + +describe('resolveValuesFileContext', () => { + it('should resolve a values file against the chart in its own directory', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': CHART_METADATA }); + + const context = await resolveValuesFileContext('/repo/chart/values.yaml', readTextFile); + + expect(context?.chart?.path).toBe('/repo/chart/Chart.yaml'); + expect(context?.chart?.appVersion).toBe('1.2.3'); + }); + + it('should resolve a values file several directories deep against the chart above it', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': CHART_METADATA }); + + const context = await resolveValuesFileContext('/repo/chart/env/production/values.yaml', readTextFile); + + expect(context?.chart?.path).toBe('/repo/chart/Chart.yaml'); + }); + + it('should resolve a subchart values file against the subchart own metadata', async () => { + const readTextFile = createFakeFileSystem({ + '/repo/chart/Chart.yaml': ['name: parent', 'appVersion: 1.0.0', ''].join('\n'), + '/repo/chart/charts/sub/Chart.yaml': ['name: sub', 'appVersion: 2.0.0', ''].join('\n'), + }); + + const context = await resolveValuesFileContext('/repo/chart/charts/sub/values.yaml', readTextFile); + + expect(context?.chart?.path).toBe('/repo/chart/charts/sub/Chart.yaml'); + expect(context?.chart?.appVersion).toBe('2.0.0'); + }); + + it('should ignore metadata written as Chart.yml, which Helm itself would not load', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yml': CHART_METADATA }); + + const context = await resolveValuesFileContext('/repo/chart/values.yaml', readTextFile); + + // In scope through the filename fallback, but governed by no chart: an + // `appVersion` Helm would never read is worse than none at all. + expect(context).toEqual({ chart: undefined }); + }); + + it('should exclude a chart own metadata file, which declares a chart rather than supplying values', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': CHART_METADATA }); + + await expect(resolveValuesFileContext('/repo/chart/Chart.yaml', readTextFile)).resolves.toBeUndefined(); + }); + + it('should treat any yaml file beneath a chart directory as in scope', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': CHART_METADATA }); + + const context = await resolveValuesFileContext('/repo/chart/production.yaml', readTextFile); + + expect(context?.chart?.path).toBe('/repo/chart/Chart.yaml'); + }); + + it('should find chart metadata sitting at the filesystem root', async () => { + const readTextFile = createFakeFileSystem({ '/Chart.yaml': CHART_METADATA }); + + const context = await resolveValuesFileContext('/values.yaml', readTextFile); + + expect(context?.chart?.path).toBe('/Chart.yaml'); + }); + + it('should exclude a file under the chart templates directory', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': CHART_METADATA }); + + await expect(resolveValuesFileContext('/repo/chart/templates/deployment.yaml', readTextFile)).resolves.toBeUndefined(); + }); + + it('should exclude a file named values.yaml under the chart templates directory', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': CHART_METADATA }); + + await expect(resolveValuesFileContext('/repo/chart/templates/values.yaml', readTextFile)).resolves.toBeUndefined(); + }); + + it('should exclude a subchart templates file against the subchart rather than against its parent', async () => { + const readTextFile = createFakeFileSystem({ + '/repo/chart/Chart.yaml': ['name: parent', 'appVersion: 1.0.0', ''].join('\n'), + '/repo/chart/charts/sub/Chart.yaml': ['name: sub', 'appVersion: 2.0.0', ''].join('\n'), + }); + + // Relative to the parent chart this path reads `charts/sub/templates/...`, + // which no `templates/` rule would exclude. + await expect(resolveValuesFileContext('/repo/chart/charts/sub/templates/values.yaml', readTextFile)).resolves.toBeUndefined(); + }); + + it('should treat a standalone values.yaml with no chart above it as in scope with no chart', async () => { + const readTextFile = createFakeFileSystem({}); + + const context = await resolveValuesFileContext('/repo/loose/values.yaml', readTextFile); + + expect(context).toEqual({ chart: undefined }); + }); + + it('should ignore an unrelated yaml file with no chart above it', async () => { + const readTextFile = createFakeFileSystem({}); + + await expect(resolveValuesFileContext('/repo/loose/config.yaml', readTextFile)).resolves.toBeUndefined(); + }); + + describe('appVersion', () => { + it('should report no appVersion when the chart declares none', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': ['name: my-chart', 'version: 0.1.0', ''].join('\n') }); + + const context = await resolveValuesFileContext('/repo/chart/values.yaml', readTextFile); + + expect(context?.chart?.path).toBe('/repo/chart/Chart.yaml'); + expect(context?.chart?.appVersion).toBeUndefined(); + }); + + it('should read appVersion from raw source text, not the value YAML parsed it into', async () => { + const readTextFile = createFakeFileSystem({ '/repo/chart/Chart.yaml': ['name: my-chart', 'appVersion: 1.10', ''].join('\n') }); + + const context = await resolveValuesFileContext('/repo/chart/values.yaml', readTextFile); + + // YAML's core schema coerces the plain scalar 1.10 to the float 1.1. + expect(context?.chart?.appVersion).toBe('1.10'); + }); + + it('should report no appVersion when the declared one contains template syntax', async () => { + const readTextFile = createFakeFileSystem({ + '/repo/chart/Chart.yaml': ['name: my-chart', 'appVersion: "{{ .Chart.Version }}"', ''].join('\n'), + }); + + const context = await resolveValuesFileContext('/repo/chart/values.yaml', readTextFile); + + expect(context?.chart?.appVersion).toBeUndefined(); + }); + }); +}); + +describe('resolveTag', () => { + const chart: ChartMetadata = { path: '/repo/chart/Chart.yaml', appVersion: '1.2.3' }; + + it('should prefer the tag written in the file over the chart appVersion', () => { + const reference = createReference('9.9.9'); + + expect(resolveTag(reference, chart)).toEqual({ source: 'file', text: '9.9.9', range: reference.tag?.range }); + }); + + it('should fall back to the chart appVersion for a tagless reference', () => { + expect(resolveTag(createReference(undefined), chart)).toEqual({ + source: 'chart-metadata', + text: '1.2.3', + metadataPath: '/repo/chart/Chart.yaml', + }); + }); + + it('should resolve nothing for a tagless reference with no chart', () => { + expect(resolveTag(createReference(undefined), undefined)).toBeUndefined(); + }); + + it('should resolve nothing for a tagless reference under a chart that declares no appVersion', () => { + expect(resolveTag(createReference(undefined), { path: '/repo/chart/Chart.yaml', appVersion: undefined })).toBeUndefined(); + }); +}); diff --git a/packages/helm/src/chart-context.ts b/packages/helm/src/chart-context.ts new file mode 100644 index 0000000..3754ae8 --- /dev/null +++ b/packages/helm/src/chart-context.ts @@ -0,0 +1,139 @@ +import { parseDocument } from 'yaml'; +import type { ImageReference } from './extract-image-references'; +import { readUsableScalar, type SourceRange } from './raw-scalar'; + +/** Reads a file's text, or `undefined` when nothing readable sits at that path. */ +type ReadTextFile = (path: string) => Promise; + +/** The chart metadata governing a values file. */ +interface ChartMetadata { + /** Path of the metadata file itself, so a message can point the reader at it. */ + readonly path: string; + /** `appVersion` exactly as written, or `undefined` when the chart declares none. */ + readonly appVersion: string | undefined; +} + +/** A YAML file this package has something to say about, and the chart that governs it. */ +interface ValuesFileContext { + readonly chart: ChartMetadata | undefined; +} + +/** + * The tag a reference is checked against, and where it came from. The two + * arms differ in what they can offer a message: a tag written in the file + * has a range to underline, a tag taken from chart metadata has a file to + * point the reader at instead. + */ +type ResolvedTag = + | { readonly source: 'file'; readonly text: string; readonly range: SourceRange } + | { readonly source: 'chart-metadata'; readonly text: string; readonly metadataPath: string }; + +/** + * The one name chart metadata goes by. Helm's own loader recognises + * `Chart.yaml` and nothing else, so accepting a second spelling would + * resolve an `appVersion` out of a file Helm would ignore — the opposite of + * what this module exists to do. + */ +const CHART_METADATA_FILE_NAME = 'Chart.yaml'; + +/** The chart subdirectory holding Go templates, whose syntax yields nothing checkable. */ +const TEMPLATES_DIRECTORY_NAME = 'templates'; + +/** + * The names a values file goes by when no chart vouches for it — outside a + * chart directory the name is the only evidence there is. + */ +const STANDALONE_VALUES_FILE_NAME_PATTERN = /^values\.ya?ml$/i; + +/** + * Resolves the chart governing a YAML file, and with it whether this package + * has anything to say about that file at all. + * + * The nearest ancestor directory holding chart metadata wins, which is what + * makes a subchart's values file resolve against the subchart's own + * `appVersion` rather than its parent's — the subchart is what Helm would + * actually deploy. Every YAML file beneath a chart directory is in scope, + * not just the conventionally named ones, because environment overlays + * rarely carry the name `values.yaml`. + * + * A file under the chart's `templates/` directory is out of scope outright, + * even one named `values.yaml`. It is Go template source, and template + * syntax resolves at render time, which this package never does. So is the + * chart's own metadata file, which declares a chart rather than supplying + * values to one. + * + * With no chart anywhere above it, a file is in scope only when its own name + * says it is a values file — that keeps a standalone values file working + * while leaving unrelated workspace YAML alone. + * + * Paths are split on `/` here rather than handed to `node:path`. Callers pass + * a URI path, which is forward-slash separated on every platform, and + * keeping platform path semantics out of this package keeps it a pure + * source-text library. + */ +async function resolveValuesFileContext(path: string, readTextFile: ReadTextFile): Promise { + const segments = path.split('/'); + + for (let depth = segments.length - 1; depth > 0; depth -= 1) { + const chart = await readChartMetadata(segments.slice(0, depth), readTextFile); + + if (chart !== undefined) { + const isUnderTemplates = segments[depth] === TEMPLATES_DIRECTORY_NAME; + + return isUnderTemplates || chart.path === path ? undefined : { chart }; + } + } + + const fileName = segments[segments.length - 1]; + + return fileName !== undefined && STANDALONE_VALUES_FILE_NAME_PATTERN.test(fileName) ? { chart: undefined } : undefined; +} + +/** + * Reads the chart metadata sitting directly in one directory, if any. + * + * The candidate path is assembled by joining segments rather than by + * concatenating a directory and a name, so the filesystem root — whose + * segments are a lone empty string — yields `/Chart.yaml` instead of + * `//Chart.yaml`. + */ +async function readChartMetadata(directorySegments: readonly string[], readTextFile: ReadTextFile): Promise { + const metadataPath = [...directorySegments, CHART_METADATA_FILE_NAME].join('/'); + const text = await readTextFile(metadataPath); + + return text === undefined ? undefined : { path: metadataPath, appVersion: readAppVersion(text) }; +} + +/** + * Reads `appVersion` exactly as the chart wrote it. YAML coerces + * `appVersion: 1.10` to the float `1.1`, so a parsed value would report a + * version the chart never declared. An `appVersion` carrying Helm template + * syntax is no more usable than an absent one, and reads as `undefined` too. + */ +function readAppVersion(source: string): string | undefined { + // `getIn` hands back parsed values by default, and a parsed value is the + // one thing this package never reads a field from. + return readUsableScalar(source, parseDocument(source).getIn(['appVersion'], true))?.text; +} + +/** + * Resolves the tag a reference is checked against. A tag written in the file + * wins; a tagless reference falls back to the governing chart's + * `appVersion`, which is what Helm would render in its place. `undefined` + * means the reference is checked against nothing at all and must produce no + * marker. + */ +function resolveTag(reference: ImageReference, chart: ChartMetadata | undefined): ResolvedTag | undefined { + if (reference.tag !== undefined) { + return { source: 'file', text: reference.tag.text, range: reference.tag.range }; + } + + if (chart?.appVersion !== undefined) { + return { source: 'chart-metadata', text: chart.appVersion, metadataPath: chart.path }; + } + + return undefined; +} + +export { CHART_METADATA_FILE_NAME, resolveTag, resolveValuesFileContext }; +export type { ChartMetadata, ReadTextFile, ResolvedTag, ValuesFileContext }; diff --git a/packages/helm/src/extract-image-references.ts b/packages/helm/src/extract-image-references.ts index f2e9498..d32c1f7 100644 --- a/packages/helm/src/extract-image-references.ts +++ b/packages/helm/src/extract-image-references.ts @@ -1,21 +1,10 @@ import { isPair, isScalar, parseDocument, visit, type Document, type Pair } from 'yaml'; - -/** A half-open character range into the original source text. */ -interface SourceRange { - readonly start: number; - readonly end: number; -} - -/** A scalar's exact source text, never the value YAML parsed it into. */ -interface RawScalar { - readonly text: string; - readonly range: SourceRange; -} +import { readUsableScalar, type RawScalar } from './raw-scalar'; /** * 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. + * `undefined` for a tagless reference, which `resolveTag` resolves through + * the governing chart's `appVersion`. */ interface ImageReference { readonly repository: RawScalar; @@ -142,17 +131,6 @@ function readSiblingScalar(source: string, items: readonly Pair[], name: string) return pair === undefined ? undefined : readUsableScalar(source, pair.value); } -/** - * Reads a scalar a consumer can act on. Helm resolves template syntax at - * render time, and this package never renders, so a templated value is no - * more usable here than a missing one and reads as `undefined` too. - */ -function readUsableScalar(source: string, node: unknown): RawScalar | undefined { - const scalar = readRawScalar(source, node); - - return scalar === undefined || containsHelmTemplateSyntax(scalar.text) ? undefined : scalar; -} - /** Whether a mapping's parent key is `image` or ends in `Image`. */ function hasCorroboratingParentKey(path: readonly unknown[]): boolean { const parent = path[path.length - 1]; @@ -166,61 +144,5 @@ 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 - * plain and single/double-quoted scalars carry a usable range here; a - * missing value, or a non-scalar value (a nested mapping or sequence), - * yields `undefined` so the caller skips the candidate entirely. - */ -function readRawScalar(source: string, node: unknown): RawScalar | undefined { - if (!isScalar(node)) { - return undefined; - } - - const range = node.range; - - if (!range) { - return 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); - - return { - text, - range: { start: start + offset, end: start + offset + text.length }, - }; -} - -// A quote pair is the shortest string that can carry one: two characters, -// one at each end. -const MIN_QUOTED_LENGTH = 2; - -/** Strips one layer of matching single or double quotes, if present. */ -function stripQuotes(raw: string): { text: string; offset: number } { - const first = raw[0]; - const last = raw[raw.length - 1]; - - if (raw.length >= MIN_QUOTED_LENGTH && first === last && (first === '"' || first === "'")) { - return { text: raw.slice(1, raw.length - 1), offset: 1 }; - } - - return { text: raw, offset: 0 }; -} - export { extractImageReferences }; -export type { ImageReference, RawScalar, SourceRange }; +export type { ImageReference }; diff --git a/packages/helm/src/index.ts b/packages/helm/src/index.ts index dc28aa5..fbc0f1a 100644 --- a/packages/helm/src/index.ts +++ b/packages/helm/src/index.ts @@ -1,2 +1,5 @@ +export { CHART_METADATA_FILE_NAME, resolveTag, resolveValuesFileContext } from './chart-context'; +export type { ChartMetadata, ReadTextFile, ResolvedTag, ValuesFileContext } from './chart-context'; export { extractImageReferences } from './extract-image-references'; -export type { ImageReference, RawScalar, SourceRange } from './extract-image-references'; +export type { ImageReference } from './extract-image-references'; +export type { RawScalar, SourceRange } from './raw-scalar'; diff --git a/packages/helm/src/raw-scalar.ts b/packages/helm/src/raw-scalar.ts new file mode 100644 index 0000000..f97bced --- /dev/null +++ b/packages/helm/src/raw-scalar.ts @@ -0,0 +1,83 @@ +import { isScalar } from 'yaml'; + +/** A half-open character range into the original source text. */ +interface SourceRange { + readonly start: number; + readonly end: number; +} + +/** A scalar's exact source text, never the value YAML parsed it into. */ +interface RawScalar { + readonly text: string; + readonly range: SourceRange; +} + +/** + * Reads a scalar a consumer can act on. Helm resolves template syntax at + * render time, and this package never renders, so a templated value is no + * more usable here than a missing one and reads as `undefined` too. + */ +function readUsableScalar(source: string, node: unknown): RawScalar | undefined { + const scalar = readRawScalar(source, node); + + return scalar === undefined || containsHelmTemplateSyntax(scalar.text) ? undefined : scalar; +} + +/** 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 + * plain and single/double-quoted scalars carry a usable range here; a + * missing value, or a non-scalar value (a nested mapping or sequence), + * yields `undefined` so the caller skips the candidate entirely. + */ +function readRawScalar(source: string, node: unknown): RawScalar | undefined { + if (!isScalar(node)) { + return undefined; + } + + const range = node.range; + + if (!range) { + return 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); + + return { + text, + range: { start: start + offset, end: start + offset + text.length }, + }; +} + +// A quote pair is the shortest string that can carry one: two characters, +// one at each end. +const MIN_QUOTED_LENGTH = 2; + +/** Strips one layer of matching single or double quotes, if present. */ +function stripQuotes(raw: string): { text: string; offset: number } { + const first = raw[0]; + const last = raw[raw.length - 1]; + + if (raw.length >= MIN_QUOTED_LENGTH && first === last && (first === '"' || first === "'")) { + return { text: raw.slice(1, raw.length - 1), offset: 1 }; + } + + return { text: raw, offset: 0 }; +} + +export { readUsableScalar }; +export type { RawScalar, SourceRange };