fix(api): refuse only what a name declaration actually breaks by lllamnyp · Pull Request #4429 · cozystack/cozystack

Timofei Larkin (lllamnyp) · 2026-09-24T06:35:38Z

What this PR does

Follow-up to #4367, taking the non-blocking findings from its approving review. Each one is a case where a name declaration (x-cozystack-name) was handled more harshly, or more quietly, than the declaration itself warrants.

An unparseable schema is no longer blamed on a declaration. A schema that is not a JSON object has no declaration in it, and the spec schema loader already reports it. The API server used to refuse every create of the kind with "declares an invalid x-cozystack-name". It now treats the schema as declaring nothing: creates keep working, and Kubernetes and Kafka keep their legacy caps.
Annotation keywords no longer take a kind offline. Any unknown key inside a declaration used to refuse creates, so a generator adding a title or an example would have done the same. JSON Schema annotation keywords (title, default, examples, deprecated, readOnly, writeOnly, $comment) and x- extensions are now ignored. A constraint keyword the server does not implement, such as format or a misspelled maxLenght, is still refused, because accepting it would drop it silently.
The docs no longer overclaim. They said a declaration no name can satisfy is refused. That is true only for lengths: a pattern is checked to compile, not to match anything, so $^ rejects every name and names the pattern as the reason. The docs now say that.
The API review gate sees the declarations the server refuses. The gate only read a declaration that was an object, and only diffed the keywords it knew. A declaration turned into a string or null, or a misspelled keyword, looked like no change. A changed declaration now goes through the same parser the server uses, and the gate reports it if it cannot be enforced. The parser moves to pkg/apis/apps/validation so there is one copy of the rules.
The declaration no longer leaks into the published OpenAPI for spec. The whole openAPISchema is published as the schema of spec, so x-cozystack-name appeared there as a vendor extension of spec. It is now stripped from the published copy. metadata.name still carries no limit in the published OpenAPI: it is the shared ObjectMeta reference, and a per-kind copy is a bigger change than this one.

Not changed: a pattern that can never match is still accepted. Deciding whether a regular expression matches any string within the declared lengths is a much larger piece of code than the mistake it would catch.

How it is tested

TestUnparseableSchemaDeclaresNothing: three non-object schemas; creates still pass, and the Kubernetes legacy cap still holds.
TestNameDeclarationIgnoresAnnotations: a declaration carrying title, examples, $comment and an x- extension still enforces its maxLength.
TestInvalidNameSchemaFailsClosed gains format as an unsupported constraint keyword.
TestClassifyNameDeclaration gains a declaration turned into a string, a misspelled keyword and a null declaration, all reported, and an added annotation, which is not.
TestPatchSpecLeavesNameDeclarationOffSpec: the published spec drops x-cozystack-name and keeps an unrelated vendor extension.

Downstream repositories

No repository is affected beyond the follow-ups already noted on #4367 (the cozystack/website pages on annotations and ApplicationDefinitions). Those are still unfiled, so no box is ticked.

Release note

fix(api): An ApplicationDefinition whose openAPISchema cannot be parsed no longer refuses every create of its kind as an invalid name declaration, and annotation keywords (`title`, `examples`, `x-` extensions and the like) inside `x-cozystack-name` are now ignored instead of taking the kind offline. The API review gate now reports a changed name declaration that the API server would refuse, and `x-cozystack-name` no longer appears on the published OpenAPI schema of `spec`.

Summary by CodeRabbit

New Features
- Application name declarations now support minimum and maximum lengths and regular-expression patterns. Matching patterns are checked for compilation; a pattern that matches no names rejects all names.
Bug Fixes
- Invalid or unsupported name constraints now produce configuration errors and block every create of the affected kind.
- Descriptive annotations and x- extensions are ignored when validating name declarations.
- The custom name declaration is omitted from the published API specification.

