Skip to content
Merged
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
1 change: 1 addition & 0 deletions internal/packagecmd/internal/verify/lint/doc/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
66 changes: 66 additions & 0 deletions internal/packagecmd/internal/verify/lint/doc/openapi.go
Original file line number Diff line number Diff line change
@@ -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.",
},
}
2 changes: 2 additions & 0 deletions internal/packagecmd/internal/verify/lint/doc/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
}
Original file line number Diff line number Diff line change
@@ -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, ".")
}
9 changes: 8 additions & 1 deletion internal/packagecmd/internal/verify/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion internal/packagecmd/packagecmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading