feat(config): lint errors on reserved-schema usage#564
Merged
Conversation
Reject any manifest that populates a parses-but-inert reserved field. A hand-verified registry (verified field-by-field against internal/generate, since the schema comments are unreliable) maps each field path to a category: not-yet-wired (telemetry, rollout.type/canary/blue_green, deploy_target, release.version_overrides) errors as not implemented in this version; callback timeout_minutes errors as belonging in the called workflow. The consumed knobs rollout.max_parallel/fail_fast and job_timeout_minutes stay valid. Walks components and external repos too. Migrates in-repo fixtures and docs, and adds an e2e scenario proving the merge-queue lint lane reds on a reserved field. Signed-off-by: Joshua Temple <joshua.temple@stablekernel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
cascade lint(step 1) rejects unknown top-level keys, but a manifest thatpopulates a field which parses but is not wired to generation still lints
clean and generates an inert result. Those reserved fields should be a hard
error, not a silent no-op.
Fix
A hand-verified reserved-field registry (
internal/config/reserved.go), wiredinto
Validate()so it errors throughcascade lintand thegeneratepath.It mirrors the
globalOnlyComponentFieldspattern and maps each field path to acategory with a distinct message:
... is reserved and not implemented in this cascade versiontimeout_minutes):... timeout belongs in the called workflow, not the callerThe walk covers the top level, every component, and external repos.
Consumed-vs-reserved evidence (verified against
internal/generate)telemetryinternal/generatedeploys[].rollout.type/.canary/.blue_greenwriteDeployStrategyOptions; no generate referencedeploys[].deploy_targetrelease.version_overridestimeout_minutesgraph.go) but never emittedrollout.max_parallel/rollout.fail_fastpromote.go:797-806,generator.go:1100-1104job_timeout_minutesgenerator.go:216target_shaEnvState(component state), not the manifest surface, so it never reaches manifest lintMigration
Reserved usage now errors, so in-repo fixtures that populated reserved fields
were updated: the four reserved-shape e2e scenarios (their premise, that a
reserved field generates byte-identically, no longer holds) are removed and
replaced by
59-lint-reserved-rejection.yaml, which proves the merge-queue lintlane reds on a reserved field. The reserved structural unit tests now assert the
rejection while retaining their direct structural-validator coverage. The
reference/manifest.mdandreference/versioning.mdreserved-field docs notethat
cascade lintrejects their use.Verification
go build ./...,go test ./...,go test -race ./internal/config/... ./internal/generate/...,and
golangci-lint run ./...all clean (root ande2emodule). New reservedtests fail before the registry and pass after.