Three cases refused creates of a kind, or were documented as refused, without the declaration being at fault. A schema that is not a JSON object was reported as an invalid x-cozystack-name, although such a schema has no declaration to blame, and the spec schema's own loader already reports it. It now declares nothing, so creates keep working and a kind with a legacy cap keeps it. Any unknown key inside a declaration took the kind offline, so a generator adding a title or an example would do the same. Keywords that constrain nothing, the JSON Schema annotations and x- extensions, are now ignored. A constraint keyword the server does not implement is still refused, because accepting it would drop it silently. The docs said a declaration no name satisfies is refused. That holds for lengths only: a pattern is checked to compile, not to match anything. The docs now say so. The parser moves to pkg/apis/apps/validation so the API review gate can reuse it rather than keep a copy of its rules. Assisted-by: LLM Signed-off-by: Timofei Larkin <[email protected]>

The gate read a name declaration only when it was an object, and diffed only the constraint keywords it knew. A declaration turned into a string or null, or a misspelled keyword, therefore looked like no change at all. Those are exactly the inputs on which the API server refuses every create of the kind, the harshest outcome a schema change can have. The gate now runs a changed declaration through the same parser the server uses and reports it when it cannot be enforced. Assisted-by: LLM Signed-off-by: Timofei Larkin <[email protected]>

The whole openAPISchema is published as the schema of spec, so x-cozystack-name showed up as a vendor extension of spec in the aggregated OpenAPI. That describes something spec does not contain. Assisted-by: LLM Signed-off-by: Timofei Larkin <[email protected]>

coderabbitai · 2026-09-24T06:35:59Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5d43d069-9540-48c3-a2dc-889c0121bd0a

📥 Commits

Reviewing files that changed from the base of the PR and between 146e06e and a461dc4.

📒 Files selected for processing (9)

docs/agents/overview.md
internal/apigate/classify_test.go
internal/apigate/schemadiff.go
pkg/apis/apps/validation/nameschema.go
pkg/cmd/server/openapi.go
pkg/cmd/server/openapi_test.go
pkg/registry/apps/application/rest.go
pkg/registry/apps/application/rest_name_schema_test.go
pkg/registry/apps/application/rest_validation_test.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

Walkthrough

The change adds shared parsing and validation for x-cozystack-name declarations. Application name validation and API gate comparisons use the shared parser. OpenAPI publication removes the declaration extension while preserving unrelated extensions.

Changes

Name declaration validation and handling

Layer / File(s)	Summary
Shared name validation and application enforcement `pkg/apis/apps/validation/nameschema.go`, `pkg/registry/apps/application/rest.go`, `pkg/registry/apps/application/rest_name_schema_test.go`, `pkg/registry/apps/application/rest_validation_test.go`, `docs/agents/overview.md`	A shared `NameSchema` parser validates declaration keywords, values, and patterns. Application storage uses it for name validation and retains legacy caps. Tests cover invalid declarations, ignored annotations, and unparseable schemas. The documentation describes these rules.
API gate declaration comparison `internal/apigate/schemadiff.go`, `internal/apigate/classify_test.go`	The API gate compares changed declarations and reports when a declaration cannot be enforced. Tests cover invalid declarations and annotations.
OpenAPI extension filtering `pkg/cmd/server/openapi.go`, `pkg/cmd/server/openapi_test.go`	OpenAPI publication removes the `x-cozystack-name` extension while preserving unrelated extensions. A test checks the published schema.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to a461d

The name-declaration changes are mergeable after normal checks; no actionable issue remains.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: API review and validation now refuse only name declarations that the system cannot enforce.
Docstring Coverage	✅ Passed	Docstring coverage is 86.67% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 8 files. (1 skipped: 1 …
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

IvanHunters

Verdict

LGTM with non-blocking notes.

A tight, well-tested follow-up. It lifts parseNameSchema into the exported pkg/apis/apps/validation package and narrows four over-harsh behaviours around x-cozystack-name. go build/vet/test/golangci-lint come back clean on the four touched packages, and each behaviour change is pinned by a test that goes red when you revert the fix. Nothing here blocks. The two notes below are refinements; one of them flags a real divergence between the api-gate and the server that is worth a follow-up.

Findings

[MINOR] internal/apigate/schemadiff.go:58, api-gate is blind to a fractional maxLength the server refuses

[MINOR] pkg/apis/apps/validation/nameschema.go:101, blanket x- acceptance silently drops a constraint keyword k8s enforces elsewhere

Notes

The malformed / non-object openAPISchema change is a deliberate, safe loosening. ParseNameSchema now returns "declares nothing" instead of an error, so a garbage schema no longer refuses every create of the kind, and the name stays bounded by the 53-char Helm budget and legacyNameCaps. TestUnparseableSchemaDeclaresNothing shows the Kubernetes 32-cap still rejecting a 33-char name under a garbage schema. Only worth putting on record: the sole remaining signal that a definition is broken is a klog.Errorf at pkg/registry/apps/application/rest.go:136, a server log rather than a create-time rejection, which is what the docstring means by "the spec schema's own loader is where that failure is reported" (buildSpecSchema is that loader).
The red CI check on this head is unrelated. Build packages/system/backupstrategy-controller died in "Set up build toolchain" while curl-downloading flux 2.9.5 (exit 22, a network/infra failure), not in the Go build.

IvanHunters · 2026-09-24T11:43:45Z

 		return nil
 	}
-	return diffSchema("metadata.name", nameDeclaration(base), headName)
+	encoded, err := json.Marshal(headRaw)


[MINOR] api-gate is blind to a fractional maxLength the server refuses

The gate parses the head declaration from a re-marshalled decoded value (json.Marshal(headRaw)), where headRaw came from json.Unmarshal into map[string]any and every JSON number is a float64. So a source declaration of {"maxLength": 32.0} (or 3.2e1) re-marshals to {"maxLength":32} and parses clean, while the server reads the original bytes and rejects them. Probe on this head:

form=32.0 server=cannot unmarshal number 32.0 into Go struct field NameSchema.maxLength of type int64 apigate=["metadata.name: maxLength constraint added (32)"] form=3.2e1 server=<same error> apigate=["metadata.name: maxLength constraint added (32)"] form=32 server=<nil> apigate=["metadata.name: maxLength constraint added (32)"]

The server refuses every create of the kind for the fractional form; the gate reports it as an ordinary enforceable constraint, the opposite of the cannot be enforced, so every create of this kind is refused signal this same change adds at line 63. The server still fail-closes and the gate is advisory, so this is non-blocking. Fix: run the head declaration through the same parser on the same bytes the server sees (feed validation.ParseNameSchema the raw openAPISchema string) rather than re-marshalling the decoded map.

IvanHunters · 2026-09-24T11:43:45Z

+	}
+	for k, v := range keywords {
+		switch {
+		case jsonSchemaAnnotations[k], strings.HasPrefix(k, "x-"):


[MINOR] blanket x- acceptance silently drops a constraint keyword k8s enforces elsewhere

case jsonSchemaAnnotations[k], strings.HasPrefix(k, "x-"): continue accepts and ignores any x-* key. The base parser refused unknown keys via DisallowUnknownFields, so this flips fail-closed to fail-open for the x- space. x-kubernetes-validations is a real CEL constraint the apiserver enforces on ordinary schema fields, but inside x-cozystack-name it is consumed by nothing (NameSchema has only type/description/minLength/maxLength/pattern, and patchSpec deletes the extension from the published spec), so a chart that declared it expecting a name constraint would have it silently dropped while the api-gate could still report it as enforced.

Same root cause, related: a non-nil but unconstrained declaration (annotation-only, or x--only) is now tolerated, and resolveNameSchema applies legacyNameCaps only when declared == nil (rest.go:1330). So on the upgrade window a Kubernetes declaration that parses but carries no maxLength drops the 32-char legacy cap; an over-long name is admitted and only rejected later, on the KubernetesNodes worker-pool release. Realism is low (generated schemas carry a maxLength) and a signal still appears eventually, hence MINOR. Suggest: allowlist the specific x- prefixes meant to be ignored and reject x-kubernetes-* as unsupported, so gate and server agree and an author's constraint is not discarded in silence.

The "Set up build toolchain" steps installed flux with a bare `curl https://fluxcd.io/install.sh | sudo bash`, letting the installer resolve its floating "latest". latest had advanced to v2.9.5, and a matrix job on PR #4429 failed when that release's checksums transiently 4xx'd during install. Pin FLUX_VERSION to 2.8.6 and guard on the installed client version, the same shape the "Set up promotion toolchain" steps already use, so the flux CLI is fixed and reproducible across CI instead of following latest. This does not remove the checksum host's transient errors; it removes the uncontrolled version bump. Related to the yq floating-latest pin in #3640. Assisted-by: LLM Signed-off-by: IvanHunters <[email protected]>

## What this PR does The flux CLI was installed across the build, PR, tag, nightly and e2e-fork workflows and in the e2e-sandbox image through the vendor `install.sh` with no version pin, so each install resolved the installer's floating `latest`. `latest` had advanced to v2.9.5, and a matrix job on #4429 failed when that release's checksums transiently 4xx'd during install. Pinning does not stop the checksum host from returning a transient error, so it is not a fix for that flake; what it fixes is the uncontrolled version. This sets `FLUX_VERSION` to 2.8.6 and guards each flux install on the installed client version (workflows, with a fixed-string match) or passes it to `install.sh` (the e2e-sandbox image), matching the already-pinned "Set up promotion toolchain" steps, so the flux CLI is fixed and reproducible across CI with no flux install resolving floating `latest`. This is scoped to the flux CLI only: other tools some of these steps install (yq, helm, mc) are not touched here, and the yq floating-latest is tracked separately in #3640. ### Screenshots  Not applicable — CI and test-image change, no UI impact. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note ci(workflows): pin the flux CLI version across CI toolchain steps and the e2e-sandbox image so the flux CLI is fixed and reproducible instead of following the installer's floating latest ```  ## Summary by CodeRabbit * **Chores** * Build, release, and end-to-end test environments now use Flux CLI version 2.8.6. * Flux is installed when it is missing or does not match version 2.8.6; an existing matching installation is reused. * The end-to-end sandbox image now installs Flux version 2.8.6 by default.

Timofei Larkin (lllamnyp) added 3 commits September 24, 2026 09:35

Timofei Larkin (lllamnyp) requested review from IvanHunters, Andrei Kvapil (kvaps), Aleksei Sviridkin (lexfrei), myasnikovdaniil, Kirill Ilin (sircthulhu) and Timur Tukaev (tym83) as code owners September 24, 2026 06:35

github-actions Bot added area/api Issues or PRs related to the cozystack-api aggregated API server kind/bug Categorizes issue or PR as related to a bug size/L This PR changes 100-499 lines, ignoring generated files labels Sep 24, 2026

IvanHunters approved these changes Sep 24, 2026

View reviewed changes

Timofei Larkin (lllamnyp) enabled auto-merge September 24, 2026 12:01

This was referenced Sep 24, 2026

ci: yq is fetched from the floating latest release and installed as root in four privileged workflows #3640

Closed

ci(workflows): pin flux version across CI toolchain steps #4442

Merged

Timofei Larkin (lllamnyp) merged commit ce3eb30 into main Sep 24, 2026
87 of 88 checks passed

Timofei Larkin (lllamnyp) deleted the fix/name-schema-followups branch September 24, 2026 14:46

Aleksei Sviridkin (lexfrei) mentioned this pull request Sep 24, 2026

fix(vm-disk): report DataVolume readiness through WorkloadsReady #4380

Merged

1 of 12 tasks

Navigation Menu

Repository navigation

fix(api): refuse only what a name declaration actually breaks - #4429

fix(api): refuse only what a name declaration actually breaks#4429
Timofei Larkin (lllamnyp) merged 3 commits into
mainfrom
fix/name-schema-followups

Timofei Larkin (lllamnyp) commented Sep 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 24, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

IvanHunters left a comment

Uh oh!

IvanHunters Sep 24, 2026

Uh oh!

IvanHunters Sep 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Timofei Larkin (lllamnyp) commented Sep 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

How it is tested

Downstream repositories

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

IvanHunters left a comment

Choose a reason for hiding this comment

Verdict

Findings

Notes

Uh oh!

IvanHunters Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

IvanHunters Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Timofei Larkin (lllamnyp) commented Sep 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 24, 2026 •

edited

Loading