diff --git a/internal/packagecmd/internal/verify/lint/doc/doc.go b/internal/packagecmd/internal/verify/lint/doc/doc.go index 9fcbf3f3..c43ec11b 100644 --- a/internal/packagecmd/internal/verify/lint/doc/doc.go +++ b/internal/packagecmd/internal/verify/lint/doc/doc.go @@ -72,6 +72,7 @@ type Example struct { // catalog is every documented linter, in the order verify runs them. var catalog = []Linter{ packageDoc, + openapiDoc, templatesDoc, docsDoc, imagesDoc, diff --git a/internal/packagecmd/internal/verify/lint/doc/openapi.go b/internal/packagecmd/internal/verify/lint/doc/openapi.go new file mode 100644 index 00000000..07cdc4ad --- /dev/null +++ b/internal/packagecmd/internal/verify/lint/doc/openapi.go @@ -0,0 +1,66 @@ +package doc + +import ( + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint" + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/openapi" + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/openapi/rules" +) + +// openapiDoc documents the openapi linter. +var openapiDoc = Linter{ + ID: openapi.LinterID, + Summary: "The OpenAPI schemas a package ships under openapi/", + Description: []string{ + "Checks the schemas that describe a package's settings, rather than the values rendered from them — a rendered manifest is the concern of the templates linter.", + "Only openapi/settings.yaml is inspected, because it is the schema the UI builds a settings form from.", + }, + Rules: []Rule{ + { + ID: rules.AdvancedRuleID, + Impact: lint.Error.Ptr(), + Summary: "x-deckhouse-ui-advanced is allowed on top-level settings only", + Description: []string{ + "The x-deckhouse-ui-advanced extension folds a setting behind an \"advanced\" toggle in the UI. It is read from the top level of openapi/settings.yaml — the root object and the entries of its properties map — and nowhere below.", + "Deeper down the extension has no effect at all: the author writes it expecting one field of a nested object to be hidden, the UI ignores it, and the field is rendered like any other. Reporting it here is the only place that mistake becomes visible.", + "The extension is optional. A schema that never sets it, and a schema that sets it on top-level settings only, both satisfy this rule.", + "Depth is counted in settings, not in YAML nesting: allOf, anyOf, oneOf and not branches constrain the same object as their parent, so a branch stays at its parent's level. A oneOf branch that re-declares a top-level setting is therefore still top level, while properties, patternProperties, items and additionalProperties each descend a level.", + }, + Reports: []string{ + "a schema more than one settings level below the root sets x-deckhouse-ui-advanced", + "openapi/settings.yaml exists but cannot be read or is not valid YAML", + }, + Example: Example{ + Reported: []string{ + "type: object", + "properties:", + " storage:", + " type: object", + " x-deckhouse-ui-advanced: true # top level, honoured", + " properties:", + " size:", + " type: string", + " x-deckhouse-ui-advanced: true # nested, ignored by the UI", + }, + Accepted: []string{ + "type: object", + "properties:", + " storage:", + " type: object", + " x-deckhouse-ui-advanced: true # hides the whole storage block", + " properties:", + " size:", + " type: string", + }, + }, + Fix: "Mark the top-level setting that owns the nested schema as advanced, or drop x-deckhouse-ui-advanced from the nested schema.", + Notes: []string{ + "The reported value is the schema pointer of the offending schema, keywords included, so it maps straight onto the lines of settings.yaml.", + "A definitions or $defs entry stands in for a top-level setting, so its own body may carry the marker but the schemas inside it may not.", + "A package without openapi/settings.yaml exposes no settings and reports nothing.", + }, + }, + }, + Notes: []string{ + "The openapi linter is a hard schema contract: it has no .pkglint.yaml settings, and its rules report at their built-in severity.", + }, +} diff --git a/internal/packagecmd/internal/verify/lint/doc/scope.go b/internal/packagecmd/internal/verify/lint/doc/scope.go index 7a53e2ea..39e2184d 100644 --- a/internal/packagecmd/internal/verify/lint/doc/scope.go +++ b/internal/packagecmd/internal/verify/lint/doc/scope.go @@ -9,6 +9,7 @@ import ( "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/docs" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/icon" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/images" + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/openapi" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/oss" pkglint "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/package" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/templates" @@ -20,6 +21,7 @@ import ( // documented targets are read from the same declarations verify enforces. var linterScopes = map[string]lint.TypeScopes{ pkglint.LinterID: pkglint.Scopes, + openapi.LinterID: openapi.Scopes, templates.LinterID: templates.Scopes, docs.LinterID: docs.Scopes, images.LinterID: images.Scopes, diff --git a/internal/packagecmd/internal/verify/lint/linters/openapi/linter.go b/internal/packagecmd/internal/verify/lint/linters/openapi/linter.go new file mode 100644 index 00000000..47040289 --- /dev/null +++ b/internal/packagecmd/internal/verify/lint/linters/openapi/linter.go @@ -0,0 +1,44 @@ +// Package openapi validates the OpenAPI schemas a package ships under openapi/. Its +// rules encode hard schema contracts rather than preferences, so the linter carries no +// .pkglint.yaml settings and its rules always report at their built-in severity. +package openapi + +import ( + "context" + + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint" + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/diag" + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/openapi/rules" +) + +// LinterID is the stable identifier used to reference this linter in diagnostics. +const LinterID = "openapi" + +// Scopes lists the verification targets this linter is processed in. openapi/ is packaged +// into both images, so the schemas are checked everywhere they ship. +var Scopes = lint.EveryType(lint.AllScopes...) + +// Config holds the path required to construct a Linter. +type Config struct { + Path string +} + +// NewLinter constructs a Linter from cfg, scoping its diagnostics to this linter. +func NewLinter(cfg Config, res *diag.Collector) *Linter { + return &Linter{ + config: cfg, + collector: res.With(diag.LinterID(LinterID)), + } +} + +// Linter runs OpenAPI schema rules against a package directory. +type Linter struct { + config Config + + collector *diag.Collector +} + +// Lint executes the openapi rules against the configured package path. +func (l *Linter) Lint(ctx context.Context) { + rules.NewAdvancedRule(l.config.Path, l.collector).Check(ctx) +} diff --git a/internal/packagecmd/internal/verify/lint/linters/openapi/rules/advanced.go b/internal/packagecmd/internal/verify/lint/linters/openapi/rules/advanced.go new file mode 100644 index 00000000..3e39d0ba --- /dev/null +++ b/internal/packagecmd/internal/verify/lint/linters/openapi/rules/advanced.go @@ -0,0 +1,182 @@ +package rules + +import ( + "context" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + + "sigs.k8s.io/yaml" + + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/diag" +) + +// Rule purpose: keep the UI-advanced marker at the top level of the settings schema, the +// only depth the UI reads it from. + +// AdvancedRuleID is the stable identifier used to reference this rule in diagnostics. +const AdvancedRuleID = "advanced" + +const ( + // openAPIDir is the package subdirectory holding the OpenAPI schemas. + openAPIDir = "openapi" + // settingsFile is the OpenAPI schema describing user-configurable settings. + settingsFile = "settings.yaml" + // advancedKey is the vendor extension that marks a setting as advanced in the UI. + advancedKey = "x-deckhouse-ui-advanced" + // maxAdvancedDepth is the deepest property level the marker is honoured at. Depth 0 is + // the schema root and depth 1 its top-level settings; below that the UI ignores it. + maxAdvancedDepth = 1 +) + +// Nested-schema keywords, grouped by the shape of the value they hold and mapped to the +// property levels descending into them adds. Traversal follows these rather than every map +// in the document, so a setting named like the extension is never mistaken for it. +// +// Iteration order does not matter: findings are sorted before they are reported. +var ( + // namedSchemaKeywords hold a map of name to nested schema. + namedSchemaKeywords = map[string]int{ + "properties": 1, + "patternProperties": 1, + // A definition stands in for a setting schema, so its body sits at the level a + // top-level setting does. Where it is referenced from is not knowable here. + "definitions": 1, + "$defs": 1, + } + + // listSchemaKeywords hold an ordered list of nested schemas. A combinator branch + // constrains the same object as its parent, so it stays at the parent's level; the + // tuple form of items describes elements, which is one level down. + listSchemaKeywords = map[string]int{ + "allOf": 0, + "anyOf": 0, + "oneOf": 0, + "items": 1, + } + + // singleSchemaKeywords hold exactly one nested schema. items appears here and in + // listSchemaKeywords because it takes both forms; each loop type-checks its value, so + // a given document shape is visited by exactly one of them. + singleSchemaKeywords = map[string]int{ + "items": 1, + "additionalProperties": 1, + "not": 0, + } +) + +// AdvancedRule checks that the settings schema carries the advanced marker at its top +// level only. +type AdvancedRule struct { + collector *diag.Collector + path string +} + +// NewAdvancedRule constructs an AdvancedRule scoped to a package directory. The +// schema location inside it is fixed, so the rule resolves it from packageDir itself. +func NewAdvancedRule(packageDir string, collector *diag.Collector) *AdvancedRule { + return &AdvancedRule{ + path: packageDir, + collector: collector.With( + diag.RuleID(AdvancedRuleID), + diag.Path(filepath.Join(openAPIDir, settingsFile))), + } +} + +// Check reports every schema below the top level that carries the advanced marker. The +// marker is optional: a schema that never sets it, and one that sets it on the root object +// or on its top-level settings, all satisfy the rule. An absent settings.yaml means the +// package exposes no settings, which is not this rule's concern. +func (r *AdvancedRule) Check(_ context.Context) { + raw, err := os.ReadFile(filepath.Join(r.path, openAPIDir, settingsFile)) + if os.IsNotExist(err) { + return + } + + if err != nil { + r.collector.Error("failed to read %s: %v", settingsFile, err) + + return + } + + var root map[string]any + if err = yaml.Unmarshal(raw, &root); err != nil { + r.collector.Error("failed to parse %s: %v", settingsFile, err) + + return + } + + for _, pointer := range deepAdvanced(root) { + r.collector.With(diag.Value(pointer)). + Error("%s is allowed only on root properties", advancedKey) + } + + r.collector.Commit() +} + +// deepAdvanced returns the pointer of every schema below the top level of root that +// carries the advanced marker, sorted so the same schema always reports in the same order. +func deepAdvanced(root map[string]any) []string { + var found []string + + collectAdvanced(root, "", 0, &found) + + slices.Sort(found) + + return found +} + +// collectAdvanced appends the pointer of each schema at or below node that carries the +// advanced marker deeper than the UI honours it, descending through the nested-schema +// keywords. depth counts the property levels between node and the schema root. +func collectAdvanced(node map[string]any, pointer string, depth int, found *[]string) { + if depth > maxAdvancedDepth { + if _, ok := node[advancedKey]; ok { + *found = append(*found, pointer) + } + } + + for keyword, delta := range namedSchemaKeywords { + named, ok := node[keyword].(map[string]any) + if !ok { + continue + } + + for name, value := range named { + if child, ok := value.(map[string]any); ok { + collectAdvanced(child, joinPointer(pointer, keyword, name), depth+delta, found) + } + } + } + + for keyword, delta := range listSchemaKeywords { + list, ok := node[keyword].([]any) + if !ok { + continue + } + + for i, item := range list { + if child, ok := item.(map[string]any); ok { + collectAdvanced(child, joinPointer(pointer, keyword, strconv.Itoa(i)), depth+delta, found) + } + } + } + + for keyword, delta := range singleSchemaKeywords { + if child, ok := node[keyword].(map[string]any); ok { + collectAdvanced(child, joinPointer(pointer, keyword), depth+delta, found) + } + } +} + +// joinPointer appends segments to a parent pointer, dot-separated. Keywords stay in the +// result so the pointer maps straight onto the lines of settings.yaml. +func joinPointer(pointer string, segments ...string) string { + if pointer != "" { + segments = append([]string{pointer}, segments...) + } + + return strings.Join(segments, ".") +} diff --git a/internal/packagecmd/internal/verify/verify.go b/internal/packagecmd/internal/verify/verify.go index bc903c3d..c92d3d16 100644 --- a/internal/packagecmd/internal/verify/verify.go +++ b/internal/packagecmd/internal/verify/verify.go @@ -21,6 +21,7 @@ import ( "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/docs" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/icon" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/images" + "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/openapi" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/oss" pkglint "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/package" "github.com/deckhouse/deckhouse-cli/internal/packagecmd/internal/verify/lint/linters/templates" @@ -341,7 +342,7 @@ func buildLinters(ctx context.Context, root *settings.Root, path string, scope l target := lint.Target{Type: lint.PackageType(def.Type), Scope: scope} collector = collector.With(diag.PackageID(def.Name)) - linters := make([]linter, 0, 6) + linters := make([]linter, 0, 7) if pkglint.Scopes.Contains(target) { linters = append(linters, pkglint.NewLinter(pkglint.Config{ @@ -351,6 +352,12 @@ func buildLinters(ctx context.Context, root *settings.Root, path string, scope l }, collector)) } + if openapi.Scopes.Contains(target) { + linters = append(linters, openapi.NewLinter(openapi.Config{ + Path: path, + }, collector)) + } + if templates.Scopes.Contains(target) { rendered, err := renderTemplates(ctx, def, path) if err != nil { diff --git a/internal/packagecmd/packagecmd.go b/internal/packagecmd/packagecmd.go index 080c662c..da227522 100644 --- a/internal/packagecmd/packagecmd.go +++ b/internal/packagecmd/packagecmd.go @@ -15,7 +15,7 @@ import ( // entry point (cmd/package/main.go) and the "version" subcommand, whose value is // injected by the plugin's own ldflags. d8 reports its version itself. // -// Vendored from d8-package-plugin v0.0.29 (1ad1db5). Keep this in sync when +// Vendored from d8-package-plugin v0.0.30 (23f3072). Keep this in sync when // re-syncing internal/, pkg/ and templates/ from upstream. func NewCommand() *cobra.Command { return pkgcmd.NewCmdRoot()