Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,16 @@ jobs:
with:
go-version-file: go.mod

- name: Verify docs are up-to-date
# `git add -A` (not `-N`) so the gate also catches generated files that
# *disappear*: `-N` stages a tracked file's deletion, leaving nothing for
# the subsequent unstaged `git diff` to report. `make docs` now `rm -f`s
# `schemas/*.json` before regenerating them, so a generator that stops
# emitting a file would otherwise pass silently.
- name: Verify docs and schemas are up-to-date
run: |
make docs
git add -N docs/reference/cli docs/configuration
git diff --exit-code -- docs/reference/cli docs/configuration
git add -A docs/reference/cli docs/configuration schemas
git diff --cached --exit-code -- docs/reference/cli docs/configuration schemas

build:
name: Build
Expand Down Expand Up @@ -212,7 +217,14 @@ jobs:
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
with:
go-version-file: go.mod
# v1.6.0, not v1.4.0: v1.4.0 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 producing any JSON, so the gate cannot run at all. Reproducible
# with v1.4.0 on `./pkg/configschema/...` alone; v1.6.0 scans the same
# tree cleanly.
- name: Install govulncheck
run: go install golang.org/x/vuln/cmd/govulncheck@v1.4.0
run: go install golang.org/x/vuln/cmd/govulncheck@v1.6.0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: is this govulncheck bump (v1.4.0 -> v1.6.0, plus the matching string in scripts/govulncheck-gate.sh) meant to ride along here? It isn't in the description's Changes section, which covers the schema pipeline and the docgen fold.

No objection to the bump - just that a dependency-gate change is easy to lose in a PR this size, and someone bisecting a CI failure later will want it on its own commit. Either call it out in the body or split it.

- name: Run vulnerability gate
run: ./scripts/govulncheck-gate.sh
16 changes: 11 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ OTEL_EXPORTER=otlp OTEL_ENDPOINT=localhost:4317 ./nic deploy -f config.yaml
cmd/nic/ # CLI entry point - thin wrapper: .env loading, telemetry, signal handling
└── main.go # Calls internal/cli.Execute(ctx)

cmd/docgen/ # Standalone tool: generates docs/reference/cli/ (from internal/cli.NewRootCmd())
# and docs/configuration/ (from provider config structs)
cmd/docgen/ # Standalone tool: generates docs/reference/cli/ (from internal/cli.NewRootCmd()),
# docs/configuration/ (go/ast over the config structs), and
# schemas/ (JSON Schema reflected off the provider registry)

internal/
└── cli/ # Cobra command tree; imported by cmd/nic (to run) and cmd/docgen (to introspect for docs)
Expand Down Expand Up @@ -340,7 +341,7 @@ func SomeFunction(ctx context.Context, ...) error {
### Adding a New Cluster Provider

1. Create `pkg/providers/cluster/<name>/`.
2. Implement the `Provider` interface (`Name`, `Validate`, `Deploy`, `Destroy`, `GetKubeconfig`, `Summary`, `InfraSettings`).
2. Implement the `Provider` interface (`Name`, `Validate`, `Deploy`, `Destroy`, `GetKubeconfig`, `Summary`, `InfraSettings`). Also implement the optional `cluster.ConfigTyped` capability (a one-line `ConfigType() reflect.Type` in `configtype.go`) so the provider's config schema and reference docs generate themselves; `pkg/nic.TestRegisteredProvidersImplementConfigTyped` fails if you skip it, and `make docs` is what commits its schema.
3. Choose the right backing tool. Embed templates with `//go:embed` if you need files (see `pkg/providers/cluster/aws/templates/`).
4. Register the provider with the `registry.Registry` built in `pkg/nic/registry.go`.
5. Populate `InfraSettings` so `pkg/argocd` and the CLI can configure software without knowing about your provider. Add new fields to `InfraSettings` (not provider-name switches) if you need to express a new capability.
Expand All @@ -353,18 +354,20 @@ func SomeFunction(ctx context.Context, ...) error {
### Adding a New DNS Provider

1. Create `pkg/providers/dns/<name>/`.
2. Implement the `DNSProvider` interface (`Name`, `ProvisionRecords`, `DestroyRecords`).
2. Implement the `DNSProvider` interface (`Name`, `ProvisionRecords`, `DestroyRecords`). Also implement the optional `dns.ConfigTyped` capability (a one-line `ConfigType() reflect.Type`) so the provider's config schema and reference docs generate themselves; `pkg/nic.TestRegisteredProvidersImplementConfigTyped` fails if you skip it.
3. Register with the `registry.Registry`.
4. Add to `examples/` (e.g., update `aws-config-with-dns.yaml`).
5. Run `make docs` and commit the regenerated `docs/configuration/` and `schemas/` (CI fails on drift).

### Adding a New Repository Provider

1. Create `pkg/providers/repository/<name>/`.
2. Implement the `Provider` interface (`Name`, `Validate`, `Provision`). `Provision` returns a `Source`: `RemoteSource` for a repository reached over the network, `LocalSource` for a directory on disk (only usable with cluster providers whose `InfraSettings` set `SupportsLocalGitOps`).
2. Implement the `Provider` interface (`Name`, `Validate`, `Provision`). `Provision` returns a `Source`: `RemoteSource` for a repository reached over the network, `LocalSource` for a directory on disk (only usable with cluster providers whose `InfraSettings` set `SupportsLocalGitOps`). Also implement the optional `repository.ConfigTyped` capability (a one-line `ConfigType() reflect.Type` in `configtype.go`) so the provider's config schema and reference docs generate themselves; `pkg/nic.TestRegisteredProvidersImplementConfigTyped` fails if you skip it.
3. Keep `Validate` offline and side-effect free: it runs from `nic validate` and dry-run deploys, before any infrastructure exists. Resolve credentials from environment variables only inside `Provision`. The config carries env-var names, never secret values.
4. Register with the `registry.Registry` in `pkg/nic/registry.go`, exporting a `ProviderName` constant as the registry key.
5. Update `examples/` configs with the new `repository:` provider block.
6. Cover the provider with table-driven unit tests (see `pkg/providers/repository/existing/` for the pattern).
7. Run `make docs` and commit the regenerated `docs/configuration/` and `schemas/` (CI fails on drift).

### Adding a New Configuration Field

Expand All @@ -374,6 +377,8 @@ func SomeFunction(ctx context.Context, ...) error {
4. Plumb it through to the backing tool.
5. Update example configs in `examples/`.
6. Add tests.
7. Run `make docs` and commit the regenerated `docs/configuration/` and `schemas/`. A field's allowed values and default belong in a `jsonschema:"enum=…,default=…"` tag, not in prose - both generators read the tag, so stating it in the godoc as well just duplicates it in every output. When the default also exists as a Go const, the tag is a second declaration of it: pin the two together with a test (see `TestChartVersionSchemaDefaultMatchesConst`), because nothing else relates them and a drifted tag publishes a wrong default to every reader.
8. **Do not add a `yaml` tag without `,omitempty` unless the field is genuinely mandatory.** Required-ness in the generated schema is inferred from the absence of `,omitempty`, so a field NIC defaults at runtime but tags without it produces a schema that rejects configs the binary accepts. `pkg/nic.TestExampleConfigsMatchGeneratedSchemas` catches this for anything an example exercises.

**Placeholder sentinel.** NIC reserves the literal, case-sensitive token `CHANGEME` as an "unfilled value" marker. `config.CheckPlaceholders` walks the parsed YAML node tree (not the Go struct) and rejects any scalar value *or mapping key* whose text *contains* `CHANGEME` (including nested provider blocks, lists, `|`/`>` block scalars, and map keys such as `node_groups: { CHANGEME: … }`), reporting every offending field in one pass. YAML comments are never scanned, though a `#` line inside a block scalar is content and is. The check runs at `validate` and `deploy` only — it is deliberately not part of `NebariConfig.Validate`, so `destroy`/`kubeconfig` are not gated on it. Starter and example configs that must be edited before deploy should use this exact token; example configs meant to validate as-is must avoid it (use descriptive-but-valid values like `nebari.example.com`). See `docs/operations/config-placeholders.md`.

Expand Down Expand Up @@ -467,6 +472,7 @@ References:
- **`docs/design-doc/`** - Living design docs (architecture / implementation / operations / appendix)
- **`docs/reference/cli/`** - Generated CLI command reference (from `internal/cli`'s cobra tree; regenerate with `make docs`)
- **`docs/configuration/`** - Generated configuration reference (from provider config structs; regenerate with `make docs`)
- **`schemas/`** - Generated JSON Schema for `nebari-config.yaml` and each registered provider, consumed by the docs site (regenerate with `make docs`)
- **`docs/local-kind-development.md`** - Local Kind workflow
- **`docs/plans/`** - In-flight implementation plans
- **[CONTRIBUTING.md](CONTRIBUTING.md)** - The human-facing contribution process
Expand Down
6 changes: 3 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ build: ## Build the binary
CGO_ENABLED=0 go build -trimpath $(LDFLAGS) -o $(BINARY_NAME) $(CMD_DIR)
@echo "Built $(BINARY_NAME) successfully"

docs: ## Generate CLI and configuration reference documentation
@mkdir -p docs/reference/cli docs/configuration
@rm -f docs/reference/cli/*.md docs/configuration/*.md
docs: ## Generate the CLI, configuration, and JSON Schema reference from the Go source
@mkdir -p docs/reference/cli docs/configuration schemas/providers
@rm -f docs/reference/cli/*.md docs/configuration/*.md schemas/*.json schemas/providers/*.json
go run ./cmd/docgen

starters: ## Generate the Nebi starter workspaces into dist/starters
Expand Down
29 changes: 25 additions & 4 deletions cmd/docgen/main.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,26 @@
//go:generate go run .

// Command docgen generates markdown documentation from Go config structs.
// Command docgen generates NIC's reference documentation from the source of
// truth in Go, in three flavours:
//
// - docs/configuration/ - markdown config reference, parsed out of the config
// structs with go/ast (struct definitions, field types, yaml tags, doc comments)
// - docs/reference/cli/ - markdown CLI reference, walked off internal/cli's cobra tree
// - schemas/ - JSON Schema for nebari-config.yaml and each registered
// provider, reflected off the live types via the provider registry
//
// Usage:
//
// go run ./cmd/docgen -output docs/configuration
// go run ./cmd/docgen # all three
// make docs # the same, plus cleaning stale output first
//
// This tool parses Go source files using go/ast to extract struct definitions,
// field types, yaml tags, and doc comments, then generates markdown documentation.
// It is an internal build/CI tool, not a user-facing subcommand of nic. All
// three outputs are committed in-tree and guarded by a CI drift check, so a
// config change that isn't accompanied by regenerated output fails the build.
package main

import (
"context"
"flag"
"fmt"
"log"
Expand Down Expand Up @@ -181,6 +191,9 @@ func discoverProviderConfigFiles(rootDir string, providerGroups []string) ([]con
func main() {
outputDir := flag.String("output", "docs/configuration", "Output directory for generated configuration documentation")
cliOutputDir := flag.String("cli-output", "docs/reference/cli", "Output directory for generated CLI reference documentation")
schemaOutputDir := flag.String("schema-output", "schemas", "Output directory for generated JSON Schema documents")
schemaProviders := flag.String("schema-providers", "", "Comma-separated provider subset to regenerate schemas for (default: all registered)")
schemaVersion := flag.String("schema-version", "", "Version string stamped into schemas/manifest.json")
rootDir := flag.String("root", "", "Root directory of the project (defaults to current directory)")
verbose := flag.Bool("verbose", false, "Enable verbose output")
flag.Parse()
Expand All @@ -197,6 +210,7 @@ func main() {
log.Printf("Project root: %s", *rootDir)
log.Printf("Configuration output directory: %s", *outputDir)
log.Printf("CLI output directory: %s", *cliOutputDir)
log.Printf("Schema output directory: %s", *schemaOutputDir)
}

outPath := filepath.Join(*rootDir, *outputDir)
Expand Down Expand Up @@ -257,6 +271,13 @@ func main() {

fmt.Printf("Configuration documentation generated successfully in %s\n", outPath)
fmt.Printf("CLI documentation generated successfully in %s\n", cliOutPath)

// Schemas are reflected off the live types rather than parsed, so this
// emitter shares only the project root with the two above.
schemaOutPath := filepath.Join(*rootDir, *schemaOutputDir)
if err := generateSchemas(context.Background(), *rootDir, schemaOutPath, *schemaProviders, *schemaVersion); err != nil {
log.Fatalf("Failed to generate schemas: %v", err)
}
}

// processConfigFile parses cf's source, writes its page, and returns the
Expand Down
26 changes: 26 additions & 0 deletions cmd/docgen/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ func (g *MarkdownGenerator) writeFieldRow(field FieldDoc, required string) {
if runes := []rune(desc); len(runes) > 200 {
desc = string(runes[:197]) + "..."
}
// Append the `jsonschema` constraints after truncation, so they are never

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// the part that gets cut. These are short and are the field's most
// actionable documentation.
desc = strings.TrimSpace(desc + constraintSuffix(field))

yamlKey := field.YAMLKey
if yamlKey == "" {
Expand All @@ -92,6 +96,28 @@ func (g *MarkdownGenerator) writeFieldRow(field FieldDoc, required string) {
field.Name, yamlKey, goType, required, desc)
}

// constraintSuffix renders a field's `jsonschema` enum/default as a sentence
// to append to its description, so the allowed values and default a field
// declares are visible in the markdown reference and not only in schemas/.
// Returns "" when the field declares neither.
func constraintSuffix(field FieldDoc) string {
var parts []string
if len(field.Enum) > 0 {
quoted := make([]string, len(field.Enum))
for i, v := range field.Enum {
quoted[i] = "`" + v + "`"
}
parts = append(parts, "One of: "+strings.Join(quoted, ", ")+".")
}
if field.Default != "" {
parts = append(parts, "Defaults to `"+field.Default+"`.")
}
if len(parts) == 0 {
return ""
}
return " " + strings.Join(parts, " ")
}

// formatType formats a Go type for markdown display.
func formatType(t string) string {
// Wrap complex types in code blocks
Expand Down
59 changes: 59 additions & 0 deletions cmd/docgen/markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,3 +277,62 @@ func TestMarkdownGenerator_WriteNote(t *testing.T) {
t.Error("note should contain the note text")
}
}

func TestConstraintSuffix(t *testing.T) {
tests := []struct {
name string
field FieldDoc
want string
}{
{
name: "no constraints",
field: FieldDoc{Name: "Region"},
want: "",
},
{
name: "enum only",
field: FieldDoc{Name: "Effect", Enum: []string{"NO_SCHEDULE", "NO_EXECUTE"}},
want: " One of: `NO_SCHEDULE`, `NO_EXECUTE`.",
},
{
name: "default only",
field: FieldDoc{Name: "ChartVersion", Default: "3.4.3"},
want: " Defaults to `3.4.3`.",
},
{
name: "enum and default",
field: FieldDoc{Name: "Mode", Enum: []string{"System", "User"}, Default: "User"},
want: " One of: `System`, `User`. Defaults to `User`.",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := constraintSuffix(tt.field); got != tt.want {
t.Errorf("constraintSuffix() = %q, want %q", got, tt.want)
}
})
}
}

// A long description is truncated, but the constraints are appended after the
// truncation so they can never be the part that gets cut.
func TestWriteFieldRowKeepsConstraintsAfterTruncation(t *testing.T) {
var buf bytes.Buffer
g := NewMarkdownGenerator(&buf)
g.writeFieldRow(FieldDoc{
Name: "Effect",
GoType: "string",
YAMLKey: "effect",
Doc: strings.Repeat("x", 300),
Enum: []string{"NO_SCHEDULE"},
}, "Yes")

out := buf.String()
if !strings.Contains(out, "...") {
t.Errorf("expected the long description to be truncated, got %q", out)
}
if !strings.Contains(out, "One of: `NO_SCHEDULE`.") {
t.Errorf("constraints dropped by truncation: %q", out)
}
}
24 changes: 24 additions & 0 deletions cmd/docgen/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,14 @@ type FieldDoc struct {
Doc string
IsInline bool
IsIgnored bool // yaml:"-"

// Enum and Default come from the `jsonschema` tag, the same tag the JSON
// Schema emitter reads. Constraints declared there would otherwise appear
// only in schemas/ and silently vanish from the markdown reference, which
// is what happened when the allowed values for aws.Taint.Effect and
// azure.NodeGroup.Mode moved out of trailing comments and into tags.
Enum []string
Default string
}

// ParseFile parses a Go source file and extracts struct documentation.
Expand Down Expand Up @@ -192,6 +200,22 @@ func parseTag(tagValue string, doc *FieldDoc) {
doc.JSONKey = parts[0]
}
}

// Parse the jsonschema tag so constraints declared for the schema emitter
// also reach the markdown reference. Only the two keys that carry
// user-facing meaning are read; the rest are schema-shaping directives.
for _, opt := range strings.Split(tag.Get("jsonschema"), ",") {
key, value, ok := strings.Cut(opt, "=")
if !ok {
continue
}
switch key {
case "enum":
doc.Enum = append(doc.Enum, value)
case "default":
doc.Default = value
}
}
}

// typeToString converts an AST type expression to a readable string.
Expand Down
21 changes: 21 additions & 0 deletions cmd/docgen/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,27 @@ func TestParseTag(t *testing.T) {
Required: false,
},
},
{
// The jsonschema tag is the schema emitter's input; the markdown
// emitter reads enum/default out of it too so a constraint declared
// once shows up in both outputs.
name: "jsonschema enum and default",
tagValue: "`yaml:\"mode,omitempty\" jsonschema:\"enum=System,enum=User,default=User\"`",
want: FieldDoc{
YAMLKey: "mode",
Required: false,
Enum: []string{"System", "User"},
Default: "User",
},
},
{
name: "jsonschema directives without a value are ignored",
tagValue: "`yaml:\"name\" jsonschema:\"required,minLength=1\"`",
want: FieldDoc{
YAMLKey: "name",
Required: true,
},
},
{
name: "yaml ignored field",
tagValue: "`yaml:\"-\"`",
Expand Down
Loading
Loading