From 5d75c3e81df1bb97b6609c76733bb1d1e7d9add6 Mon Sep 17 00:00:00 2001 From: CarmenDou <15951653662@163.com> Date: Wed, 7 Oct 2026 16:42:44 -0700 Subject: [PATCH 1/2] fix(template): a health check path is optional, and draft and info show it The platform stopped requiring a health check path on a web service in #600, but the local manifest check still refused a web service without one, and it accepted any given path that began with a slash. A web service may now omit the path, and a given path is checked with the platform's HEALTHCHECK_RE, so //host is refused here as the platform refuses it. A declared empty or non-string path stays refused, and so does any health check on a worker. insta template draft and insta template info now print a web service's health check in their human output: health check /healthz, or no health check. A worker or a managed service prints nothing about it. --json is unchanged. --- src/commands/template-author.ts | 4 +- src/commands/template.ts | 7 +++- src/template-manifest.ts | 11 ++++- test/template-author.test.ts | 22 +++++++++- test/template.test.ts | 73 ++++++++++++++++++++++++++++----- 5 files changed, 100 insertions(+), 17 deletions(-) diff --git a/src/commands/template-author.ts b/src/commands/template-author.ts index 0105d45..b887da5 100644 --- a/src/commands/template-author.ts +++ b/src/commands/template-author.ts @@ -15,7 +15,7 @@ export type TemplateDraftVariable = { name: string; kind: string; value?: string; description?: string; brokenReference?: string; originalName?: string } export type TemplateDraftService = { - name: string; type: string; image: string | null; port: number | null; volume: boolean; mountPath: string | null + name: string; type: string; image: string | null; port: number | null; healthcheck?: string | null; volume: boolean; mountPath: string | null public?: boolean; pgVersion?: number; removed: boolean; variables: TemplateDraftVariable[] } export type TemplateDraft = { @@ -100,6 +100,8 @@ function serviceText(s: TemplateDraftService): string { const bits = [ kind, s.port != null ? `port ${s.port}` : undefined, + // Only a web service has one, and a platform that does not serve the field says nothing. + s.type === 'web' && s.healthcheck !== undefined ? (s.healthcheck ? `health check ${s.healthcheck}` : 'no health check') : undefined, s.image ? `image ${s.image}` : undefined, // A null mount path on a volume is the default /data. s.volume ? `volume at ${s.mountPath ?? '/data'}` : undefined, diff --git a/src/commands/template.ts b/src/commands/template.ts index bf6f7e9..b8bb9ae 100644 --- a/src/commands/template.ts +++ b/src/commands/template.ts @@ -42,12 +42,13 @@ export function templateListLines(templates: TemplateIndexEntry[]): string[] { // `volume` is the boolean a manifest declares now; `volumeGib` is a size a registry published // before sizing moved to the platform. Both are read: the catalog serves whichever the row carries, // and dropping the size on its own would quietly stop saying the service HAS a disk. -type InfoService = { name: string; type?: string; port?: number; volumeGib?: number; volume?: boolean; mountPath?: string; pgVersion?: number; public?: boolean } +type InfoService = { name: string; type?: string; port?: number; healthcheck?: string; volumeGib?: number; volume?: boolean; mountPath?: string; pgVersion?: number; public?: boolean } // The info endpoint may list services as an array or keep the manifest's map shape — render both. export function normalizeInfoServices(raw: unknown): InfoService[] { const one = (name: string, s: any): InfoService => ({ name, type: s?.type, port: s?.port, + healthcheck: typeof s?.healthcheck === 'string' ? s.healthcheck : undefined, volumeGib: s?.volumeGib ?? s?.volume?.size, volume: s?.volume === true || s?.volumeGib != null || s?.volume?.size != null, mountPath: typeof s?.mountPath === 'string' ? s.mountPath : undefined, @@ -112,7 +113,9 @@ export function templateInfoLines(t: TemplateInfo, bold: (s: string) => string = // A size only when the registry still carries one: a manifest names no size any more, so // "persistent /data" is all there is to say until the service exists. const disk = s.volumeGib ? `${s.volumeGib}Gi volume` : s.volume ? `persistent ${s.mountPath ?? '/data'}` : undefined - const bits = [infoKind(s), s.port ? `port ${s.port}` : undefined, disk].filter(Boolean) + // Only a web service has a path to probe, and a manifest that names none has no health check. + const health = s.type === 'web' ? (s.healthcheck ? `health check ${s.healthcheck}` : 'no health check') : undefined + const bits = [infoKind(s), s.port ? `port ${s.port}` : undefined, health, disk].filter(Boolean) return `${s.name}${bits.length ? ` (${bits.join(', ')})` : ''}` }) lines.push(`services (${services.length}): ${summary.join(', ')}`) diff --git a/src/template-manifest.ts b/src/template-manifest.ts index 90c8794..e8a0c2d 100644 --- a/src/template-manifest.ts +++ b/src/template-manifest.ts @@ -65,6 +65,8 @@ export type TemplateVar = { const CODE_RE = /^[a-z0-9][a-z0-9-]{0,38}$/ export const ENV_NAME_RE = /^[A-Z][A-Z0-9_]{0,63}$/ const GENERATOR_RE = /^secret:([1-9]\d{0,2})$/ +// The platform's HEALTHCHECK_RE (src/provisioning/templateManifest.ts), verbatim. +const HEALTHCHECK_RE = /^\/(?!\/)[A-Za-z0-9\-._~!$&'()*+,;=:@%/?]*$/ // The compute vocabulary every CLI has known. Any other type is the platform's to accept or refuse. const COMPUTE_TYPES = ['web', 'worker'] @@ -154,8 +156,13 @@ export function validateManifest(m: TemplateManifest): string[] { if (svc.healthcheck !== undefined) problems.push(`${where}.healthcheck: a worker has no HTTP endpoint to probe — its health is the machine's state; remove it (or declare type: web)`) if (svc.alwaysOn === false) problems.push(`${where}.alwaysOn: a worker cannot scale to zero — nothing is routed to it, so nothing would wake it; remove alwaysOn or set it true`) } - if (svc.type === 'web' && !svc.healthcheck) problems.push(`${where}: web services must declare a healthcheck path`) - if (svc.healthcheck && !String(svc.healthcheck).startsWith('/')) problems.push(`${where}: healthcheck must be an absolute path (start with /)`) + // Optional on a web service, and a given path must pass the platform's grammar. + if (svc.healthcheck !== undefined) { + const healthcheck = scalarString(svc.healthcheck) + if (healthcheck === null) problems.push(`${where}.healthcheck must be a string`) + else if (!healthcheck.startsWith('/')) problems.push(`${where}: healthcheck must be an absolute path (start with /)`) + else if (!HEALTHCHECK_RE.test(healthcheck)) problems.push(`${where}: healthcheck must be a single-slash absolute path on the service itself (no '//host', scheme, backslash or control characters), got: ${healthcheck}`) + } // Sizing is the platform's, capped for the org's plan. Same answers the // publish endpoint gives, said here so an author does not upload to find out. Read as unknown: // the type above admits only what is SUPPORTED, and the document is a cast over YAML.parse, so diff --git a/test/template-author.test.ts b/test/template-author.test.ts index eddf9c3..1a49305 100644 --- a/test/template-author.test.ts +++ b/test/template-author.test.ts @@ -137,7 +137,7 @@ describe('draft rendering', () => { ' readme 2 bytes', ` updated ${UPDATED}`, 'services (4):', - ' app (web, port 8080, image ghcr.io/acme/app:1.2, volume at /data)', + ' app (web, port 8080, health check /health, image ghcr.io/acme/app:1.2, volume at /data)', ' BASE_URL required', ' DATABASE_URL reference ${{services.db.DATABASE_URL}}', ' SESSION_SECRET generated', @@ -190,6 +190,26 @@ describe('template draft', () => { await templateDraft('my-app', { json: true }, { api, project: linked() }) expect(JSON.parse(printed())).toEqual({ template, editorUrl: EDITOR }) }) + it('says each web service health check, or that it has none, and nothing for a worker or a datastore', async () => { + const svc = (over: Record) => ({ image: null, port: null, volume: false, mountPath: null, removed: false, variables: [], ...over }) + const template = view({ services: [ + svc({ name: 'site', type: 'web', port: 80, healthcheck: '/healthz' }), + svc({ name: 'root', type: 'web', port: 81, healthcheck: '/' }), + svc({ name: 'bare', type: 'web', port: 82, healthcheck: null }), + svc({ name: 'old', type: 'web', port: 83 }), + svc({ name: 'queue', type: 'worker', healthcheck: '/ignored' }), + svc({ name: 'db', type: 'postgres', pgVersion: 16, healthcheck: null }), + ] }) + const { api } = platform({ [`GET ${DRAFT_PATH}`]: () => ({ body: { template } }) }) + await templateDraft('my-app', {}, { api, project: linked() }) + const lines = printed().split('\n') + expect(lines).toContain(' site (web, port 80, health check /healthz)') + expect(lines).toContain(' root (web, port 81, health check /)') + expect(lines).toContain(' bare (web, port 82, no health check)') + expect(lines).toContain(' old (web, port 83)') + expect(lines).toContain(' queue (worker)') + expect(lines).toContain(' db (postgres 16)') + }) it('gives no link on a host without the api label', async () => { const { api } = platform({ [`GET ${DRAFT_PATH}`]: () => ({ body: { template: view() } }) }, 'http://localhost:7070') await templateDraft('my-app', { json: true }, { api, project: linked() }) diff --git a/test/template.test.ts b/test/template.test.ts index 00bc892..0f82b33 100644 --- a/test/template.test.ts +++ b/test/template.test.ts @@ -227,11 +227,40 @@ describe('validateManifest', () => { } expect(validateManifest(m)).toEqual([]) }) - it('requires web services to declare an absolute healthcheck path', () => { + // The platform made the path optional (#600), so a web service without one is accepted. + it('accepts a web service without a healthcheck path, and refuses a relative one', () => { const m: TemplateManifest = { code: 'x', version: '1', services: { a: { type: 'web', image: 'a:1' }, b: { type: 'web', image: 'b:1', healthcheck: 'health' } } } - const problems = validateManifest(m) - expect(problems).toContain('services.a: web services must declare a healthcheck path') - expect(problems).toContain('services.b: healthcheck must be an absolute path (start with /)') + expect(validateManifest(m)).toEqual(['services.b: healthcheck must be an absolute path (start with /)']) + }) + // The grammar is the platform's HEALTHCHECK_RE: one leading slash, then a path and query charset. + it('checks a given healthcheck path with the platform grammar', () => { + const web = (healthcheck: unknown) => + ({ code: 'x', version: '1', services: { app: { type: 'web', image: 'a:1', healthcheck } } }) as unknown as TemplateManifest + for (const ok of ['/', '/healthz', '/api/health?full=1', '/a/b-c_d.e~f%20g']) expect(validateManifest(web(ok)), ok).toEqual([]) + for (const bad of ['//evil.example', '/x\\y', '/a b', '/a#b', '/a\nb']) { + expect(validateManifest(web(bad)).join('\n'), JSON.stringify(bad)).toMatch(/^services\.app: healthcheck must be a single-slash absolute path/) + } + expect(validateManifest(web('http://x/y'))).toEqual(['services.app: healthcheck must be an absolute path (start with /)']) + expect(validateManifest(web('//evil.example'))).toEqual([ + "services.app: healthcheck must be a single-slash absolute path on the service itself (no '//host', scheme, backslash or control characters), got: //evil.example", + ]) + }) + // Declared but empty or not a string is not "none", and the platform refuses it. + it('refuses an empty or non-string healthcheck on a web service', () => { + const web = (healthcheck: unknown) => + ({ code: 'x', version: '1', services: { app: { type: 'web', image: 'a:1', healthcheck } } }) as unknown as TemplateManifest + expect(validateManifest(web(''))).toEqual(['services.app: healthcheck must be an absolute path (start with /)']) + for (const odd of [null, {}, ['/']]) expect(validateManifest(web(odd)), JSON.stringify(odd)).toEqual(['services.app.healthcheck must be a string']) + }) + it('still refuses a healthcheck on a worker, with or without a valid path', () => { + const worker = (healthcheck: string) => + ({ code: 'x', version: '1', services: { bg: { type: 'worker', image: 'a:1', healthcheck } } }) as unknown as TemplateManifest + expect(validateManifest(worker('/healthz')).join('\n')).toMatch(/a worker has no HTTP endpoint/) + expect(validateManifest(worker('//evil.example')).join('\n')).toMatch(/a worker has no HTTP endpoint/) + }) + it('parses YAML for a web service with no healthcheck path', () => { + const m = parseManifestYaml(['code: demo', 'version: "1.0"', 'services:', ' app:', ' type: web', ' image: nginx:1.27'].join('\n')) + expect(m.services?.app?.healthcheck).toBeUndefined() }) it('requires a description on required vars unless a generator answers for the user', () => { const m: TemplateManifest = { @@ -392,7 +421,7 @@ describe('templateInfoLines', () => { maintainer: 'insforge', source: 'github.com/plausible/community-edition', upstream: { pinned: 'ghcr.io/plausible/community-edition:v2.1.1' }, services: [ - { name: 'app', type: 'web', port: 8000 }, + { name: 'app', type: 'web', port: 8000, healthcheck: '/api/health' }, { name: 'worker', type: 'worker', volumeGib: 10 }, ], variables: { @@ -408,11 +437,11 @@ describe('templateInfoLines', () => { // that half of the line the moment the catalog is republished. it('says a boolean-volume service has a disk, and still prints a size when the registry has one', () => { const boolTpl = { ...tpl, services: [{ name: 'agent', type: 'web', port: 7681, volume: true }] } - expect(templateInfoLines(boolTpl)).toContain('services (1): agent (web, port 7681, persistent /data)') + expect(templateInfoLines(boolTpl)).toContain('services (1): agent (web, port 7681, no health check, persistent /data)') const sizedTpl = { ...tpl, services: [{ name: 'agent', type: 'web', port: 7681, volumeGib: 10 }] } - expect(templateInfoLines(sizedTpl)).toContain('services (1): agent (web, port 7681, 10Gi volume)') + expect(templateInfoLines(sizedTpl)).toContain('services (1): agent (web, port 7681, no health check, 10Gi volume)') const customMountTpl = { ...tpl, services: [{ name: 'agent', type: 'web', port: 7681, volume: true, mountPath: '/app/storage' }] } - expect(templateInfoLines(customMountTpl)).toContain('services (1): agent (web, port 7681, persistent /app/storage)') + expect(templateInfoLines(customMountTpl)).toContain('services (1): agent (web, port 7681, no health check, persistent /app/storage)') }) it('says the Postgres version and whether a bucket is public', () => { @@ -426,12 +455,34 @@ describe('templateInfoLines', () => { }, } expect(templateInfoLines(t)).toContain( - 'services (4): app (web, port 8000), db (postgres 17), assets (storage, public), scratch (storage, private)', + 'services (4): app (web, port 8000, no health check), db (postgres 17), assets (storage, public), scratch (storage, private)', ) }) it('names a postgres with no version plainly, and ignores pgVersion and public on other types', () => { const t = { ...tpl, services: { db: { type: 'postgres' }, app: { type: 'web', port: 80, pgVersion: 17, public: true } } } - expect(templateInfoLines(t)).toContain('services (2): db (postgres), app (web, port 80)') + expect(templateInfoLines(t)).toContain('services (2): db (postgres), app (web, port 80, no health check)') + }) + it('says a web service health check, or that it has none, and nothing for any other type', () => { + const t = { + ...tpl, + services: { + site: { type: 'web', port: 80, healthcheck: '/healthz' }, + root: { type: 'web', port: 81, healthcheck: '/' }, + bare: { type: 'web', port: 82 }, + queue: { type: 'worker', healthcheck: '/ignored' }, + db: { type: 'postgres', healthcheck: '/ignored' }, + }, + } + expect(templateInfoLines(t)).toContain( + 'services (5): site (web, port 80, health check /healthz), root (web, port 81, health check /), bare (web, port 82, no health check), queue (worker), db (postgres)', + ) + }) + it('carries a healthcheck only when it is a string', () => { + expect(normalizeInfoServices({ a: { type: 'web', healthcheck: '/healthz' }, b: { type: 'web', healthcheck: 5 }, c: { type: 'web' } })).toEqual([ + { name: 'a', type: 'web', healthcheck: '/healthz', volume: false }, + { name: 'b', type: 'web', volume: false }, + { name: 'c', type: 'web', volume: false }, + ]) }) it('carries pgVersion and public only when they are the right kind of value', () => { expect(normalizeInfoServices({ @@ -450,7 +501,7 @@ describe('templateInfoLines', () => { expect(lines).toContain(' version 2.1.1') expect(lines).toContain(' source github.com/plausible/community-edition') expect(lines).toContain(' upstream ghcr.io/plausible/community-edition:v2.1.1') - expect(lines).toContain('services (2): app (web, port 8000), worker (worker, 10Gi volume)') + expect(lines).toContain('services (2): app (web, port 8000, health check /api/health), worker (worker, 10Gi volume)') const text = lines.join('\n') expect(text).toMatch(/required:\n\s+BASE_URL\s+public URL\n\s+ADMIN_PWD\s+\(generated: secret:32\)/) expect(text).toMatch(/optional:\n\s+SMTP_HOST\s+SMTP relay \(default: localhost\)/) From 2f70a6ce20e3177cc17fc7870914af01891a78dc Mon Sep 17 00:00:00 2001 From: CarmenDou <15951653662@163.com> Date: Wed, 7 Oct 2026 17:02:34 -0700 Subject: [PATCH 2/2] fix(template): a worker with a health check gets one problem, not two The path grammar check also ran on a worker, so a worker with a health check such as //x got the worker refusal plus a second line telling the author to fix a path they should remove. The grammar check now skips a worker, so it only gets the refusal. --- src/template-manifest.ts | 2 +- test/template.test.ts | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/template-manifest.ts b/src/template-manifest.ts index e8a0c2d..4fa0074 100644 --- a/src/template-manifest.ts +++ b/src/template-manifest.ts @@ -157,7 +157,7 @@ export function validateManifest(m: TemplateManifest): string[] { if (svc.alwaysOn === false) problems.push(`${where}.alwaysOn: a worker cannot scale to zero — nothing is routed to it, so nothing would wake it; remove alwaysOn or set it true`) } // Optional on a web service, and a given path must pass the platform's grammar. - if (svc.healthcheck !== undefined) { + if (svc.type !== 'worker' && svc.healthcheck !== undefined) { const healthcheck = scalarString(svc.healthcheck) if (healthcheck === null) problems.push(`${where}.healthcheck must be a string`) else if (!healthcheck.startsWith('/')) problems.push(`${where}: healthcheck must be an absolute path (start with /)`) diff --git a/test/template.test.ts b/test/template.test.ts index 0f82b33..5fcd10a 100644 --- a/test/template.test.ts +++ b/test/template.test.ts @@ -252,11 +252,15 @@ describe('validateManifest', () => { expect(validateManifest(web(''))).toEqual(['services.app: healthcheck must be an absolute path (start with /)']) for (const odd of [null, {}, ['/']]) expect(validateManifest(web(odd)), JSON.stringify(odd)).toEqual(['services.app.healthcheck must be a string']) }) - it('still refuses a healthcheck on a worker, with or without a valid path', () => { - const worker = (healthcheck: string) => + // A worker is told to remove the path, so it gets no second problem about the path grammar. + it('refuses a healthcheck on a worker once, whatever the value', () => { + const worker = (healthcheck: unknown) => ({ code: 'x', version: '1', services: { bg: { type: 'worker', image: 'a:1', healthcheck } } }) as unknown as TemplateManifest - expect(validateManifest(worker('/healthz')).join('\n')).toMatch(/a worker has no HTTP endpoint/) - expect(validateManifest(worker('//evil.example')).join('\n')).toMatch(/a worker has no HTTP endpoint/) + for (const value of ['/healthz', '//x', '//evil.example', 'health', '', null]) { + const problems = validateManifest(worker(value)) + expect(problems, JSON.stringify(value)).toHaveLength(1) + expect(problems[0], JSON.stringify(value)).toMatch(/^services\.bg\.healthcheck: a worker has no HTTP endpoint/) + } }) it('parses YAML for a web service with no healthcheck path', () => { const m = parseManifestYaml(['code: demo', 'version: "1.0"', 'services:', ' app:', ' type: web', ' image: nginx:1.27'].join('\n'))