mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 10:37:21 +09:00
docs/agents-registries
4434
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
d2e3eb4f82 |
fix(kubernetes, e2e): unblock autoscaling below min size, size the computeplane pool, fix the backupcontroller build (#4768)
## What this PR does Makes the backupcontroller test package compile again on main. #4587 added a flavor argument to `createCNPGBackupArtifact` and `marshalCNPGBackupSnapshot`, and #4669, whose CI ran before that, added two test calls with the old argument lists. Both PRs were green on their own, and together they break `go test ./internal/backupcontroller/`. The two calls now pass the same default flavor as the other tests in the file. The second commit fixes an e2e suite that fails the required E2E check here. It is the `computeplane` suite from #4713, and its worker pool was one node with a single vCPU. After the kubelet reservations that node has 900m of CPU to allocate. The tenant addons (cilium, coredns, csi, ingress-nginx, metrics-server, cert-manager) request exactly 900m between them. If cert-manager's startupapicheck hook pod is created last, it can't be scheduled and the install times out after 10m. The reinstall after remediation runs into the same full node. Whether that pod comes last depends on install order, so the suite passed on #4713 and failed here. The pool now has two workers with one vCPU each, the same shape the kubernetes suites use. One vCPU per worker stays on purpose, because of #3513. Two workers exposed a deadlock in the tenant cluster-autoscaler. The suite's pool is created at zero replicas and then gets a min size of 2, which CAPI does not apply to the live replica count on upgrade (#4786). The autoscaler's scale-up for a pending pod asks for one node, and the clusterapi provider refuses it because one is under the min size, although it is an increase. That refusal backs the group off, and the min-size enforcement skips groups in backoff, so the pool never leaves zero. Upstream had the same report in kubernetes/autoscaler#4942, closed as stale, and it is now tracked in kubernetes/autoscaler#10400. Our autoscaler image already patches the mirror case for max size, and the patch now refuses only a decrease below the min size. `go vet` and `go test ./internal/backupcontroller/...` pass, and so do the bats pins for the computeplane suite. The autoscaler patch adds a test for each direction, and the clusterapi package tests pass with it applied to 1.32.0. ### Screenshots Not a UI change. ### 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 NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved cluster autoscaling for worker groups outside configured size limits: groups below their minimum can scale up toward it, and nodes can be removed when a group exceeds its maximum. Scale-down requests below the minimum remain restricted. * **Tests** * Updated backup and restore test calls with PostgreSQL and empty flavor values. * Updated ComputePlane checks for a two-worker pool and clarified cert-manager installation timing expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
82ae9f6cf7 |
fix(kubernetes): let the autoscaler grow a node group that sits below min
The clusterapi provider refuses any SetSize under the group's min size, including an increase. A pool whose min size is raised while it stands at fewer replicas then deadlocks: the scale-up for pending pods asks for fewer nodes than the min, is refused, and puts the group in backoff, and the min-size enforcement skips groups in backoff. The group stays where it is for good. Refuse only a decrease below min size. The guard it keeps is the one DecreaseTargetSize relies on, and DeleteNodes checks min size itself. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9620be0103 |
feat(postgres): add per-user createRole and bypassRls attributes (#3505)
## What this PR does - Adds `users.<name>.createRole` to the postgres chart, which sets the `CREATEROLE` role attribute on a managed login user. - Adds `users.<name>.bypassRls`, which sets `BYPASSRLS` on the same user. The two attributes are needed together to provision a privileged service role, see below. - Sets `createrole_self_grant` as a role-level default alongside `createRole`, without which the attribute is granted but not usable. - Refuses `createRole` on PostgreSQL 15 and older, where it is an escalation path rather than a bounded privilege. - Makes the init job record the roles it manages in a superuser-only marker instead of trusting role comments and refuses to adopt a role it did not create once `createRole` has been used, see below. - Adds `tests/user_createrole_test.yaml`: defaults, both explicit values for each attribute, composition with `replication` and with each other, the `createrole_self_grant` set and reset paths, and the version guard on v15 and v13. Adds `tests/managed_roles_test.yaml` for the marker and the adoption checks. Today a tenant that needs to create its own roles has no way to do it through the chart, because role attributes are not inherited through role membership: granting `CREATEROLE` to the generated `<db>_admin` group role has no effect on the users that are members of it, and `<db>_admin` is `NOINHERIT` anyway. The attribute has to be set on the login user itself, which only the init script can do. The remaining option is to hand the tenant the `postgres` superuser, which also gives it `COPY ... FROM PROGRAM`, arbitrary file access and command execution inside the instance pod. These flags are the narrow version of that: the tenant manages its own roles, and the superuser stays internal. Both attributes are reconciled on every init-script run, like `replication`, so setting one back to `false` emits the `NO...` form and revokes it. ## Why `bypassRls` belongs in the same PR `CREATEROLE` on its own does not let a tenant reproduce a role layout that uses row-level security, which is the common case this feature exists for. PostgreSQL only lets a role create other roles carrying `BYPASSRLS` if it holds the attribute itself, so `CREATE ROLE service_role ... BYPASSRLS` fails with `Only roles with the BYPASSRLS attribute may create roles with the BYPASSRLS attribute`. Exposing `createRole` without `bypassRls` therefore ships a feature that stops one statement short of the goal. `bypassRls` is not gated on a PostgreSQL version, because it carries no escalation of its own: `BYPASSRLS` waives row-level security inside the tenant's own databases, and the database owner already reads every row in them. It grants no reach outside the tenant's data. ## Why `createrole_self_grant` is set with the attribute This is the non-obvious half. PostgreSQL 16 grants a `CREATEROLE` holder only `ADMIN OPTION` on the roles it creates, governed by `createrole_self_grant`, whose default is empty. `ADMIN OPTION` allows granting the role onward but not `SET ROLE` to it, and creating an object owned by another role requires `SET`. The attribute therefore looks functional -- `CREATE ROLE` succeeds -- while the very next statement a tenant writes, `CREATE SCHEMA ... AUTHORIZATION <new role>`, fails with `must be able to SET ROLE`. Every tenant granted `createRole` would hit this and have to discover the parameter, so the chart sets it as a role-level default and resets it when the attribute is withdrawn. ## Why the init job stops trusting role comments and role names The init job runs as the superuser, and it trusted two things a CREATEROLE holder controls for the roles it creates. It picked the users and roles to drop by their comment, and on PostgreSQL 16 a CREATEROLE holder may `COMMENT ON ROLE` every role it created, so it could hand the chart for deletion a role the chart never created. It also adopted any role that already existed under a declared name, so a role created under a name an administrator declares later stayed under its creator's control, for example `debezium` before `users.debezium.replication: true`, or `newdb_admin` before `databases.newdb`. Neither case can be caught from the catalog. Creating a role through an intermediate role and then dropping that role leaves the squatted role with no row in `pg_auth_members` at all, while the objects, role settings and default privileges planted in it stay. On 17.11 such a `debezium` had zero memberships, and after the chart adopted it `migrator` could still run code as `debezium` with `REPLICATION` through a function it had planted. The chart therefore keeps its own record: - Chart-managed roles are members of `cozystack:managed`, a NOLOGIN role that only the superuser can grant. On 16+ the membership is `WITH INHERIT FALSE, SET FALSE`. The name is outside the username and database charsets. The delete passes act only on these members. On its first run the chart marks every role it had commented before, when no role could yet hold CREATEROLE through it. - Once any user has held `createRole`, recorded as `cozystack.createrole_granted` on the `postgres` database (the same mechanism as `cozystack.adopted_restore`), the chart refuses to adopt an existing role it did not mark, both for users and for `<db>_admin`/`<db>_readonly`. The init job fails with the role's name, and a platform administrator must drop the role. Before that point adoption works as before, so a fresh install that declares `users.app` still adopts the role CNPG bootstraps. ## The PostgreSQL 16 floor on `createRole` The floor is a security boundary, not a cosmetic check. Before PostgreSQL 16, `CREATEROLE` allowed granting membership in any non-superuser role, so a user holding it could grant itself `pg_execute_server_program` and reach command execution on the instance, which is exactly what not exposing the superuser is meant to prevent. I verified both sides on a real cluster: on 15.15 `GRANT pg_execute_server_program TO <createrole_user>` succeeds and `COPY ... FROM PROGRAM 'id'` then returns `uid=26(postgres)`; on 16.11 and 18.1 the same grant is refused because it now requires ADMIN option on the role. PostgreSQL 16 restricted `CREATEROLE` to roles the grantee administers, which is what makes the attribute safe to expose. The guard uses the same fail-fast `SECURITY:` idiom as the blocked `postgresql.parameters` list in `templates/db.yaml`. Note that a grant obtained on PostgreSQL 15 survives a major upgrade: after moving an instance to 16.11 the user kept its membership in `pg_execute_server_program` even though new attempts were refused. Operators who exposed `CREATEROLE` out of band on an older major should audit and revoke memberships in `pg_execute_server_program`, `pg_read_server_files` and `pg_write_server_files` before relying on the floor. ## Verification The marker, adoption and `createRole` paths were rendered and run against live PostgreSQL 13, 15, 16 and 17 containers. An upgrade from the previous chart marks the roles it already managed and removes a dropped user. What happens to a dropped role's objects comes from #4574 and is covered by its tests. A fresh install adopts the CNPG `app` role in the same run that grants `createRole`. A forged comment is ignored, and squatted `debezium` and `newdb_admin` roles stop the init job with nothing granted to them. The orphaned-role pass still drops the roles of a removed database, and every re-run is clean. Reverting each part of the fix in turn fails `tests/managed_roles_test.yaml`. Beyond the chart unit tests, I replayed a real migration checklist against a live PostgreSQL 17.7 instance, connected as the managed login user and never as the superuser: creation of four roles with their attributes including `CREATEROLE` and `BYPASSRLS`, `CREATE SCHEMA ... AUTHORIZATION` landing on the requested owner, trusted extensions into a named schema, per-role and per-database `search_path`, per-role `statement_timeout` and `idle_in_transaction_session_timeout`, RLS including `FORCE ROW LEVEL SECURITY` verified end to end (a plain role sees zero rows, the `BYPASSRLS` role sees them all), and `SECURITY DEFINER` functions. All of it succeeds without the superuser. The one thing that remains out of reach is `CREATE EVENT TRIGGER`, which PostgreSQL restricts to superusers and no chart flag can change. Other attributes and the predefined roles that a managed-service admin role usually wants (`pg_monitor`, `pg_read_all_settings`, `pg_maintain`) can follow the same per-user pattern in a later PR, minus the three that grant OS access (`pg_execute_server_program`, `pg_read_server_files`, `pg_write_server_files`). ### Screenshots No UI change. ### Downstream repositories `packages/apps/postgres/values.schema.json` gains two fields, which per the trigger map reaches `cozystack/terraform-provider-cozystack`: its postgres resource carries a hand-maintained schema plus an expand/flatten pair for `users`, with no codegen from `values.schema.json`, so `createRole` and `bypassRls` will be invisible to Terraform users until they are added there by hand. I searched that repository and found no open PR or issue covering it (the only open PR is a dependabot Go module bump). I have not filed the follow-up, so I am leaving the box unticked rather than claiming work that does not exist. Happy to open the issue or the provider PR if a maintainer confirms that is the preferred route. I walked the rest of the map against the diff and found nothing else: no package is added, renamed or removed, no version enum changes, no default changes, `release.prefix` and the output Secret and Service names are untouched, and no CRD outside the regenerated `packages/system/postgres-rd/cozyrds/postgres.yaml` is affected. The `README.md` parameter table is regenerated here and postgres is already in the website generator's app list, so the website path stays automated. - [ ] 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: not filed yet, see the note above - [ ] [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: ### Release note ```release-note feat(postgres): add `users.<name>.createRole` and `users.<name>.bypassRls`, which grant the CREATEROLE and BYPASSRLS attributes to a managed user so a tenant can create and manage its own roles, including roles that bypass row-level security, without the postgres superuser. `createRole` also sets `createrole_self_grant` on the user so that roles it creates can own objects. `createRole` requires PostgreSQL 16 or newer: on older majors CREATEROLE can be used to grant oneself pg_execute_server_program and reach command execution on the instance, so the chart refuses to render the attribute there. The init job now records the roles it manages as members of a superuser-only `cozystack:managed` role instead of trusting role comments. Once `createRole` has been used on an instance, it refuses to adopt an existing role it did not create. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * PostgreSQL user configuration now supports `createRole` and `bypassRls` privileges. * These privileges can be combined with replication access. Enabling `createRole` also configures the role’s self-grant behavior. * `createRole` requires PostgreSQL 16 or newer; configuring it on older versions is rejected. * **Documentation** * Updated configuration schemas, examples, and guides to describe the new options and PostgreSQL version requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
f5325425d6 |
fix(vm-instance): supply cloud-init network-data to netplan guests (#4306)
<!-- 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
A guest whose interfaces are rendered by netplan ignores two things the
platform already puts on the wire for it, and this adds an opt-in
`netplanDhcp` value that supplies the cloud-init network-data making it
ask for both.
`virt-launcher` starts a DHCP server on **every** bridge-bound
interface, secondary ones included — `newDHCPConfigurator` in
`pkg/network/setup/launcher/configurator.go` gates only on `iface.Bridge
!= nil`, and `BridgeConfigGenerator.Generate` serves the address the CNI
put on the pod-side interface along with that link's MTU. The chart
renders `bridge: {}` for every network, so a VM joined to a VPC subnet
is offered its reserved address and its MTU. cloud-init's fallback
network configuration covers a single interface, so on Debian and Ubuntu
nothing ever sends a DHCPDISCOVER on the second NIC and the interface
stays inert, while AlmaLinux (NetworkManager auto-profile) and Windows
come up. That asymmetry is the tell: with no DHCP server at all, none of
the three would work.
The same DHCP server advertises the launcher pod's search domains as
option 119, and `UseDomains=` defaults to `no` in systemd-networkd, so a
netplan-managed link keeps none of them. A cluster domain under `.local`
then reaches the mDNS exclusion in resolved and every lookup answers
`REFUSED`, which is #2085. `dhcp4-overrides: use-domains: true` is
enough on its own: the domains on the wire already carry whatever
cluster domain the cluster actually uses, so nothing needs to be
hardcoded here and #2372 does not collide with this.
### This is a regression, not a missing feature
`packages/apps/virtual-machine` carried exactly this, added in
|
||
|
Aleksei Sviridkin
|
bcda5c893a |
feat(kubevirt)!: drop HotplugVolumes for declarative volume hotplug (#4453)
## What this PR does This turns off the deprecated KubeVirt `HotplugVolumes` gate and moves the platform to declarative volume hotplug. It changes how every volume hotplug on the platform works, so the title carries `!`. Closes #4032. This PR is stacked on #4451 and has to merge after it. #4451 adds the kubevirt-csi flag this PR flips. The replacement gate is already on in KubeVirt 1.9, and the old gate is the only thing keeping it off. `DeclarativeHotplugVolumes` is Beta at v1.9.0, and Beta gates are on by default. virt-controller skips `handleDeclarativeVolumeHotplug` while `HotplugVolumes` is on, so dropping the old gate is what switches the new one on. The kubevirt-csi fallback goes off in the same change. With the old gate off, virt-api rejects the VMI-level `removevolume` on a VM-owned VMI, so the chart now passes `--enable-vmi-hotplug-fallback=false`. The comments next to the gate and the flag point at each other. This needs the rebuilt kubevirt-csi image. A driver built before #4451 does not know the flag and exits on it at startup. The image is stamped at release preparation and ships in the same release as this change, so only a `make apply` from main before then would crash-loop the CSI controller. What changes at runtime on an upgrade: - The VM-level `addvolume` and `removevolume` calls now edit `VM.spec` directly. virt-controller then makes the VMI hotplug volumes match it, instead of going through `VM.status.volumeRequests`. - Volumes hot-plugged through the VM subresource are in `VM.spec`, so they stay attached. - During a tenant teardown, when the VM goes before its VMI, the driver no longer calls the VMI subresource. It waits for the VMI status to drop the volume, and the garbage collector deletes the VMI with its VM. Upstream waits up to two minutes. If the VMI is still there after that, the unpublish fails and is retried. A new platform migration keeps old VMI-only volumes attached. On its first pass virt-controller removes every hotpluggable VMI volume that is missing from `VM.spec`. Until July 2025 kubevirt-csi attached volumes at the VMI level (the Role then granted `virtualmachineinstances/addvolume`), and live migration carries such a VMI across KubeVirt updates. Without help, a tenant node not recreated since then would lose a mounted disk on upgrade. Migration 59 copies each such volume and its disk from the VMI into the VM template before anything else changes. Adding a hotplug volume to the VM spec is a valid live update in KubeVirt, so no VM needs a restart. It runs as the platform chart's pre-upgrade hook, before the upgrade re-points `cozystack-packages`. The KubeVirt CR and the tenant kubernetes charts both render from that source, so the gate and the flag cannot change before the migration has finished. It only touches VMs that need it, logs each one, and a second run changes nothing. The migration also makes ephemeral hotplugs permanent. A volume a user added at the VMI level, for example with `virtctl addvolume` without `--persist` from an older virtctl, has the same shape as kubevirt-csi's old attachments, because virt-api builds both the same way. There is nothing reliable to tell them apart by, so both are copied. Leaving the user's volume out would not keep it ephemeral: the first reconcile would unplug it from the running VM. After the upgrade it is part of `VM.spec` and attaches again on a restart, and `virtctl removevolume` drops it. Ephemeral hotplug stops working on VMs. With the old gate off, virt-api refuses VMI-level `addvolume` and `removevolume` on a VMI that a VM owns (`vmiVolumePatch` in v1.9.0). A volume on such a VMI is added and removed through the VM from then on. A VMI without a VM is not affected. Only running VMIs are copied. virt-controller reconciles no other VMI, and a stopped VM builds its next VMI from `VM.spec`, so copying from a stopped one would bring back volumes the restart was going to drop. Migrations run only on the isp-full, isp-full-generic and isp-hosted variants. KubeVirt comes from the platform only on isp-full and isp-full-generic, so those are covered. On the default variant, where packages are installed by hand, nothing runs the migration. This optional check lists the VMIs and volumes the migration would copy. It uses the same rule. On the default variant, run it before upgrading and recreate each tenant node it lists: ```bash kubectl get vm,vmi --all-namespaces --output json | jq --raw-output ' ([.items[] | select(.kind == "VirtualMachine") | {key: "\(.metadata.namespace)/\(.metadata.name)", value: [.spec.template.spec.volumes[]?.name]}] | from_entries) as $vm | .items[] | select(.kind == "VirtualMachineInstance") | ([.metadata.ownerReferences[]? | select(.controller == true and .kind == "VirtualMachine" and (.apiVersion | startswith("kubevirt.io/"))) | .name][0]) as $owner | "\(.metadata.namespace)/\($owner)" as $key | select(.status.phase == "Running" and $owner != null and $vm[$key] != null) | .spec.volumes[]? | select(.dataVolume.hotpluggable == true or .persistentVolumeClaim.hotpluggable == true) | .name as $n | select($vm[$key] | index($n) | not) | "\($key) \($n)"' ``` Empty output means nothing is affected. I checked the query against the migration's test fixtures only. Neither the query nor the migration has run on a live cluster. #4287 also adds a migration 59. If it merges first, this one moves to 60: the file, the version it stamps, `migrations.targetVersion`, the bats file (its `run_migration` argument and expected `STAMP`) and `hack/testdata/migration-59-vmi-hotplug`. A VM with a pending hotplug request at upgrade time may also get stuck. virt-controller still processes `VM.status.volumeRequests` before the declarative handler, and it applies them through the VMI subresource, which virt-api rejects once the old gate is off. The sync then fails before the VM spec is updated, so nothing clears the request, and every later sync of that VM fails the same way. A retry through the VM subresource edits the spec and clears it, which covers kubevirt-csi's own requests. A one-shot request stays stuck until the VM restarts. I read this from the v1.9.0 code and haven't reproduced it. Waiting until this lists nothing before upgrading narrows the window, but a request made during the rollout can still hit it: ```bash kubectl get vm --all-namespaces --output json | jq --raw-output '.items[] | select((.status.volumeRequests // []) | length > 0) | .metadata.namespace + "/" + .metadata.name' ``` The two halves live in different HelmReleases, so there is a short window during the rollout where the driver and the gate disagree. In that window an unpublish fails and is retried until both are updated. A tenant cluster whose `kubernetes` HelmRelease is suspended or failing keeps its old driver for longer. Drivers from before July 2025 attach through the VMI subresource, so on such a tenant every attach is refused until its release reconciles. Such a driver can also attach a volume after the migration has run and before the gate flips. The migration doesn't see that volume, and the first declarative reconcile unplugs it from the running VM. The migration lists the VMs once and patches each one with a `resourceVersion` test, so a VM that changes in between fails the hook Job. The Job has `backoffLimit: 3`, and each retry lists again and skips the VMs already patched. Re-reading only the conflicting VM would avoid the retry; I left that out. The new gate is listed in the CR even though it is on by default. With the old gate gone it is the only hotplug path, and the chart refuses a render that disables it. I found no other user of `HotplugVolumes` in the platform or in the downstream repositories. The VMI-level fallback may have been failing already. The kcsi Role has no rule for the VMI-level `removevolume` (#4452), so that call likely got 403 with or without the gate. I haven't checked that on a cluster. `hack/migration-59-vmi-hotplug-volumes.bats` runs the migration against a fake kubectl: a VM that needs the copy, VMs and VMIs that must be left alone (a VMI that is not Running among them), a second run, a cluster without KubeVirt and the fail-closed paths. The helm-unittests for the KubeVirt CR pin the whole gate list, accept `HotplugVolumes` in `disabledFeatureGates` and refuse `DeclarativeHotplugVolumes` there. The CSI test pins the flag at false, and the `make update` idempotency test still passes. None of that exercises hotplug itself. The real test is e2e with volume attach and detach on a tenant cluster, including a teardown. No CI lane upgrades from the previous release yet, so an upgrade with a live VMI-only volume is untested; #4463 tracks that test. ### Screenshots No UI change. ### 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 feat(kubevirt)!: KubeVirt now runs with DeclarativeHotplugVolumes instead of the deprecated HotplugVolumes feature gate, and kubevirt-csi no longer uses the VMI-level hot-unplug. Volume hotplug requests are applied through VM.spec. The upgrade copies volumes that older kubevirt-csi versions attached only to the VMI into VM.spec, so they stay attached; the check in the PR description is optional. This also applies to ephemeral volumes added to running user VMs at the VMI level, such as with virtctl addvolume without --persist: they become part of VM.spec and attach again after a VM restart. virt-api now refuses VMI-level addvolume and removevolume on a VMI owned by a VM, so such volumes are added and removed through the VM. A tenant cluster whose kubernetes release is suspended or failing keeps its old kubevirt-csi, and a driver from before July 2025 cannot attach volumes there until that release reconciles, and a volume it attaches during the upgrade itself can be unplugged again. A values override that puts DeclarativeHotplugVolumes into disabledFeatureGates now fails the kubevirt render. Clusters on the default variant do not run platform migrations and should run that check before upgrading. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Declarative hotplug volumes are enabled by default for KubeVirt. * Running virtual machines keep eligible hotplugged volumes and disks when their configuration is migrated. * **Bug Fixes** * The migration avoids adding duplicate volumes or disks and does not mark the migration complete if required operations fail. * The Kubernetes CSI driver no longer uses the VMI hotplug fallback. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
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 --> |
||
|
Mattia Eleuteri
|
21fa1fcdb1 |
feat(postgres): add per-user createRole and bypassRls attributes
Without these, a tenant that needs to manage its own roles has no option but the postgres superuser, which also hands over filesystem access and command execution on the instance. Role attributes are not inherited through role membership, so the generated <db>_admin group role cannot carry them: they have to be set on the login user itself. users.<name>.createRole and users.<name>.bypassRls set CREATEROLE and BYPASSRLS on a managed login user. Both are reconciled on every init-script run, so setting one back to false emits the NO... form and revokes the attribute. createRole is refused on PostgreSQL 15 and older. Before PostgreSQL 16, CREATEROLE allowed granting membership in any non-superuser role, so a user holding it could grant itself pg_execute_server_program and reach command execution on the instance, which is exactly what not exposing the superuser prevents. PostgreSQL 16 restricted CREATEROLE to roles the grantee administers. bypassRls carries no such gate: BYPASSRLS only waives row-level security inside the tenant's own databases, whose owner already reads every row, so it adds no reach. The two attributes are needed together, not separately. PostgreSQL only lets a role create other roles carrying BYPASSRLS if it holds the attribute itself, so provisioning a privileged service role fails with "Only roles with the BYPASSRLS attribute may create roles with the BYPASSRLS attribute" when only createRole is granted. createRole also sets createrole_self_grant as a role-level default. PostgreSQL 16 grants a CREATEROLE holder only ADMIN OPTION on the roles it creates, governed by that parameter, whose default is empty. ADMIN OPTION allows granting the role onward but not SET ROLE to it, and creating an object owned by another role requires SET. The attribute therefore looks functional -- CREATE ROLE succeeds -- while the next statement a tenant writes, CREATE SCHEMA ... AUTHORIZATION <new role>, fails with "must be able to SET ROLE". The default is reset when the attribute is withdrawn. The init job runs as the superuser, and before this change it trusted two things a CREATEROLE holder controls for the roles it creates: role comments and role names. It picked the users and roles to drop by their comment, and a PostgreSQL 16 CREATEROLE holder may comment every role it created, then hand it to the chart for deletion. It adopted any role that already existed under a declared name, so a role created under a name an administrator declares later stayed under its creator's control. Neither can be caught from the catalog: creating a role through an intermediate role and then dropping that role leaves the squatted role with no member at all, while the objects, settings and default privileges planted in it stay. The chart therefore records the roles it manages as members of "cozystack:managed", a role only the superuser can grant, and the delete passes act only on those. Its first run marks every role the chart had commented before, when no role could yet hold CREATEROLE through it. Once any user has held createRole, recorded as a setting of the postgres database, the chart refuses to adopt a role it did not mark and fails with the role's name, rather than hand a squatted role REPLICATION or the ownership of a database. Verified against live PostgreSQL instances. On 17 a migration checklist replayed as the login user, with no superuser: role creation with attributes, CREATE SCHEMA ... AUTHORIZATION with the requested owner, extensions in a named schema, per-role search_path and timeouts, RLS including FORCE ROW LEVEL SECURITY, and SECURITY DEFINER functions all succeed. On 17 the forged-comment and squatting paths are refused or ignored, the upgrade from the previous chart marks the roles it already managed, and a fresh install still adopts the role CNPG bootstraps for users.app. On 13, 15 and 16 the init script runs and re-runs cleanly. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
9734a16692 |
feat(postgres): add a postgis image flavor (#4587)
<!-- 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 Adds a `flavor: postgresql | postgis` enum to the `postgres` app. `postgis` runs the cluster on the CloudNativePG PostGIS operand image, so tenants can enable `postgis`, `postgis_raster`, `postgis_topology`, `postgis_sfcgal`, `postgis_tiger_geocoder` and `address_standardizer` through the existing `databases.<name>.extensions`. The default stays `postgresql` and renders exactly the image it rendered before this change. This picks up #2672 by @matthieu-robin, which the stale bot closed after a rebase request, with these changes: - The postgis tags in #2672 (`17.7-3.5`, `16.11-3.5`, ...) do not exist in `ghcr.io/cloudnative-pg/postgis`; the registry answers 404, so the flavor would have ended in `ImagePullBackOff`. The map now uses tags that exist, `<minor>-<postgis>-standard-trixie`, on the same PostgreSQL minor as `files/versions.yaml`. - v18 is supported: PostGIS images for 18 now exist upstream. - `standard` rather than `system`: the chart archives through the barman-cloud plugin since the backup rework, so the operand no longer needs the barman binaries of the deprecated `system` variant. - `hack/update-versions.sh` derives the postgis map from the minors it picks, and `hack/check-postgres-postgis-versions.bats` fails if the two maps drift to different minors or away from `standard-trixie`. - The chart refuses a flavor change on a live `Cluster`. The default images are built on Debian bullseye (glibc 2.31) and the postgis ones on trixie (glibc 2.41): CNPG would roll the swap out as a plain image update, under an initialised data directory, which silently invalidates every index on collatable text, and the postgis-to-postgresql direction also drops the libraries behind every PostGIS object. Only the image name is compared, so minor bumps and registry mirrors keep passing. - `pgrouting` is not claimed: it is absent from the `18.1-3.6.1` build the chart pins and only appears in later postgis builds. A general mechanism (CNPG ImageVolume extensions) was considered and left out: it needs PostgreSQL 18 and Kubernetes 1.35 or the `ImageVolume` gate, so it cannot serve v13-v17 nor most management clusters today, and PostGIS is the only extension CloudNativePG ships as a full operand image. No other extension request exists in this repository. Testing: - `helm unittest packages/apps/postgres`: 145/145, including 6 new cases in `tests/flavor_test.yaml` (default image, postgis on the Cluster and on the init Job, the guard in both directions with a mocked live Cluster, and a mirrored digest-pinned image taking a new minor). Removing the guard makes exactly the two guard cases fail. - `hack/cozytest.sh hack/check-postgres-postgis-versions.bats` passes, and fails on a shifted minor or a `system` tag. - `make -C packages/apps/postgres generate` leaves no diff. - The rendered `ghcr.io/cloudnative-pg/postgis:18.1-3.6.1-standard-trixie` was started locally: `CREATE EXTENSION` for citext, postgis, postgis_raster and postgis_topology succeeds and `ST_Buffer` answers. A three-instance cluster on the `system` sibling of that image has been running PostGIS in production since 2026-09-29. ### 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: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#63 (issue: the provider models every field by hand, and the default handling needs a decision there) - [ ] [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 feat(postgres): add a `flavor` field accepting `postgresql` (default) or `postgis`. `postgis` bootstraps the cluster from the CloudNativePG PostGIS image, so PostGIS extensions can be enabled per database through `databases.<name>.extensions`. The flavor is fixed at creation: changing it on an existing cluster is refused, and so is restoring a backup into a cluster of the other flavor, whether through a RestoreJob or `bootstrap.enabled`; move data across flavors with a logical dump. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added a `flavor` option for PostgreSQL clusters. The default `postgresql` flavor remains available, while `postgis` selects an image with PostGIS extensions. - PostGIS images are available for PostgreSQL versions 13–18. - Cluster flavor is fixed at creation. To switch flavors, create a new cluster and migrate data with a logical dump. - **Bug Fixes** - Restores are refused when the backup and target cluster use different flavors. - **Documentation** - Updated PostgreSQL configuration guidance to describe flavor options and limitations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Mattia Eleuteri
|
39bdc9c404 |
fix(vm-instance): make the netplan network-data opt-in
Keying the network-data on instanceProfile reached every VM that never set the field, because the profile defaults to ubuntu whatever image the disk holds. The sysconfig, NetworkManager and eni renderers turn each match stanza into an interface literally named after the netplan ID, so a non-netplan guest left on the default profile would lose the DHCP it gets today. A VM with neither cloudInit nor sshKeys would also gain a cloud-init disk on upgrade, and KubeVirt's NoCloud instance-id is new to that guest, so its next boot would regenerate host keys as a first boot. The configuration also shadows a static NIC set up through netplan: netplan emits one networkd file per ID, networkd applies only the first file in lexical order that matches a link, and 10-netplan-cozy-en sorts before the file for enp2s0. The external-VLAN recipe claimed otherwise. An explicit netplanDhcp value, off by default, leaves every existing VM rendering exactly as before and lets the documentation say when not to turn it on. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
b5181b0b9b |
fix(vm-instance): supply cloud-init network-data to netplan guests
A netplan guest configures only the interface cloud-init's fallback configuration covers, and keeps none of the search domains DHCP offers it, so two things the platform already puts on the wire never reach it. virt-launcher runs a DHCP server on every bridge-bound interface, secondary ones included, yet a VM joined to a VPC subnet leaves that interface inert for want of a client asking. The same server advertises the launcher pod's search domains as option 119, which systemd-networkd discards because UseDomains= defaults to no, and a cluster domain under .local then falls through to the mDNS exclusion in resolved and answers REFUSED. The virtual-machine chart carried a network-data template for the first of those, gated on systemDisk.image. Splitting that chart into vm-disk and vm-instance removed the template along with the field it read, and neither came back. Matching en* and eth* rather than naming interfaces by index keeps the configuration correct whatever the guest enumerates its NICs as, which is where the previous template was fragile. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Alexey Artamonov
|
e63b9e439c |
fix(monitoring): count a central request as the receiver admitted it
The limiter refuses for good any request it counts above burst, and burst was the receiver's 4 MiB cap. A request grows on its way to the limiter: the sampler adds a tracestate to every span it keeps, and the limiter counts a request as the collector re-encodes it, which writes the empty value of every attribute the wire may omit, so a valid request of spans with empty-valued attributes is counted 1.4x. A request the receiver admitted near its cap was therefore refused on every retry. The limiter now runs right after memory_limiter, so it counts the request before sampling. The receiver caps a request at 3 MiB and burst stays at 5 MiB, which holds 1.4x the cap; a larger burst would also be what each pod start lets through above the rate. The rate is counted before sampling, so the store keeps only the sampled share of it. The texts say that a request above the cap gets a 400, which senders do not retry, and that each collector rollout a tenant causes lets that burst through again on every pod. The replica bound that keeps that allowance small now applies only to a central tenant's collector, whose render fails above ten replicas: a collector writing to the tenant's own store has no rate, and the replica counts it shipped with stay valid. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
65f0caae2b |
fix(monitoring): keep a central tenant's write login out of its Grafana
A tenant's Grafana pod may reach the shared traces store's vmauth, and
the Secret a tenant names in oidc.customConfig.secretRef was mounted
into it whole. The apps API does not prune keys the Monitoring schema
leaves out, so a tenant could also set grafana.image: an image of its
choosing, with the collector's Secret mounted, writes past the rate
limit the collector enforces.
Grafana's image is the platform's, like the collector's, and for a
central tenant only the auth.ini key Grafana reads is mounted from the
named Secret. Other tenants have no collector login to keep out and
keep the whole Secret, which an auth.ini reading a sibling key with
$__file{} relies on.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
cc51cac63d |
fix(monitoring): give a central tenant's Grafana a login that cannot write
The collector holds a central tenant to its write rate, but the one login it writes with was also the login of the tenant's Grafana datasource, and it could both read and write. vmauth cleans the request path before it picks a route, so any user the tenant admits to its Grafana, a Viewer included, could POST through the datasource proxy to ../insert and write into the shared store past the limiter. Each central tenant now has two VMUsers on its one account: the collector's, which may only write, and Grafana's, which may only read, each with its own chart-owned Secret and password. A namespace name holds no dot, so the reader's username never matches another tenant's. On upgrade the datasource moves to the new login in the same release that creates it, so reads may fail until vmauth loads the new user. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
3497855e15 |
fix(platform): read the tracing rate only as plain decimal digits
Sprig's int64 reads a string with a leading zero as octal and turns a parse error into 0, so "0100" would reach the collectors as 64, and "08" or digits beyond int64 would leave the default in place without a word. A float beyond int64 converts differently on each CPU architecture, and the collector adds its replica count to the rate before splitting it. A string rate must now be plain decimal digits, and any rate above 1 TiB/s fails the render; the collector also falls back to the default on digits it cannot hold rather than start with a rate of zero, which the processor refuses. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
a0aea139fc |
fix(platform): refuse a malformed shared-central tracing rate
A rate written as a quantity or a fraction ("1Mi", "100KiB", 1.5) was
read as zero, so the platform wrote no key and every collector kept
the default without a word. The platform render now fails on anything
but a whole number of bytes per second. Helm reads a number from
values as a float64, which prints a million as 1e+06, so a whole
number is checked as a number and written as int64 digits.
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
|
||
|
Alexey Artamonov
|
fc0db49187 |
fix(monitoring): bound what a central tenant writes above its rate
Each collector pod starts with a full token bucket, and burst was at least 64 MiB, the largest request vtinsert accepts. A tenant controls when its collector pods start, through any Monitoring change or by deleting them, and how many there are, so it could write tens of mebibytes above its rate per pod start and gigabytes per rollout of many replicas, which the two-day bound does not survive. The limiter only sees requests the receiver admits, so in central mode the receiver now caps them at 4 MiB, decompressed for HTTP, and burst drops to that floor; a pod start lets at most 4 MiB through above the rate. The replica count is capped at 10. The texts now say what the default rate guarantees: one tenant writes at most a default store's disk cap over two days counted uncompressed, rather than that it cannot fill it, and they name the per-pod-start allowance and that a sender pinned to one replica gets that replica's share. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
f8deb8e171 |
fix(platform): write the tracing rate only when it is positive
apps.yaml wrote the rate for any non-zero value, so a negative one reached every tenant's cozystack-values Secret and upgraded every release in every tenant, while the collectors ignored it and kept the default. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
177b207a27 |
feat(monitoring): hold each shared-central tenant to a write rate
The design has the per-tenant collector rate-limit its tenant, so that a noisy or hostile tenant cannot exhaust a shared backend. The disk cap alone does not do that: VictoriaTraces always keeps the last two days, so a tenant writing faster than the cap allows over two days fills the disk for every tenant on the store. The platform sets the rate with monitoring.tracingCentralTenantBytesPerSecond, and it reaches every tenant in _cluster through the cozystack-values Secret. A change there is a change in the values each monitoring HelmRelease reads, so Flux upgrades them all; a rate read with lookup at render time would never reach a tenant whose release is already in sync. The platform writes the key only when it is set, since every release in every tenant reads that Secret. Without it the collector holds the tenant to the rate of a default 10Gi store. Each central tenant's collector splits the rate across its replicas and refuses data above it before the batch processor, so the sender sees the refusal. An OTLP/HTTP sender gets a 429 and retries; the limiter answers gRPC with RESOURCE_EXHAUSTED and no RetryInfo, which the OTLP specification makes final, so central tenants should export over HTTP. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
32a6c71972 |
fix(monitoring): keep the collector image out of a tenant's reach
The apps API defaults a Monitoring spec from its schema but does not prune keys the schema leaves out, and the Monitoring chart hands its values to the inner chart as they are. A tenant could therefore set tracingCollector.image and run any collector it liked in its namespace; with a per-tenant rate limit in that collector, the same key would let a tenant drop the limit. The image is the platform's, so it is no longer passed through. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
474377e8ca |
build(monitoring): build the traces collector with a rate limiter
A shared-central traces store needs each tenant held to a write rate, or one tenant can fill the disk every tenant shares. No upstream OpenTelemetry distribution or processor limits throughput. Elastic's EDOT Collector registers Elastic's ratelimitprocessor among its default components, but it ships under the Elastic License 2.0 and carries far more components than the chart uses. The collector is therefore built here with the OpenTelemetry Collector Builder from the upstream components the chart already uses, at the same v0.136.0, plus that ratelimitprocessor (Apache-2.0). Its v0.8.2 is the release built against v0.135.0, the last before v0.137.0. The image is versioned by the collector release with a cozystack respin counter, runs as the upstream image's user, and is built with the other package images. The builder stage is alpine, which has no git, so a proxy error during its go install would have had neither the direct fallback nor a retry; the stage installs git. The module-fetch contract saw only go build, so it now checks go install too, and takes only git as cover for a versioned go install, which resolves outside the stage's module. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
f4a6eb809c |
fix(nats): enable JetStream for the generated account (#4675)
## What this PR does With `users` configured and `jetstream.enabled: true`, the application chart enables JetStream on the server but generates account `A` without it. Authentication and ordinary NATS messaging work, while `$JS.API.INFO` returns error 10039, "JetStream not enabled for account". The generated account now gets `jetstream: true` when persistent messaging is enabled and the account does not set JetStream itself. Tenant-supplied limits, explicit disablement, other accounts and the existing generated-user merge are kept. nats-server matches config keys case-insensitively and walks the account map in no fixed order, so a `JetStream` key in any casing counts as set. Otherwise the account would carry both keys and the server would apply either one, changing from restart to restart. Existing instances with `users` pick this up on upgrade. The rendered NATS config changes, so the StatefulSet rolls its pods once, and account `A` gets JetStream without account limits unless `config.merge.accounts.A.jetstream` sets them. ### Validation - `helm unittest` passes all 75 application-chart tests. Eight cases cover this change. Against the original template the generated-account and null-override cases fail. The other six pin what the change must keep, and the mixed-case one fails as soon as the check is case-sensitive. - Dropping the `jetstream.enabled` check, the case folding or the explicit-setting check turns at least one case red each time. - The application chart and the vendored system chart were rendered, and the resulting account configuration was run on NATS Server 2.11.10 in an isolated standalone setup. The original template reproduces error 10039. With this change a file-backed stream can be created, published to and read, and explicit disablement and a one-stream limit stay enforced. - `make generate` with cozyvalues-gen v1.7.0 leaves the generated files unchanged. ### Screenshots Not a UI change. ### Downstream repositories The diff changes an application template and chart tests only. Public values, schemas, output resource names and tooling contracts are unchanged. The website NATS documentation contains no account-level workaround to remove. No downstream trigger applies. - [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(nats): enable JetStream on the account generated for users when jetstream.enabled is true, keeping any explicit account-level JetStream setting. Instances with users roll their NATS pods once on upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Generated NATS accounts now enable JetStream when configured, without overriding an existing JetStream setting. Configured passwords are added while preserving existing account settings. * **Tests** * Added coverage for enabled and disabled JetStream, explicit limits and false or empty values, case-insensitive setting names, null overrides, and manually configured accounts when generated users are absent. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
7bf6906a80 |
fix(platform): keep the legacy etcd-operator stopped after etcd adoption (#4738)
## What this PR does Migration 50 scales the legacy etcd-operator to 0, adopts every legacy `etcd.aenix.io` cluster with `etcd-migrate`, and then scales the operator back to 1. At that point the Deployment still runs the legacy operator, and after a successful adoption it has nothing left to manage. Bringing it back undoes part of the adoption. Observed on a production cluster upgraded from 1.4.3 to 1.6.4. The API audit log and the Helm history of a tenant's `etcd` release show this sequence, in UTC on the same day. - 13:21:12 migration 50 scales `cozy-etcd-operator/etcd-operator-controller-manager` to 0. - 13:22:59 `etcd-migrate` updates `etcd-headless`, pointing its owner references at the adopted EtcdMembers, and orphan-deletes the legacy EtcdCluster and StatefulSet. - 13:23:04 migration 50 scales the operator back to 1. It is still the legacy operator, the v1alpha2 one replaced it only at 13:40. - 13:23:42 Flux upgrades the tenant's `etcd` release with the old chart (Helm revision 87), because the new chart artifact appeared only at 13:24:40. The old chart re-creates the legacy EtcdCluster at 13:23:43. - 13:23:43 to 13:23:45 the legacy operator reconciles it, updates `etcd-headless` twice and re-creates the legacy StatefulSet `etcd`. - 13:36:49 Flux upgrades the release to the new chart (revision 88), which prunes the legacy EtcdCluster. - 13:36:50 the garbage collector deletes `etcd-headless` and the re-created StatefulSet. The same happened in all five tenants that run an etcd. The pods survived because EtcdMembers own them, but they keep `spec.subdomain: etcd-headless`, so their `etcd-<i>.etcd-headless.<ns>.svc` names stopped resolving. From then on every member logged `no such host` for its peers, the v1alpha2 EtcdCluster had no status conditions, and the `etcd-defrag` CronJob failed every hour with `context deadline exceeded`. The transitional `etcd-headless` Service the etcd chart ships for this window (#3270) does not cover it. Helm patched the existing Service 202 ms before the garbage collector removed it, and did not create it again because it was already part of the release. With this change the operator stays at 0 after a successful adoption. The etcd-operator HelmRelease replaces the Deployment with the v1alpha2 operator later in the same upgrade, and no later migration waits for the operator. A failed or interrupted adoption still restores it to 1 through the existing EXIT trap, so a failed run does not leave the tenant etcds without an operator. If that HelmRelease cannot reconcile, the adopted clusters wait for it with or without this change, because the legacy operator does not reconcile v1alpha2 EtcdClusters. Clusters already upgraded with this bug can be repaired without restarting anything. Creating the Service from the tenant's own `etcd` release is enough. ``` helm -n <ns> get manifest etcd | awk 'BEGIN{RS="\n---\n"} /\nkind: Service\n/ && /\n name: etcd-headless\n/' | kubectl -n <ns> create -f - ``` On the cluster above the v1alpha2 EtcdCluster reported `Available` with reason `QuorumHealthy` within 11 seconds of the Service being created, in every tenant, and the members stopped logging `no such host`. ### Validation - `hack/cozytest.sh hack/migration-50-etcd-adopt.bats` passes all 29 tests. A new test asserts that a successful adoption scales the operator to 0 and never back to 1. Against the current script it fails with `FAIL: the legacy operator was scaled back up after a successful adoption`. - The existing test for a failed adoption still passes, so the EXIT trap still restores the operator to 1. - The new test checks the absence with `if grep ...; then echo "FAIL: ..."; false; fi`, the form the other suites use. A bare `! grep` assertion cannot fail a test under `hack/cozytest.sh`, so it could not have caught this. The other negated assertions in this file had the same weakness, and #3790 has rewritten them on main. With all of them made able to fail in a scratch copy, the suite still passes against this change. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. It changes a migration script and its bats test. No package, values, schema, CRD, make target or `hack/` layout changes, so no entry matches. - [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(platform): keep the legacy etcd-operator stopped after migration 50 adopts the legacy etcd clusters. Scaling it back up let it reconcile the legacy EtcdCluster that Flux re-created from a not yet upgraded etcd chart, re-create the legacy StatefulSet and take the legacy etcd-headless Service away from the adopted members, which the garbage collector then deleted. The adopted members lost their DNS names, the EtcdCluster never reported a status and etcd defrag failed every hour. Clusters already upgraded can recreate the Service from the etcd release manifest. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Migration Updates** * After successful etcd adoption, the legacy operator remains scaled to zero replicas for replacement by the etcd operator; it is no longer scaled back up before the migration is stamped. * If adoption fails or exits before completion, the legacy operator is restored to one replica. * The dry-run message now describes the zero-replica outcome of successful adoption. * Successful adoption continues to record the migration version stamp. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
0639ce39aa |
fix(kamaji): drop any subchart nested under the vendored kamaji chart (#4459)
## What this PR does A subchart nested under the vendored kamaji chart would deploy no matter what the values say, so this PR removes that directory in `make update` and adds a test that fails if anything renders from it. The vendored chart declares a `kamaji-etcd` dependency gated on `kamaji-etcd.deploy`, and the subchart itself is not vendored. The issue proposed pinning `kamaji-etcd.deploy: false` in the package values. I tested that, and it would not have helped: through this package the condition is never evaluated at all. Helm skips the condition because the umbrella chart declares no dependencies. `processDependencyEnabled` returns immediately when the chart it starts from has no `dependencies` in its `Chart.yaml`, and `cozy-kamaji` has none. The vendored `kamaji` chart is loaded from `charts/` without being declared, so Helm never recurses into it and never reads its `kamaji-etcd.deploy` condition. The same early return is in Helm v3 and v4. I put `kamaji-etcd-0.15.0.tgz` into `charts/kamaji/charts/` and rendered the package three ways: - default values: StatefulSet, Job, DataStore and four Certificates from `kamaji-etcd` - `kamaji.kamaji-etcd.deploy: false` in the package values: the same etcd - `deploy: false` edited into the vendored `charts/kamaji/values.yaml`: the same etcd Rendering `charts/kamaji` on its own with `kamaji-etcd.deploy=false` renders no etcd, which confirms the umbrella is what skips the condition. So any chart that ends up under `charts/kamaji/charts/` deploys whatever the values say, and no values change can close the hazard. This PR changes two things: - `make update` removes `charts/kamaji/charts` after extracting the upstream chart. Upstream does not ship that directory today, so the recipe still reproduces the current tree exactly. I checked that by running it on a copy of the package. I also ran it against a fake upstream tarball that carried the subchart: the directory was extracted and then removed. - The package gets its first helm-unittest suite. It asserts that no template of a subchart nested under `charts/kamaji/charts/` renders a document, and a `test` target lets `hack/helm-unit-tests.sh` pick it up in `make unit-tests`. With the subchart tgz dropped in, the suite fails. With the `rm` line removed from the recipe, the fake-upstream update leaves the tgz behind and the suite fails too. `/tests` is anchored in `.helmignore`, so the packaged chart's file list is unchanged. I left the `kamaji-etcd` block in the vendored values alone. It is not quite dead: `certmanager_issuer.yaml` reads it to decide whether to keep the self-signed issuer. That only matters when `certManager.issuerRef.name` is set, and this package does not set it. `helm template` output is byte-identical to main. Closes #4397 ### Screenshots Not a UI change. ### Downstream repositories The change touches only the kamaji package's update recipe, its test suite and its `.helmignore`. The closest trigger is package `Makefile`s → the website's `development.md`, but that page describes `make update` and `make test` in general terms that still hold, and the rendered chart is unchanged. - [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(kamaji): `make update` now drops any subchart nested under the vendored kamaji chart, and a unit test fails if one appears, because such a subchart (for example the bundled kamaji-etcd) would deploy regardless of `kamaji-etcd.deploy`. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Helm chart updates now avoid including nested charts that could deploy unexpectedly. * Helm chart packages exclude test files, keeping them out of distributed chart artifacts. * **Tests** * Added a chart check to verify nested charts render no resources. <!-- 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 --> |
||
|
IvanHunters
|
86bf2ba8ec |
fix(kubernetes): let kubevirt-csi hot-unplug from an orphaned VMI (#4563)
<!-- 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 This lets the kubevirt-csi controller detach a volume from a VM instance that no VM owns. Right now that call is always denied. Closes #4452. The driver has a fallback for a volume that is still attached to the VMI but is no longer in the VM spec. It calls the VMI-level `removevolume` subresource. The `-kcsi` Role grants only the two VM-level subresources, so the call ends in 403. Nothing else reaches this service account with that grant. Only its own Role and ClusterRole name it as a subject, and the group bindings that cover every service account grant nothing on this subresource. At v1.9.0, `kubevirt.io:default` grants only `kubevirts`, `version` and `guestfs`. `kubevirt.io:edit` and `kubevirt.io:admin` do include the VMI subresource, but they aggregate only into the built-in `edit` and `admin` roles, and no tenant binding gives those to this account. The cozystack tenant roles aggregate on their own labels and don't list it. The fallback is still needed after the flag in #4453 goes to false. The flag only skips VMIs that a VM owns. When a VM is deleted with `propagationPolicy=Orphan`, the KubeVirt VM controller drops its finalizer and leaves the VMI running (`vm.go` at v1.9.0). The tenant node stays up with its volumes attached. When a pod moves off that node, the driver has to detach through the VMI, and on a 403 the hot-plug pod keeps holding the infra volume. Until #4453 lands, the fallback also runs for VM-owned VMIs and gets the same 403. The grant is `update` on `virtualmachineinstances/removevolume` and nothing more. The account can already remove a volume from any VM in the namespace, so it gets no new reach. virt-api also rejects the VMI-level call on a VM-owned VMI once `HotplugVolumes` is off. `virtualmachineinstances/addvolume` stays out because the driver only attaches through the VM. The new helm-unittest pins the Role's `subresources.kubevirt.io` rule to these three subresources with `update`. I haven't reproduced the 403 on a cluster. This shows it on a tenant namespace, before and after: ```bash kubectl auth can-i update virtualmachineinstances.subresources.kubevirt.io --subresource=removevolume \ --namespace <tenant-ns> --as system:serviceaccount:<tenant-ns>:<release>-kcsi ``` Upstream kubevirt/csi-driver has the same gap in `deploy/infra-cluster-service-account.yaml`. ### 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. --> No UI change. ### 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(kubernetes): the kubevirt-csi controller can now hot-unplug a volume from a VMI that no VM owns, instead of failing with 403 Forbidden. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Kubernetes CSI volume removal for virtual machine instances, including cases where a volume is not listed in the virtual machine specification. This helps the driver complete volume removal in scenarios where the Kubernetes virtualization API accepts the instance-level request. Existing permissions for adding and removing volumes from virtual machines remain unchanged. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
ec4d26df54 |
fix(kubernetes): accept a golden worker image that exists only as a PVC (#4578)
## What this PR does A worker pool on `osImage.builtin` refused a golden image that exists in `cozy-public` as a PVC without its DataVolume, and the whole KubernetesNodes release failed to render. CDI clones the PVC, not the DataVolume, so that golden is a valid source, and vm-disk already accepts it. This can happen once CDI garbage-collects DataVolumes (`dataVolumeTTLSeconds`) or when someone restores a golden PVC on its own. Now the render fails only when neither the DataVolume nor the PVC exists. The StorageClass and size checks already fall back to the PVC, so a PVC-only golden is still checked against the pool, and the new test suite pins that too. Closes #4565 ### Screenshots ### 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 fix(kubernetes): a worker pool on osImage.builtin now accepts a golden image that exists in cozy-public only as a PVC, matching vm-disk; it is refused only when neither the DataVolume nor the PVC exists. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Built-in golden images can now be used when available as either a DataVolume or a PVC. * **Bug Fixes** * Missing golden images now produce an error that identifies the missing image and suggests ways to resolve the issue. The error is suppressed when the pool already has a successful clone. * PVC-based golden images are checked against the pool’s storage class and minimum disk size requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
f9f7562e56 |
fix(kamaji): resolve Gateway API availability once at startup (#4501)
## What this PR does Carries the fix proposed upstream in clastix/kamaji#1327 (not yet merged there; issue clastix/kamaji#1326) as a build patch on the 26.9.4-edge tag that cozystack builds Kamaji from. In Kamaji 26.9.4-edge, every TenantControlPlane reconcile checks whether the Gateway API is available. The discovery call ignores the reconcile context, so it is bounded only by the client's 32s default timeout, which is longer than the 30s reconcile budget. The watches, however, read availability only once, at startup. With the patch, Kamaji resolves availability once when the manager starts. The watches, the indexer, the reconciled resources and the admission webhook's Gateway API check all use that one value. A discovery error at startup now stops the manager, so it is no longer mistaken for "Gateway API not installed". A Gateway API installed after Kamaji starts needs a Kamaji restart. On this platform that case does not come up: the kamaji package depends on cert-manager, and cert-manager depends on gateway-api-crds. The webhook's separate TLSRoute check still runs discovery on each request, as before, and is listed as a follow-up in clastix/kamaji#1327. Kamaji's chart sets no `timeoutSeconds` on the webhook, so the apiserver's 10s default applies, and a discovery call that can take up to the client's 32s can make that admission time out. The patch is the upstream commit applied unchanged except for the dropped docs hunk, which is not part of the image. It applies cleanly to 26.9.4-edge with no offsets. It modifies different files than the existing `fix-kubelet-config-compat.diff`, so the two apply in either order. The upstream tests stay in the patch so that it matches the upstream commit; the image build does not run them. On the patched tree, `go build ./...` passes, and `go test -race` passes for `./controllers/...`, `./internal/webhook/...`, `./internal/utilities/...`, `./internal/kubeadm/...` and `./cmd/...`. I also checked that the new tests can fail: they go red when the availability check is inverted and when the old per-reconcile lookup is put back. If clastix/kamaji#1327 changes before it merges, this patch should be updated to match. It is dropped once Kamaji ships the change. Refs #4489. This does not close #4489, because this change does not explain the long stall reported there. ### Screenshots No UI change. ### 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 fix(kamaji): Kamaji now checks for the Gateway API once at startup instead of on every TenantControlPlane reconcile, so a slow discovery endpoint can no longer use up the reconcile timeout. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Gateway API availability is now checked at startup and consistently applied to Gateway-related setup and configuration validation. * Gateway configuration is rejected if the Gateway API was unavailable when Kamaji started. If you install the required CRDs afterward, restart Kamaji before configuring Gateway resources. * Discovery errors now stop startup rather than allowing Kamaji to continue with an uncertain Gateway API status. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
50dec5d46a |
fix(platform): resume a retried migration Job from the stamped version (#4583)
## What this PR does A retried pod of the platform migration Job no longer runs migrations that already finished in the same upgrade. The Job gets `CURRENT_VERSION` once, at render time, and a failed pod is replaced by one with the same environment. By then every finished migration has stamped the `cozystack-version` ConfigMap, but the new pod ignored that and started from the rendered version again. `run-migrations.sh` now starts from the version stamped in the ConfigMap. If reading it fails, the pod fails and the Job retries it, because falling back to the rendered version is exactly the re-run this fixes. The rendered value is used only when the ConfigMap holds no integer version, which is also when the chart itself could not get a real version from it. Without that fallback a value like `v58` gave a green Job that ran nothing. The new `hack/run-migrations.bats` runs the script in the migrations image's alpine base against a fake kubectl that keeps the stamp in a file, so one pod's stamps are what the next pod reads. It needs docker, like the other migration suites. I also found that the ConfigMap existence check at the top of the script treats any API error as a missing ConfigMap and skips every pending migration. That is older than this change and is filed as #4579. Closes #4555 ### Screenshots ### 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 fix(platform): a retried platform migration Job now resumes from the version stamped in the cozystack-version ConfigMap instead of re-running migrations that already finished in the same upgrade. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Migration startup now resumes from a valid stamped version, avoiding reruns of migrations that have already completed. * Empty or non-numeric stamps fall back to the rendered version. Out-of-range stamps or failed stamp reads stop migrations before they run. * If the migration ConfigMap is missing, migrations do not run and the target version is stamped. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
bb2946990d |
fix(vm-instance)!: refuse guest memory below KubeVirt's 1M floor (#4569)
<!-- 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-instance resource guard now refuses guest memory below 1M, the same floor KubeVirt admission enforces. Before, a value such as `500m` or `1Ki` rendered and then failed when the VirtualMachine was applied. This is a breaking change. A memory above zero and below 1M rendered before and is now refused at render time. KubeVirt v1.9.0 would not admit such a VM either: `validateMemoryRequestsNegativeOrNull` refuses a non-zero request below 1M (`pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go:1135`). The VirtualMachine admitter runs the same check on its template. The cpu, sockets and memory descriptions now list the values the guard refuses, so the schema states the same rule as the error. The error cannot quote an unquoted number as it was written, because YAML has decoded it before the chart sees it. A large one also printed in exponent form, so `4294967296` came out as `4.294967296e+09`. A string is still quoted exactly as written. A number is now printed in plain digits and marked "read as", which is the closest honest wording I found. Closes #4413 ### 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. --> No UI change. ### 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(vm-instance)!: refuse `resources.memory` below 1M at render time, since KubeVirt does not admit a VM with less. A value above zero and below 1M rendered before and is now refused. The cpu, sockets and memory descriptions list the refused values, and the guard error marks an unquoted number as read rather than written. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * VM resource values are now checked when rendering: CPU cores and sockets must be positive whole numbers no greater than 4294967295. Unit-suffixed, fractional, zero, negative, and quoted values with leading zeros are rejected. * Memory values below `1M` are rejected, with clearer messages showing the value as entered or interpreted. * **Documentation** * Updated resource guidance to describe these limits and validation behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
ed20c53449 |
fix(vm-disk): refuse to clone a golden image that does not exist (#4567)
<!-- 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 Stacked on #4392, so the base is `fix/vm-default-images-health-check`. Retarget to `main` once #4392 merges. A new vm-disk with `source.image` now fails to render when the golden image it names does not exist, and the error names the image and `cozy-public`. Before this the chart cloned `cozy-public/vm-default-images-<name>` without looking, and CDI waited for that PVC forever while the VMDisk stayed Ready. That happens on a typo, and on every cluster where the opt-in `cozystack.vm-default-images` package is off, so the message also says to add it to `bundles.enabledPackages`. The golden counts as missing only when neither its DataVolume nor its PVC exists. CDI clones the PVC, so a PVC with no DataVolume is still a usable source, and a DataVolume whose PVC is not created yet is an import about to start. Under `helm template` every lookup is empty, so the check runs only when the release namespace itself is visible. The same trick is already used in the kubernetes and seaweedfs charts. Existing disks are exempt the same way #4392 exempts them, because their DataVolume spec is immutable. A disk refused while the catalog is off renders by itself once the golden appears, since app HelmReleases retry on failure. Tests are in `packages/apps/vm-disk/tests/dv_golden_image_test.yaml`: refusal of a missing golden, a PVC without a DataVolume, a DataVolume before its PVC, rendering with no cluster, and an existing disk whose golden was removed. Closes #4548 ### 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. --> No UI change. ### 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(vm-disk): a new disk with source.image now fails with a clear error when the golden image does not exist in cozy-public, instead of waiting forever for a clone that never starts. ``` |
||
|
IvanHunters
|
c7b8009964 |
fix(kubernetes): log kubevirt-csi temp NFS mount dir removal failure (#4562)
<!-- 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 kubevirt-csi node plugin now logs a warning when it cannot remove the temp dir it uses for NFS publish. Before, the error was dropped, so a dir left behind after a failed unmount was never reported. The publish path temp-mounts the export root to create `/data`, then removes the temp dir in a defer. The failure is logged, the same way the unmount failure next to it already is, and not returned. A leftover temp dir does not break the volume, and failing a publish that otherwise worked only makes kubelet retry it. A new unit test publishes an NFS volume through the fake mounter, where the removal always fails, and checks that a warning naming the dir is logged. This is independent of #4451. That PR touches the same Go module but not `node.go`. A failed unmount still leaves the NFS root mounted. That is older than this change and is tracked in #4561. Closes #4466 ### 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. --> No UI change. ### 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(kubernetes): the kubevirt-csi node plugin now logs a warning when it cannot remove its temporary NFS mount dir instead of dropping the error ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Failed cleanup of a temporary mount directory now produces a warning with the directory path, making the issue easier to identify. The volume publish operation continues to succeed when cleanup fails. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
2ddd55ce5b |
fix(build): honour PUSH and LOAD from the environment and in image-talos (#4654)
## What this PR does A local build that asks to load images into docker no longer pushes them. `hack/common-envs.mk` set `PUSH` and `LOAD` with plain `:=`, and a makefile assignment beats the environment, so `LOAD=1 PUSH=0 make image` quietly pushed a multi-arch build. Both now take the environment value, and an empty one falls back to the default. I also made `PUSH` default to 0 when `LOAD=1`, so `LOAD=1` alone only loads. Before this, `make image LOAD=1` pushed and loaded at once. An explicit `PUSH` from the environment or the command line still wins, and `OCI_EXPORT_DIR` still forces both off. Every buildx target gets the values through `BUILDX_ARGS`, so the fix lives in that one file. The Talos installer was the one image that ignored `PUSH`. It is published with skopeo, not buildx, so it never saw `--push=$(PUSH)`. Its registry copies now run only when `PUSH=1`. A build that pushes nothing no longer writes `PUSHED_TAGS_LOG`. That log tells the multi-arch stitch which tags to move, so it must hold only tags that were really pushed. The cache helper lost an argument nobody used. No caller passes the second argument of `cache-args`, and an explicit tag there would bypass `CACHE_TAG`, so the amd64 and arm64 builds of that image would share one cache ref. CI sets neither `PUSH` nor `LOAD`, so CI builds do the same thing as before. Each fix has a `make --dry-run` test in `hack/common-envs_test.bats`. Closes #4499 Closes #4504 Closes #4551 ### Screenshots Not a UI change. ### 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: The ccp skills pass `PUSH=1` explicitly, which still works. The website docs do not mention `PUSH`, `LOAD` or `cache-args`. ### Release note ```release-note fix(build): `LOAD` and `PUSH` set in the environment are now honoured, `LOAD=1` no longer pushes unless `PUSH=1` is also set, and the Talos installer is no longer published when `PUSH=0`. ``` |
||
|
Aleksei Sviridkin
|
2af8b0c229 |
ci(build): fall back to direct, retry and cache Go module downloads (#4660)
<!-- 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 One transient error from proxy.golang.org is enough to fail an image build or the `Unit & controller tests` job today. The error is an HTTP/2 stream reset (`stream error ... INTERNAL_ERROR`), and nothing retries or falls back after it. This PR adds three things: a direct fallback, a bounded retry and a module cache. Image builds now fall back to fetching modules from their origin when the proxy fails. They get `GOPROXY=https://proxy.golang.org|direct` as a build arg from `hack/common-envs.mk`, and every Go builder stage declares `ARG GOPROXY`. With a pipe Go goes direct on any proxy error. With Go's default comma it does that only on a 404 or 410. The value comes from `IMAGE_GOPROXY` and not from the host's `GOPROXY`, because buildkit can't reach a mirror on localhost or a `file://` proxy. Every workflow job that runs Go on the runner sets the same value as env. Explicit module downloads in Dockerfiles are retried three times with a growing pause. The loop is inline, in one form that the contract test pins. I tried a shared script from a named build context first, but buildx refuses any `--build-context` for the four Dockerfiles still pinned to the `dockerfile:1.2` frontend. The images also build from different contexts, so no single file can be copied in. Raising those pins changes how those images build and needs a real build to check, so I left it for a follow-up. Two alpine stages, token-proxy and shadowbox's `ss-server`, left the download to `go build`. The alpine golang images have no git, so the direct fallback can't fetch for them, and they now get the same download step. The runner jobs use `hack/go-mod-download.sh` with the same bound. The unit test job now starts from a module cache instead of downloading everything. It restores `GOMODCACHE` with `actions/cache`, keyed on every `go.sum` in the tree, and then prefetches all tracked modules through the retry, so a cache miss still works. A cache saved by a PR run is visible only to that PR, but one saved on the default branch can be restored by any PR ([docs](https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching#restrictions-for-accessing-a-cache)). That is why the build cache warmer on main writes the same key before its image build. This way the first run of a PR, which is the one that flakes, starts warm. The warmer restores only an exact key, so old module versions don't pile up, and its Go steps can't fail the image build. Both runner labels are Linux, so the key matches. Both also check out under `/home/ubuntu/_work`, so `go env GOMODCACHE` should give the same cache path, but I have not seen it on the warmer's runner yet. The release jobs in `tags.yaml` and `promote-rc.yaml` get the GOPROXY env only, no cache. `hack/go-module-fetch-contract.bats` covers all three. Each test is red on main, and I checked each guard with a mutation. Closes #4591 ### 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. --> - [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 ci(build): Go module downloads in image builds and CI jobs now fall back to direct fetching and retry on transient proxy.golang.org errors. The unit test job reuses a module cache written from main. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Reliability** - Go module downloads now fall back to direct access when the public proxy encounters errors. - Builds and automated checks retry failed module downloads, helping them recover from temporary network issues. - **Performance** - CI caches Go modules to reduce repeated downloads and speed up builds and tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
9c025fa2b1 |
fix(installer): skip the packages artifact push and stamp with PUSH=0 (#4663)
## What this PR does A local build with `PUSH=0` no longer publishes the packages artifact. Before this change, `make image-packages` in `packages/core/installer` ran `flux push artifact` no matter what `PUSH` was set to, and then wrote the pushed digest into `values.yaml`. The `chart` target in the same Makefile already respects `PUSH`. With `PUSH` set to anything other than 1, the target now skips both the push and the digest stamp. It prints one line saying so and leaves the committed `platformSourceUrl` and `platformSourceRef` as they are. It still writes `packages/core/platform/.build-revision`, which is gitignored. I chose to skip the stamp instead of computing a local digest, because every reader of that value needs a digest that exists in the registry. The chart passes `platformSourceRef` to the operator as `--platform-source-ref`, and Flux then pulls `cozystack-packages@<digest>` from the registry. The release scripts `verify-promoted-packages.sh`, `promote-packages-artifact.sh`, `stitch-multiarch.sh` and `nightly-mirror.sh` also treat it as a registry digest. A digest of an artifact that was never pushed points at nothing. On main an unpushed run would not even get that far, since the recipe dies at `test -n "$DIGEST"`. The gate is a make-level `ifeq ($(PUSH),1)`, so it reads whatever `PUSH` ends up as after `hack/common-envs.mk`. On main that means `PUSH=0` on the command line. I also applied the `common-envs.mk` change from #4654 locally, which takes `PUSH` from the environment and makes `LOAD=1` default it to 0. With that applied, a dry run shows no push and no stamp for `PUSH=0` in the environment, for `LOAD=1`, and for `PUSH=0` on the command line, while `LOAD=1 PUSH=1` and an empty `PUSH` still push. CI is not affected, because no workflow sets `PUSH` or `LOAD` and `PUSH` stays at 1. I dry-ran the three CI invocations of this Makefile before and after the change, on main and again with #4654 applied: - `tags.yaml`, release stitch step: `make -C packages/core/installer image-packages chart` with `REGISTRY`, a release tag as `IMAGE_TAG`, and `PUSHED_TAGS_LOG` - `pull-requests.yaml`, installer step of a same-repo PR: `make -C packages/core/installer image` with a `pr-N-sha` tag, `PLATFORM=linux/amd64`, `BUILDER`, and an empty `OCI_EXPORT_DIR` - `build-main.yaml` and `build-release.yaml`: `make build`, which runs the installer `image` target with defaults ```text $ diff --recursive before/ after/ # tags.txt pr.txt build.txt $ echo $? 0 ``` There is one side effect. `OCI_EXPORT_DIR` forces `PUSH := 0`, so running `image-packages` with it set no longer pushes, where before it would have tried an anonymous push. No CI path does that. The fork PR build already runs only `pre-checks image-operator` for the installer, and the `make -n ... image OCI_EXPORT_DIR=/oci` scan in `e2e-fork.yaml` greps only the `.oci.tar` names, which this change does not touch. The new test dry-runs the target in both modes. With `PUSH=0` there must be no `flux push artifact` and no `platformSourceRef` stamp; with `PUSH` at its default, both must be present. The first test fails on main. I mutated the guard three ways, and each time only the matching test went red: - condition forced true: the `PUSH=0` test fails - compared against 2: the default-path test fails - `endif` moved up so only the push is gated: the `PUSH=0` test fails on the stamp check One limitation is left as a follow-up. The test's `env -u PUSH -u LOAD` only isolates it once #4654 lands, because on main a file assignment wins over the environment anyway. It does not cover `PUSH=0` coming in through `MAKEFLAGS`, so `make bats-unit-tests PUSH=0` would fail the default-path test here and the ordering check in `hack/stitch-multiarch_test.bats`. Nothing runs the suite that way today. If that changes, passing an explicit `PUSH=1` on the default-path `make` lines, or unsetting `MAKEFLAGS` the way `hack/bats-unit-keep-going.bats` does, would fix it. ### Screenshots Not a UI change. ### 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: The website's `development.md` tells developers to run `make image-packages` with `REGISTRY` set to their own registry. That is the default `PUSH=1` path, and it works as before. Neither the website nor ccp mentions `PUSH=0` for this target. Closes #4656 ### Release note ```release-note fix(installer): `make image-packages` no longer pushes the cozystack-packages artifact or stamps its digest into values.yaml when PUSH=0. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * You can run `image-packages` with `PUSH=0` to skip publishing the artifact and keep the committed platform source reference unchanged. * When pushing is enabled, the command publishes the artifact and updates the platform source URL and digest-based reference. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
7ea71042b7 |
ci(e2e): add a daily isp-hosted install lane on kind (#4712)
## What this PR does
This adds a daily CI lane that installs the `isp-hosted` platform
variant through the `hosted` installer variant. No lane ran either of
them before. The PR, nightly and tag lanes all install `isp-full` on
Talos, where Cozystack brings its own CNI and storage, and that is how
#3376 shipped unnoticed.
What `isp-hosted` changes is what it leaves to the host. The system
bundle swaps kube-ovn and Cilium for a `noop` networking Package and
drops multus, LINSTOR, MetalLB, cozy-proxy, snapshot-controller and the
rest of the shared data-plane set. The iaas bundle refuses to render.
Migrations are not a difference, `isp-full` enables them too.
So the lane runs on kind. kindnet, kube-proxy and local-path come from
the cluster, and there is no Cilium, no LINSTOR and no `replicated` or
`local` StorageClass. It installs the latest GHCR nightly instead of
building anything, at 05:00 UTC after the nightly mirror publishes, and
on `workflow_dispatch`. There is no pull request trigger, because a run
would install the nightly images and not the PR's.
A run passes when every HelmRelease is Ready, `cozystack-platform` and
`tenant-root` included. It also checks that the platform emitted the
`noop` networking Package and none of LINSTOR, MetalLB, multus or
KubeVirt, and that the cluster has no Cilium CRD and no `replicated` or
`local` class.
The lane reuses the e2e sandbox container for tooling and
`collect-report`, attached to the kind network.
`hack/isp-hosted-lane_test.bats` pins the wiring: the Package selects
`isp-hosted`, the install uses `variant=hosted`, the kind DNS domain
matches `networking.clusterDomain`, and kind is pinned by checksum and
digest. Six of its seven tests were red before the implementation
existed.
`hack/sandbox-runner-headroom.bats` gets a named exemption. That check
treats any workflow touching `packages/core/testing` as a lane that
boots the QEMU guests, and this one boots none. The exemption is checked
after the job match, and removing it turns the test red again.
The pins come from the kind v0.32.0 release: the binary sha256
`50030de2…` from its `kind-linux-amd64.sha256sum`, and
`kindest/node:v1.33.12@sha256:3f5c8443…` from its release notes.
v1.33.12 is the Kubernetes version the Talos lanes use, so the variant
is the only thing that differs. All three are plain `env` values in the
workflow, where a Renovate custom manager can match them.
### Real runs
The first real run on this branch,
[36961209755](https://github.com/cozystack/cozystack/actions/runs/36961209755),
went red on `tenant-root`:
```text
tenant-root False Helm install failed for release tenant-root/tenant-root with chart tenant@0.0.0+629b61300902: unable to build kubernetes objects from release manifest: resource mapping not found for name: "allow-external-communication" namespace: "tenant-root" from "": no matches for kind "CiliumNetworkPolicy" in version "cilium.io/v2"
```
This is #3387 reproduced live: `packages/apps/tenant` renders Cilium
policies on a cluster without Cilium. The other releases went Ready,
except `cozystack-basics`, which was still in an upgrade at the last
listing. The fix landed in #4710.
The run after it,
[37089699259](https://github.com/cozystack/cozystack/actions/runs/37089699259),
is green on the first attempt. It installed nightly
`0.0.0-nightly.20261003.37089458387.1`, built from
|
||
|
IvanHunters
|
1621c471f2 |
ci(dashboard): run eslint on the console and clear its errors (#4692)
## What this PR does
The console now passes its own eslint config, and CI runs it. On main
`pnpm lint` exits 1 with 54 problems, 51 of them errors, and
`ui-test.yaml` never ran it. On this branch it exits 0 with no errors,
and a Lint step sits between Typecheck and Test.
How each rule was handled:
- `no-explicit-any` (38 errors): replaced with real types. These commits
build a vite bundle byte-identical to main's.
- `react-refresh/only-export-components` (6 errors): the hooks, context
objects, `resolveSource` and the RJSF registry moved out of component
files into their own modules. `useTenantContext` stays in
`tenant-context.tsx`, so its importers and the tests that mock that
module are untouched.
- `no-unused-vars` (1 error): dropped the unused `ad` binding in
`OverviewTab`.
- `react-hooks/set-state-in-effect` (6 errors): relaxed per line with a
reason, not in the config.
The last rule is the only relaxation. Each of these effects re-syncs
local state to something that changes outside the component. The rule
wants the state derived during render, which changes when the value is
committed or what the first render shows, so it would be a behaviour
change. Per file:
- `components/ResourceQuotasField.tsx` re-syncs the inputs after the
form value is reset from outside.
- `components/SourceWidget.tsx` re-syncs the selected option when the
value loads asynchronously in edit forms.
- `components/command-palette/command-palette.tsx` resets query and
level whenever the palette closes.
- `components/command-palette/use-keyboard-nav.ts` moves the highlight
back to the top when the item list changes.
- `hooks/use-is-mac.ts` would return `true` on the first render on macOS
with a lazy initial state. Today it returns `false`.
- `lib/tenant-provider.tsx` falls back to an existing tenant once the
tenant list has loaded.
The eslint config itself is unchanged, and `no-explicit-any` stays on
everywhere.
`pnpm typecheck` and `pnpm test` (442 tests) pass at every commit. After
the file moves the bundle has the same chunk set, and only the entry
chunk differs, by reordered and renamed minified functions.
Three `react-hooks/exhaustive-deps` warnings from main are left as they
are, because fixing them changes when the effects run. One is in
`SourceWidget.tsx`, where the effect reads `options` but depends only on
`value`. The other two come from `ads = adList?.items || []` in
`use-command-items.tsx`, which gives two `useMemo` hooks a new
dependency on every render.
Things I did not do here:
- The step has no `--max-warnings`, so a new warning will not fail CI.
Once the three warnings are fixed, `--max-warnings 0` can go in.
- `SourceField.tsx` and `SourceWidget.tsx` each declare the same small
`SourceOptionSchema` interface. I kept them local since the two widgets
share nothing else.
- The job name stays `Typecheck and test`, so a required check with that
name keeps matching.
- `pnpm -r lint` only reaches `apps/console`.
`packages/{k8s-client,ui,types}` have no `lint` script, same as on main.
Closes #3911
Closes #4566
### Screenshots
No visual change: types, module layout and CI only.
### Downstream repositories
The diff touches only the console workspace and `ui-test.yaml`. The
console has no downstream copy since `cozystack-ui` was archived, and no
trigger map entry matches.
- [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
ci(dashboard): the console's eslint now runs in the UI test job, and the console passes it with no errors.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Improvements**
* Tenant selection now uses available tenant namespaces, restores a
saved selection when possible, and falls back to the root tenant or the
first available tenant.
* Tenant options are sorted, and the selected tenant is kept in sync
with the available choices.
* **Chores**
* Automated checks now include linting alongside type checks and unit
tests.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
IvanHunters
|
02fbe76418 |
fix(ci): close three blind spots in the e2e test selectors (#4658)
<!-- 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 e2e selectors miss Chainsaw suites that have no `chainsaw-test.yaml` or `.yml` directly in their directory. Chainsaw v0.2.15 also runs a directory that holds only numbered step files such as `01-install.yaml`, and its discovery walks every subdirectory of the path the runner hands it. A suite in either layout ran only when nothing was selected. No narrowed selection named it, the full-suite list left it out, and `select-install.sh --validate` never asked for its mapping. Now `hack/select-e2e.sh` and `hack/select-install.sh` list a top-level directory as a suite when Chainsaw would load a test from it or from anywhere below it. No suite in the current tree uses either layout, so today's suite list stays the same. A file placed directly in `examples/backups/` (a shared `common.sh`, for example) selected nothing, because it fell through to the inert `examples/` rule. It now escalates to the full suite, same as `examples/backups/_lib/`. `README.md` there stays inert through the `*.md` rule. The `test-chainsaw` comment in `packages/core/testing/Makefile` said Chainsaw discovers every `chainsaw-test.yaml`, which was too narrow. I fixed it, and the matching sentences in `docs/agents/e2e-testing.md` too. Every behaviour change has a bats test that fails against the scripts on main. Known gaps I left for a follow-up, none of them a wrong answer today: - The `full_suite_list` helper in `hack/select-e2e_test.bats` repeats the selector's `find`/`sed`, so a bug in that shared pattern would pass every full-suite assertion. The step-file and nested tests compare against literal lists, which covers the new layouts. - The selectors are a little wider than Chainsaw: a broken `chainsaw-test.y*ml` symlink, or one with no Test in it, still lists its suite. That errs toward running more. - A few test comments describe what used to happen rather than why the rule exists. Closes #4630 ### 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. --> - [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(ci): the e2e test selectors now see Chainsaw suites made of numbered step files or nested below a suite directory, and run the full suite for an edit to a file placed directly in examples/backups/. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * E2E suite discovery recognizes supported manifest and numbered-step files at any depth and maps them to the top-level suite. * Changes to files directly under `examples/backups/` trigger full-suite selection. * Suite selection reports an error if discovery fails or finds no suites. * **Documentation** * Clarified supported suite layouts, disabled suite-file handling, discovery and escalation rules, and per-path coverage guidance. * **Tests** * Added coverage for nested suites, numbered steps, unmapped suite layouts, and backup-path selection. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
94abf6940b |
feat(kubernetes): let kubevirt-csi detach without the HotplugVolumes gate (#4451)
## What this PR does This lets kubevirt-csi detach volumes without the deprecated KubeVirt `HotplugVolumes` gate. The default stays as it is, so nothing changes at runtime yet. Part of #4032. The driver got a switch upstream for exactly this, and I bumped to it. kubevirt/csi-driver#382 added `--enable-vmi-hotplug-fallback`, merged as cb4770e4e475. With it set to false, the driver no longer calls the VMI-level `removevolume` on a VMI owned by a VM. virt-api rejects that call once `HotplugVolumes` is off. Our wrapper now passes the flag to upstream and no longer has its own hot-unplug path. `unpublishHotplugVolume` was a backport of kubevirt/csi-driver#184, and the previous pin already contained that change. Upstream detaches from the VMI when the VM is gone, then waits for the VMI status to drop the volume before it returns. So the wrapper's re-check never found the volume attached. Adding the ownership check there would have been dead code. Closes #4450 The chart leaves the flag at its default of true and does not render it. The driver image in the chart is stamped only at release preparation, so between releases main runs the previous release's binary, which does not know the flag. Go's `flag` package exits on an unknown flag. Rendering `=true` would crash-loop the CSI controller for no effect. The right value depends on the KubeVirt CR, not on the tenant, so the comment next to `HotplugVolumes` in `kubevirt-cr.yaml` points at the controller Deployment. The follow-up PR drops the gate and renders the flag as false. Nothing changes on the node side. Main already pins f5de10d39556, and cb4770e4e475 is that commit plus kubevirt/csi-driver#382. A Go unit test runs an unpublish through the wrapper during a teardown, with the flag on, off with a VM-owned VMI, and off with an orphaned VMI. It checks that the flag reaches upstream and that only upstream makes the VMI-level call. Upstream's own tests cover the ownership rule. A second test holds that an NFS volume only releases the node from its CiliumNetworkPolicy and never reaches the upstream hot-unplug. A helm-unittest pins that the chart renders no such argument. I haven't built the image or run e2e. Volume attach and detach on a tenant cluster in e2e is the real check. ### Screenshots No UI change. ### 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 feat(kubernetes): kubevirt-csi supports --enable-vmi-hotplug-fallback, so it can detach volumes without the KubeVirt HotplugVolumes feature gate. The flag stays true for now and behaviour is unchanged. ``` |
||
|
Yan Bondarenko
|
15b7092f3a |
fix(nats): enable JetStream for the generated account
With users configured and jetstream.enabled true, the chart enables JetStream on the server but generates account A without it, so every JetStream call from those users fails with error 10039, "JetStream not enabled for account". The generated account now gets jetstream: true unless it already sets JetStream itself. nats-server matches config keys case-insensitively and walks the account map in no fixed order, so a JetStream key in any casing counts as set. Otherwise the account would carry both keys and the server would apply either one, changing from restart to restart. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Yan Bondarenko
|
6248d458ad |
fix(platform): keep the legacy etcd-operator stopped after etcd adoption
Migration 50 scales the legacy etcd-operator to 0, adopts every legacy etcd.aenix.io cluster with etcd-migrate and then scales the operator back to 1. At that point it is still the legacy operator, and it has nothing left to manage. Until a tenant's etcd HelmRelease reconciles the new chart, Flux can upgrade it with the old one, which re-creates the legacy EtcdCluster that etcd-migrate just orphan-deleted. The legacy operator then reconciles it, re-creates the legacy StatefulSet and rewrites the legacy etcd-headless Service, dropping the owner references etcd-migrate pointed at the adopted EtcdMembers. When the new chart prunes the legacy EtcdCluster, the garbage collector deletes the Service with it, and the adopted members' etcd-<i>.etcd-headless names stop resolving. After a successful adoption the operator now stays at 0 until the etcd-operator HelmRelease replaces the Deployment with the v1alpha2 operator later in the same upgrade. A failed or interrupted adoption still restores it through the EXIT trap. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
f32e0693c1 |
ci(dashboard): run the console's eslint in the UI test job
The console defines a lint script next to typecheck and test, but no workflow ran it, so new findings landed unseen next to the old ones. The backlog is cleared down to three react-hooks/exhaustive-deps warnings. eslint exits zero on warnings alone, so the lint script caps them at that count: the step gates from its first run, and a new warning fails it. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4d3796e41d |
refactor(dashboard): mark the deliberate setState calls inside effects
Each of these effects re-syncs local state to something that changes outside the component. The rule's suggested rewrite derives the state during render instead, which changes when the new value is committed and what the first render shows, so each site carries a scoped disable with its reason rather than a rewrite. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
be9b148e6c |
refactor(dashboard): move non-component exports out of component files
Vite's fast refresh can only hot-swap a module that exports nothing but components; a file that also exports a hook, a helper or a registry object forces a full page reload on every edit to it. Hooks, context objects and the RJSF registry now live in their own modules, and the components stay where they were. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
f1281e6176 |
refactor(dashboard): drop the unused ad binding in OverviewTab
The tab never reads it; the prop stays in the interface because the detail page passes the same props to every tab. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
8298c43e05 |
refactor(dashboard): type command palette instances as K8sResource
client.list was called without a type argument, so its items came back as unknown and were cast to any to read metadata.name. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
05ff14dbc1 |
refactor(dashboard): type the RJSF templates and source fields instead of any
RJSF's generic defaults are any, and the templates restated them, so every schema read went untyped. The templates now default to unknown and FormContextType, and the source widgets read option schemas through a small local interface. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
def917a5d5 |
refactor(dashboard): type the backup form pages instead of any
The console's own rules forbid any, and eslint flags every one of these. The create pages only read a handful of reference fields, so a local interface per page covers them; the rest of the spec still passes through from RJSF untouched. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
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
|
3e36b35795 |
test(e2e): add an install suite for the isp-hosted variant on kind
isp-hosted shipped unable to converge and nothing noticed, because no lane installs it: every e2e lane runs isp-full on Talos, where Cozystack provides the CNI and storage itself. This suite installs it the way it is meant to run, through the hosted installer variant on a cluster that brings its own CNI, kube-proxy and default StorageClass, and passes only when every HelmRelease is Ready and the variant really left out the data plane it delegates to the host. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
89a1fe52d9 |
build(multus): say why the image build runs no tests of the patched package
The other patched Go images now test the packages their patches touch in the builder stage. The one package the multus patch touches, pkg/multus, creates network namespaces in its tests, which a build container is not allowed to do, so that image keeps building without a test step. The Dockerfile records the reason, so the absence reads as a decision and not as an omission. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |