mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-29 16:17:20 +09:00
feat/cozystack-controller-gpu-only
189
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
9fc57e7e07 |
fix(gpu): count a pod as a VMI's launcher only when the VMI lists it
The binder, the VMI webhook, the catalog option source and the usage records each leave out of a pod's GPU usage the units its VMI's devices already hold. They matched the pod to the VMI by its owner reference, the usage records also by the device-reference label, and whoever creates the pod writes both, so a pod naming a VMI with attached devices could hide its GPUs from capacity, raw-key budgets and the usage records. A pod now counts as a VMI's launcher only when that VMI lists the pod's UID in status.activePods; any other pod's GPUs count as raw-name usage. One shared reader decides it for all four. The binder takes the active pods from the VMIs it already lists, the webhook from the VMI informer every replica warms, the usage records from a read of the named VMI, and cozystack-api reads the named VMI only for a pod that requests a raw GPU name, for which it gains get on virtualmachineinstances. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
964bbcaf95 |
fix(gpu): share the tenant, quota, coverage and capacity readers
The binder, the admission webhooks, the catalog option source and the tenant quota gate carried copies of the tenant release name mapping, the resourceQuotas reader, the permittedHostDevices reader and the per-VMI device coverage, and the copies had started to drift: the catalog skipped a budget it could not read, where the others failed. The VMI webhook still counted one unit per healthy GPU function when a node had lost capacity, so it refused a VM whose devices an mdev parent's remaining slots still served while the binder would have kept them; the units a node serves are now counted once, for both. They now live in the shared model, next to the tenant walk they depend on. Coverage keeps two named readers on purpose: for budgets every attached device covers its pod, since a Lost device still counts itself, while for node capacity only a device holding units does. Merging them would count a Lost device twice or not at all. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
e488a7ad78 |
feat(api): add gpuclass and gpudevice option sources
The dashboard orders GPUs by class and attaches them by device name, so it needs the catalog and the tenant's own devices without reading GPUClass or GPUDevice objects itself. The gpuclass source lists every class that still takes orders, a deleted one excluded, with its display name, a short description of the offer, the free units and the most free units on one node from the class status, the per-VM maximum, and the requesting tenant's remaining budget. That budget comes from the shared model over the tenant and each ancestor's subtree, from their declared resourceQuotas and every device, and is 0 for a vGPU class without the entitlement. Pods are read only when an ancestor limits a class's resource name with a raw key, one list per namespace of the outermost such ancestor's subtree, and a virt-launcher pod does not count the units its attached devices already cover. No field names a node. The gpudevice source lists the devices in the request namespace. cozystack-api gets read access to both kinds, and get and list on pods for the raw-key case. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
afac2a0a86 |
feat(api): hide raw GPU options when raw names are forbidden
Once the hoster sets gpu.rawResourceNames to forbidden, a VM gets GPUs only through device references, and admission refuses a GPU entry anywhere else. The gpu source would still offer raw resource names, and the instancetype source would still offer instance types such as the gn1 family that bring their own GPU entries, so the form would lead to a VM that admission then refuses. Read the setting and the GPU resource-name set from the controller's settings ConfigMap. Under forbidden the gpu source is empty, and an instance type with any gpus entry, or a hostDevices entry on a GPU resource name, is left out; a NIC passed as a host device keeps its instance type listed. An unreadable setting serves the full lists: the pickers are advisory and admission enforces the policy. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f2f1d59727 |
feat(api): refuse raw GPU quota keys when raw names are forbidden
With gpu.rawResourceNames set to forbidden, GPUs reach tenants only as GPUDevices, budgeted by gpu.cozystack.io keys. A hoster sets it only once no quota key on a GPU resource name itself is left, and tenant validation now keeps it that way: a raw GPU key, bare or as requests.* or limits.*, is refused, so nobody can bring the old budget form back. The setting and the effective GPU resource-name set are read from the settings ConfigMap the controller writes, through the uncached client the API already uses for quota reads, so the Deployment needs no new flags. Without the ConfigMap raw names stay allowed. The read happens only when the tenant declares a key that can be a resource name. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
32c90b66cf |
feat(api): keep GPU budget keys out of the tenant quota gate
A gpu.cozystack.io/<class> or gpu.cozystack.io/total key caps the whole subtree of the tenant that declares it and is enforced by the GPUDevice admission and the binder, not by this gate. The tenant chart does not render these keys into the namespace ResourceQuota, so a tenant declaring only GPU keys gets no quota objects and draws from its parent's pool like a tenant that declares nothing. The declaration gate read the same values verbatim. It would carve a child's GPU key out of its parent's and siblings' as if it were a pod resource, and it treated a GPU-only tenant as bounded, leaving that tenant's usage out of its parent's pool. Drop the GPU keys where the declared quotas are parsed, so the child, the parent, the siblings and the pool membership all see the same keys the chart renders. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3b17abcf2d |
fix(api): read pool usage under the quota key the controller counts
The tenant declaration gate reads the parent pool's current usage from
ResourceQuota status.used under the key that carries a resource's
limit-side budget, limits.<name>. The quota controller fills limits.*
usage only for cpu, memory and ephemeral-storage. For an extended
resource, a GPU name included, and for hugepages it counts only
requests.<name>, so the limits.<name> key the chart also renders
always reports zero.
The gate therefore saw such a resource as unused and could grant a
child tenant quota the parent pool was already consuming. Read the
usage of every other resource the chart renders with a prefix under
requests.<name>, where the chart applies no ratio, so that key holds
the same declared amount, and read a key that is already a usage key
as declared.
Fixes:
|
||
|
frolov.so
|
9b35fa0b48 |
docs(api): clarify kind-wide Helm timeout overrides
Timeout annotations are read from an ApplicationDefinition and therefore affect every Application of that kind. Describing them as per-Application hides their actual scope and the upgrade override precedence. Fixes cozystack/cozystack#4382 Assisted-by: LLM Signed-off-by: frolov.so <opsmon.sh@yandex.com> |
||
|
Myasnikov Daniil
|
0474613482 |
fix(api): preserve nulls without applicable chart defaults
A schema default cannot fill arbitrary map entries or replacement array items. Keep those nulls, nullable fields and required fields without a non-null default unchanged so Helm retains their validation semantics. Validate the normalized copy on both write paths so admission sees the values that reach storage without mutating the caller object. Assisted-By: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
2e88b39620 |
fix(api): repair an empty required field instead of failing the release
An Application spec reaches flux verbatim as HelmRelease .spec.values,
and Helm's value coalescing deletes a null-valued key together with the
chart default underneath it, before the result is validated against
values.schema.json. A required field written with nothing under it is
therefore rejected for being absent:
talos:
registryMirrors:
fails the whole release with "at '/talos': missing property
'registryMirrors'", naming a property the spec does contain, and nothing
reaching the user points at the empty key they typed.
Remove such a key on the write path, so the chart's own default applies
to it. Filling in the schema default instead would pin today's value
into the stored HelmRelease and freeze the application on it, so a later
bump of that default in the chart would never reach an application that
had once been created with an empty key.
Only required properties are repaired. A required property's null cannot
be load-bearing for anyone, because today it fails the render outright,
so resolving it is pure repair. Where the schema allows the field to be
absent, a null keeps the meaning it has now - the chart default is
suppressed and the key does not reach the template - and that meaning is
worth keeping: 167 optional fields across the catalogue carry a
non-trivial default, placeholder credentials and example S3 buckets
among them, so resolving a user's null to one of those would be worse
than leaving the key out.
Free-form subtrees, null map entries under a user-chosen key (a node
group, a registry host) and array elements are left as written, because
there a null may be the payload rather than an omission. Every removal
is reported back to the client as a warning, so the repair is visible
rather than silent.
Assisted-By: LLM
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
IvanHunters
|
f05718b2de |
feat(redis)!: refuse external access without TLS at admission (#4639)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does An admission policy now refuses a Redis release that would expose a plaintext endpoint outside the cluster. Redis never turns TLS on by itself, so `external: true` without `tls.enabled: true` is exactly that case. The error names the release and a remedy that works. The rule has three parts, because TLS is fixed when a Redis instance is created: the RedisFailover schema refuses a change, while the Redis API accepts one and the HelmRelease then fails. A spec's `tls.enabled` is therefore a request, not proof of what the instance runs. - On create, `external: true` needs `tls.enabled: true`. - On update, turning `external` on is refused whatever `tls.enabled` says. - While `external` stays on, `tls.enabled` cannot change. Before the first reconcile nothing else stops TLS being dropped from a release admitted as external with TLS. Withdrawing `external` in the same edit lifts the freeze, and the callers trusted by `cozystack-tenant-host-policy` are exempt, so an instance whose TLS was flipped earlier can still be unblocked. Edits to a release that was already external pass, so releases exposed in plaintext before this policy keep working. The guarantee covers the Redis app. A generic publisher such as `tcp-balancer` can still forward to a plaintext Redis, by design. This diverges from the Security section of the `external-database-exposure` proposal, which refuses only an explicit `tls.enabled: false` and covers every engine. That rule was written before redis had TLS and matches only postgres. Postgres is left out here on purpose: CloudNativePG keeps `ssl=on` and its default pg_hba accepts plaintext from any client that asks, whatever the chart's `tls.enabled` says, so an admission rule on that field closes nothing. That gap is #4619. The proposal update and a decision record are cozystack/community#91. The policy follows `cozystack-tenant-host-policy`: a ValidatingAdmissionPolicy in `cozystack-basics`, enforced inside `cozystack-api` for the aggregated kind. Its decisions are tested in Go by compiling the shipped template with the apiserver's own CEL compiler and running Redis objects through it, including the bypass sequences found in review. A structural chart test can't show what the policy admits. The redis README, the `external` field description and the threat model describe the new rule. Related: #4622 (valkey `external` is always plaintext), #4624 (the redis `tls.enabled` description implies a live switch). ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [ ] 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: I walked the trigger map against the diff and nothing in it fires, but I left the boxes empty because cozystack/community is reached anyway: the `external-database-exposure` proposal still states the old admission rule. Its update and a decision record are cozystack/community#91. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note feat(redis)!: a Redis release can be exposed outside the cluster only when it is created with `tls.enabled: true`. Creating one with `external: true` and no TLS, turning `external` on for an existing instance, or changing `tls.enabled` while `external` is on is refused at admission. Instances already external without TLS keep working and stay editable. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Redis admission checks now require TLS when external access is enabled at instance creation. They block enabling external access on an existing instance and changing TLS while external access remains enabled. Disabling external access in the same update allows TLS changes. * Existing instances that already had external access without TLS before the policy was introduced remain editable. * **Documentation** * Updated Redis configuration and security guidance to explain these restrictions and TLS migration behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
6a98acb61d |
fix(api): leave failed golden images out of the image options (#4568)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The VM image picker no longer offers golden images whose import failed. A golden keeps its PVC in `cozy-public` after the import fails, so a picker built on the PVC list offered an image whose clone never completes. The `image` option source in cozystack-api now reads the `vm-default-images-*` DataVolumes next to the PVCs and drops names whose DataVolume is in phase `Failed`. I filter rather than mark them, because such an option can never be used and `OptionItem` has no disabled state. A golden that is still importing stays in the list, since CDI holds a clone until the source is populated, and so does a PVC with no DataVolume. When the DataVolumes cannot be read, the PVCs go out unfiltered. The Option list drops a source whose provider errors, and an empty picker would be worse than the old behaviour. cozystack-api gets a read grant on `cdi.kubevirt.io` DataVolumes for this. The console did not use that source at all. `SourceField` listed the PVCs itself, as the tenant, so the fix above would not reach the picker users see. It now reads the same Option list the other dynamic dropdowns use. With that, nothing in the console lists `cozy-public` PVCs, so I removed the read grant on them from the `cozy:tenant:dashboard` Role. That is a visible change: a tenant loses `kubectl get pvc --namespace cozy-public`, which was only ever granted for this picker in #2843. One limitation: in CDI v1.66.1, which cozystack ships, a golden import reaches `Failed` only on a checksum mismatch, and a broken URL keeps it in `ImportInProgress`. So this filter covers less than the issue assumed. I filed #4564 for that. Closes #4553 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> I have no screenshot. The picker looks the same; only the list behind it changes. `apps/console/src/components/SourceField.test.tsx` covers it: the picker shows the images the option source serves and not the raw PVC names. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [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 <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(api): the VM image picker no longer offers golden images whose import failed. The console now reads images from the image option source, and tenants no longer get read access to PersistentVolumeClaims in cozy-public. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * The image picker now displays available images from the tenant’s image options. * **Bug Fixes** * Images whose imports have failed are excluded from the picker. Images remain available when import status is still in progress, has succeeded, or cannot be retrieved. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
800bb0369d |
feat(redis)!: refuse external access without TLS at admission
A Redis release with external: true and TLS off publishes a plaintext endpoint outside the cluster, and nothing stopped a tenant from asking for one. The chart never infers TLS from external, because enabling it moves Redis to a TLS-only listener, so external: true without an explicit tls.enabled: true is exactly that case. A ValidatingAdmissionPolicy on the Redis kind now refuses it with a message naming the release and a remedy that works. TLS is fixed when a Redis instance is created: the operator's RedisFailover schema refuses a change, but the Redis API accepts it. A spec's tls.enabled is therefore a request, not the instance's state, and cannot prove on UPDATE that an instance has TLS; trusting it would let a tenant flip it, expose the instance, and remove it again. So external access can be enabled only at creation, where tls.enabled does decide the instance, and any UPDATE turning it on is refused. The RedisFailover schema does not exist before the first reconcile, so a release admitted as external with TLS could have TLS dropped before the instance is created. tls.enabled is therefore frozen while external access stays on. Withdrawing external access in the same edit lifts the freeze, which leaves a tenant a way to unblock an instance whose TLS was flipped earlier, and the callers trusted by the tenant host policy are exempt so an administrator can repair one in place. Other edits to an instance that is already external pass, so releases exposed in plaintext before the policy keep working. Postgres is left out on purpose. CNPG pins ssl=on, and its default pg_hba accepts plaintext from any client that asks, whatever the chart's tls.enabled says; that field only decides whether the external hostname goes into the certificate. A policy keyed on it would close nothing. The decisions are tested by compiling the shipped template with the apiserver's own CEL compiler, since a structural chart test cannot show what the policy admits. BREAKING CHANGE: creating a Redis with external: true and without tls.enabled: true, which applied before, is now refused, including a GitOps re-apply of such a manifest into a fresh cluster. Turning external on for an existing Redis is refused as well, so exposing an instance means creating a new one with TLS, and tls.enabled can no longer be changed on an instance with external access. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
bf03c84711 |
chore(deps): move to the Kubernetes 1.37 libraries without the apimachinery 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 --> |
||
|
Aleksei Sviridkin
|
99f3d3b2b7 |
feat(controller): tag a DataVolume cause with a condition reason
WorkloadsReady=False covers different situations: a stopped VM has no pod and its monitor reports it not operational on purpose, while a disk whose DataVolume never populates is broken. A consumer that wants to react to one and not the other had only the message text to go by. The WorkloadMonitor status gains a Reason next to Message, set to DataVolumeNotReady by the DataVolume check, and the Application API carries it into the WorkloadsReady condition in place of the generic reason whenever a monitor that is not operational names one. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
b867ea1eda |
feat(api): show WorkloadsReady in the Application table
Ready follows the Helm release, so for a kind whose install does not wait on its workloads, a vm-disk among them, READY in kubectl get says only that the manifests were applied. Whether the workloads came up lives in WorkloadsReady, which the default table did not show, so a disk CDI never populated looked healthy on the surface an operator reads first. The table now carries a WORKLOADS column beside READY. It reads <none> for an Application with no WorkloadMonitor, which has no WorkloadsReady condition at all. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
48ece79ffc |
feat(controller): name the DataVolume that keeps a disk not ready
For a vm-disk, WorkloadsReady is the one condition that reports whether CDI populated the disk, and its generic "One or more workloads are not operational" gives an operator nothing to act on: a disk has no workloads, and the stuck DataVolume and its phase are exactly what they need to know. The WorkloadMonitor status gains a message that the controller fills with each selected DataVolume that is not ready and its phase, and the Application API uses the messages of the monitors that are not operational in WorkloadsReady instead of the generic text whenever one of them names a cause. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Yan Bondarenko
|
46b5a542fe |
fix(api): let the garbage collector finalize Applications
An Application and its HelmRelease share one UID, so the garbage collector keeps a single graph node for both and may finalize it through the Application endpoint. The API never returned finalizers on an Application, and every update restored the HelmRelease finalizers from the live object. When the collector came to drop foregroundDeletion or orphan through the Application, removeFinalizer found no such finalizer and returned as if it were already gone. The HelmRelease then kept foregroundDeletion after Helm had uninstalled the release, and the deletion never completed. The Application now carries the two garbage collection finalizers, and an update that adds or removes one of them applies the same change to the HelmRelease. Controller finalizers such as finalizers.fluxcd.io stay owned by the live object, so helm uninstall still runs before the release goes away. The deletion timestamp and grace period are copied from the live object, which keeps an update of a terminating release valid. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Pujitha Paladugu
|
e11e239044 |
fix(api): pass the stored object to UpdatedObject in tenantsecret Update
Motivation: a PATCH (kubectl patch, label, edit) against an existing
TenantSecret answered 404 NotFound while GET on the same name succeeded.
REST.Update called objInfo.UpdatedObject(ctx, nil). The apiserver patch
handler builds the patched object from that old object alone, so with nil it
takes its create-new-object branch and returns NotFound from inside the
handler. The same nil also made the patch reach admission as CREATE rather
than UPDATE.
Approach: hand UpdatedObject the stored Secret converted with
secretToTenant when it exists, and nil only when it does not (the
forceCreate path, unchanged), matching what the generic store and the
sibling application and securitygroup registries do.
Validation: added TestUpdate_PassesStoredObjectToUpdatedObjectInfo, which
drives Update with a patch-style UpdatedObjectInfo that fails on a nil old
object. It fails before the change ("UpdatedObject received a nil old
object") and passes after. Ran `go test ./pkg/registry/...` and
`go build ./pkg/... ./cmd/...`, all passing. Not run: a live cluster
kubectl patch, or the real apiserver patch handler.
Known limitation, unchanged by this fix: tenantToSecret prefers Data over
StringData when Data is non-empty, so a patch that only sets stringData on a
secret that already has data is a no-op, exactly as a GET-then-PUT edit of
stringData already is. Patching data or metadata works. That precedence is a
separate design question and is left alone here.
Report: https://github.com/cozystack/cozystack/issues/3601
Assisted-by: LLM
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
|
||
|
Mattia Eleuteri
|
31948c8838 |
fix(api): redo the HelmRelease metadata carry-over on a conflict retry
A 409 on the HelmRelease PUT means the live object changed after the read the carry-over was built from, and the writer behind it is normally helm-controller - the controller whose finalizer the carry-over exists to protect, and which re-adds it on the reconcile following a metadata-only update. Retrying with the metadata read before the conflict wrote that stale copy back and dropped the finalizer the reconcile had just restored, reopening the orphaning window the carry-over closes. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
mattia-eleuteri
|
6d541c63eb |
fix(api): make mergeMaps always allocate, never return an input map
mergeMaps returned one of its inputs whenever the other was nil. Create and Update seed the HelmRelease labels with mergeMaps(r.releaseConfig.Labels, ...) and then write the application metadata labels into the result in place, so for an Application with no labels the writes landed on the long-lived config map shared by every request for the kind: requests cross-contaminate (application.name varies per request) and concurrent map writes crash the apiserver. Signed-off-by: mattia-eleuteri <mattia@hidora.io> |
||
|
mattia-eleuteri
|
a18cbc581c |
fix(api): preserve foreign HelmRelease metadata on Application update
Update rebuilds the HelmRelease from the Application and issues a full-replace PUT, which stripped metadata owned by other controllers: finalizers, ownerReferences, and any labels or annotations outside the apps.cozystack.io- prefix. Losing helm-controller's finalizers.fluxcd.io finalizer is the harmful case: a delete landing before helm-controller re-adds it removes the HelmRelease without running helm uninstall, orphaning every resource of the release (VMs, databases, PVCs). A label patch immediately followed by a delete - a common client pattern - hits that window almost every time. Fetch the current HelmRelease before the PUT and carry over its finalizers, ownerReferences, and non-prefixed labels and annotations. Keys under the apps.cozystack.io- prefix remain owned by the API and are recomputed from the Application, so label/annotation deletions still propagate. Signed-off-by: mattia-eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
b201a0f878 |
fix(api): leave failed golden images out of the image options
The image option source lists every vm-default-images-* PVC in cozy-public. CDI creates that PVC as soon as an import starts, so a golden whose DataVolume ended in phase Failed is still offered, and a disk cloned from it never populates: the user picks an image the platform just suggested and ends up with a disk that never becomes usable. Read the DataVolumes next to the PVCs and drop the names whose import Failed. Filtering beats marking: the option can never be used, and OptionItem has no disabled state to show it in. A golden still importing stays, since CDI holds a clone until the source is populated, and so does a PVC with no DataVolume. When the DataVolumes cannot be read, without CDI or without the grant, the PVCs go out unfiltered: the Option list drops a source whose provider errors, and an empty picker would be a regression rather than a safeguard. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
3dde6503ec |
refactor(api): keep each app's kind without the apimachinery fork
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. The replace directive goes, and with it the Renovate rule that kept the fork's stale release tags from being proposed as updates. The serializer 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. One path converts through the Scheme rather than the serializer and goes back to the first registered kind: the row objects of a Table requested with includeObject=Object, which kubectl asks for only with --sort-by. Admission webhooks and ValidatingAdmissionPolicy get the request kind set explicitly. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
93ba1472ed |
fix(api): send the initial-events-end bookmark only to WatchList clients
The apiserver defaults SendInitialEvents on for every watch whose resourceVersion is empty or "0", and leaves AllowWatchBookmarks off. The aggregated storages read SendInitialEvents alone as a WatchList request, so a plain `kubectl get -w` on applications, tenant modules, namespaces, secrets or security groups got a bookmark annotated as the end of initial events. The k8s.io/apiserver v0.35 watch handler lets that through. From v0.36 it treats the annotated bookmark as the end of a WatchList and calls the WatchList completion hook, which it only sets up when SendInitialEvents and AllowWatchBookmarks are both on, so the handler dereferences nil and the watch dies. The handler is the one compiled into cozystack-api, so bumping the library is what breaks it, whatever the cluster version. The marker now requires both flags, the same condition the watch cache uses, and the backing watches ask for bookmarks only when the client did, since every backing bookmark is forwarded. Security groups still forward SendInitialEvents to their backing watch so it replays existing objects as ADDED. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2c2c0301f7 |
chore(deps): roll up pending Renovate updates and move to Go 1.27.1 (#4447)
## What this PR does This replaces the open Renovate PRs with one rollup. Go modules move to their newest releases that keep the Kubernetes libraries and controller-runtime where they are, the toolchain and builder images move to Go 1.27.1, and the first-party code gets the Go 1.27 modernizers. Tests for the code the bumps touch come first, in their own commits, and pass on both the old and the new dependencies. Before the bumps: - The e2e sandbox image builds again: dl.min.io answers 410 for mc since MinIO archived it, so mc now comes from its last GitHub release with a pinned checksum. - Renovate stops offering tags of the apimachinery fork. They mirror upstream, so #2999 is a downgrade that drops our patch. - `make unit-tests` now runs the tests of the nested Go modules (kubevirt-csi-driver, token-proxy, kubeovn-webhook, api/apps/v1alpha1). They never ran in CI before. - New tests: JWT verification in the dashboard token proxy, the kube-ovn plunger metrics, cert-manager object idempotency in the tenant gateway, values-file layering in the operator, and the kubevirt CSI driver wrapper. - One real fix. The lineage webhook decoded admitted objects with encoding/json, so an int64 above 2^53 came back rounded. jsonpatch v2.5 compares numbers exactly and would emit a replace that rewrites such a field in the pod. The webhook now decodes with apimachinery's json. Held back on purpose, with the reasons in the commit messages: - Kubernetes libraries stay on v0.35. #3001 needs the apimachinery fork rebased onto release-1.37 first. - cel-go v0.29.2 and etcd v3.6.15, the lines k8s.io/apiserver v0.35 is built and tested with. - grpc v1.83.2. v1.84.0 is affected by GO-2026-6443 and no v1.84 patch release carries the fix yet. - kubevirt.io/api and the CDI API stay on what upstream csi-driver builds against. From CDI v1.60 it no longer compiles. - api/apps/v1alpha1 keeps go 1.26.4, so consumers of the published types are not forced onto Go 1.27. - go fix runs without embedlit (Go 1.27-only syntax, breaks backports to release branches on Go 1.26) and omitzero (would make metadata, spec and status required in the published OpenAPI schemas), and one errorsastype rewrite that did not compile is left out. govulncheck reports nothing reachable in any of the five modules, standard library included. Supersedes #4311, #4310, #4309, #3452, #3993, #3927, #3902, #3805, #3451, #3415, #3414, #3413, #3302, #4050 and #3363. #3001 stays open. Not checked locally: the images that build upstream projects (kube-ovn, kamaji, cilium, linstor-csi and others) with the Go 1.27 builder. The image build jobs of this PR cover them. Closes #4487. ### 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): move the toolchain and builder images to Go 1.27.1 and raise Go dependencies to their latest compatible releases, closing the published advisories; the lineage webhook no longer rewrites int64 fields above 2^53 in admitted objects. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a restore resource policy option for Velero strategies. * Added a test target to run Go tests across tracked modules. * **Bug Fixes** * Updated the end-to-end test environment to download and verify the MinIO client from its current release location. * Added coverage for token validation, webhook patching, controller behavior, and reconciliation. * **Maintenance** * Updated the Go toolchain, dependencies, and container image versions. * Refreshed pinned workflow actions and base image digests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
9afdc49894 |
feat(apps): warn when a removed field is still set on a database app
The apps API hands the stored spec to the HelmRelease unchanged, so a users[].password left on a Postgres or MariaDB application is silently ignored by the chart. Return a warning on create and update for each such user, and for the superuser key each chart refuses to manage, so the operator editing it learns it has no effect. Also warn when an update introduces a "_"/"-" collision between usernames, database names or Grant names, which the charts reject only on install. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Timofei Larkin
|
ce3eb30e03 |
fix(api): refuse only what a name declaration actually breaks (#4429)
## What this PR does Follow-up to #4367, taking the non-blocking findings from its approving review. Each one is a case where a name declaration (`x-cozystack-name`) was handled more harshly, or more quietly, than the declaration itself warrants. - **An unparseable schema is no longer blamed on a declaration.** A schema that is not a JSON object has no declaration in it, and the spec schema loader already reports it. The API server used to refuse every create of the kind with "declares an invalid x-cozystack-name". It now treats the schema as declaring nothing: creates keep working, and `Kubernetes` and `Kafka` keep their legacy caps. - **Annotation keywords no longer take a kind offline.** Any unknown key inside a declaration used to refuse creates, so a generator adding a `title` or an example would have done the same. JSON Schema annotation keywords (`title`, `default`, `examples`, `deprecated`, `readOnly`, `writeOnly`, `$comment`) and `x-` extensions are now ignored. A constraint keyword the server does not implement, such as `format` or a misspelled `maxLenght`, is still refused, because accepting it would drop it silently. - **The docs no longer overclaim.** They said a declaration no name can satisfy is refused. That is true only for lengths: a pattern is checked to compile, not to match anything, so `$^` rejects every name and names the pattern as the reason. The docs now say that. - **The API review gate sees the declarations the server refuses.** The gate only read a declaration that was an object, and only diffed the keywords it knew. A declaration turned into a string or null, or a misspelled keyword, looked like no change. A changed declaration now goes through the same parser the server uses, and the gate reports it if it cannot be enforced. The parser moves to `pkg/apis/apps/validation` so there is one copy of the rules. - **The declaration no longer leaks into the published OpenAPI for `spec`.** The whole `openAPISchema` is published as the schema of `spec`, so `x-cozystack-name` appeared there as a vendor extension of `spec`. It is now stripped from the published copy. `metadata.name` still carries no limit in the published OpenAPI: it is the shared `ObjectMeta` reference, and a per-kind copy is a bigger change than this one. Not changed: a pattern that can never match is still accepted. Deciding whether a regular expression matches any string within the declared lengths is a much larger piece of code than the mistake it would catch. ### How it is tested - `TestUnparseableSchemaDeclaresNothing`: three non-object schemas; creates still pass, and the Kubernetes legacy cap still holds. - `TestNameDeclarationIgnoresAnnotations`: a declaration carrying `title`, `examples`, `$comment` and an `x-` extension still enforces its `maxLength`. - `TestInvalidNameSchemaFailsClosed` gains `format` as an unsupported constraint keyword. - `TestClassifyNameDeclaration` gains a declaration turned into a string, a misspelled keyword and a null declaration, all reported, and an added annotation, which is not. - `TestPatchSpecLeavesNameDeclarationOffSpec`: the published `spec` drops `x-cozystack-name` and keeps an unrelated vendor extension. ### Downstream repositories No repository is affected beyond the follow-ups already noted on #4367 (the `cozystack/website` pages on annotations and ApplicationDefinitions). Those are still unfiled, so no box is ticked. - [ ] 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 fix(api): An ApplicationDefinition whose openAPISchema cannot be parsed no longer refuses every create of its kind as an invalid name declaration, and annotation keywords (`title`, `examples`, `x-` extensions and the like) inside `x-cozystack-name` are now ignored instead of taking the kind offline. The API review gate now reports a changed name declaration that the API server would refuse, and `x-cozystack-name` no longer appears on the published OpenAPI schema of `spec`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application name declarations now support minimum and maximum lengths and regular-expression patterns. Matching patterns are checked for compilation; a pattern that matches no names rejects all names. * **Bug Fixes** * Invalid or unsupported name constraints now produce configuration errors and block every create of the affected kind. * Descriptive annotations and `x-` extensions are ignored when validating name declarations. * The custom name declaration is omitted from the published API specification. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
7a0aab2bdc |
refactor: apply the Go 1.27 modernizers
Run go fix over every first-party module: interface{} becomes any,
hand-written loops become slices, maps, min/max and range-over-int,
errors.As becomes errors.AsType, pointer helpers become new(expr) and
are inlined at their call sites, and so on. Every rewrite keeps
behaviour.
Three rewrites stay out. embedlit flattens embedded-field literals
such as ObjectMeta, syntax only Go 1.27 accepts, which would break
backports to release branches still on Go 1.26. omitzero drops
omitempty from struct fields, and openapi-gen reads omitempty as
optional, so metadata, spec and status would turn required in the
published schemas. And errorsastype rewrote an errors.As whose
target, apierrors.APIStatus, does not implement error, which
errors.AsType rejects at compile time.
Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
myasnikovdaniil
|
e9868b4f62 |
fix(api): keep a suspended HelmRelease suspended across an app update (#4411)
## What this PR does This PR makes `REST.Update` keep live `spec.suspend` of the HelmRelease, on first write and on conflict retry. Update rebuilds HelmRelease from Application and writes it back whole, Application has no suspension field, so every edit through aggregated API resumed a release that operator or controller had suspended. Caller that depends on it is cnpg restore driver. It suspends target release, patches Postgres app through this API, purges Cluster and resumes only after purge is done, so next chart render lands `bootstrap.recovery` on empty namespace. When the patch resumes release, helm-controller upgrades right away, chart can render recovery Cluster while purge is still running, then driver deletes it as the Cluster being replaced and nothing renders it again, because values do not change anymore. RestoreJob stays `Running` until its deadline. On main it works only because of timing. Same write also drops `finalizers.fluxcd.io`, and helm-controller v1.5.0 spends its first pass after that on putting finalizer back and requeue (750ms minimum retry delay) before it upgrades, this delay usually lets purge finish first. #3426 keeps controller finalizers across this write, so delay is gone, and `postgres-2-backup-roundtrip` failed on both its runs there while it passes on other PRs. In [35714873620](https://github.com/cozystack/cozystack/actions/runs/35714873620) stuck restore is `pg-src-to-pg-target-pitr`: `startedAt` 11:40:37, helm upgrade v3 at 11:40:37, new Cluster created at 11:40:38 and gone by 11:40:47, no render after that. [35652855170](https://github.com/cozystack/cozystack/actions/runs/35652855170) has same picture on first restore, all inside 22:10:51. Tests are in `rest_suspend_test.go`: edit leaves `spec.suspend` as it was (both values) with values still taken from Application, and suspend that lands between read and write survives conflict retry. Removing either line fails its test. ### Downstream repositories Change is in how aggregated API writes HelmRelease back, no downstream repo restates that. Rollback guide on website (`guides/concepts.md`) tells operators to suspend HelmRelease while they work, with this fix that suspension holds across app edit, so docs need no change. - [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 fix(api): app edit through cozystack API no longer resumes a suspended HelmRelease, so postgres restore waits for its purge before cluster is rendered again. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Application updates now preserve a Helm release’s suspended or active state instead of unintentionally changing it. * The release state is also preserved when an update encounters a conflict and retries, preventing a concurrent suspension from being lost. * Application updates continue to use the application’s configured values while retaining the release’s current suspension state. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
180030be72 |
fix(api): refuse only what a name declaration actually breaks
Three cases refused creates of a kind, or were documented as refused, without the declaration being at fault. A schema that is not a JSON object was reported as an invalid x-cozystack-name, although such a schema has no declaration to blame, and the spec schema's own loader already reports it. It now declares nothing, so creates keep working and a kind with a legacy cap keeps it. Any unknown key inside a declaration took the kind offline, so a generator adding a title or an example would do the same. Keywords that constrain nothing, the JSON Schema annotations and x- extensions, are now ignored. A constraint keyword the server does not implement is still refused, because accepting it would drop it silently. The docs said a declaration no name satisfies is refused. That holds for lengths only: a pattern is checked to compile, not to match anything. The docs now say so. The parser moves to pkg/apis/apps/validation so the API review gate can reuse it rather than keep a copy of its rules. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
146e06e75e |
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](https://github.com/cozystack/cozyvalues-gen/pull/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 -->
|
||
|
Myasnikov Daniil
|
5df219b37a |
fix(api): keep a suspended HelmRelease suspended across an app update
Update rebuilds the HelmRelease from the Application and writes it back whole, and the Application carries no suspension, so every edit made through the API resumed a release that an operator or a controller had suspended. The CNPG restore driver depends on that suspension. It suspends the target release, patches the Postgres app through this API, purges the Cluster, and resumes only once the purge is done, so the chart's next render lands bootstrap.recovery on an empty namespace. Resumed by the patch instead, helm-controller upgrades straight away: the chart can render the recovery Cluster while the purge is still running, the driver then deletes it as the Cluster being replaced, and nothing renders it again because the values have not changed since. The RestoreJob sits in Running until its deadline. On main this has been masked by timing rather than prevented. The same write also dropped the Flux finalizer, so helm-controller's first pass after it only put the finalizer back and requeued, which held the upgrade back by at least its 750ms minimum retry delay. A branch that keeps the finalizer removes that delay, and the backup round-trip failed on both of its runs there. The suspension is re-read on a conflict retry as well, since the write that caused the conflict may be the suspend itself. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Timofei Larkin
|
a3911811c7 |
feat(api): take Application name limits from the application's schema
The API server capped Kubernetes and Kafka names by knowing what those two charts render: worker pool releases under a 17-character prefix, controller pods with a 13-character suffix. That is the charts' knowledge sitting in the platform, and it does not generalise. The next application with a sub-resource budget needs another branch on Kind, and an application served from outside this repository cannot get one at all. An application now declares the limits on its own name in its schema, under x-cozystack-name, and the server applies whatever it finds there for whatever kind the ApplicationDefinition describes. A declaration only tightens: prefix + name must still fit the 53-character Helm release name limit. A rejected name is answered with the chart's own explanation of the limit. The schema is untrusted input, and definitions from other repositories are written by hand. A declaration that cannot be enforced as written, whether an unknown keyword, a wrong type, a limit no name satisfies or a pattern Go cannot compile, refuses creates of that kind with a configuration error. Dropping it would admit names the chart cannot render; applying it would reject every name and blame the user. An upgrade brings cozystack-api up before the definitions it serves are re-rendered, so for a while the new server can read a Kubernetes or Kafka definition that predates its declaration. The two old caps stay as a floor for exactly that case, a definition that declares nothing, until no supported upgrade starts from one. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Aleksei Sviridkin
|
36d5c31eaa |
docs(api): stop listing helm-install-timeout carriers in rest.go
The comment next to the timeout overrides named the kinds that set helm-install-timeout and gave each one's reason. The list had already fallen behind the ApplicationDefinitions that carry the annotation, and nothing checks it, so it drifts every time a kind opts in. Say where the annotations live and when a kind should set them instead. Which kinds set them is recorded in their ApplicationDefinitions, and a comment should not restate it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
scooby87
|
7d07bbe3ce |
[kafka] Migrate from ZooKeeper to KRaft mode (#2352)
## Summary - Replace ZooKeeper-based Kafka deployments with KRaft (Kafka Raft) metadata mode - Existing ZooKeeper instances migrate automatically and in place via a Helm `pre-install,pre-upgrade` hook Job that drives Strimzi's ZK-to-KRaft migration state machine - New instances deploy directly in KRaft mode with broker and controller `KafkaNodePool`s - The node pools are owned by the migration hook, not by regular chart manifests (see "Why the pools are hook-managed") - Kafka version is user-configurable (v3.9 default, v3.8 available) - Strimzi operator is unchanged in this PR (stays at 0.45.1-rc1); 0.45.x is the last line that supports ZooKeeper→KRaft migration - ZooKeeper removed from the dashboard UI (ApplicationDefinition) ## Migration strategy Both fresh installs and ZooKeeper→KRaft migrations are driven by a single Helm hook Job (`templates/migration-hook.yaml`, `pre-install,pre-upgrade`) that owns the `KafkaNodePool`s. On every install and upgrade the Job: 1. Ensures both node pools exist (idempotent): a broker pool named exactly `kafka` for a migrating cluster — so Strimzi adopts the existing `<cluster>-kafka-N` brokers and their PVCs in place — or `b-<hash>` for a fresh cluster, plus a controller pool `c-<hash>`. The controller pool pins `strimzi.io/next-node-ids` above the broker range so the broker pool keeps node IDs `[0..N-1]` and Strimzi cannot hand the existing broker IDs to the fresh controllers. The shared `kafka` name is claimed atomically (via `kubectl create`) so only one migration per namespace can win it. 2. Inspects the Kafka CR: no CR → fresh install (pools ensured; the main apply then creates the KRaft Kafka CR, nothing to migrate); already `strimzi.io/kraft=enabled` or `status.kafkaMetadataState: KRaft` → done; any other read error → fail closed. Otherwise (a ZooKeeper cluster) it migrates. 3. Annotates `strimzi.io/node-pools=enabled` then `strimzi.io/kraft=migration`, polls `status.kafkaMetadataState` through `ZooKeeper → KRaftMigration → KRaftDualWriting → KRaftPostMigration → PreKRaft → KRaft`, and only once a safe state is reached flips `strimzi.io/kraft=enabled`. If it times out in an intermediate state it aborts without finalising, so Helm fails the upgrade and the next reconcile retries from where it left off. Data is migrated in place: brokers keep their node IDs and reuse their existing PVCs, nothing is copied. Migration is one-way — Strimzi has no supported KRaft→ZooKeeper rollback once complete, so lowering the `version` field or rolling the app back to a pre-KRaft chart is unsupported (take a backup first). ### Why the pools are hook-managed The hook must create the pools and walk the migration state machine *before* the main release apply. A resource a hook creates cannot also be a regular chart manifest across the ZK→KRaft boundary: Helm's main apply finds the pool live in the cluster but absent from the previous (ZooKeeper) release revision and aborts with `original object KafkaNodePool ... not found`, leaving the release `failed` even though the cluster migrated fine (reproduced under server-side apply, the helm-controller default). Managing the pools only from the hook avoids that, so `templates/kafkanodepools.yaml` and the deployed-version gate ConfigMap (`templates/version.yaml`) are removed and the pools are applied idempotently by the Job on every install and upgrade. ## Bug fixes included - ZooKeeper `metricsConfig` referenced wrong ConfigMap key (`kafka-metrics-config.yml` instead of `zookeeper-metrics-config.yml`) - ZooKeeper `storageClass` used `kafka.storageClass` instead of `zookeeper.storageClass` - WorkloadMonitor used `.Values.replicas` instead of `.Values.kafka.replicas` - `validateNameLength` now caps a Kafka app name at the KRaft controller pod-hostname budget (`<release>-c-<hash>-<id>` must fit 63 chars), instead of admitting names that later fail the chart render forever ## What's next A follow-up PR will upgrade Strimzi to 0.51.0 and Kafka to 4.2 once all instances are confirmed migrated to KRaft. That PR is already prepared on branch `KRaft-upgrade-strimzi-051`. ## Test plan - [x] `helm template` renders correctly for new KRaft instances (no ZooKeeper refs) - [x] `helm template` with custom values (1 replica, external, topics, v3.8) - [x] Version validation rejects unsupported versions via JSON schema - [x] helm-unittest: 86/86 across 9 suites - [x] `hack/kafka-migration-hook.bats`: 12/12 — the real hook script executed against a stub kubectl (resolution branches, fail-closed reads, atomic collision guard, finalize guard) - [x] Operator template renders with the unchanged Strimzi 0.45.1-rc1 - [x] Live migration on a disposable cluster under server-side apply: fresh install → healthy KRaft, release `deployed`; in-place migration of a 20000-message ZooKeeper cluster → release `deployed`, broker PVCs adopted in place (same UIDs), and the KRaft data byte-for-byte identical to the pre-migration ZooKeeper data (matching sha256 over every `partition:offset:key:value`) ### Release note ```release-note [kafka] Enable KRaft metadata mode with automatic, in-place ZooKeeper-to-KRaft migration. Kafka version is now configurable (v3.9, v3.8). The Strimzi operator is unchanged (0.45.1-rc1). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added chart-level Kafka version selector (default v3.9) and KRaft-aware node pool rendering with controller storage sizing. * **Updates** * Default Kafka appVersion updated to 3.9.0; operator and image tag defaults aligned to 0.45.0. * Templates now omit ZooKeeper resources and metrics when KRaft is in use. * **Tests** * New Helm unit tests for Kafka CR, node pools, metrics, topics, dashboard mappings, and workload monitors. * **Documentation** * Values, schema, and README updated to document new version and controller storage options. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
aa20495df3 |
feat(marketplace): register a tapped repository's apps on connect
Connecting a marketplace repository materialized its PackageSource but created no Package, so its ApplicationDefinitions never rendered and its apps never appeared in the dashboard catalog until a manual `cozypkg add` the dashboard cannot drive (the package list is computed from the ApplicationDefinitions, which do not exist yet). The tap materializer now creates a tap-managed registration Package for each materialized PackageSource, the way the platform ships a Package per built-in app, so the ApplicationDefinition registrations deploy on connect and the apps become browsable; the app components carry no install block and are still instantiated per user resource. Privileged install components of an unconfirmed tap auto-registration (a Package that still carries the marketplace-tap label) are refused on several best-effort layers: the Package reconciler skips their HelmRelease and prunes a lingering one at the install site, and the PackageSource reconciler withholds their ExternalArtifact so the helm-controller has no privileged content to deploy, re-running on confirmation via a Package watch. `cozypkg add --allow-privileged` sheds the label as an explicit ownership handover, which is the confirmation; a platform Package never carries the label. A benign component flipped to privileged in a new revision can still be materialized by source-watcher before these layers observe the flip; closing that race fully requires decoupling the catalog registration from install and is tracked in issue #4359. `cozypkg untap` and the dashboard disconnect remove the registration Package (de-registering the apps) under a UID + ResourceVersion precondition, so a concurrent `cozypkg add` handover is never clobbered; running instances are left but unmanaged. `cozypkg add` re-opens a tap auto-registration for variant selection and hands it over. The cozystack-api ClusterRole gains delete on packages for the disconnect path. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Alexey Artamonov
|
8ef230f0bf |
Merge remote-tracking branch 'origin/main' into kafka-multi-instance
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> # Conflicts: # docs/storage-immutability.md # packages/apps/kafka/tests/topics_test.yaml # packages/system/kafka-rd/cozyrds/kafka.yaml |
||
|
Yan Bondarenko
|
1b576bc638 |
fix(registry): drop the backing object's UID from remapped error details
WrapPreservingStatus rewrites Group, Kind and Name of a backend status to the aggregated resource but left Details.UID as the HelmRelease's, so a client that keys on details saw an identity the tenant cannot look up. The UID goes with the rest of the remap, and a test pins it. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
29351bd7fa |
fix(registry): report aggregated resource identity in errors
Backing resource names are not part of the aggregated API contract. Clients need the requested resource identity and the original status code and reason to handle rejected writes consistently. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
39da2300d2 |
fix(registry): keep backend status codes on aggregated write errors
Aggregated endpoints must retain backend HTTP codes and reasons so clients can distinguish conflicts and admission failures. Dry-run deletion also depends on current finalizers, which a cached read can miss. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
a20c706940 |
fix(registry): align dry-run with storage semantics
Dry-run must expose the same deletion state as a real request without persisting it. Shared option objects can hide dropped fields in tests, and an unreachable patch method offers no API coverage. Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Yan Bondarenko
|
a9812477e2 |
fix(registry): preserve write options in aggregated storage
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> Assisted-by: LLM |
||
|
Alexey Artamonov
|
5cbf16088f |
fix(api): cap Kafka app name to the KRaft controller pod-hostname budget
A Kafka cluster's KRaft controller pods are named "<release>-c-<hash>-<id>" (8-hex hash, id at least one digit), and that derived pod hostname must fit the 63-char DNS-1123 label limit. That is tighter than the 53-char Helm release-name budget validateNameLength enforced, so a release name of 51-53 characters was admitted but then failed the chart render (kafka.assertNameLength) forever, an un-recoverable upgrade dead-end for an existing cluster whose name is immutable. Cap the Kafka app name at admission, mirroring the existing Kubernetes worker-pool cap, so the overflow surfaces on the Kafka CR the operator is editing instead of at render time on a pod Strimzi can never create. The chart's render guard remains the second line of defense and also catches larger clusters whose higher node ids need more digits. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
5b91c6b7c6 |
fix(registry): run validating admission on TenantSecret writes (#4226)
## What this PR does The apiserver hands validating admission to a storage as the `ValidateObjectFunc` it passes into `Create`, `Update` and `Delete`. Mutating admission runs in the handler, but validating webhooks and `ValidatingAdmissionPolicy` execute only when the storage calls that callback, as `rest.AdmissionToValidateObjectFunc` in `k8s.io/apiserver` builds it for exactly that purpose. TenantSecret bound all three callbacks to `_`, so every write answered as if no policy existed. The practical effect is that the cluster-wide `cozystack-no-delete-guardrail` policy, and any webhook an operator writes for `tenantsecrets`, are inert on this resource. `application` and `securitygroup` call all three callbacks; TenantSecret was the only write-capable storage in the tree that did not. The callbacks are now called on all four write paths, including the create-on-update branch, which is a create and so carries `createValidation` rather than `updateValidation`. What goes to admission is the aggregated TenantSecret the caller submitted, or the stored one it replaces, not the backing Secret, so a policy sees the resource it was written against. This is a behaviour change for anyone who has a policy matching `tenantsecrets` today: writes that silently passed will start being evaluated, and may now be denied. That is the point of the fix, but it is worth saying out loud before it ships. ### Tests Each write path is covered by a denying admission callback, asserting both that the callback runs and that the backing Secret is left alone when it denies. A nil callback keeps every path working, which is what the apiserver passes when no plugin handles the operation. Removing any one of the four calls, one at a time, turns the suite red. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. It touches one aggregated storage's write paths: no package added, renamed or removed, no `values.schema.json`, no version enum, no changed default, no `kind` or `plural`, no `release.prefix`, no renamed output Secret or Service, no hand-typed CRD, and nothing under `hack/`. - [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 fix(registry): run validating admission on TenantSecret writes, so ValidatingAdmissionPolicies and webhooks matching tenantsecrets are no longer bypassed ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - TenantSecret create, update, force-create, and delete operations now apply validation checks before completing changes. - Denied create or update requests are stopped before data is saved. - Delete validation runs after the current secret is retrieved and before removal. - TenantSecret write operations remain available when no validation callback is provided. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
64eff1f3c1 |
fix(registry): run validating admission on TenantSecret writes
The apiserver hands validating admission to a storage as the ValidateObjectFunc it passes into Create, Update and Delete: mutating admission runs in the handler, but validating webhooks and ValidatingAdmissionPolicies only execute when the storage calls that callback. TenantSecret bound all three to _, so every write answered as if no policy existed. The cluster-wide no-delete guardrail and any webhook written for tenantsecrets were inert on this resource, while application and securitygroup honour them. Call the callbacks on all four write paths, including the create-on-update branch, which is a create and so carries createValidation rather than updateValidation. The object handed to admission is the aggregated TenantSecret the caller submitted or the stored one it replaces, not the backing Secret, so a policy sees the resource it was written against. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
6d42223208 |
fix(registry): preserve dry-run in tap storage
Tap Create and Delete bound the options parameter to _ and built their own metav1.CreateOptions and DeleteOptions for the dynamic client, so the caller's dry-run directive never reached the backing write. A --dry-run=server connect created the Flux OCIRepository and a dry-run disconnect deleted the PackageSource and its source for real, which is the opposite of what a server-side dry-run promises. Forward the directive to all five backing writes, including the orphan-source fallback that had no access to the options at all. Field manager stays as the storage sets it: the objects are synthesized here rather than supplied by the caller, so who owns them is a separate question from whether the write happens. The fake dynamic client's tracker ignores DryRun and persists regardless, so the tests assert the options recorded on the backing action instead of the absence of an object; reverting any one of the five turns the suite red. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
scooby87
|
afc003c464 |
feat(postgres): database read-replica autoscaling via KEDA (#3954)
## What this PR does Adds automatic horizontal autoscaling of a PostgreSQL database's **read replicas** in response to load, implementing the merged design proposal [Database Horizontal Autoscaler](https://github.com/cozystack/community/tree/main/design-proposals/database-horizontal-autoscaling) (community#44). The mechanism is entirely stock — no bespoke operator, no new CRD: - **KEDA as an optional platform component** (`packages/system/keda`, wired into `core/platform` as a source + system-bundle entry). It claims the `external.metrics.k8s.io` APIService that nothing served before. - **`cozy-lib.keda.scaledObject` helper** (`packages/library/cozy-lib/templates/_keda.tpl`) renders a KEDA `ScaledObject`, fail-closed on missing/invalid arguments. - **`packages/apps/postgres` autoscaling** — an `autoscaling` values block (`enabled`, `minReplicas`, `maxReplicas`, `metric`, `target`, `maxReplicationLagSeconds`, `dryRun`, `transition`) validated by `values.schema.json`; under active autoscaling the chart renders a `ScaledObject` and seeds `spec.instances` to a constant floor (`max(replicas, effectiveMin)`), while the postgres release is forced to client-side (two-way) Helm apply so the seed is a merge no-op on upgrade and the KEDA-managed HPA — the runtime writer via the CNPG `Cluster` scale subresource — is not reverted. - **Single-value metric (§1)** — one PromQL query returns `Σ(read load over standbys) + target`, so a stock HPA computes `desired = ceil((Σ+target)/target) = 1 + ceil(Σ/target) = primary + desiredRead`, with no per-pod emission. - **The four static brakes (§5)** — quorum floor `max(minReplicas, maxSyncReplicas+1, 2)` (quorum wins over `maxReplicas`), scale-down pacing (`Pods/1/600s`), a replication-lag clamp folded into the query (freeze both directions while lag exceeds the threshold and the primary is writing, with `max_over_time` hysteresis), and dry-run (render paused). - **Two-phase enable/disable migration** via a `transition` sub-flag: phase 1 stages `spec.instances` to the live count and brings KEDA up with its HPA paused in both directions; flipping to phase 2 hands the runtime count to KEDA, and because the seed re-renders the same count the flip is a merge no-op with no dip. - **Observability** — a database-autoscaling Grafana dashboard and PrometheusRule alerts. **Live validation (dev cluster, CNPG 1.30.0).** The full path works end-to-end: the CNPG `Cluster` scale subresource serves `status.selector`, KEDA renders the HPA, the metric reaches it (`ScalingActive=ValidMetricFound`, `TARGETS 151/150`, `desiredReplicas=2 = 1 primary + 1 replica` — the §1 arithmetic), and the lag-clamp query returns the base value on live VictoriaMetrics. Two packaging fixes were needed and are included: KEDA must run on the cozystack `cozy.local` cluster domain (the upstream `cluster.local` default makes the metrics gRPC target NXDOMAIN and every HPA hangs on `FailedGetExternalMetric`), and the operator's client-go throttle is raised so a single-replica operator does not lose its leader lease under scale-subresource polling. Implements #3953. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. Two repositories are affected; both are follow-ups to open once this lands (and after its dependency #3951 merges and the values surface is final), left unticked here for a maintainer to confirm rather than claimed with a speculative link: - **cozystack/terraform-provider-cozystack** — this adds the nested `autoscaling` block to `packages/apps/postgres/values.schema.json`. The provider hand-models each app's fields (schema + model + expand/flatten), so it needs the new block to expose it in Terraform. - **cozystack/website** — KEDA is a new platform component (`guides/platform-stack`), and the postgres reference page gains the `autoscaling` params. The managed-app reference is regenerated from `README.md` by the release bot; the platform-stack component list is hand-maintained. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: to open (new platform component + postgres autoscaling params) - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: to open (new `autoscaling` block in postgres values.schema.json) ### Depends on - #3951 — bump CloudNativePG to 1.30.0. The HPA can only drive the CNPG `Cluster` scale subresource on CNPG ≥ 1.28.4, where it serves `status.selector`; without it the HPA fails with `InvalidSelector`. This PR should not merge before #3951. ### Release note ```release-note feat(postgres): add optional horizontal autoscaling of read replicas via KEDA, configured through the new `autoscaling` values block; KEDA ships as a new optional platform component. Requires CloudNativePG >= 1.28.4 for the Cluster scale subresource selector. Note: every Postgres release now applies client-side (`release.cozystack.io/helm-server-side-apply: "false"`) so the autoscaler's live `/scale` count is not reverted by Helm; a kind-wide side effect is that chart-rendered fields on Postgres releases are no longer re-asserted by server-side-apply drift correction (driftDetection is unset), a documented trade-off with a per-instance apply strategy as follow-up. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added configurable PostgreSQL read-replica autoscaling with KEDA. * Added CPU and read-connection metrics, replica bounds, replication-lag safeguards, dry-run mode, and transition handling. * Added optional KEDA platform support, Grafana autoscaling dashboard, and Prometheus alerts. * **Documentation** * Clarified integer thresholds, prerequisites, configuration, and transition guidance. * **Bug Fixes** * Improved validation and actionable errors for unavailable KEDA or invalid settings. * **Tests** * Expanded coverage for autoscaling, observability, package selection, and validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
54621bc046 |
feat(migration): tenant self-service VM import from VMware vSphere
Hidora's customers are leaving VMware now, and a tenant has to be able to import their own machines without a platform administrator. The chart approach could not express that: a tenant can neither create the Secret holding vCenter credentials nor name the VDDK image, so both had to come from somewhere they cannot reach. Two CRDs in a new `forklift.cozystack.io` group, split by lifecycle. A VMImportSource is a long-lived connection -- type, endpoint, credentials, readiness -- and a VMImportTask is the one-shot operation that names machines and produces VMDisks and VMInstances. The outputs carry no owner reference back to the task, so deleting a finished import leaves the machines alone. Konveyor Forklift does the transfer underneath; the controller renders its objects, mirrors its verdicts, and hands each finished volume into a VMDisk without copying it. What a live vCenter migration forced out, and no amount of source reading would have: - A guest copied verbatim keeps the drivers it had under VMware and no virtio, so on a virtio disk it does not fail the import -- it succeeds and then does not boot. Imported disks go on SATA. - VDDK connects straight to the ESXi host at the address vCenter advertises, which is routinely unreachable, and on the first cluster collided with the Service CIDR. `spec.hosts` redirects per host. - Firmware must be carried across or a UEFI guest imports as BIOS and never boots (stacked on #3002). - Deleting a task mid-transfer orphaned its DataVolume; a finalizer now removes the engine's volumes and leaves the outputs. The console gains a Migration section: connections are editable, and the VM field is a picker built from a machine list the controller publishes, since the aggregated API cannot reach the inventory and should not hold a source's credentials to try. Two general mechanisms fall out of that -- an option source may take an argument, and one named in a CRD annotation may reference a sibling field. Design: cozystack/community#62. Docs: cozystack/website#673. Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
IvanHunters
|
2af19c4378 |
fix(marketplace): Get surfaces a pending tap instead of 404
List emits a pending Tap for a tap source (labeled OCIRepository) that has no materialized PackageSource yet, but Get looked up only the PackageSource and returned NotFound for the same object. A dashboard detail view (list then detail) of a connecting or collision-blocked tap would 404 instead of showing the block reason. Get now falls back to the pending tap source by name, the same lookup Delete already uses, so Get, List, and Delete agree. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |