Repository navigation
fix(platform): order cozystack-basics after the APIs its admission policies type-check - #2842
Conversation
…licies type-check cozystack-basics ships cluster-wide ValidatingAdmissionPolicies matching tenants.apps.cozystack.io and the Gateway API types. KCM's VAP status type-checker panics on a nil schema when it type-checks a policy against a matched type that isn't resolvable yet; during a cold install this crashes KCM, which stalls cert/token issuance, so cozystack-api never starts and the tenants schema never resolves — a bootstrap deadlock. Add dependsOn edges so the package installs only after cozystack-engine (cozystack-api) and gateway-api-crds are Ready. Drift detection is off on operator-generated HelmReleases, so a template capability gate would drop the policy at first install and never re-add it; ordering is the reliable fix. Assisted-By: Claude <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
github-actions
Bot
added
size/M
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 resolves a cold-install deadlock where the kube-controller-manager would crash due to a nil-pointer dereference when validating admission policies against APIs that had not yet been served. By enforcing explicit dependency ordering, we ensure that 'cozystack-basics' only initializes after the necessary engine and gateway components are ready, preventing the race condition that led to the crash loop. 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
|
|
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)
📝 WalkthroughWalkthroughThis PR fixes ValidatingAdmissionPolicy ordering issues by adding explicit ChangesValidatingAdmissionPolicy ordering
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 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 |
![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 adds cozystack-engine and gateway-api-crds as dependencies for cozystack-basics to prevent a deadlock caused by kube-controller-manager panicking when type-checking ValidatingAdmissionPolicies before their corresponding schemas are resolved. It also introduces a regression test to verify these dependencies. The reviewer requests confirmation on whether this change preserves backward compatibility for existing installations during upgrades, in accordance with the repository style guide for changes to packages/core/platform/.
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.
| - cozystack.cozystack-engine | ||
| - cozystack.gateway-api-crds |
There was a problem hiding this comment.
According to the repository style guide, any changes to packages/core/platform/ require a corresponding migration update or an explicit note/verification that backward compatibility is preserved for existing installations. While the PR description explains why there is no cycle risk, please explicitly confirm if this change preserves backward compatibility for existing clusters during upgrades without requiring a migration hook update.
References
- Changes to packages/core/platform/ require a corresponding migration update or an explicit note that backward compatibility is preserved. (link)
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — adds dependsOn edges so cozystack-basics installs only after the APIs its ValidatingAdmissionPolicies match are served, removing the cold-install deadlock at its trigger; ships a regression test pinning the edges.
Business context: cozystack-basics ships cluster-wide VAPs matching tenants.apps.cozystack.io and the Gateway API types; on a cold install it can land before cozystack-api/gateway-api-crds are Ready, kube-controller-manager's VAP status type-checker nil-panics against the unresolvable schema and CrashLoops, which stalls cert/token issuance so the API never starts — a self-sustaining deadlock that times out every HelmRelease.
Verified: all three dependsOn targets exist as platform sources (cozystack-engine, gateway-api-crds, tenant-application); cozystack-basics is a dependency-graph leaf and the engine consumes none of its outputs, so the new edges add no cycle; the regression test asserts the edges. No migration is needed despite touching a platform source — dependsOn is a declarative HelmRelease field that Flux reconciles on upgrade, and on an existing cluster the targets are already Ready, so the edge is satisfied immediately with no imperative transformation to run (the deadlock is cold-install-only). The capability-gate alternative is correctly rejected: operator-generated HelmReleases are checksum-driven with drift detection off, so a .Capabilities.APIVersions.Has gate would render the policy out at first install and never re-add it once the API appears.
Non-blocking: the underlying KCM nil-panic is an upstream Kubernetes bug (the best-effort status type-checker should degrade, not crash kube-controller-manager); the PR notes it will be reported separately — worth filing so the platform-side ordering stays a defense-in-depth measure rather than the only guard.
2 participants
What this PR does
cozystack-basicsships cluster-wide ValidatingAdmissionPolicies — the gateway / route / tenant host policies — that matchtenants.apps.cozystack.io(served bycozystack-api) and the Gateway API types (gateways,httproutes,tlsroutes). kube-controller-manager's ValidatingAdmissionPolicy status type-checker panics with a nil-pointer dereference ink8s.io/apiserver/pkg/cel/openapiwhen it type-checks a policy against a matched type whose schema it cannot resolve yet.During a cold install this is a deadlock.
cozystack-basicscan install beforecozystack-apiis Ready; the tenant-host policy is created; KCM panics type-checking it against the not-yet-servedtenantsschema and CrashLoops; that stalls cert/token issuance, socozystack-apinever starts and the schema never resolves — KCM keeps crashing and every HelmRelease times out onDependencyNotReady.This adds
dependsOnedges socozystack-basicsinstalls only aftercozystack-engine(which servestenants.apps.cozystack.ioviacozystack-api) andgateway-api-crdsare Ready. The policies are then never created before the APIs they type-check are served.A template capability gate (
.Capabilities.APIVersions.Has) was considered and rejected: operator-generated HelmReleases run without drift detection, so helm-controller is checksum-driven — it would render the policy out at first install and never re-add it once the API appears, silently dropping the protection. Ordering viadependsOngates the first install on Ready, which is reliable.No cycle risk: nothing
dependsOncozystack-basics, and nocozystack-enginecomponent consumes acozystack-basicsoutput, so engine becomes Ready independently.Includes a helm-unittest regression test asserting the
dependsOnedges stay.The underlying nil-pointer panic is an upstream Kubernetes bug (the best-effort status type-checker should never crash kube-controller-manager); it is unguarded as of the current release and will be reported separately. This change removes the trigger on the cozystack side.
Release note
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests