fix(template): a health check path is optional, and draft and info show it

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,

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(', ')}`)

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.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 /)`)
+      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

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<string, unknown>) => ({ 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() })

test/template.test.ts

@@ -227,11 +227,44 @@ 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'])
+  })
+  // 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
+    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'))
+    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 +425,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 +441,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 +459,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 +505,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\)/)