Repository navigation
refactor(api): keep each app's kind without the apimachinery fork - #4472
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Conversation
Every apps kind is registered on the one Application Go type, and the stock Scheme.ConvertToVersion picks the first kind registered for a type, so without help a Redis comes back from the API as whichever app kind was registered first. So far the fix lived in a fork of k8s.io/apimachinery that patches ConvertToVersion. Upstream has not taken the change, and the fork has to be rebased onto every Kubernetes minor before the libraries can move. The apps group now encodes through a serializer that picks the kind the object carries, along the lines upstream suggested instead of changing Scheme, and the replace directive goes. It relies on the storage stamping the kind on every object it returns, which it does. A test drives create, get, list, update, watch, apply and table through the real handler chain for two kinds and fails without the serializer on upstream apimachinery. Two paths convert through the Scheme rather than the serializer and go back to the first registered kind: the row objects of a Table requested with includeObject=Object, which kubectl asks for only with --sort-by, and the object a MutatingAdmissionPolicy hands to the plugins after it. Admission webhooks and ValidatingAdmissionPolicy get the request kind set explicitly, and no mutating policy targets apps.cozystack.io. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository: cozystack/cozystack/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
Folded into #4473 together with the rest of the stack, so the Kubernetes bump lands as one PR. |
…hinery fork (#4473) ## What this PR does This moves the root module's `k8s.io/*` from v0.35 to v0.37.1 and controller-runtime from v0.23 to v0.25.1, and drops the `k8s.io/apimachinery` fork on the way. Supersedes #3001, #4467 and #4472; those were a stack of the same commits. The four commits are meant to be read one by one, and each builds and passes the tests on its own: 1. **Plain watches stop getting the initial-events-end bookmark.** The apiserver defaults `sendInitialEvents=true` onto any watch with an empty or `0` resourceVersion and leaves bookmarks off, and our aggregated storages took that flag alone as a WatchList request. From `k8s.io/apiserver` v0.36 the watch handler compiled into cozystack-api panics on that bookmark, so `kubectl get -w` on any cozystack-api resource would die after the bump. The marker now needs both flags, the same condition the watch cache uses. There are tests per storage and one through the real handler chain. 2. **The apimachinery fork goes.** Every apps kind shares the `Application` Go type, and stock `Scheme.ConvertToVersion` picks the first kind registered for it. The fork patched that; the apps group now encodes through a serializer that keeps the kind the object carries. A handler-chain test covers create, get, list, update, watch, apply and table for two kinds and fails without it. 3. **MutatingAdmissionPolicy stays out of admission.** From v0.36 the plugin is on by default and refuses every write until it has synced `mutatingadmissionpolicies`, which cozystack-api has no RBAC to list and which clusters before 1.36 do not serve. Without this, every create, update and delete would fail after the bump. It is also re-applied in `Complete`, since `--disable-admission-plugins` replaces the list. 4. **The bump itself**, with the regenerated clients, openapi and strategy CRDs. Commits 2 and 4 are where the review effort goes; 1 and 3 are small. Known changes in behaviour: - Without the fork, the row objects of a Table requested with `includeObject=Object` carry the first registered app kind. kubectl asks for that only with `--sort-by`, and it sorts on fields, not on the kind. Nothing at this layer can fix it, because the handler converts through `APIGroupInfo.Scheme`, a concrete `*runtime.Scheme`. Admission webhooks and ValidatingAdmissionPolicy still see the right kind. - The alpha PodSpec field `workloadRef` became `schedulingGroup` upstream. Once the new strategy CRDs are applied, the apiserver prunes `workloadRef` from stored strategies it serves, and their next write erases it. It only ever worked behind an alpha feature gate. The branch is rebased onto the Renovate rollup (#4447), with `go mod tidy` and codegen rerun on top of it. ### Screenshots No UI changes. ### 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 chore(deps): cozystack-api and the controllers build against the Kubernetes 1.37 libraries without the apimachinery fork. Plain watches no longer receive the WatchList initial-events-end bookmark, and objects in Table rows requested with includeObject=Object may carry another app's kind. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application API responses now preserve the requested resource kind across operations, including lists and watches. * Backup strategy resources support additional Kubernetes pod settings, including probe protocols, volume ownership options, and scheduling groups. * Backup strategy schemas include updated Kubernetes pod fields and descriptions. * **Bug Fixes** * Initial-event watch bookmarks are sent only when requested; plain watches continue without an ending bookmark. * Mutating Admission Policy is disabled by default, including when admission plugin options are customized. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
Stacked on #4467; the diff here is one commit on top of it.
This drops the
k8s.io/apimachineryfork. Every apps kind (Postgres, Redis, and the rest) is registered on the oneApplicationGo type, and stockScheme.ConvertToVersionpicks the first kind registered for a type, so without help a Redis comes back from the API labelled as whichever app was registered first. The fork patchedConvertToVersionto fix that. Upstream has not merged the patch (kubernetes/kubernetes#135537), and the fork has to be rebased onto every Kubernetes minor before we can move the libraries.The apps group now encodes through a serializer that uses the kind the object carries, which is the direction the upstream reviewers pointed to instead of changing
Scheme. The storages already stamp the kind on everything they return. A new test runs create, get, list, update, watch, server-side apply and table through the real apiserver handler chain for two kinds, and it fails on upstream apimachinery without the serializer.Two paths still convert through the
Scheme, so they go back to the first registered kind that the fork used to correct:includeObject=Object. kubectl only asks for that with--sort-by, and it sorts on fields, not on the kind.apps.cozystack.io.Admission webhooks and ValidatingAdmissionPolicy are not affected: they get the request kind set explicitly. Neither gap can be closed at this layer, because the handler takes the converter straight from
APIGroupInfo.Scheme, a concrete*runtime.Scheme.GOVERNANCE.md no longer lists the fork as compiled into the product, and two comments that justified the in-tree Cilium mirror by the fork now give the actual reason.
Screenshots
No UI changes.
Downstream repositories
Release note