fix(api): publish open spec as x-kubernetes-preserve-unknown-fields, not additionalProperties:true - #2867
Conversation
…not additionalProperties:true
cozystack-api published the free-form ".spec" of aggregated apps.cozystack.io
resources (Tenant, Application, ...) as the boolean `additionalProperties: true`.
The Kubernetes ValidatingAdmissionPolicy status type-checker run by
kube-controller-manager nil-dereferences when it recurses into that node, so as
soon as a VAP matches one of these resources (cozystack-tenant-host-policy on
tenants) KCM CrashLoopBackOffs cluster-wide and stalls reconciliation, failing
cold installs once the 3x install retry was removed.
Emit `x-kubernetes-preserve-unknown-fields: true` instead. It is semantically
equivalent ("arbitrary fields allowed") and the type-checker handles it safely.
All app custom schemas already use the safe object-form additionalProperties,
so patchSpec was the only source of the boolean form.
Adds a regression test that reproduces the exact crash through
k8s.io/apiserver/pkg/cel/openapi.SchemaDeclType (the entry point KCM uses): it
panics on the pre-fix code and passes after the fix.
Fixes #2863
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
github-actions
Bot
added
size/L
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReplace boolean-form ChangesVAP Type-Checker Crash Fix
Sequence Diagram(s)sequenceDiagram
participant patchSpec
participant sanitizeBooleanAdditionalProperties
participant markOpenObject
participant OpenAPIStore
patchSpec->>sanitizeBooleanAdditionalProperties: sanitize(userSchema)
sanitizeBooleanAdditionalProperties->>markOpenObject: convert additionalProperties:true -> marked-open object
patchSpec->>markOpenObject: mark root open if top-level AdditionalProperties was nil
patchSpec->>OpenAPIStore: publish sanitized schema (no boolean additionalProperties)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue where the cozystack-api's OpenAPI specification caused a cluster-wide crash in the kube-controller-manager. By switching from the boolean 'additionalProperties: true' to the semantically equivalent 'x-kubernetes-preserve-unknown-fields: true' extension, the API now provides a schema that is compatible with the Kubernetes ValidatingAdmissionPolicy status type-checker, ensuring stable cluster operations. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the boolean additionalProperties: true with the x-kubernetes-preserve-unknown-fields extension when marking schemas as free-form objects in pkg/cmd/server/openapi.go. This prevents a nil-dereference crash in the Kubernetes ValidatingAdmissionPolicy status type-checker. Comprehensive unit tests are added in pkg/cmd/server/openapi_test.go to verify this behavior and prevent regressions. The review feedback suggests a minor improvement to use the idiomatic spec.Extensions{} type instead of a generic map when initializing extensions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if s.Extensions == nil { | ||
| s.Extensions = map[string]any{} | ||
| } |
There was a problem hiding this comment.
To be more idiomatic and consistent with the kube-openapi package types, consider initializing s.Extensions using the defined type spec.Extensions{} instead of the generic map[string]any{}.
| if s.Extensions == nil { | |
| s.Extensions = map[string]any{} | |
| } | |
| if s.Extensions == nil { | |
| s.Extensions = spec.Extensions{} | |
| } |
There was a problem hiding this comment.
Done in 379bc584 — markOpenObject (and sanitizeForV2) now initialize s.Extensions with spec.Extensions{} instead of the generic map[string]any{}, matching the kube-openapi types and the generated definitions in zz_generated.openapi.go. No behavioral change, since spec.Extensions is defined as map[string]interface{}.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/cmd/server/openapi.go`:
- Around line 91-97: The code currently mutates an existing
target.Properties["spec"] which can retain a $ref or properties; instead,
replace the .spec entry with a fresh empty schema before marking it open. Ensure
target.Properties is initialized if nil, create a new empty spec schema (e.g., a
zero-value spec.Schema), call markOpenObject on that new schema, and then assign
it to target.Properties["spec"] so any prior $ref/properties are discarded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3cca421a-43ed-440f-a58e-d776a54b0ccb
📒 Files selected for processing (2)
pkg/cmd/server/openapi.gopkg/cmd/server/openapi_test.go
There was a problem hiding this comment.
NOT LGTM — the recursive sanitizer correctly closes the crash class (verified, including the false form), but its only remaining type-checker-reachable branch — a boolean additionalProperties nested inside an object-form additionalProperties — has zero test coverage: removing that recursion line leaves the whole suite green while the input panics SchemaDeclType again.
Business context: cozystack-api published the boolean additionalProperties: true on the free-form .spec of aggregated apps.cozystack.io resources, and KCM's ValidatingAdmissionPolicy status type-checker nil-dereferences on that construct, crash-looping KCM cluster-wide as soon as a VAP matches those resources (#2863).
Verified this round: the sanitizer covers a strict superset of what SchemaDeclType traverses (properties, items, additionalProperties.Schema); the false form indeed crashes too (the Allows value is never consulted before the nil dereference — confirmed against k8s.io/apiserver@v0.34.1); the new table test is a genuine red→green against the pre-sanitizer code; the fresh-schema rewrite of the schemaless branch drops the inherited $ref to apiextensions/v1.JSON (an improvement, and it addresses the CodeRabbit finding); the enforcement path (buildSpecSchema) consumes the raw schema string separately, so dropping additionalProperties: false only relaxes the published documentation, not server-side validation.
Blockers
B1: the one type-checker-reachable sanitizer branch is untested
File: pkg/cmd/server/openapi_test.go:218
Issue: TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties has no case with a boolean additionalProperties nested inside an object-form additionalProperties — e.g. {"type":"object","additionalProperties":{"type":"object","additionalProperties":true}}. That is the only nested location SchemaDeclType actually recurses into (besides properties/items, which are covered).
Evidence: mutation check on this branch's HEAD — deleting the else if ap.Schema != nil { sanitizeBooleanAdditionalProperties(ap.Schema) } branch keeps the entire suite green, while the input above then panics SchemaDeclType with the same SIGSEGV as #2863.
Impact: a future refactor of the sanitizer can silently reintroduce the cluster-wide KCM crash-loop; CI would not notice.
Fix: add the map-value case to the existing table. While there, one case each for anyOf/oneOf/not (currently only allOf is exercised, so deleting any of those recursion lines is also undetected) — those are defense-in-depth rather than reachable crash paths, but they are four more table rows.
Non-blocking follow-ups
- The comment on the
nested-under-allofcase says "Each case below panics SchemaDeclType before this sanitizer" — for the allOf case the pre-fix panic actually came from the then-unconditional top-leveladditionalProperties: trueinjection, not from the node insideallOf(SchemaDeclTypedoes not traverseallOf/anyOf/oneOf/not). Worth a half-sentence so future readers don't assume those branches are type-checker-reachable.
| } | ||
| if custom.AdditionalProperties == nil { | ||
| custom.AdditionalProperties = &spec.SchemaOrBool{Allows: true} | ||
| markOpenObject(&custom) |
There was a problem hiding this comment.
This still lets the crash construct through on two paths: a custom schema that explicitly declares boolean additionalProperties: true has a non-nil AdditionalProperties, so this branch is skipped and the boolean form is republished verbatim; and a boolean-form node nested deeper (under properties, items, allOf/anyOf/oneOf) is never visited at all. Both reproduce the same SchemaDeclType SIGSEGV on this branch's HEAD. Since kindSchemas comes from ApplicationDefinition.openAPISchema without validation, a recursive sanitizer here would close the class, not just the in-tree instance.
There was a problem hiding this comment.
Good catch — confirmed and fixed in 244de9d.
You're right that the previous revision only handled the wrapper injected when AdditionalProperties == nil, so both an explicitly-declared top-level boolean and a boolean nested under properties/items/allOf/anyOf/oneOf slipped through and re-triggered the SchemaDeclType panic. Since openAPISchema is untrusted input, I replaced the narrow guard with a recursive sanitizeBooleanAdditionalProperties that walks the whole schema and neutralizes every boolean-form additionalProperties at any depth.
One addition to your proposal: additionalProperties: false panics too — verified against pristine apiserver@v0.34.1:
additionalProperties:true -> panics=true
additionalProperties:false -> panics=true
The trigger is the nil inner *spec.Schema (SchemaOrBool{Schema: nil}), which IsXEmbeddedResource() dereferences before Allows is ever read — so false is just as fatal. The sanitizer handles both: true → x-kubernetes-preserve-unknown-fields: true (equivalent), false → drop the node (it can't become preserve-unknown without flipping "closed" to "open"; the published schema is type-check/doc-only, enforcement is in the apiserver, so "declared properties only" is preserved without the crash-prone node).
Regression tests now cover top-level true/false and nested boolean under properties, items, and allOf, asserting the published JSON carries no boolean additionalProperties node and that SchemaDeclType returns non-nil — each fails (panics) on the pre-sanitizer code.
…hed spec Addresses review feedback on #2863. The previous fix only converted the open spec wrapper cozystack-api injects, so a user-supplied ApplicationDefinition openAPISchema carrying a boolean additionalProperties — declared explicitly at the top level, or nested under properties/items/allOf/anyOf/oneOf — was republished verbatim and re-triggered the kube-controller-manager VAP type-checker crash. openAPISchema is untrusted input (external-apps and operator-authored definitions flow through unvalidated), so close the whole class rather than only the schemas cozystack ships. Recursively rewrite every boolean-form additionalProperties (nil inner schema): additionalProperties:true becomes x-kubernetes-preserve-unknown-fields:true, additionalProperties:false is dropped. Both JSON forms carry a nil inner schema and crash the type-checker identically; object-form maps are recursed into and left intact. Also publish the schemaless spec from a fresh schema so no inherited $ref or properties leak into the open object. Regression tests cover top-level true/false and nested (properties, items, allOf) boolean forms, asserting the published schema carries no boolean additionalProperties node and does not panic SchemaDeclType. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/cmd/server/openapi_test.go (1)
221-228: ⚡ Quick winCover
anyOfandoneOfin the sanitizer regression matrix.
sanitizeBooleanAdditionalPropertiesnow walks both branches inpkg/cmd/server/openapi.go, but this table only locks downproperties,items, andallOf. Adding one representativeanyOfandoneOfcase would keep those recursion paths from regressing silently.Suggested table additions
cases := map[string]string{ "top-level-boolean-true": `{"type":"object","additionalProperties":true}`, "top-level-boolean-false": `{"type":"object","additionalProperties":false}`, "nested-under-properties": `{"type":"object","properties":{"foo":{"type":"object","additionalProperties":true}}}`, "nested-under-items": `{"type":"object","properties":{"list":{"type":"array","items":{"type":"object","additionalProperties":true}}}}`, "nested-under-allof": `{"type":"object","allOf":[{"type":"object","additionalProperties":false}]}`, + "nested-under-anyof": `{"type":"object","anyOf":[{"type":"object","additionalProperties":true}]}`, + "nested-under-oneof": `{"type":"object","oneOf":[{"type":"object","additionalProperties":false}]}`, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/cmd/server/openapi_test.go` around lines 221 - 228, The test TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties misses recursion paths for anyOf and oneOf; update the cases map in pkg/cmd/server/openapi_test.go (inside TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties) to include representative entries for "nested-under-anyof" and "nested-under-oneof" (JSON strings where an object contains anyOf/oneOf with an object that has "additionalProperties":true or false) so sanitizeBooleanAdditionalProperties in pkg/cmd/server/openapi.go is exercised for those branches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/cmd/server/openapi_test.go`:
- Around line 221-228: The test
TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties misses recursion
paths for anyOf and oneOf; update the cases map in
pkg/cmd/server/openapi_test.go (inside
TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties) to include
representative entries for "nested-under-anyof" and "nested-under-oneof" (JSON
strings where an object contains anyOf/oneOf with an object that has
"additionalProperties":true or false) so sanitizeBooleanAdditionalProperties in
pkg/cmd/server/openapi.go is exercised for those branches.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f4a1bba2-bebf-4352-a77e-1ef482a06c1b
📒 Files selected for processing (2)
pkg/cmd/server/openapi.gopkg/cmd/server/openapi_test.go
dosubot
Bot
added
priority/critical-urgent
…anic The e2e cluster bootstrap let `talosctl gen config` pick its bundled default Kubernetes version (v1.33.1 for Talos v1.13.0). That version predates the fix for the kube-controller-manager ValidatingAdmissionPolicy status type-checker nil-pointer panic on `additionalProperties: true` schemas (kubernetes/kubernetes#135155, backported to release-1.33 in #136958, first released in v1.33.10). cozystack-api's aggregated Tenant schema publishes `additionalProperties: true`, so on the older default KCM crash-loops cluster-wide during install and fails E2E. Pin the e2e management cluster to v1.33.12 (latest 1.33 patch) so it runs a Kubernetes that contains the fix. This complements the schema-side fix in #2867 and is independent of it. Refs: #2863 Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
myasnikovdaniil
added
debug
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — re-reviewed at head e043ddc7e: the diff against the previously reviewed 244de9d85 is empty, so B1 from the previous review carries over unchanged, and the branch has picked up a new empty CI-trigger commit on top.
Blockers
B1 (carried over): the nested-in-additionalProperties sanitizer branch is still untested
File: pkg/cmd/server/openapi_test.go
Issue: unchanged since the previous review — TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties still has no case with a boolean additionalProperties nested inside an object-form additionalProperties, the only nested location SchemaDeclType actually recurses into besides properties/items.
Evidence: git diff 244de9d85..e043ddc7e is empty — no test changes since the finding was raised.
Fix: as before — add the map-value case to the table (plus optional anyOf/oneOf/not rows for defense-in-depth).
B2: drop the empty CI-trigger commit before merge
Issue: e043ddc7e ("ci: trigger e2e debug run") is an empty commit (no file changes) pushed to trigger a CI run. This repo merges with merge commits only, so it would land in main history permanently.
Evidence: git show e043ddc7e --stat lists zero changed files.
Fix: once the debug run has served its purpose, drop it (git reset --hard 244de9d85 + force-push). Same situation as on #2868.
…n sanitizer regression Address review feedback from lexfrei and coderabbitai on pkg/cmd/server/openapi_test.go: TestPatchSpecSanitizesUserSuppliedBooleanAdditionalProperties only exercised the top-level, properties, items, and allOf positions, leaving the one remaining type-checker-reachable sanitizer branch untested: a boolean additionalProperties nested inside an object-form additionalProperties. Deleting that recursion line kept the whole suite green while the input panicked SchemaDeclType again (#2863). Add the nested-under-additionalproperties case (the reachable blocker, verified red->green by mutation) plus anyOf/oneOf/not cases (defense-in-depth: those branches are recursed by the sanitizer but never traversed by SchemaDeclType, so the hasBooleanAdditionalProperties assertion guards them). Rewrite the test docstring to distinguish the type-checker-reachable cases from the defense-in-depth ones, replacing the inaccurate "each case panics SchemaDeclType" note. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Address review feedback from gemini-code-assist on pkg/cmd/server/openapi.go:
markOpenObject and sanitizeForV2 initialized s.Extensions with the generic
map[string]any{}; use the named spec.Extensions{} type instead, matching the
kube-openapi types and the generated definitions in zz_generated.openapi.go.
No behavioral change — spec.Extensions is defined as map[string]interface{}.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
e043ddc to
379bc58
Compare
|
Aleksei Sviridkin (@lexfrei) both blockers addressed:
|
|
CodeRabbit (@coderabbitai) added the |
|
Tip For best results, initiate chat on the files or code changes.
The distinction in the docstring between type-checker-reachable cases (guarded by the If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
…anic (#2868) ## What this PR does The e2e cluster bootstrap (`hack/e2e-prepare-cluster.bats`) let `talosctl gen config` pick its bundled default Kubernetes version — **v1.33.1** for Talos v1.13.0. v1.33.1 predates the fix for the kube-controller-manager `ValidatingAdmissionPolicy` status type-checker nil-pointer panic on `additionalProperties: true` schemas ([kubernetes/kubernetes#135155](kubernetes/kubernetes#135155), backported to `release-1.33` in [#136958](kubernetes/kubernetes#136958), first released in **v1.33.10**). Because `cozystack-api` publishes `additionalProperties: true` on the aggregated Tenant schema, KCM crash-loops cluster-wide during install on the old default and fails E2E (#2863). This pins the e2e **management** cluster to **v1.33.12** (latest 1.33 patch), which contains the fix. It complements — and is independent of — the schema-side fix in #2867; either alone avoids the panic. Refs: #2863 ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated end-to-end test cluster configuration to explicitly pin the Kubernetes version for more consistent test execution. * Added explanatory comments clarifying the version choice and kept existing secret/configuration wiring intact. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — both blockers from my prior reviews are resolved with proof; the fix is correct and go test/go vet are clean.
Business context: cozystack-api published the boolean additionalProperties: true on the free-form .spec of aggregated apps.cozystack.io resources; KCM's ValidatingAdmissionPolicy status type-checker nil-dereferences on that construct, crash-looping KCM cluster-wide once a VAP matches those resources (#2863).
Resolved since the previous review:
- B1 (the
additionalProperties.Schemarecursion branch was untested) — the table now carriesnested-under-additionalproperties({"type":"object","additionalProperties":{"type":"object","additionalProperties":true}}) plusanyof/oneof/not. Verified by mutation: deleting theap.Schemarecursion line makes that case panicSchemaDeclTypewith the #2863 SIGSEGV, so the test now genuinely guards the crash rather than passing vacuously. - B2 (empty CI-trigger commit) — dropped; the branch is four clean commits, none empty.
- The earlier note on the misleading
allOfcomment is addressed: the comment now correctly separates the type-checker-reachable cases (properties / items / additionalProperties.Schema) from defense-in-depth (allOf / anyOf / oneOf / not, whichSchemaDeclTypenever traverses).
Also verified: patchSpec captures topLevelAPAbsent before sanitizing, so an explicit additionalProperties:false is respected rather than re-opened; the schemaless branch starts from a fresh schema, dropping the inherited $ref; the spec.Extensions{} refactor is a semantically-identical idiomatic alias. Full pkg/cmd/server suite passes and go vet is clean against vendored k8s.io/apiserver@v0.34.1.
Non-blocking follow-ups
## What this PR does > **Supersedes #2610.** Same work, rebased cleanly onto current `main` so it now sits on top of #2867 (the `kube-controller-manager` VAP type-checker fix). That fix is what was killing the base install in #2610's E2E *before* the `kubernetes` app test ever ran, leaving the Talos worker-bootstrap rewrite verified only by code reading. #2610's 67-commit branch is squashed here into 8 logical commits with original authorship preserved (@kvaps and @IvanHunters as authors); the tree is **byte-identical** to #2610's head — no code changes, just the rebase onto `main` (to pick up #2867) plus history cleanup. The detailed review history lives on #2610. Phase 1 of the Kubernetes-app split design (see cozystack/community#8): replace the Ubuntu+kubeadm worker bootstrap path with Talos, driven by `cluster-api-bootstrap-provider-talos` (CABPT) and a `clastix/talos-csr-signer` sidecar embedded in the Kamaji control-plane pod. Existing tenants keep working — old machines roll out and get replaced by Talos workers without manual intervention. Highlights: - Add CABPT (v0.6.12) as a second `BootstrapProvider` alongside the existing kubeadm one in `capi-providers-bootstrap`. - Bump the Kamaji control-plane provider with an upstream-bound patch that exposes `KamajiControlPlane.spec.network.additionalServicePorts`, used to publish trustd (50001/TCP) on the apiserver service. - Generate Talos PKI (Ed25519 CA + trustd TLS) via cert-manager and stable random Talos secrets (`token`, `clusterId`, `clusterSecret`, `bootstrapToken`) using the helm lookup-and-reuse pattern. - Materialise a kubeadm-format `bootstrap-token-<id>` Secret inside the tenant `kube-system` via a Helm post-install/upgrade Job. - Render a `TalosConfigTemplate` (worker machineconfig, `generateType: none`) and switch the `MachineDeployment.bootstrap` reference from `KubeadmConfigTemplate` to `TalosConfigTemplate`, gated on Talos secrets and the Kamaji apiserver service being ready. - Boot workers from the Talos openstack image via a CDI `DataVolume` (`source.http.url` → `factory.talos.dev`), expose the system disk as virtio-blk with `blockSize.custom: logical=512, physical=4096` so 4Ki-native block storage backends (LINSTOR/DRBD) play nicely with QEMU's O_DIRECT writes and SeaBIOS still boots, drop the separate kubelet disk (Talos lays out EPHEMERAL itself). - Pin pod/service CIDRs in the worker machineconfig to the tenant ranges (`10.243.0.0/16` / `10.95.0.0/16`) to avoid Talos's address-overlap diagnostic against the host pod CIDR. - Set `cluster.controlPlane.endpoint` and `cilium.k8sServiceHost` to the Kamaji apiserver Service ClusterIP (looked up at render time), so bootstrap survives the chicken-and-egg moment where tenant DNS does not exist yet and the host CoreDNS does not serve the tenant zone. End-to-end on a dev cluster: a fresh tenant brings up a Talos worker, kubelet CSRs get approved (apiserver-client by Kamaji, kubelet-serving by the talos-csr-signer sidecar), Cilium initialises with hostNetwork against the apiserver ClusterIP, CoreDNS comes up, and the node transitions to `Ready`. ### Release note ```release-note feat(kubernetes): bootstrap tenant workers with Talos Linux instead of Ubuntu+kubeadm. Existing clusters roll over to Talos workers automatically; new tenants come up on Talos from the start. Powered by cluster-api-bootstrap-provider-talos and the talos-csr-signer sidecar in the Kamaji control-plane pod. Note: the tenant `kubernetes` HelmRelease now reports Ready as soon as Helm completes (Install/Upgrade DisableWait) and no longer blocks on worker/addon readiness — worker-rollout health is tracked by WorkloadMonitor, not the HelmRelease. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes **New Features** * Added Talos worker configuration for `talos.version` and `talos.schematicID` * Introduced `nodeHealthCheck` (`maxUnhealthy`, `nodeStartupTimeout`) for worker remediation tuning * Added image overrides for `kubectl` and `talosCsrSigner` * Exposed additional tenant control-plane Service ports support **Breaking Changes** * Removed Kubernetes v1.30; supported versions start at v1.31 * Renamed `nodeGroups.ephemeralStorage` → `nodeGroups.diskSize` with consolidated Talos system-disk semantics **Removals** * Removed Ubuntu container-disk images and related containerd patch mechanisms * Air-gapped tenant workers temporarily unsupported during the Talos transition **Documentation** * Updated chart docs, parameters, and GitOps upgrade guidance for version bumps <!-- end of auto-generated comment: release notes by coderabbit.ai -->
2 participants
What this PR does
cozystack-apipublished the free-form.specof aggregatedapps.cozystack.ioresources (Tenant, Application, …) as the booleanadditionalProperties: true. The Kubernetes ValidatingAdmissionPolicy status type-checker run bykube-controller-managernil-dereferences when it recurses into that node (k8s.io/apiserver/pkg/cel/openapi.isExtension). As soon as a VAP matches one of these resources —cozystack-tenant-host-policymatchestenants— KCMCrashLoopBackOffs on every control-plane node, stalls reconciliation, and times out the install (kubectl wait hr --allnever completes). With the 3× install retry removed, a single occurrence fails E2E outright.This emits
x-kubernetes-preserve-unknown-fields: trueinstead, inpkg/cmd/server/openapi.go::patchSpec(both the schemaless and custom-schema branches, covering the OpenAPI v2 and v3 post-processors). It is semantically equivalent — "arbitrary fields allowed" — and the type-checker handles it safely. All managed-app custom schemas already use the safe object-formadditionalProperties, sopatchSpecwas the only source of the boolean form.Regression test
TestPatchedSpecDoesNotPanicVAPTypeCheckerreproduces the exact crash throughk8s.io/apiserver/pkg/cel/openapi.SchemaDeclType— the entry point KCM'svalidatingadmissionpolicystatuscontroller uses. It panics on the pre-fix code and passes after the fix. Companion structural tests assert the published spec carriesx-kubernetes-preserve-unknown-fields: trueand never the booleanadditionalProperties: true, and that genuine object-form maps are left untouched.Verified against the pristine vendored
k8s.io/apiserver@v0.34.1(the version cozystack-api builds against), which reproduces the panic, and previously confirmed end-to-end onkindest/node:v1.33.1.Upstream
The underlying nil-dereference is a Kubernetes KCM bug — the VAP status type-checker should treat
additionalProperties: trueas dynamic rather than panic. A separate upstream report is being prepared; cozystack fixes the symptom immediately with the schema change above.Fixes #2863
Release note
Summary by CodeRabbit
Bug Fixes
Tests