feat(schema): generate config JSON Schema, folded into docgen

[viniciusdc]

Supersedes #507, which carried the same work against main and predates the docgen tool.

Rebased onto main (c786e05) on 2026-09-03, picking up #642, #655 and #659. The two original commits are followed by four addressing @dcmcand's review: the drift-gate correction, the filtered-run reporting fix, the ADR addendum trim, and the govulncheck pin (split out so a bisect hits the tool bump, not the schema emitter).

What

Generates JSON Schema documents for nebari-config.yaml and every registered provider's Config struct, straight from the Go types, and commits them under schemas/ for the docs site to consume.

Rather than ship a second generator with its own make target and its own CI gate, this folds the pipeline into cmd/docgen - the tool #567 established for everything NIC derives from Go source. One tool, one make docs, one drift gate.

Lineage: #362 built this against the old pkg/provider/ (singular) layout and was superseded by #507, which re-homed it onto the current pkg/providers/{cluster,dns}/ split. This PR folds #507's standalone generator into docgen.

Changes

The schema pipeline

• ConfigTyped, an optional capability (ConfigType() reflect.Type) that a cluster, DNS, or repository provider may implement, discovered by type assertion rather than added to the Provider interface - so out-of-tree providers (ADR-0004) are unaffected. One line per registered provider.
• pkg/configschema wraps invopop/jsonschema: reads yaml tags, package-qualifies $defs keys (so aws.Config and longhorn.Config don't merge), closes objects, describes inline provider maps (InlineMaps), and pulls field descriptions from godoc. Absolute PackagePaths are refused rather than silently yielding description-free schemas.
• pkg/nic/config_types.go exposes RegisteredConfigTypes through the registry, so the generator enumerates providers with no hard-coded list. TestRegisteredProvidersImplementConfigTyped fails if a registered provider skips the capability, and TestConfigTypesCoversEveryRegistryCategory compares the number of category maps against registry.Registry's ProviderList fields - so a future category can't go undocumented the way the repository category did. A category missing from categoryTable (or from RegisteredConfigTypes) emits no schema, and CI cannot see the gap. The drift gate does catch a generated file that disappears - it regenerates and diffs against what is committed - but only for a category that was emitted at least once, leaving a tracked file to go missing. A category that was never walked has no committed schema to diff against at all, which is why counting is the check.
• schemas/ committed in-tree: manifest.json ({providers, dns, repository, top_level}), nebari-config.json, providers/<name>.json. Cluster and DNS providers keep bare filenames so the docs site's URLs stay valid; other categories are qualified on disk (repository-local.json), because cluster/local and repository/local share a name and one would otherwise silently overwrite the other.

Folded into docgen

• cmd/schemagen/main.go -> cmd/docgen/schema.go, a third emitter beside the config-markdown and CLI-markdown ones. Flags namespaced: -schema-output, -schema-providers, -schema-version.
• make schemas folds into make docs, which cleans and regenerates docs/configuration/, docs/reference/cli/ and schemas/ in one pass.
• No standalone schema workflow. feat(schema): generate config JSON Schema from the provider registry #507 carried a .github/workflows/schemas.yml with its own drift check and a 67-line paths filter - a second list to keep in sync with the files the generator reads. That never lands: the drift check is the ci.yml step docs: config + CLI reference generation (docgen) #567 added, widened to cover schemas/. (Correction from review: since that workflow only ever existed on the superseded feat(schema): generate config JSON Schema from the provider registry #507, this diff deletes nothing - it just doesn't add it.) After the rebase that check lives in Move verify docs steps in CI to its own job #655's standalone verify-docs job, so it runs in seconds with no cluster and no dependency on deploy-and-journeys; this PR widens it to cover schemas/, and fixes it to use git add -A + git diff --cached so a generated file that disappears is caught (git add -N stages the deletion, and the following unstaged diff then has nothing to report).
• cmd/docgen/schema_test.go added, matching the per-file coverage the rest of the tool has.

Bugs the merge surfaced

All of these were invisible while the generators were separate, which is most of the argument for merging them.

The markdown reference was silently losing enum values. Moving the allowed values for aws.Taint.Effect and azure.NodeGroup.Mode out of trailing comments and into jsonschema tags gave the schema an enum and left the corresponding cell in docs/configuration/aws.md blank - the markdown emitter never read that tag. It does now, rendering enum/default and appending them after description truncation so a constraint can never be the part that gets cut. Prose that restated a tagged default is trimmed, leaving the tag as the single declaration.

Every godoc description had vanished from the schemas. AddGoComments uses the path it is handed as both the directory to walk and the import-path suffix it matches comments against (gopath.Join(base, dir)). Hand it an absolute path and every schema still generates, still validates, and carries no descriptions at all. Package paths are now module-relative with the working directory pinned to the module root for that step, and TestGenerateSchemasIncludesGodocDescriptions asserts a description actually lands.

The published schemas rejected every config they describe. cluster, dns and repository each hold their providers in a single yaml:",inline" map. invopop reports such a field as embedded and then declines to walk it because it is not a struct, so all three reflected as objects with no properties - and additionalProperties: false turned that into a schema nothing can satisfy: 9 of 9 examples failed. pkg/configschema grows an InlineMaps option and docgen supplies the provider names from the registry, so the three wrappers now state what the validator actually enforces (one registered provider, no more than one). Found in review.

Two repository fields were marked required though NIC defaults them at runtime. required is inferred from a yaml tag without ,omitempty, and repository/local's path/branch plus repository/existing's path/branch are all filled in during Provision - so the schema rejected repository: {local: {}}, which ships in examples/local-config.yaml. Found in review.

A default= tag had already drifted from the const it duplicates. AWSLoadBalancerControllerConfig.ChartVersion was tagged default=3.2.1 while defaultLBCChartVersion is 3.4.3 - and both generators publish the tag, so the wrong default would have shipped to every reader of the reference and the schema. Corrected, and TestChartVersionSchemaDefaultMatchesConst now compares the tag against the const, since nothing else in the pipeline relates them.

Docs updated alongside

AGENTS.md gains the ConfigTyped + make docs step in all three provider checklists (the enforcing test always covered all three) and a warning that a yaml tag without ,omitempty publishes a schema that rejects valid configs. ADR-0005's update section asserted that this pipeline "did not ship", which is no longer true; an addendum records what shipped, keeps the still-accurate half (cmd/schemagen does not exist and will not), and carries the one-extraction-path deferral.

Docs consumption

The docs site renders these schemas live - see the config-schema reference PoC and its Netlify preview (nebari-docs PR: https://redirect.github.com/nebari-dev/nebari-docs/pull/710). It currently tracks this branch via raw.githubusercontent.com; once this lands on a tagged release the docs pin to the tag.

Also in here

govulncheck v1.4.0 -> v1.6.0 (ci.yml + scripts/govulncheck-gate.sh). Not a drive-by: v1.4.0 cannot scan this branch. It vendors x/tools v0.46.0, whose ssautil.AllFunctions panics with ForEachElement called on type containing *types.TypeParam on the generics reached through pkg/configschema's jsonschema dependency. The scan aborts with exit 2 before emitting any JSON, so scripts/govulncheck-gate.sh never runs at all.

v1.4.0  main         ./...                   exit 0   clean
v1.4.0  this branch  ./...                   exit 2   panic
v1.4.0  this branch  ./pkg/configschema/...  exit 2   panic  (the trigger)
v1.4.0  this branch  ./cmd/docgen/...        exit 2   panic  (imports it)
v1.4.0  this branch  ./pkg/nic/...           exit 0   clean  (does not)
v1.6.0  this branch  gate script             exit 0   6 advisories, verdict OK

#659 cleared the gate on main with v1.4.0, which is why this looked redundant - but main has no pkg/configschema. The ci.yml step carries this reasoning as a comment, and the bump is its own commit.

The guard that was missing

All three of the schema bugs above share one root cause: nothing compared the generated schemas to a real config. They are legal JSON Schema documents whether or not they describe what NIC accepts, so the drift gate passes on a schema that rejects everything.

TestExampleConfigsMatchGeneratedSchemas closes that: every file under examples/ is validated against nebari-config.json, and each provider block against that provider's schema. Run against the schemas as they were, it fails on all nine examples.

Landing it required completing examples/custom-tls-config.yaml, which had no node_groups though the AWS provider requires them - a pre-existing gap nic validate cannot catch, because cluster-provider validation is not reached on that path.

Test plan

• go build ./..., go vet ./..., gofmt clean
• go test -short ./... passes (23 packages)
• make docs is deterministic - regenerating on a clean tree is byte-identical, and the drift gate passes locally after the rebase onto main
• Regenerated output picks up main's Longhorn instance_manager_cpu_percent with no code change - registry-driven discovery working
• go run ./cmd/docgen -schema-providers aws narrows correctly and leaves the manifest and top-level schema untouched
• Full CI (Build, Test, Lint, Vulnerabilities, Deploy, workflow pins) - now runs, since the PR targets main

Deferred

Broader examples/default struct tags and provider godoc coverage - most provider fields still have no description; the schema surfaces them the moment the godoc lands. Publishing schemas/ per release tag plus a schema-versions.json so the docs version toggle can list real versions.

The two emitters still extract independently - go/ast for markdown, reflection for JSON Schema. Rendering the markdown config reference from the schema would collapse them to one extraction path and retire docgen's configFiles allowlist, which is what produced both blockers on #567. Worth doing now that both have landed.

[dcmcand]

Summary

Reviewed at bbc9ca0. I'd started this review at 9707118 and you pushed while I was verifying, so everything below was re-checked against the current head - including the findings that are now fixed.

The design is right and the tooling work is good: folding schema generation into cmd/docgen rather than shipping a second binary is the correct call, registry-driven discovery genuinely works with no hard-coded provider list, and the two silent-omission guards are well built. The three boundary rules in AGENTS.md "Abstraction Boundaries" all hold - cmd/ imports no concrete provider package, pkg/config gains no provider knowledge, and ConfigTyped follows the optional-capability pattern already used by backupRoleARNResolver and the repository.Source variants. No architectural blockers.

What blocks it is the output rather than the pipeline: the primary artifact this PR exists to publish does not validate the configs it describes.

Blockers

1. schemas/nebari-config.json rejects all 9 examples/*.yaml. cluster, dns and repository reflect as closed empty objects, because invopop does not expand a yaml:",inline" field of map kind and AllowAdditionalProperties: false then closes them. cluster + repository break all nine; dns breaks the one example with a dns: block. Inline on pkg/configschema/configschema.go.
2. required inferred from missing ,omitempty rejects configs NIC accepts. repository: {local: {}} - shipped in examples/local-config.yaml, accepted by nic validate at this commit - fails repository-local.json. repository-existing.json requires path while its own generated description calls it optional, and likewise rejects a path-less config the binary accepts. Two inline comments.
3. docs/adr/0005-nic-config-cli-surface.md:140-144 now asserts something false. Its Update section says, in the present indicative, that "the cmd/schemagen / schemas/ pipeline described in the Context did not ship" and that NIC emits finished markdown "rather than NIC emitting JSON Schema for nebari-docs to render into markdown downstream". At this commit schemas/ ships and nebari-docs#710 renders it. To be precise about scope: the cmd/schemagen half is still true (no such binary exists), and the next bullet's "not a second binary" prescription is satisfied by this PR - it's the outcome claim that is now wrong. That file isn't in the diff so I can't anchor a comment to it; an addendum recording what actually shipped would close it, and it's the natural home for the deferral in Question A below.
4. Checklist parity in AGENTS.md - the ConfigTyped step is documented for cluster providers only, while the enforcing test covers DNS and repository too. Inline.

Blockers 1 and 2 share one fix shape and one missing test: validate examples/*.yaml against the generated schemas in CI. That single test would have caught both, plus the chart_version default you just fixed. pkg/nic/examples_test.go already exists on exactly that argument ("the examples are the documented starting point for users, so a schema change that leaves them behind is a user-visible break"), and no test currently references schemas/.

Fixed since my first pass - all re-verified, thank you: chart_version now correctly declares 3.4.3 in the tag, the markdown page and aws.json; schemas/README.md now documents the repository key and the group-qualified filename rule; the six orphaned // Name returns the provider name comments are separated from ConfigType; both cmd/schemagen references now say cmd/docgen; and the configschema package doc no longer claims YAML is supported. The filepath.Abs(outDir) fix and the unexported-field skip in the parser are both good catches.

Questions

A. Two extraction paths over the same structs. cmd/docgen/schema.go is explicit that the schema emitter shares only the project root with the go/ast emitters. ADR-0005:150-158 decided the letter - "a docgen output format, not a second binary" - and this PR satisfies it. But the rationale was avoiding two extractors that can drift, and this PR had to fix exactly that bug class (markdown silently dropping jsonschema enums). The deferral is disclosed in the description, which I appreciate; my question is whether it gets an ADR addendum or a tracking issue so "two sources of truth" doesn't become permanent by default.

B. Category enumeration is back to a hand-maintained list. cmd/docgen/schema.go's categories table is a literal three-entry list, pkg/nic/config_types.go is three near-identical loops, and schemaFileName re-hardcodes bareNameGroups - three files to touch for a fourth category, where the markdown side solved the same problem with glob discovery precisely because (per main.go:44-48) a literal list is what let the repository category ship undocumented. TestConfigTypesCoversEveryRegistryCategory means it fails loudly rather than silently, so this is a question, not a blocker: is driving the category list off reflection over registry.Registry worth doing now, or is the test considered sufficient?

C. Is whole-file validation a supported use of nebari-config.json? schemas/README.md names only the docs renderer, and ADR-0005 lists editor/LSP validation as a future need - so in fairness, "validate a config with this" isn't a promise the repo makes yet. I'm still calling blocker 1 a blocker because pkg/configschema/configschema.go explicitly justifies closing the schema as mirroring the validator, and because the empty blocks are equally broken for a renderer. If renderer-only is the intent for now, saying so in the README (and dropping the validator-fidelity claim) would at least make the limitation honest.

Kudos

Left inline, but the short list: the AddGoComments diagnosis is the most valuable comment in the PR; TestConfigTypesCoversEveryRegistryCategory guards the next category without naming it; appending constraints after truncation so a constraint can never be the part that gets cut; manifest.json keeping providers as the cluster list with the compatibility reason recorded in code; and three emitters behind one make docs with one drift gate, retiring a 67-line paths filter. The claim that merging the generators surfaced two invisible bugs holds up - and the same argument now applies to the schema/example mismatch, which merging hasn't surfaced only because nothing compares them.

Verification notes

Everything was reproduced in a clean worktree, then independently re-checked by reviewers instructed to refute each claim with "probably right" counting as refuted. Nothing was refuted; three claims were tightened (the 9/9 vs dns scope, blocker 2 being worse than first stated, and the cmd/schemagen half of blocker 3 surviving).

• make docs at bbc9ca0 leaves the tracked tree untouched: determinism and the drift gate are clean, including after the rebase onto #583.
• The one open item in your test plan passes: golangci-lint run is runnable in a worktree at this commit and reports 0 issues. go build ./..., go vet ./... and go test -short ./... all pass.
• Schema validation used the draft each document declares (2020-12), with check_schema passing first - the documents are legal, just unsatisfiable for real configs.
• nic validate behaviours cited inline were checked against a binary built at this commit, including confirming that a passing exit code means the provider validator actually ran (a deliberately-invalid path is rejected from the expected line).
• Two corrections to the description: .github/workflows/schemas.yml never existed on this base (only on the superseded #507), so no standalone gate is removed by this diff; and the drift gate lives inside the deploy job, so a flaky deploy means docs and schemas go unchecked - pre-existing from #567, but this PR widens that gate to schemas/.
• Not verified: anything requiring cloud credentials, and the nebari-docs#710 consumer side.

One pre-existing issue the new schema usefully caught: examples/custom-tls-config.yaml has no node_groups, which pkg/providers/cluster/aws/provider.go:200 requires. nic validate can't catch it because cluster-provider validation isn't reached on that path. The schema is right and the example is wrong - worth fixing separately.

[dcmcand]

Blocker. AllowAdditionalProperties: false combined with the inline provider maps makes the published top-level schema reject every valid config. Re-verified at bbc9ca0.

cluster, dns and repository each hold their providers in a single Providers map[string]any with yaml:",inline" (pkg/config/config.go:80,140,201). Under FieldNameTag: "yaml", invopop v0.14.0 does not expand an inline field of map kind (reflect.go:1074-1076 returns ("", shouldEmbed=true), then reflectStructFields aborts at 484-490 for a non-struct kind). All three therefore reflect as:

{"properties": {}, "additionalProperties": false, "type": "object"}

Validated against the draft the document declares (2020-12; check_schema passes first, so the document is legal - just unsatisfiable): 9 of 9 examples/*.yaml fail. cluster and repository break all nine, dns breaks aws-config-with-dns.yaml, the only example with a dns: key:

['cluster'] Additional properties are not allowed ('aws' was unexpected)
['repository'] Additional properties are not allowed ('existing' was unexpected)

No $ref, patternProperties, allOf or unevaluatedProperties anywhere in schemas/ wires the per-provider schemas in, and no type in the repo implements JSONSchemaExtend/JSONSchema(), so nothing rehabilitates them today.

Two notes on the justification in the comment above:

1. It isn't accurate. There is no strict/KnownFields decoding in pkg/config, and at this commit nic validate accepts both an unknown top-level key and an unknown key inside a provider block (exit 0 both times). The schema is stricter than the real validator in one direction and looser in the other.
2. The blocker holds even if the docs renderer is the only intended consumer: a renderer reading these three defs shows three fields with no children and no hint that a provider key belongs there.

Suggested fix: give the three wrapper types a JSONSchemaExtend emitting additionalProperties/patternProperties for the provider blocks - ideally a oneOf over the registered provider names, which the generator already knows - or stop closing structs that carry an inline catch-all. Then add the regression test that would have caught it: validate examples/*.yaml against the generated schemas. pkg/nic/examples_test.go already exists on exactly that argument, and nothing under *_test.go references schemas/ today.

[dcmcand]

Blocker. required: ["path", "branch"] rejects a config NIC accepts. Re-verified at bbc9ca0.

examples/local-config.yaml ships repository: / local: {}. Built from this commit:

$ ./nic validate -f examples/local-config.yaml
✓ Configuration file is valid
EXIT=0

That exit 0 is a real accept, not a skipped path - repository validation does reach the provider (pkg/nic/validate.go:130-134), which I confirmed by feeding it a relative path and getting path must be an absolute directory (exit 1) from pkg/providers/repository/local/config.go:23. Both fields are genuinely defaulted at runtime: resolveDir (provider.go:62-64) resolves an empty Path to ~/.nic/gitops/<project_name>, and provider.go:149 is cmp.Or(localCfg.Branch, defaultBranch) with defaultBranch = "main" (:26). extractConfig states the contract outright: "An empty or absent provider config is valid and yields the zero Config, which Provision fills with per-project defaults."

Root cause is the ,omitempty-absence heuristic: local/config.go:14,17 carry yaml:"path" / yaml:"branch" with no ,omitempty, so invopop infers required. ADR-0005's own addendum already flags this signal as imperfect and recommends adding omitempty to genuinely-optional tags - that's the fix here.

Worth a sweep of the other root-level required arrays in the same pass. existing.json's required context looks correct (its godoc says it is deliberately mandatory); the repository pair does not.

[dcmcand]

Blocker (same root cause as repository-local.json). This document contradicts itself, and rejects configs NIC accepts.

path is in required here, while line 36 of this same file describes it as "Path is an optional subdirectory within the repository" - faithfully generated from the godoc at pkg/providers/repository/existing/config.go:21-23, which says "optional". examples/aws-config.yaml also annotates the key inline as "Optional subdirectory within the repository".

Verified against the binary: a config with url, branch and auth.ssh.env but no path validates clean (✓ Configuration file is valid, exit 0) and fails this schema with 'path' is a required property.

Fix is the same one-word change - yaml:"path,omitempty" on existing/config.go:23 - plus the examples-vs-schemas test, which would have caught both instances at once.

[dcmcand]

Good addition, and pointing at the enforcing test is exactly the right thing to put in a checklist. But the parity is still off, and the omission has teeth.

TestRegisteredProvidersImplementConfigTyped loops over all three categories (pkg/nic/config_types_test.go:26,33,39), and both dns.ConfigTyped and repository.ConfigTyped exist - yet the DNS checklist below (4 steps) and the repository checklist (6 steps) mention neither the capability, the test, nor make docs. A contributor following either to the letter hits a failing test it never warned about. Verified by adding a DNS provider and a repository provider that satisfy their interfaces but omit ConfigType():

config_types_test.go:35: dns provider "throwaway-dns" does not implement dns.ConfigTyped
config_types_test.go:41: repository provider "throwaway-repo" does not implement repository.ConfigTyped
--- FAIL: TestRegisteredProvidersImplementConfigTyped

schemas/README.md names the capability in prose, but descriptively rather than as a required step, with no signature, no test name and no cross-reference from either checklist. Please add the step to both, ideally with the make docs reminder that currently lives only in "Adding a New Configuration Field".

[dcmcand]

A cluster provider and a DNS provider sharing a name silently overwrite each other's schema - which is the exact failure mode this function's comment describes preventing for the other categories.

Both cluster and dns are in bareNameGroups (main.go:39), so both return the same bare filename and the categories order decides the winner. Reproduced: registering a DNS provider named aws produced 9 files instead of 10, with providers/aws.json containing "$ref": "#/$defs/cloudflare.Config" and a zone_name property - the DNS schema overwrote the AWS cluster schema, exit 0, no warning, while manifest.json advertised aws under both keys.

cmd/docgen already has the machinery: checkOutputNameCollisions (main.go:467, called from the markdown path) covers markdown filenames only. Extending it over schemaFileName output closes this. TestSchemaFileNameQualifiesNonBareGroups iterates a hardcoded list with no cluster+dns same-name case, so it can't catch it as written.

Not hypothetical for long - an existing DNS provider, or a route53 that exists in both categories, would trip it.

[dcmcand]

Kudos - this is the most valuable comment in the PR. It names the precise upstream mechanism (gopath.Join(base, dir) being used as both walk root and import-path suffix), which is not discoverable from the API surface, and it explains why the failure mode is so nasty: every schema still generates and still validates, just with no descriptions at all.

Pairing it with TestGenerateSchemasIncludesGodocDescriptions is the right move, because a missing description has no other signal - nothing errors and nothing looks wrong until someone reads the output.

[dcmcand]

Nice catch in the latest push - resolving outDir before the chdir closes a footgun that would only have shown up with a relative -root, and the comment explains the compounding clearly.

[dcmcand]

Kudos for recording why providers holds only the cluster providers instead of renaming it for symmetry. Consumer-compatibility decisions are the ones that get "cleaned up" a year later by someone who can't see the constraint; having the reason inline is what prevents that.

[dcmcand]

Kudos for the ordering here. Appending constraints after truncation so a constraint can never be the part that gets cut is a small decision that keeps paying off - the 200-rune limit would otherwise silently eat exactly the most actionable half of a long description. The comment says why, and TestConstraintSuffix exercises the boundary.

Catching the silent enum loss (aws.Taint.Effect, azure.NodeGroup.Mode) as a side effect of merging the two generators is a good argument for the merge.

[dcmcand]

This is a genuinely clever guard - comparing NumField catches the next category without naming it, which is the failure mode that let pkg/providers/repository/ ship undocumented in the first place. Keeping the "why" in the comment above it is what makes it maintainable.

One caveat worth a line of defence: it couples to registry.Registry's field count, not to its provider lists. Adding any non-provider field to Registry (a mutex, a logger, a clock) fails this test with a message telling the author to add a category map - the wrong instruction. Filtering to fields whose type is a *ProviderList[...] would make it precise and let it fail for the right reason.