Skip to content

refactor(api): keep each app's kind without the apimachinery fork - #4472

Closed
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
fix/watch-initial-events-bookmark-needs-bookmarksfrom
feat/apps-api-without-apimachinery-fork
Closed

Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
fix/watch-initial-events-bookmark-needs-bookmarksfrom
feat/apps-api-without-apimachinery-fork

Conversation

@lexfrei

Copy link
Copy Markdown
Contributor

What this PR does

Stacked on #4467; the diff here is one commit on top of it.

This drops the k8s.io/apimachinery fork. Every apps kind (Postgres, Redis, and the rest) is registered on the one Application Go type, and stock Scheme.ConvertToVersion picks 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 patched ConvertToVersion to 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:

  • The row objects of a Table requested with includeObject=Object. kubectl only asks for that with --sort-by, and it sorts on fields, not on the kind.
  • The object a MutatingAdmissionPolicy hands to the admission plugins after it. There is no mutating policy on 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

refactor(api): cozystack-api no longer depends on a fork of k8s.io/apimachinery; objects in Table rows requested with includeObject=Object may carry another app's kind.

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]>
@coderabbitai

coderabbitai Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

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

Review profile: CHILL

Plan: Advanced

Run ID: 21deae6c-cf23-4362-b90e-16838db32b44

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@github-actions github-actions Bot added area/api Issues or PRs related to the cozystack-api aggregated API server kind/cleanup Categorizes issue or PR as related to cleanup of code, process, or technical debt size/L This PR changes 100-499 lines, ignoring generated files labels Sep 24, 2026
@lexfrei

Copy link
Copy Markdown
Contributor Author

Folded into #4473 together with the rest of the stack, so the Kubernetes bump lands as one PR.

Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Oct 2, 2026
…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 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api Issues or PRs related to the cozystack-api aggregated API server kind/cleanup Categorizes issue or PR as related to cleanup of code, process, or technical debt size/L This PR changes 100-499 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant