Skip to content

Commit 146e06e

Browse files
authored
feat(api): take Application name limits from the application's own schema (#4367)
## What this PR does The aggregated API server capped two kinds' names by knowing what their charts render. A `Kubernetes` cluster name was held to 32 characters because every worker pool is a `KubernetesNodes` release named `kubernetes-nodes-<name>-<pool>`, and the default `md0` pool has to fit the 53-character Helm release name limit. A `Kafka` name was held to 44 because the KRaft controller pod `kafka-<name>-c-<hash>-<id>` has to fit the 63-character DNS-1123 label limit. The arithmetic is right, but it is the charts' knowledge living in the platform: the next application with a sub-resource budget needs another `if r.kindName == ...` branch, and an application served from outside this repository — the whole point of `ApplicationDefinition` — cannot get one at all. An application now declares the limits on its own name in its own schema, and the server applies whatever it finds there for whatever kind the definition describes. The chart's `values.yaml` carries a `## @name` annotation, `cozyvalues-gen` v1.7.0 publishes it at the root of `values.schema.json` as `x-cozystack-name`, and `hack/update-crd.sh` carries it into the `ApplicationDefinition`: ```yaml ## @name {string} - Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit. ## @maxlength 32 ``` ```json "x-cozystack-name": { "type": "string", "description": "Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.", "maxLength": 32 } ``` `maxLength`, `minLength` and `pattern` are honoured; `pattern` is unanchored, as JSON Schema specifies. A declaration only tightens: `prefix + name` must still fit the 53-character Helm limit. A rejected name is answered with the chart's own description, or with "limit declared by the <Kind> application" when the chart gives none. The schema is untrusted input — definitions from other repositories are written by hand — so a declaration that cannot be enforced as written (not an object, a type error, an unknown keyword such as a misspelled `maxLenght`, `maxLength` below 1, a negative `minLength`, `minLength` above `maxLength` or above the Helm budget, a pattern that is not RE2) refuses every create of that kind with `configuration error: the <Kind> ApplicationDefinition declares an invalid x-cozystack-name: …`, and logs the same at start-up. Dropping the declaration would admit names the chart cannot render; applying it would reject every name and blame the user. Other kinds are unaffected, and existing objects of the affected kind can still be read, updated and deleted. An upgrade brings `cozystack-api` up before `kubernetes-rd` and `kafka-rd` re-render (`kubernetes-rd` depends on `cozystack-engine`), so the new server can briefly read a definition that predates its declaration. For exactly that case — a definition that declares nothing — the two previous caps stay in `legacyNameCaps` as a floor, to be removed once no supported upgrade starts from such a definition. That is the only per-kind knowledge left in the name checks. Also in this PR: - The `pre-commit` job pins `cozyvalues-gen` v1.7.0, the first release that understands `@name` ([cozystack/cozyvalues-gen#33](cozystack/cozyvalues-gen#33)). - The API review gate now diffs the root `x-cozystack-name` of every definition as `metadata.name`, so a chart that lowers its cap or adds a pattern is reported like any other tightened field. On this PR it reports the Kubernetes and Kafka caps as added; their effective limits are unchanged. - The migration 56 comment that pointed at the removed `maxKubernetesClusterName` now points at the chart's declaration. Not in scope, because none of it is a static limit a chart can declare: the `Tenant` name pattern in `pkg/apis/apps/validation`, the computed tenant-namespace length check (it depends on the parent namespace at request time), the tenant quota hierarchy, and the Phase 2 removed-field warning for `Kubernetes`. ### How it is tested `TestNameCapComesFromTheShippedSchema` reads the shipped `kubernetes` and `kafka` ApplicationDefinitions off disk, builds storage through `NewREST` as the server does, and checks the cap and the chart's explanation; it then rewrites `maxLength` inside the shipped schema and checks the limit moves with it, which a cap held in code would not. `TestInvalidNameSchemaFailsClosed` covers each malformed declaration through the same path, `TestLegacyNameCapsCoverUndeclaredDefinitions` pins the upgrade floor and that a declaration replaces it, and `TestClassifyNameDeclaration` covers the gate. ### Downstream repositories Two repositories are affected and neither follow-up is filed yet, so no box is ticked. - `cozystack/website` — `content/en/docs/next/development.md` documents the `values.yaml` annotations and needs `@name`; `content/en/docs/next/cozystack-api/application-definitions.md` describes `openAPISchema` and needs `x-cozystack-name`. - `cozystack/external-apps-example` — it pins no `cozyvalues-gen` version and regenerates against whatever is current; nothing in it declares a name constraint, so it is unaffected until it chooses to. - [ ] 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 feat(api): Application name limits now come from the application's own schema instead of being hardcoded per kind in the aggregated API server. A chart declares them with a `## @name` annotation in `values.yaml`, which cozyvalues-gen v1.7.0 publishes as `x-cozystack-name`; the API server enforces `maxLength`, `minLength` and `pattern` on `metadata.name` for any kind, answers a rejected name with the chart's own explanation, and refuses creates of a kind whose declaration it cannot enforce with a configuration error. The Kubernetes (32) and Kafka (44) name caps are unchanged in effect and are now declared by their charts. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Applications can declare minimum and maximum name lengths and formatting patterns. * Kubernetes cluster names are limited to 32 characters, and Kafka cluster names to 44 characters, to meet generated resource naming limits. * Names that violate declared constraints are rejected with an explanatory message. Tighter name constraints are also reported as breaking changes. * Declared maximum lengths cannot exceed the platform’s existing Helm release-name budget. * **Documentation** * Added guidance on declaring and enforcing chart-specific resource-name limits. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 parents ff52aeb + 8f2a6bc commit 146e06e

16 files changed

Lines changed: 549 additions & 110 deletions

File tree

‎.github/workflows/pre-commit.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ jobs:
5454
5555
- name: Install generate
5656
run: |
57-
curl -sSL https://github.com/cozystack/cozyvalues-gen/releases/download/v1.6.0/cozyvalues-gen-linux-amd64.tar.gz | tar -xzvf- -C /usr/local/bin/ cozyvalues-gen
57+
curl -sSL https://github.com/cozystack/cozyvalues-gen/releases/download/v1.7.0/cozyvalues-gen-linux-amd64.tar.gz | tar -xzvf- -C /usr/local/bin/ cozyvalues-gen
5858
5959
- name: Run pre-commit hooks
6060
run: |

‎docs/agents/overview.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ Build environment variables:
9999

100100
Package `values.yaml` files carry annotations (`@param`, `@typedef`, `@field`, `@enum`, `@section`) that `cozyvalues-gen` turns into `values.schema.json` and `README.md`. Run `make generate` in the package after editing annotations. A pre-commit hook runs `make generate` across `packages/apps/*` and `packages/extra/*` on every commit to keep these in sync — see [`contributing.md`](./contributing.md) for the regenerate-before-commit rule.
101101

102+
A chart also declares the limits on its own resource name there, with `## @name {string} - <why>` followed by the constraint tags (`@maxLength`, `@minLength`, `@pattern`) it needs. `cozyvalues-gen` publishes the declaration at the root of `values.schema.json` as `x-cozystack-name`, from where `hack/update-crd.sh` carries it into the ApplicationDefinition, and the aggregated API server enforces it on `metadata.name` for whatever kind the definition describes. The description is the message a rejected name is answered with, so write it as the explanation of the limit. A `@pattern` is unanchored, as JSON Schema specifies, so one meant for the whole name is written `^...$`. A declaration the server cannot enforce as written — an unknown keyword, a wrong type, a limit no name satisfies, a pattern that is not RE2 — refuses every create of that kind with a configuration error rather than being dropped. A cap only ever tightens: `prefix + name` must still fit the 53-character Helm release name limit, and a declaration looser than that budget does not widen it. The limits belong in the chart because that is where the facts behind them are — which sub-resources it renders and what release names they take — and a chart that needs nothing beyond the platform-wide limits declares nothing.
103+
102104
## Testing
103105

104106
- **Helm unit tests:** `make helm-unit-tests` (runs `hack/helm-unit-tests.sh` over every package that defines a `test` target). A `test` target is expected to run helm-unittest: the sweep fails a package whose run reports no suite, because exiting 0 having asserted nothing is indistinguishable from passing. A target that drives something else belongs under another name, as `packages/core/testing` already does with its sandbox flow. `make unit-tests` runs the unit suite — Helm, BATS, Go, and the preset/readiness checks. It does not reach the controller and contract tests under `./internal/...`, which have their own target, `make test-controllers`, so covering both locally means running that target as well; CI runs them as two steps of one job.

‎internal/apigate/classify.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ func diffResource(base, head Resource) []string {
157157
for _, c := range diffSchema("spec", base.Versions[v], headSchema) {
158158
out = append(out, fmt.Sprintf("[%s] %s", v, c))
159159
}
160+
for _, c := range diffNameSchema(base.Versions[v], headSchema) {
161+
out = append(out, fmt.Sprintf("[%s] %s", v, c))
162+
}
160163
}
161164
return out
162165
}

‎internal/apigate/classify_test.go‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,40 @@ func TestClassify(t *testing.T) {
126126
}
127127
})
128128
}
129+
130+
// TestClassifyNameDeclaration covers the constraints an application declares
131+
// on its own metadata.name at the schema root: tightening one rejects names
132+
// the base accepted, loosening or dropping one does not.
133+
func TestClassifyNameDeclaration(t *testing.T) {
134+
kube := func(declaration map[string]any) Resource {
135+
s := Schema{"type": "object", "properties": map[string]any{}}
136+
if declaration != nil {
137+
s["x-cozystack-name"] = declaration
138+
}
139+
return res("apps.cozystack.io", "Kubernetes", "kuberneteses", SourceCozyRD, s)
140+
}
141+
capAt := func(n float64) map[string]any { return map[string]any{"type": "string", "maxLength": n} }
142+
143+
for _, tc := range []struct {
144+
name string
145+
base, head Resource
146+
breaking bool
147+
}{
148+
{"cap added", kube(nil), kube(capAt(32)), true},
149+
{"cap lowered", kube(capAt(32)), kube(capAt(30)), true},
150+
{"pattern added", kube(capAt(32)), kube(map[string]any{"type": "string", "maxLength": 32.0, "pattern": "^[a-z]+$"}), true},
151+
{"cap raised", kube(capAt(32)), kube(capAt(40)), false},
152+
{"cap dropped", kube(capAt(32)), kube(nil), false},
153+
{"unchanged", kube(capAt(32)), kube(capAt(32)), false},
154+
} {
155+
t.Run(tc.name, func(t *testing.T) {
156+
got := countCategory(Classify(snap(tc.base), snap(tc.head)), Breaking)
157+
if tc.breaking && got != 1 {
158+
t.Fatalf("expected 1 breaking finding, got %d", got)
159+
}
160+
if !tc.breaking && got != 0 {
161+
t.Fatalf("expected no breaking finding, got %d", got)
162+
}
163+
})
164+
}
165+
}

