mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-05 19:47:37 +09:00
docs/agents-registries
1781
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
|
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> |
||
|
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 --> |
||
|
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
|
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
|
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 --> |
||
|
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> |
||
|
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
|
b2a5f12e73 |
build(kubernetes): run the tests of the packages the patches touch
The cluster-autoscaler and kubevirt-cloud-provider image builds ran go build only, and go build never compiles a _test.go, so nothing ran the tests of the code the patches change. Run go test in the builder stage on the packages the patches touch. The list is read from the patches themselves, so a new patch is covered without an edit to the Dockerfile. Both builder stages export GOOS and GOARCH for the target, so the tests unset them: a local make image builds amd64 and arm64 by default, and a test binary for the other architecture runs on the build host only under binfmt emulation. The first run of that step on cluster-autoscaler found the carried downscale patch out of step with the upstream tests it leaves in place: SetSize no longer refuses a size above maxSize, while one case of TestNodeGroupIncreaseSizeErrors still expected that refusal. The patch now drops that case, and carries a test for the downscale it is named for instead. DeleteNodes lowers a group one node at a time through SetSize, so with the refusal in place a group standing above its max size could not shed a node at all. The new test deletes a node from such a group and expects the replica count to drop; against the unpatched SetSize it fails with the refusal the patch removes. Scale-up past maxSize stays bounded by the core scale-up orchestrator, which skips a node group already at MaxSize. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9809b22280 |
fix(vm-disk): report DataVolume readiness through WorkloadsReady (#4380)
## What this PR does Nothing on a vm-disk application tells whether CDI ever finished populating the disk. Its WorkloadMonitor has no pods to count, and `Ready` follows the Helm release. So an import or clone that never completes can leave a green application over a disk nobody can use. cozystack/cozystack#2908 is one way to get there: a clone into a StorageClass that cannot place a replica where the source data sits. The expensive part there was the silence, not the stuck clone. The disk state was in the DataVolume phase all along, and nothing read it. With this change the WorkloadMonitor in cozystack-controller reads it. The monitor lists the DataVolumes its selector matches and reports not operational while any of them is in flight, Failed, Unknown or has no phase. An upload counts as in flight until it completes. That surfaces on the application as `WorkloadsReady=False` with reason `DataVolumeNotReady` and a message that names the DataVolume and its phase, for example `DataVolume vm-disk-x is ImportInProgress`. `Ready` still follows the HelmRelease, as for any other kind (`convertHelmReleaseToApplication` in `pkg/registry/apps/application/rest.go`), and for a vm-disk it no longer waits on the DataVolume at all. So the disk state has to be visible outside `Ready`, and this PR puts it where operators look by default: - `kubectl get` on any application kind gets a `WORKLOADS` column next to `READY`. It shows the `WorkloadsReady` status, or `<none>` when the application has no WorkloadMonitor. - The console badge turns amber and reads `DataVolumeNotReady` while `Ready` is True and `WorkloadsReady` is not True for that reason. A failing `Ready` still wins. The console shows only the disk case: other kinds need a separate decision, because a stopped VM legitimately has no pod and reports `WorkloadsReady=False` too. A Helm readiness gate on the DataVolume would be the obvious alternative, and it is not used on purpose. helm-controller runs that wait inside its reconcile, so an importing disk would hold one of the shard's worker slots for the whole install window, and a disk stuck for good would keep taking the slot back. cozystack/cozystack#4145 has the details. The monitor costs no slot, keeps watching after install, and covers existing disks without anyone writing to them. Without any gate, the install would still wait on the DataVolume. helm-controller's default poller falls back to a resource's `Ready` condition for kinds it has no rule for (`checkReadyCondition` in fluxcd/cli-utils `pkg/kstatus/status/status.go`), so a vm-disk install could hold a worker slot for the whole install timeout. The ApplicationDefinition therefore gets a `healthCheckExprs` entry for DataVolume with `current: "true"`, the same idiom postgres-rd uses for its ScaledObject. With it Helm never waits on the DataVolume. The entry is pinned by a helm-unittest in `packages/system/vm-disk-rd`, which gets a `test` target for it. Release settings reach a HelmRelease only when the application is created or updated, so an existing disk gets the entry on its next update. The reconciler that syncs definitions onto existing releases copies chartRef, valuesFrom and labels only. That affects any kind with release settings, so it is filed as cozystack/cozystack#4409 and not fixed here. `WorkloadsReady` covers every disk right away, since the monitor needs no write to the application. CDI copies the DataVolume labels onto the PVC it creates and makes the DataVolume its controller, so the monitor also publishes a `pvc-<name>` Workload for the disk. That Workload reads not operational while its DataVolume is not ready, even though the PVC is already Bound, so `kubectl get workloads` agrees with `WorkloadsReady`. While the DataVolumes cannot be read it keeps the verdict it has, like the monitor. The WorkloadMonitor CRD now requires at least one selector label. An empty selector would match every Pod, PVC, Service and DataVolume in the namespace. No chart renders one. When an application has several monitors that are not operational and only some of them name a cause, `WorkloadsReady` carries the named messages and reason only. A vm-disk has one monitor, so it does not hit this. ### Which phases count as ready Settled phases pass: `Succeeded`, `PendingPopulation`, `WaitForFirstConsumer`, `Paused`. A disk with `bindImmediately: false` on a `WaitForFirstConsumer` class rests in `PendingPopulation` or `WaitForFirstConsumer` until a VM consumes it. A multi-stage import rests in `Paused` until its next checkpoint, and the vm-disk chart renders no checkpoint source. These report ready while holding no data, because that is where such a disk is supposed to sit. `UploadReady` is not one of them. CDI enters it when the upload server pod is running and ready, and leaves it for `Succeeded` only when that pod succeeds (`updateStatusPhase` in `pkg/controller/datavolume/upload-controller.go` at v1.66.1), so the one phase covers a disk nobody has uploaded to and a half-uploaded one. A `source: upload` disk therefore reports `WorkloadsReady=False` from creation until `virtctl image-upload` completes. A phase the controller does not know also reads as ready, so a CDI release that adds or renames a phase cannot turn working disks red. The cost goes the other way: a renamed in-flight phase silently reads as settled. The list wants a re-read whenever the CDI version moves. It matches the named phases of the CDI v1.66.1 API, the version vendored in `packages/system/kubevirt-cdi-operator`. ### Clusters without CDI CDI comes with the IaaS bundle, while cozystack-controller runs everywhere, and CDI can be installed while the controller is running. A watch on a kind whose CRD is absent never syncs, and then the whole controller does not start. So the watch is not set up at startup. A runnable adds the DataVolume watch once the kind is served. It waits for the first reconcile, because only from then on does a new watch start at once, and then checks discovery right away and every 30s after that. It installs the DataVolume reader only after the DataVolume informer has synced within a timeout, because a reconcile context has no deadline and a List on an informer that never syncs would block every WorkloadMonitor reconcile. The watch source is started once: a controller-runtime `Kind` source adds an event handler to the informer and never removes it, so on a timeout the next check waits on the source it already has instead of starting another one. RBAC needs no change: the controller's ClusterRole already has read on everything. When the DataVolumes cannot be read, because the reader is not installed yet or the List fails, the monitor still publishes its replica counts and keeps its last DataVolume verdict, and a List error is returned so the request is retried and recorded as a `DataVolumesUnavailable` Warning event on the monitor. The status of a monitor without DataVolumes is not affected by a failing List. A disk whose verdict is already recorded does not read as ready in the window after a controller restart. A monitor with no recorded verdict yet, which is every monitor right after the upgrade that brings this change and any disk created right after a restart, reads ready until the DataVolume watch has synced. Because the watch starts on the first reconcile rather than on the next discovery tick, that window is the time to sync the DataVolume informer plus at most one 5s requeue, not a 30s poll interval. The window is not zero, since a monitor reconciled in it is published before its DataVolumes can be read. From then on, a reconcile that runs before the watch has synced is requeued after a few seconds, so the DataVolume events that queue it are not lost. A pass that changes nothing in the monitor status does not write it, so that requeue sends no status writes to the API server. Once the reader is installed, every monitor that holds a DataVolume verdict is queued once more. So a verdict whose DataVolume was deleted while the controller was down is recomputed then, not at the periodic resync. ### Tests The table test in `workloadmonitor_controller_test.go` covers the named phases of CDI v1.66.1, plus Failed, Unknown, an empty phase and a missing status. A second table checks the message and reason for an importing, a failed, a phaseless and a populated disk, and a third test lists two stuck DataVolumes in both orders and expects one message. It also covers a DataVolume outside the selector, the kind not being served, a List error that still publishes the replica counts, and a missing reader that keeps the last verdict. The watch tests check that the watch starts on the first reconcile and not before it. A source that never syncs leaves no reader and is retried on the same source, with the reconciles in between still requeued. A List failure records the Warning event, the sync window records none, and an unchanged pass writes no status. A manager that stops during the wait gets the cancellation back, and a reconcile that runs while the watch syncs is requeued without an error. A table test covers the PVC Workload: importing, populated, a PVC with no DataVolume controller or a controller of another kind or group, and the unread cases. A helm-unittest in `packages/system/cozystack-controller` pins the selector's `minProperties`. `rest_table_test.go` pins the `WORKLOADS` column, `rest_workloads_test.go` the condition message and reason, and `status.test.ts` the console badge, including a halted VM that stays green. In `hack/e2e-chainsaw/vminstance` the existing VMDisk scenarios now wait for the DataVolume to be Ready, then for `Ready` and `WorkloadsReady` on the VMDisk itself. That replaces the wait on the HelmRelease. Those asserts would pass even if the monitor ignored DataVolumes, so a new scenario imports from an `.invalid` URL. It first waits for the importer to have failed, which CDI reports on the DataVolume as `Running=False` with reason `Error`, and then for `Ready=True` with `WorkloadsReady=False` and reason `DataVolumeNotReady` on the VMDisk. Without the first wait the VMDisk assert passed in about a second, on a DataVolume that had no phase yet. The e2e runs on a fresh install only, so nothing here exercises an existing release across an upgrade. ### What it does not do #2908 lists possible fix directions, and none of them is here. The cross-class clone still never completes, it just stops being invisible. Keep the issue open. While the DataVolume watch syncs, every monitor is still requeued. Only the ones whose selector reaches a DataVolume need it, but the controller cannot tell which those are before it can read DataVolumes. Closes #4449 ### Screenshots There is no screenshot: I had no running console to take one from. The badge change is pinned by the vitest cases in `packages/system/dashboard/images/console/apps/console/src/lib/status.test.ts`: a VMDisk whose `WorkloadsReady` has reason `DataVolumeNotReady` goes amber, and a halted VMInstance with the generic reason stays green. ### Downstream repositories I walked the trigger map against the diff. cozystack/terraform-provider-cozystack is affected: its `wait_for_ready` reads only `Ready`, so on a `cozystack_vmdisk` it returns before the disk is populated once this lands. That needs a decision on the provider side, so it is an issue there. The release-time bot regenerates `packages/apps/vm-disk/README.md` into the website's virtualization pages, and `vm-disk` is in the `VMS` list that generator reads, so the website needs no follow-up. The ApplicationDefinition keeps its kind, plural, prefix and path. - [ ] 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: https://github.com/cozystack/terraform-provider-cozystack/issues/59 - [ ] [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(vm-disk): report whether CDI finished populating a disk in the WorkloadsReady condition, which names the stuck DataVolume and its phase. kubectl get shows WorkloadsReady in a new WORKLOADS column for every application kind, and the console badge turns amber while a disk is not populated, so a disk whose import or clone never completes no longer looks healthy. Ready of a new or updated vm-disk no longer waits for the DataVolume, so wait on WorkloadsReady to know the disk is populated. ``` |
||
|
IvanHunters
|
dad5924b21 |
fix(computeplane): do not grant tenants the cluster admin kubeconfig (#4722)
## What this PR does A tenant user could read the admin kubeconfig of their own ComputePlane cluster, which the module promises never to hand out. The module deploys the `kubernetes` app into the tenant namespace as `computeplane-cluster`. The secrets allowlist kept the Secret out of `tenantsecrets`, but the chart's dashboard Role still gave the tenant's `use` subjects get/list/watch on `computeplane-cluster-admin-kubeconfig`. So a plain `kubectl get secret` worked. The fix drops that one rule for the ComputePlane cluster only. The kubernetes chart now skips it when the internal value `withholdAdminKubeconfig` is set, and the computeplane module sets that value inline in the cluster HelmRelease. The tenant can't reach those values, because the module reads only `nodeGroups` and passes an allowlisted subset of it to the pool releases. The value is kept out of `values.yaml` and the schema, same as `clusterReleaseName` in kubernetes-nodes. A regular Kubernetes app keeps the rule, so the dashboard still offers its kubeconfig. Only the v1.7.0 alphas ship the module. After this lands, the `kubectl auth can-i` check can go back into the e2e suite in #4713. ### Screenshots No UI changes. ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(computeplane): tenant users no longer get read access to the ComputePlane cluster's admin kubeconfig Secret through the kubernetes app's dashboard Role. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security** * Computeplane dashboards no longer grant access to the cluster’s admin kubeconfig Secret. Other dashboard resource permissions remain available. This limits access to sensitive cluster credentials through the dashboard while preserving the service-related permissions needed for dashboard resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
ded2354b65 |
fix(tenant): skip Cilium policies on isp-hosted without Cilium (#4710)
## What this PR does The tenant chart no longer fails to install on a hosted cluster without Cilium. The isp-hosted variants install no CNI of their own, but the chart rendered its Cilium policies unconditionally, so `tenant-root` failed with `no matches for kind "CiliumNetworkPolicy" in version "cilium.io/v2"`. Everything that depends on `cozystack-basics` then never converged. This also unblocks the isp-hosted e2e lane, which fails on exactly this. The guard is keyed on the platform variant. The platform already passes it to every tenant as `_cluster.bundle-name`. It covers every Cilium object the chart renders, including the shared-central tracing egress policies. - On variants where Cozystack installs Cilium itself, nothing changes and a missing CRD still fails the install. - On `isp-hosted` and `isp-hosted-slim`, the policies render only when the host serves both Cilium policy kinds, since a hosted cluster may run Cilium. The render on Cilium variants is byte-identical to `main`. I checked it with `helm template` over all four Cilium variants and an absent `_cluster`, with optional services on and off, with and without the Cilium API, for root, child and grandchild tenants. I didn't use the capability check alone because it fails open. helm-controller upgrades a deployed release only when the chart or the values digest changes, so a change in API discovery never triggers a re-render. That is `VerifyRelease` in `internal/action/verify.go` at v1.5.0, the version Cozystack ships. If the Cilium CRDs were not discoverable at install time on a Cilium cluster, a capability-only guard would leave the tenant without policies until the next chart or values change. An upgrade rendered in that window would delete existing ones. On isp-hosted without Cilium, tenants get no network isolation from Cozystack. Isolating them is up to the host. I didn't fall back to standard `NetworkPolicy`, because the host CNI may not enforce it. The tenant policies also use Cilium features that don't map one to one: the `world` and `kube-apiserver` entities, clusterwide policies, selectors on ancestor namespace labels. A partial translation would look like isolation without really being one. The tenant README now documents this, including that Cilium added to the host later takes effect only on the tenant's next chart or values change. The threat model and the self-assessment list it under non-goals. The new helm-unittest suite fails on `main` for the hosted cases without Cilium. Each guard mutation I tried turns at least one case red: no variant check, no capability check, `and` changed to `or`, `isp-hosted-slim` dropped from the list. I didn't add an explicit "require Cilium" value, since the Cilium variants already behave that way. One thing to keep in mind: the list of hosted variants now lives in the platform bundle and in the tenant helper, so a new hosted variant has to go into both. A platform `_cluster` flag saying whether Cozystack manages the CNI would remove that, but it's a platform change. Closes #3387 ### 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 tenant README change reaches the website through the release bot that regenerates the managed-apps pages from each package README. ### Release note ```release-note fix(tenant): the tenant chart no longer fails to install on the isp-hosted variants when the host has no Cilium. There, the Cilium network policies are created only if the host serves the Cilium policy API; otherwise tenants get no network isolation from Cozystack. Other variants are unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Tenant network-isolation policies are rendered for `isp-hosted` and `isp-hosted-slim` only when the required Cilium APIs are available. If Cilium becomes available later, policies are applied after a subsequent chart or values change. Tracing policies also require the Cilium APIs. * **Documentation** * Clarified when Cozystack provides tenant network isolation and that isolation is the host’s responsibility when the required Cilium APIs are unavailable. * **Tests** * Added coverage for policy rendering across bundle types and Cilium API availability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
11910e0b2c |
fix(kubernetes-nodes): refuse a resources.cpu that cannot be a core count (#4386)
## What this PR does A worker pool's CPU value could land in the VM as something KubeVirt cannot accept. `resources.cpu` is typed as a quantity, but on the kubevirt substrate the template wrote it unchanged into `domain.cpu.cores`, which is `uint32` in KubeVirt and an integer in the KubevirtMachineTemplate CRD. With `resources.cpu: 500m` the chart rendered `cores: 500m`. That string was truthy, so it also counted as explicit sizing and dropped the pool's `instanceType` from the VM, and then the API server refused the template anyway. The render now decodes the value as YAML, the same way `cores: <raw>` is decoded, and stops unless the result is a whole number from 1 to 4294967295. `500m`, `"1.5"` and `"1e10"` now fail at render time with an error naming the field, before the value reaches the VM template or the instancetype decision. `"0"` and `"-2"` were already refused on `main` and still are. Only inputs that could not produce a working pool are refused. Every value that rendered a pool before renders the same bytes now, so the content-hashed template keeps its name and nothing rolls. That includes `2`, `"2.0"`, `"+2"` and `"1e1"`, and also two odd cases I kept on purpose because pools may already carry them. An unquoted `0` is falsy, so the pool renders without `domain.cpu` and is sized by its instancetype, as on `main`. A leading-zero value such as `"010"` is still emitted as written and keeps the reading it gets on `main` today, where YAML treats it as octal. I checked both against a render of `main`, and the tests pin the template names that `main` produces for them. This is where kubernetes-nodes and vm-instance now differ. Since #4405, vm-instance refuses a leading zero and an unquoted `0`, and this chart does not. Stored worker pools can carry these values, because migration 56 copies `resources` verbatim, so refusing them here without a migration would fail those pools' releases on their next upgrade. #4554 tracks the migration and the refusal after it. The proxmox substrate is not checked. It converts the quantity to millicores and rounds up to whole cores, so `500m` is a valid size there and becomes one core. The field description now says what the value means on each substrate. Memory is left alone. It goes to `domain.memory.guest`, which is a `resource.Quantity`, so a quantity string is valid there. The issue suggested coercing to an integer, as vm-instance did before #4405. I didn't, because `int64` turns `500m` into `0` without an error. The new helm-unittest suite covers each input form, both preserved cases, the upper bound, the instancetype case and the proxmox path. Two neighbouring descriptions still say only "CPU available.": `nodeGroups[].resources.cpu` in `packages/extra/computeplane` and in the parent `kubernetes` chart. computeplane passes `resources` straight into this chart, so the new check applies to its pools as well. I left both alone to keep this change to one chart. Closes #4150 ### 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(kubernetes-nodes): on the kubevirt substrate, a worker pool whose resources.cpu cannot be a whole vCPU core count (for example 500m, 1.5 or a value above 4294967295) now fails at render time with a clear error, instead of rendering a VM template that is refused later or dropping the pool's instanceType. Values that rendered a working pool before render unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * KubeVirt CPU settings now accept whole-core values from 1 to 4,294,967,295. Fractional, millicore, negative, and larger values are rejected; an unquoted `0` is treated as unset. * Proxmox rounds fractional and millicore CPU values up to whole cores. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
fb7e780517 |
fix(postgres): reassign dropped roles' objects to an unprivileged role (#4574)
## What this PR does Objects owned by a role the Postgres init-job drops no longer end up owned by the `postgres` superuser. This covers a user removed from `users` and an orphaned managed `<db>_admin` or `<db>_readonly` role. Before dropping such a role the job reassigned everything it owned to `postgres`, and objects a tenant role created should not become superuser-owned. They now go to `cozystack:orphaned`, a NOLOGIN role that is granted nothing. The init-job creates it and resets its attributes on every run. The `:` in the name is outside the charset allowed for user and database names, so values cannot declare it. There is a trade-off. Grants and memberships of the dropped role do not carry over, so a view or function it owned that ran with its owner's privileges can lose access it had through them. To keep such objects working, keep the user in values or move the objects to another owner before removing it. `docs/operations/backup-classes.md` is updated accordingly. Role names read from the catalog now reach SQL as psql variables instead of being spliced in by the shell. The queries are echoed after substitution, so the job log shows which role is dropped. Tested with helm-unittest and by running the rendered init script against PostgreSQL 13 and 18. ### 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 fix(postgres): objects owned by a user removed from `users`, or by an orphaned managed database role, are now reassigned to the unprivileged `cozystack:orphaned` role instead of the `postgres` superuser. A view or function of the removed role that ran with its owner's privileges can lose access it had through that role's grants or memberships. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Clarified that after restore, the first initialization releases undeclared managed databases without deleting their data and removes users not declared in the chart. * Removed users’ objects are reassigned to a restricted, non-login role. Grants and memberships are not preserved; functions use invoker rights, and PostgreSQL 15+ views are set to use invoker security. * Added guidance for finding eligible objects still owned by the PostgreSQL superuser for manual reassignment. * Declare a user in `users` to keep it; the chart will generate a new password. * **Bug Fixes** * Removed users’ objects are reassigned across connectable, non-template databases before their users are dropped. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
339be8d42d |
fix(vm-default-images,kubernetes-worker-image,vm-disk): wait for golden images to import and refuse cloning a failed one (#4392)
## What this PR does Both golden image catalogs, `vm-default-images` and `kubernetes-worker-image`, went Ready before any image had imported. Each one renders a DataVolume per image with immediate bind, so every import starts at install, but neither PackageSource had `healthCheckExprs`. The HelmRelease went Ready as soon as helm applied the DataVolumes, and an image that failed or stalled stayed green. This PR makes both releases wait for the imports. Each component's `install:` block now has a DataVolume health check. `current` holds only on `Succeeded`, and `failed` holds on `Failed`. Any other phase, `Unknown` included, matches neither, and flux keeps waiting. A DataVolume with no `status` key makes the CEL evaluation error out. Flux reports that as Unknown and keeps waiting too. The other settled phases are safe to treat as not ready for these charts. `PendingPopulation` and `WaitForFirstConsumer` are where a volume waits for a consumer, and immediate bind removes that wait. `UploadReady` and `Paused` belong to upload and multi-stage sources, and both charts render only `source.http`. There is no per-component timeout, so I did not set one. The window is the operator-wide `--helmrelease-install-timeout`, 10m by default. The operator sets `RetryOnFailure` on both install and upgrade, so a download that outlasts the window is retried as an upgrade until the import finishes. Nothing uninstalls the DataVolumes in between, and the import keeps running. One image that never imports keeps its whole catalog not Ready until the image imports or its entry is removed. There is no time limit, and I kept it that way on purpose. A group/kind health check cannot tell one image from another, so it cannot be made per image. A per-component timeout would be an API change for one opt-in package. Both charts' `values.yaml` document this. Nothing on `main` lists either package in `dependsOn`, and the images that did import stay usable in the meantime. Tenant worker pools look up each golden directly and never wait on the catalog's status. The price is a helm-controller slot. The operator labels the generated HelmRelease with `cozystack.io/package`, plus `cozystack.io/privileged` when privileged. The flux-shard-operator acts only on HelmReleases that already carry `sharding.fluxcd.io/key`. So both catalogs go to the system helm-controller, which picks up releases without that key and runs with `--concurrent=5`. A DataVolume in `Failed` ends each attempt at once and costs next to nothing. An import that stalls without reaching `Failed` is the expensive case, for example an importer that keeps retrying an unreachable URL. Each attempt waits out the full window, and the next one starts 30s later. So one of those five slots stays busy about 95% of the time for as long as the image is stuck. #4145 describes the same cost on the shard controllers. The status change alone does not protect a vm-disk, because vm-disk clones from a golden without checking it. A golden still importing is fine, since CDI holds the clone until the source is populated. A golden whose import has Failed is never populated, so the clone would wait forever. vm-disk now refuses to render a new disk on a Failed golden and names the image. This matches the check the worker pools already have. A disk that already exists is exempt, so a golden that fails later does not break that disk's upgrades. The check fires only when CDI sets Failed, and at CDI v1.66.1 an import reaches Failed only on a checksum mismatch. The goldens carry no checksum, so a broken URL keeps the importer retrying and the new disk still waits, with only the catalog's not-Ready status pointing at the image. #4564 tracks that gap. A golden that does not exist at all is a separate gap, tracked in #4548. Tests pin the expressions for both catalogs. A helm-unittest case checks them on the rendered PackageSource. A Go test builds the HelmRelease spec from the shipped file and checks that they arrive with the poller wait strategy. The repo has no harness that evaluates flux CEL, so the phase behaviour above is argued from flux's evaluator and not exercised by a test. The vm-disk check has helm-unittest cases for a Failed, a Succeeded, and a still-importing golden, for a render with no cluster, and for an existing disk. Closes #4169 ### 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 - [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(vm-default-images, kubernetes-worker-image): the golden image catalogs now report Ready only after their images have finished importing, and a failed or stalled import keeps the catalog not Ready. A vm-disk created from a golden image whose DataVolume is in phase Failed is now refused with an error naming the image, instead of waiting forever on a clone that cannot complete. ``` |
||
|
Aleksei Sviridkin
|
64f43f4f77 |
fix(computeplane): do not grant tenants the cluster admin kubeconfig
The computeplane module deploys the kubernetes chart into the tenant namespace, and that chart's dashboard Role lets the tenant's "use" subjects read <release>-admin-kubeconfig. Withholding the Secret from the tenant secrets view does not help when RBAC grants it directly, so any tenant user at "use" level or above could fetch the cluster-admin credential of a cluster the module promises to keep operator-owned. The kubernetes chart now drops that rule when the internal value withholdAdminKubeconfig is set, and the module sets it in the cluster HelmRelease's inline values. The tenant cannot reach those values: the module reads only nodeGroups from them and splices an allowlisted subset into the pool releases, and Flux applies spec.values over valuesFrom. A regular kubernetes app keeps the rule, so its dashboard can still offer the kubeconfig. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
10a6cf2a0b |
test(tests): make absence-only helm-unittest cases fail when they check nothing (#4664)
## What this PR does Twenty-one helm-unittest cases asserted only that something is absent from a document, and could pass without ever looking at the document they name. This PR makes each of them fail when that happens. 1. Thirteen cases picked their document by position: mongodb (4), nats certmanager (3), opensearch (3), qdrant certmanager (2), seaweedfs cluster_scoped_names (1). When a template gains or reorders a document, the index points at some other object that also lacks the field, and the case stays green. These now use `documentSelector` on `kind` or `metadata.name`, like the earlier harbor conversion. 2. Seven cases had no selector at all, on templates wrapped in a top-level `if`: kubernetes cilium_gateway_alpn, mongodb restore, linstor-scheduler admission, seaweedfs s3_traffic_distribution and three in vertical-pod-autoscaler. If a values change turns the condition off, the render is empty and the absence check passes on nothing. A `documentSelector` turns an empty render into `document not found`. 3. The redis null-namespace case checked that the in-cluster name is present, but never that the external name is absent. It now asserts that neither `my-redis.` nor `my-redis.example.com` (the suite-level host that `_namespace: null` must override) is in the SAN list. The `kubernetes cluster` entry in the #3634 catalog no longer applies, since `packages/apps/kubernetes/tests/cluster_test.yaml` has been removed from main. I did not add the `my-redis.redis.` check that #4621 mentions, because the per-engine SAN it refers to has not landed. Each case was checked by mutation. I broke the template in a way the old case could not see, then ran the chart's `make test` with the new test file and with the one from main. For group 1 I removed the guard and put an extra document in front of the target. For group 2 I turned off the top-level condition. For group 3 I let the empty-host branch emit the bare SAN. Every time the main version stayed green and the new one went red, and the fixed cases were the only extra failures. All templates are back to their committed bytes. Follow-ups, not in this PR: - The same absence-only shape has appeared in more cases since #3634 was filed: mongodb `tls_test.yaml` (8 cases), mariadb `tls_test.yaml` (6), redis `tls_test.yaml` (2 more), kubernetes `kamaji_managed_args_test.yaml` (2), and one each in kafka `dashboard-resourcemap_test.yaml`, cert-manager-issuers `solver_test.yaml` and seaweedfs `volume_resize_hook_test.yaml`. They need a separate issue. The four platform `bundles_proxy_protocol*` cases are #3864, handled in #3929. Closes #3634 Closes #4621 ### Screenshots Not applicable, test-only 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 * **Tests** * Updated chart validation to identify rendered resources by their type or name rather than relying on document order, improving consistency across Kubernetes applications. * Expanded Redis TLS checks to verify that certificates omit generic and external hostnames when no namespace data is provided. * Kept existing expected behavior checks for gateway, database, messaging, search, storage, and autoscaling resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
f447ab8ebd |
docs(vm-disk): explain which condition reports the disk populated
Ready follows the Helm release; whether CDI finished is in WorkloadsReady. Say so and where an operator sees it, say that a disk created before this change keeps a release that may wait on the DataVolume until its next update, say that an upload disk reads not ready until the upload completes, and say that a disk waiting for its first consumer counts as ready, so it is not read as broken. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
2df29639df |
fix(clickhouse, workloadmonitor): add the cluster domain to keeper hosts and stop a PVC nil dereference (#4608)
## What this PR does Two small bug fixes, one per commit. Both were first proposed by @officialasishkumar in #2334 and #2336. Those PRs went stale, and part of each was fixed on main in the meantime, so this carries only what is still broken. ClickHouse Keeper hosts rendered without a cluster domain, as `chk-<name>-keeper-cluster1-0-0.<namespace>.svc.` with nothing after `svc.`. The zookeeper block in `clickhouse.yaml` redeclared `$clusterDomain` from `.Values.clusterDomain`, which the chart never defines, and that shadowed the value read from `_cluster.cluster-domain` at the top of the template. Keeper is enabled by default, so every ClickHouse release got these hosts. The keeper name test set `clusterDomain` itself and hid the bug. It now sets only `_cluster.cluster-domain`, like the platform does, and fails without the fix. The WorkloadMonitor controller panicked on a PVC with no `storageClassName`, which is any PVC that takes the cluster default class. The check used `||` where it needed `&&`. controller-runtime recovers the panic, but the reconcile of that monitor fails on every retry and never reaches its status update. A PVC with an empty `storageClassName` also got a resource key with an empty class name instead of `default`. A new table test covers unset, empty and explicit classes. The duplicate `template` key in the ClickHouse backup CronJob from #2334 and the `Operational` status fix from #2336 are already on main, so they are not in this PR. ### 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 fix(clickhouse, workloadmonitor): ClickHouse Keeper node hostnames now include the cluster domain, and the WorkloadMonitor controller no longer panics on a PVC without a storage class. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Workload resource labels now use the default storage class when a PVC’s storage class is unset or empty, while continuing to reflect explicitly configured storage classes. * ClickHouse ZooKeeper hostnames now consistently use the configured cluster domain, or `cozy.local` when no domain is set. This helps hostnames align with the cluster’s domain configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
3b5af9dc55 |
fix(tenant): skip Cilium policies on isp-hosted without Cilium
The isp-hosted variants install no CNI of their own, so on a host without Cilium the cilium.io/v2 kinds do not exist and the tenant install fails, taking the root tenant and everything that depends on cozystack-basics down with it. The policies now render on isp-hosted only when the host serves both Cilium policy kinds. On every other variant Cozystack installs Cilium itself, so they keep rendering unconditionally: a CRD that is missing at render time there fails the install loudly instead of leaving the tenant without network isolation, because helm-controller does not re-render a release when only API discovery changes. On isp-hosted without Cilium the tenant gets no network isolation from Cozystack; isolating tenants is then up to the host's CNI. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
360f03bb8b |
feat(monitoring,tenant): shared-central tracing topology (phase 4) (#3775)
## What this PR does Adds the opt-in shared-central traces topology from the distributed tracing design (cozystack/community `design-proposals/distributed-tracing`, rollout item 4): a tenant can send its spans to one VictoriaTraces store in tenant-root instead of running its own, with both write and read isolation between the tenants that share it. **Switching it on.** A tenant opts in with the new Tenant field `tracingCentral`, which needs the tenant's own Monitoring. The tenant chart publishes it to the tenant's monitoring charts as `_namespace.tracingCentral` in the `cozystack-values` Secret, which the apps API does not let an application write, and renders the two network paths the topology needs under the same condition. tenant-root decides whether it hosts the store, with the new Monitoring field `tracingCentralHost` next to a cluster-mode `tracingStorages` entry named `generic`. **Isolation.** VictoriaTraces takes the tenant account from the `AccountID`/`ProjectID` headers and does no authorization of its own, so other tenants reach the shared store only through a vmauth in tenant-root. Each central tenant gets a VMUser that pins an account derived from its namespace name and UID, for both its collector's writes and its Grafana reads, and vmauth overrides any account a client sends. The collector and Grafana log in with credentials the chart generates and keeps. The tenant egress rules allow only pods carrying the collector's or Grafana's label to reach the vmauth proxy port, and a pod that carries such a label without the tenant's credentials gets only 401 from it; the vmauth internal routes, whose `/metrics` lists every tenant's user, are served on a port tenants cannot reach. The tenant-root namespace itself is inside the trust boundary, as it already is for the metrics of tenants that rely on a parent's monitoring: its workloads reach the store directly, and one that learns a tenant's account ID can read and write that account, and whoever tenant-root admits to its Grafana reads the store through a datasource that bypasses vmauth (with the System oidc mode, tenant-root's own groups, which already hold the same access level in every tenant under it); the `tracingCentralHost` description says so. **Sharing the store.** Each tenant's VMUser is capped at four requests in flight per vmauth replica, reads and writes together, and each collector replica exports with two senders, so more collector replicas do not raise a tenant's throughput; above the cap vmauth queues for up to 10s and answers 429, which the exporter retries. The store has one disk and one retention for all of its tenants and nothing reserves space per tenant; `tracingCentralHost` states that, since hosting is tenant-root's decision. The vmauth runs two replicas spread across nodes under a PodDisruptionBudget, so draining one node does not stop every central tenant at once. Every traces store is also disk-bounded now, as the design requires: without `retentionDiskUsageBytes` it drops its oldest day once its filesystem is 80% full. That alone does not keep one tenant from filling the shared disk, since VictoriaTraces always keeps the last two days. **States that would drop spans silently are refused, one race aside.** A live render fails, with a message naming the prerequisite, when a tenant turns central on while tenant-root hosts nothing, and when tenant-root stops hosting while central tenants remain; tenant-root's render also refuses `tracingCentralHost` without a store to host. The second check does not wait on a VMUser that is already being deleted, since its tenant is leaving. Each check reads the other side's objects when it renders, so a tenant turning central on while tenant-root stops hosting can pass both and export to a vmauth that is gone; the `tracingCentralHost` description names this race and its recovery. At runtime, tenant-root's Monitoring release is not Ready while the vmauth has failed. These checks run on a render, so removing the tenant-root Monitoring altogether is not refused, and from then on every Monitoring render of a central tenant fails until it turns central off; the `tracingCentralHost` description says to turn central off on the tenants first. Turning central on keeps the tenant's own local stores and their datasources, so traces stored before the switch stay readable until the tenant removes them; turning it off again leaves the spans written meanwhile in the shared store, unreadable from the tenant until it turns central on again, and spans sent while the collector switches back may be dropped; a deleted tenant's spans likewise stay until retention drops them. Right after a tenant turns central on, or after its credentials Secret is re-created, spans its collector sends before the vmauth has loaded the new login get a 401, which the exporter does not retry, so they are dropped; the tracing guide in #3772 covers it. **Where this departs from the design.** The design has the per-tenant collector inject the account headers and adds exactly one egress rule. Here the account is pinned by the tenant's VMUser at the vmauth and the collector sends none, because the collector runs in the tenant namespace and whatever runs with its pod label could otherwise choose an account; vmauth is also what the design already requires for reads, so both directions go through it. That needs a second egress rule, for the tenant's Grafana to reach the vmauth, beside the collector's. The per-tenant write rate the design gives the collector is not in this PR: no OpenTelemetry distribution has a rate-limiting processor, so #4648 builds the collector with one and holds each central tenant to a rate tenant-root publishes. Until it lands, a tenant writing faster than the disk cap allows over two days can fill the disk the other tenants share. **Consent is per host, not per tenant.** `tracingCentralHost` lets every tenant that sets `tracingCentral` in, nested tenants included, and each gets its own VMUser and its own request cap; there is no allow-list. A tenant-root that needs to limit who shares its store keeps `tracingCentralHost` off. **Upgrade.** Tenants that do not opt in render the same objects as before, and their `cozystack-values` Secret is unchanged. The monitoring charts render the same objects too, with one exception: a VTSingle or VTCluster without `retentionDiskUsageBytes` gains `retention.maxDiskUsagePercent: "80"` in its `extraArgs`, so its spec changes and its storage pods restart once, and a store already more than 80% full drops its oldest days when it starts. The inner monitoring HelmRelease's values gain the new `tracingCentralHost: false` default, as with any new field, and its `healthCheckExprs` gain a `VMAuth` entry, which matches nothing outside a hosting tenant-root. Both change the HelmRelease spec, so every tenant's monitoring release upgrades once, rendering the same objects. A tenant-root that has a cluster-mode `generic` store gets no vmauth until it sets `tracingCentralHost`. **Testing.** helm-unittest covers the three charts the change touches (tenant, monitoring, extra monitoring) and every guard was checked by mutation; a bats test covers what helm-unittest cannot, that the generated password and the collector's rollout checksum agree on a first install. On a test cluster: a spoofed `AccountID` from a tenant is overridden by vmauth, one tenant cannot read another's spans, the vmauth starts under the real tenant network policies (and stays in init without its apiserver label), the guards refuse as described, and the rendered collector config passes `otelcol validate` in the pinned image. There is no end-to-end test yet; one that covers cross-tenant isolation is planned as a follow-up. ### Screenshots No UI changes. ### Downstream repositories - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: covered by cozystack/cozystack#3772, the tracing guide - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#64 - [ ] [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 Tenant gains the user-facing field `tracingCentral` (bool, default false), which the trigger map sends to terraform-provider-cozystack: its hand-written Tenant schema and model need the field. The follow-up is cozystack/terraform-provider-cozystack#64. The Monitoring gains `tracingCentralHost` (bool, default false); the provider's `cozystack_tenant_module` is a marker without values, so it needs no change. The website reference pages for both are generated from the READMEs on release. The tracing user guide in #3772 covers the per-tenant backend only. A central-mode section in the tracing guide of #3772 (hosting and opting in, the shared disk and retention, turning central off on the tenants before tenant-root stops hosting, and the recovery from the opt-in/stop-hosting race) also takes the lifecycle details the `tracingCentral` description no longer carries. ### Release note ```release-note feat(monitoring,tenant): add an opt-in shared-central traces store. A tenant sets tracingCentral to send its spans to one VictoriaTraces store in tenant-root instead of its own, and tenant-root opts in to hosting it with tracingCentralHost on its Monitoring. Tenants sharing the store are isolated on write and read through a vmauth in tenant-root, and each is capped in the requests it can hold in flight. ``` |
||
|
Aleksei Sviridkin
|
fccb7fee06 |
test(tests): fail absence-only cases when the template renders nothing
Each of these templates is wrapped in a top-level condition, and the case asserts only that a field is absent. If a values change turns the condition off, the render is empty and the absence holds trivially, so the case passes without looking at anything. A document selector makes an empty render fail with "document not found". Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ef5ef06806 |
test(tests): select documents by identity in absence-only test cases
These cases picked a document by position and then asserted only that something is absent from it. When a template gains or reorders a document, the index lands on a different object that lacks the field just as well, and the case stays green while the property it names is no longer checked. Selecting by kind or metadata.name keeps each case on the object it names, and fails with "document not found" when that object stops rendering. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
77f6cd1af8 |
test(redis): assert no external SAN when no namespace map is injected
The null-namespace case only checked that the in-cluster name is present, so a chart that emitted a bare "my-redis." SAN, or fell back to the suite-level host, would still pass. It now carries the same absence check as the empty-host case, plus the suite host itself. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4583ee595f |
fix(kubernetes-nodes): refuse a resources.cpu that cannot be a core count
On the kubevirt substrate a worker pool's resources.cpu, although schema-typed as a Kubernetes quantity, is written verbatim into domain.cpu.cores, which KubeVirt declares as uint32 and the KubevirtMachineTemplate CRD as an integer. A value such as 500m or "1.5" therefore rendered a template the API server refuses, and because the same raw string was truthy it also counted as explicit sizing and dropped the pool's instancetype matcher first. A whole count past the uint32 ceiling rendered too, although KubeVirt cannot hold it. The render now decodes the value the way the emitted cores field is decoded and stops unless that is a whole number between 1 and the uint32 maximum, before the value reaches the VM template or the precedence decision. Only inputs that could not produce a working pool are refused. Every value that rendered one still renders the same bytes, so no pool rolls: a bare numeric zero stays falsy and means unset, and a leading-zero spelling is still emitted as written. The proxmox substrate is left out of the check, because it converts the quantity to millicores and rounds up to whole cores, so 500m is a valid size there. The field description now says what the value means on each substrate. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
548d33f01f |
build(images): fall back to direct and retry Go module downloads
A transient proxy.golang.org HTTP/2 stream reset fails an image build outright: Go's default GOPROXY falls back to direct only on a 404 or 410. Image builds now get GOPROXY=https://proxy.golang.org|direct as a build arg from hack/common-envs.mk, declared by every Go builder stage, so any proxy error falls back to fetching from the origin. The value comes from IMAGE_GOPROXY rather than the host's GOPROXY, which may name a mirror that buildkit cannot reach. The alpine golang images carry no git, so in their stages a module hosted on GitHub cannot be fetched directly and the retry is the cover. Explicit `go mod download` steps retry three times with a growing pause. The loop is inline, in one form the contract test pins: the images build from different contexts, so no shared script can be copied in, and a named build context in the shared buildx arguments makes buildx refuse every Dockerfile pinned to a frontend older than dockerfile:1.4. The two alpine stages that left the download to `go build` get the same download step, because without git neither the fallback nor a retry covered them. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Mattia Eleuteri
|
f714ada567 |
fix(postgres): refuse a physical restore across image flavors
A recovery replays the source's data directory, so it carries the same risk as switching the image of a live cluster: the postgresql and postgis images are built on different Debian releases, and the glibc change invalidates collatable indexes, while a postgis-to-postgresql move also loses the libraries behind every PostGIS object. The chart only guarded the live cluster, so a fresh release bootstrapped from a backup of the other flavor went through, and the guard's own advice was to restore a backup, which is that very move. The backup-controller now records the source flavor in the snapshot it stores on each Backup and fails a RestoreJob whose target runs another flavor, before the purge deletes the target's Cluster and PVCs. A snapshot without a flavor predates flavors and so comes from a postgresql cluster. For the chart-managed bootstrap, the chart compares against the recovery source's Cluster while it still exists. Both paths, and the live-cluster guard, point a flavor change to a logical dump. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Mattia Eleuteri
|
e4810d3268 |
fix(postgres): require a postgis image for every postgres major
The updater used to warn and leave a major out of postgis-versions.yaml
when the registry had no postgis build for its minor yet, while
versions.yaml still advertised it. `flavor: postgis` on that major then
failed to render. Postgis images trail the PostgreSQL ones, so this is a
routine state rather than an edge case: stop before writing either map
and let the next run pick up both once the image is published.
The version check now also compares the majors of the two maps, so a
map edited by hand cannot drop one silently. It is written in POSIX sh:
cozytest.sh sources it into /bin/sh, which is dash on the CI runners and
rejected the ${var//} expansions the first version used.
Assisted-by: LLM
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
|
||
|
Mattia EleuteriandMatthieu
|
dc2971264d |
feat(postgres): add a postgis image flavor
Tenants migrating spatial data need PostGIS, but the chart always runs the CloudNativePG PostgreSQL image, which does not ship it, and exposes no way to pick another one. CloudNativePG publishes a PostGIS operand image; a closed `flavor` enum selects it without handing tenants a free image field. The flavor pins the same PostgreSQL minor as the default one, on the standard-trixie variant: backups go through the barman-cloud plugin, so the deprecated system variant is not needed. update-versions.sh now derives the postgis map from the minors it picks, and a bats check fails if the two maps drift apart. The default images are built on bullseye and the postgis ones on trixie. Switching a live cluster between them would swap glibc under an initialised data directory and silently break every collatable index, and CNPG would roll it out as a plain image update. The chart therefore refuses a flavor change on an existing Cluster. Co-authored-by: Matthieu <matthieu@hidora.com> Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Alexey Artamonov
|
69a16bd2da |
docs(tenant): shorten the tracingCentral description
The description ships in the godoc, the schema, the README, the ApplicationDefinition and the dashboard's field help, and at about 2,800 characters it buried what an operator needs to decide on the field. It now says what the field does, what tenant-root has to host first, and who is inside the shared store's trust boundary; the lifecycle details move to the user guide. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
889bb79236 |
feat(harbor): restrict the portal and API to listed source ranges (#4300)
## What this PR does Adds `managementWhitelist` to the Harbor app, a list of IP addresses or CIDR ranges allowed to reach the web portal and the Harbor API, and I made it a typed list on purpose. The obvious alternative, a free-form `ingress.annotations` map, gives tenants a way into the shared ingress controller. The platform ingress-nginx runs with `allowSnippetAnnotations: true` (the kubernetes app needs a `configuration-snippet`), and v1.11.5 defaults `annotations-risk-level` to `Critical`, so it accepts any annotation, `server-snippet` included. Tenants can't create Ingress objects themselves, but they can write any field of an app spec. An annotation map on the Harbor spec would let any tenant put arbitrary nginx config into a controller that serves every tenant. A typed list is also what the repo already does for this: `filer.whitelist` in `extra/seaweedfs`, `whitelist` in `extra/ingress` and `tcp-balancer`. The use case is a registry that serves customers: pulls and pushes come from anywhere, the admin UI and API should answer only to an office or VPN range. Right now the portal, `/api/v2.0` and `/v2/` sit on one Ingress and one public host, and a tenant can't restrict any of them. With a non-empty list the existing Ingress gets `nginx.ingress.kubernetes.io/whitelist-source-range`, and `/v2` and `/service/token` move to a second Ingress, `<release>-registry-ingress`, on the same host. ingress-nginx applies that annotation only to the locations of the Ingress that carries it, so those two stay open. They are what a registry client needs: Harbor answers an anonymous `/v2/` request with a Bearer challenge whose realm is `/service/token` on the same host. Everything else under `/service/` is `/service/notifications/*`, internal job callbacks that the bundled nginx already answers with 404, so it stays on the restricted side. The second Ingress uses the same TLS secret and has no cert-manager annotations, otherwise ingress-shim gets two owners for one Certificate. The HTTP-01 solver is its own Ingress and is not restricted. With the default empty list the chart renders exactly what it renders today. An HTTPRoute can't express a source restriction, so in Gateway API mode a non-empty list fails the render. Dropping it silently would leave the portal public while the spec says it is restricted. Each entry has to be an IP address or CIDR range, and a bad one fails the render. ingress-nginx reads an empty `whitelist-source-range` as no restriction and turns an unparseable one into a 503 for the whole location, and the values schema generator can't validate list items. Three things users should know. Harbor-to-Harbor replication that pulls from this registry goes through its API, so the other registry's address has to be on the list. The check uses the client address that ingress-nginx sees: with `externalTrafficPolicy: Local` or PROXY protocol that is the real client, behind another proxy it is the proxy. And for the portal and API the list replaces the tenant ingress `whitelist` rather than narrowing it. Disabling the portal is not here. The vendored Harbor chart (1.19.1) always renders the portal Deployment and its internal nginx always proxies `/` to it, so there is no small switch to wire. The new Ingress is added to the dashboard Role and to the ingress selector in the ApplicationDefinition. ### Screenshots No UI code change. The field reaches the dashboard form through the regenerated schema and `keysOrder`. ### Downstream repositories The terraform provider models Harbor by hand, so the new field needs a follow-up there: cozystack/terraform-provider-cozystack#46. The website reference page is generated from `README.md` on release. Nothing else in the trigger map is touched: no package added or renamed, no platform, installer or `hack/` change. - [ ] 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: https://github.com/cozystack/terraform-provider-cozystack/issues/46 - [ ] [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(harbor): add `managementWhitelist`, a list of IP addresses or CIDR ranges allowed to reach the Harbor web portal and API. Only `/v2/` and `/service/token` stay reachable from any address. The default is an empty list, which keeps the current behaviour. Not supported with Gateway API publishing: a non-empty list fails the render there. Each entry must be an IP address or CIDR range; for the portal and API the list replaces the tenant ingress `whitelist`. ``` |
||
|
Alexey Artamonov
|
2d10565873 |
feat(tenant): let a tenant opt into shared-central tracing
tracingCentral turns the shared-central topology on for a tenant with its own Monitoring: it publishes _namespace.tracingCentral for the tenant's monitoring charts and renders the only two cross-boundary paths that topology needs, the collector and Grafana each to the tenant-root vmauth. Both come from one condition, so a tenant never exports to the shared store without the path, or gets the path without opting in, and tenant-root, which hosts the store, gets neither. The flag is written only for tenants that opt in: every application in a tenant reads that Secret, and a new key for all of them would re-upgrade them all on the platform upgrade. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
2c2f2c194e |
fix(dashboard, foundationdb): show the connection ConfigMap to tenants (#4612)
## What this PR does Fixes #4611. Since #4148 a tenant may read the `<release>-config` ConfigMap that holds the FoundationDB cluster file, but the console has no view for ConfigMaps, so the connection string was still reachable only with kubectl. Chart: - `<release>-resourcemap` is rendered regardless of `monitoring.enabled`, because the console now reads it; - the tenant Role grants `get`, `list` and `watch` on `<release>-resourcemap` next to `<release>-config`, still by `resourceNames`, with no wider ConfigMap access. Console: - the application page gets a ConfigMaps tab when the application's resource map lists at least one ConfigMap in the same namespace; - each listed ConfigMap is read and watched with a `metadata.name` field selector, which RBAC checks against `resourceNames`, so a tenant never needs `list` on all ConfigMaps; - values are shown as plain text with a copy button, the cluster file needs no decoding; - a resource map that is missing or forbidden hides the tab, a listed ConfigMap that cannot be read shows the API error instead of looking empty, and a malformed map or an invalid name is reported. The tab is generic, but today only FoundationDB ships a resource map, so it is the only application that gets it. #3490 keeps `monitoring.enabled` as the switch for the resource map and asks whether the map should render unconditionally. This PR answers that for FoundationDB by rendering it always, so the wording in #3490 would need to follow if both land. Tests: `helm unittest` for the chart (6 tests), `tsc --noEmit`, `vitest run` (438 tests in 60 files) and `eslint` on the changed files pass. The new tests cover reading only the listed names in the active namespace, the watch, copying, a clipboard failure, a read denial, and malformed maps. The chart renders with `monitoring.enabled` true and false. ### Screenshots The ConfigMaps tab of a FoundationDB application, rendered by the console from this branch against a local stub API with synthetic data:  ### Downstream repositories Walked the trigger map against the diff: it changes a FoundationDB chart template and its test, and the console. No `values.yaml`, `values.schema.json`, README, CRD or `hack/` file is touched. The website FoundationDB page is generated from the chart README and says nothing about connecting, and the Terraform provider does not read these ConfigMaps. The console is vendored in this repository. - [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(dashboard, foundationdb): the application page shows the FoundationDB connection ConfigMap with the cluster file to tenants who may read it ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Application details now include a ConfigMaps tab when configuration maps are available. View their values and copy them to the clipboard. * The tab displays loading, empty, unavailable, and error states. * **Bug Fixes** * The resource map remains available when monitoring is disabled, and its access permissions are included. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Yan Bondarenko
|
e3ae8033e6 |
fix(foundationdb): let tenants read the dashboard resource map
The resource map is the only object that names the connection ConfigMap for a client such as the dashboard. It was rendered only with monitoring enabled, and the tenant role could not read it, so a tenant session had no way to find the ConfigMap it is allowed to read. Assisted-by: LLM Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com> |
||
|
Aleksei Sviridkin
|
a905fef9dd |
fix(clickhouse): use the cluster domain in keeper node hostnames
The zookeeper block redeclared $clusterDomain from .Values.clusterDomain, a value the chart never defines, so it shadowed the template-level variable read from _cluster.cluster-domain and every keeper host rendered as "<pod>.<namespace>.svc." with no domain. Keeper is enabled by default, so this hit every ClickHouse release. The keeper name test set clusterDomain itself, which hid the bug; it now supplies only _cluster.cluster-domain, as the platform does. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Mattia Eleuteri
|
9afe7f1293 |
fix(kubernetes): assemble the node group once in kubernetes-nodes
The TalosConfigTemplate name is a hash of the machine-config spec, and two
templates have to agree on it: talos-reconcile-job.yaml creates the object,
nodegroup.yaml points MachineDeployment.bootstrap.configRef at it. Sharing the
rendering was not enough — each file also assembled its own $group dict, one
carrying every pool value and one only the few the spec reads, so the two
names agreed only by coincidence of the keys the spec helper happens to read.
Dropping "gpus" from the Job's dict diverged them on a GPU pool with the whole
suite still green, which is the "templates do not exist" deadlock this fix
exists to make impossible.
Move the assembly into a single kubernetes-nodes.group helper, read back with
fromYaml at both call sites, so there is one input as well as one rendering.
Cover it too, since the seam can be reintroduced. The chart's helm-unittest
suites pin each name against its own literal in its own file, which catches a
divergence but reports it as one literal having moved — indistinguishable from
a stale fixture, so re-pasting the new value is the obvious and wrong repair.
hack/check-talosconfigtemplate-name-agreement.bats renders the chart once and
compares the two values against each other, so the failure states the invariant
instead of a number. It asserts on a GPU-shaped pool as well as the default
one: the default renders gpus [], kubelet {} and instanceType "" as falsy, so a
consumer reading a subset of the group is indistinguishable there from one
reading all of it. kernelModules gets a case of its own, because it has no
default and stays unset on both of those shapes. A last case pins that the
shapes hash differently, so the others cannot hold vacuously.
The same guard compares the talos-reconcile Role's resourceNames on
TalosConfigTemplates against that name. The Job applies server-side, and
that is a PATCH on the name even when it creates the object, so a Role
naming anything else leaves the Job Forbidden and the pool without a
template, with the same silent outcome.
Document that talos.registryMirrors and kernelModules sit inside the hashed
spec, so changing either rolls the pool's existing workers instead of only
affecting nodes created later. The kernelModules text described the
fixed-name template as the present state and the content-hash name as a
future one.
No rendered output changes: the golden render snapshots are unchanged.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-by: LLM
|