‎internal/apigate/schemadiff.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"fmt"
2121
"sort"
2222
"strings"
23+
24+
appsv1alpha1 "github.com/cozystack/cozystack/pkg/apis/apps/v1alpha1"
2325
)
2426

2527
// diffSchema walks a base and head OpenAPIv3Schema node in lockstep and
@@ -39,6 +41,32 @@ func diffSchema(path string, base, head Schema) []string {
3941
return out
4042
}
4143

44+
// diffNameSchema reports a tightened constraint on metadata.name. An
45+
// application declares those beside "properties" at the schema root, where the
46+
// walk over spec never looks. The type is left out: a name is a string whether
47+
// or not the declaration says so.
48+
func diffNameSchema(base, head Schema) []string {
49+
headName := nameDeclaration(head)
50+
if headName == nil {
51+
return nil
52+
}
53+
return diffSchema("metadata.name", nameDeclaration(base), headName)
54+
}
55+
56+
func nameDeclaration(s Schema) Schema {
57+
declared, ok := s[appsv1alpha1.NameSchemaExtension].(map[string]any)
58+
if !ok {
59+
return nil
60+
}
61+
out := Schema{}
62+
for k, v := range declared {
63+
if k != "type" {
64+
out[k] = v
65+
}
66+
}
67+
return out
68+
}
69+
4270
func diffNode(path string, base, head Schema, out *[]string) {
4371
// A nil head means the schema at this path was removed entirely. Dropping a
4472
// constraint is a widening, never a break, so there is nothing to report.

‎packages/apps/kafka/values.schema.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
{
22
"title": "Chart Values",
33
"type": "object",
4+
"x-cozystack-name": {
5+
"type": "string",
6+
"description": "Kafka cluster name. The KRaft controller pods are named `kafka-\u003cname\u003e-c-\u003chash\u003e-\u003cid\u003e` with an 8-character hash, and that pod hostname must fit the 63-character DNS-1123 label limit even for the smallest cluster.",
7+
"maxLength": 44
8+
},
49
"properties": {
510
"external": {
611
"description": "Enable external access from outside the cluster.",

‎packages/apps/kafka/values.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
## @name {string} - Kafka cluster name. The KRaft controller pods are named `kafka-<name>-c-<hash>-<id>` with an 8-character hash, and that pod hostname must fit the 63-character DNS-1123 label limit even for the smallest cluster.
2+
## @maxLength 44
3+
14
##
25
## @section Common parameters
36
##

‎packages/apps/kubernetes/values.schema.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
{
22
"title": "Chart Values",
33
"type": "object",
4+
"x-cozystack-name": {
5+
"type": "string",
6+
"description": "Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-\u003cname\u003e-\u003cpool\u003e`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.",
7+
"maxLength": 32
8+
},
49
"properties": {
510
"storageClass": {
611
"description": "Default StorageClass inside the tenant cluster. Remote-accessible LINSTOR classes are auto-propagated to the tenant under the same name.",

‎packages/apps/kubernetes/values.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
## @name {string} - Cluster name. Every worker pool is a separate KubernetesNodes release named `kubernetes-nodes-<name>-<pool>`, so the name must leave room for the default `md0` pool inside the 53-character Helm release name limit.
2+
## @maxLength 32
3+
14
##
25
## @section Common Parameters
36
##

‎packages/core/platform/images/migrations/migrations/56‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,9 @@ kubectl get helmrelease --all-namespaces \
423423
continue
424424
fi
425425

426-
# Helm refuses release names longer than 53 characters. rest.go today caps a
427-
# new cluster name at 32 (maxKubernetesClusterName = 53 - len("kubernetes-nodes-")
428-
# - len("-md0")) precisely so its pools fit, but a legacy or hand-created
426+
# Helm refuses release names longer than 53 characters. The kubernetes chart
427+
# caps a new cluster name at 32 (its `## @name` declaration, enforced by
428+
# cozystack-api) precisely so its pools fit, but a legacy or hand-created
429429
# cluster name can be longer, and combined with the longer kubernetes-nodes-
430430
# prefix the child name can overflow. Do NOT fail the whole migration
431431
# here: exiting would deadlock the platform pre-upgrade hook for EVERY

0 commit comments

Comments
 (0)