mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-09 22:13:18 +09:00
eeee87f0b1f39e2cb869aebb9815ea984bf1b844
5176
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 IvanHunters
|
eeee87f0b1 |
Merge branch 'main' into feat/kubernetes-nodes-phase2b
Resolve the migrations pin conflict in packages/core/platform/values.yaml: keep targetVersion 55 (this branch owns migration 54) and take the newer platform-migrations image v1.6.0 from main. The in-tree image pin intentionally lags targetVersion until the release retag, and run-migrations.sh fails loud if a targeted migration is missing from the image, so the lag is safe by design. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
cbb2692db0 |
fix(platform,kubernetes-nodes): fail worker-pool adoption and unpin reads closed (#3450)
## What this PR does Stacked on #3315. The worker-pool adoption path treats `kubectl` reads as fail-open: `kubectl` exits 1 for NotFound and for transient server failures (timeout, 429, etcd hiccup) alike, and three code paths classified every non-zero read as "object absent". Each sits exactly where absence and failure must not be confused, because "absent" there means "do not pin" or "do not unpin": - **Migration 54, `adopt_one`/`pin_keep`**: a transient error while reading a worker object skipped the keep pin and the ownership rewrite, the migration still completed and stamped version 55 (so it never re-runs), and the parent's control-plane-only upgrade would then prune the un-pinned live MachineDeployment — deleting every running worker VM in the pool. The `cur=$(... || true)` owner read additionally bypassed the foreign-owner refusal guard. - **Migration 54, KubevirtMachineTemplate listings** (adopt loop and `pin_pool_objects`): masked twice (`2>/dev/null` plus `|| true`), so a failed list adopted or pinned zero KMTs and the parent upgrade would prune a preserved older-revision KMT still referenced by an in-flight MachineSet — permanently, since the child chart only re-emits KMTs already annotated with its own release name. - **Pre-delete unpin hook, existence check**: `kubectl get ... >/dev/null 2>&1 || return 0` four lines below the comment declaring "Failures are NOT swallowed". A timeout during uninstall read as absent, the Job reported success, and Helm skipped the still-keep-annotated MachineDeployment — leaking the adopted pool's worker VMs into the tenant namespace, running and unmanaged, after the pool was deleted. The fix applies one contract everywhere: absence is detected with `--ignore-not-found` — a missing object is success with empty output — so a non-zero read exit can only be a real failure and aborts (the migration exits 1 before the version stamp so `run-migrations.sh` retries on the next upgrade; the hook Job fails via `set -e` and is retried by `backoffLimit` with logs retained). Two properties fall out of choosing `--ignore-not-found` over classifying stderr text. stderr is never captured into data: kubectl prints warnings on *successful* reads (API deprecation headers — deterministic for a deprecated GVK — and client-side throttling notices), and a warning glued onto a `2>&1`-captured annotation value would trip the foreign-owner refusal or fake a pinned state. And absence is never inferred from message text: a transient conversion-webhook outage fails a read with `... service "capi-webhook-service" not found`, which a `*"not found"*` match would misclassify as absence, silently re-opening the prune hazard. Mechanically: `adopt_one` reads `-o json --ignore-not-found` and extracts the owner annotation with jq (already a dependency), keeping absence distinct from present-but-unannotated; `pin_keep` reads `-o name`; the hook's `unpin()` folds absence and "present but unannotated" into one empty-output no-op, which is exactly its semantics, and the whole error-classification branch disappears. `list_kmts` is the one read that still needs stderr — for the two deterministic missing-CRD messages (migration-47 precedent) — so it captures stderr to a temp file, never into the data variable; KMT listings capture into a variable first because `for x in $(...)` discards the substitution's exit status even under `set -e`, leaving `|| true` to cover only the empty-grep-match case. The one deliberately tolerant read — the child-HR existence check — is annotated as such: its fallback is an idempotent apply that itself fails closed. `adopt_one` also stamps `meta.helm.sh/release-namespace` alongside the release name, closing the corner where an object with out-of-band-stripped Helm annotations would fail the child's ownership validation. Every kubectl invocation in migration 54 and the hook is now either fail-closed or carries a comment stating why fail-open is safe at that call site. Fail-closed reads cut the other way for names that cannot exist: kubectl rejects a name containing `/` client-side and deterministically, so a pin-path read for one would deadlock the migration on every retry. `pin_pool_objects` therefore skips statically when the composed `kubernetes-<cluster>-<group>` is not a valid DNS-1123 subdomain — such a name provably never had an object applied under it, so there is nothing to pin and no read to misclassify; keys that are label-invalid but still valid object names (dotted) keep the pin behavior. The PR also re-homes three control-plane-chart comments that still described the talos-reconcile Job as living in that chart (it runs per-pool in kubernetes-nodes releases since the split, and a pool-less cluster keeps DNS SANs only — a best-effort extra nothing depends on). Tests, each red against the code it guards (verified by running the pre-fix scripts under the same injections): - The migration-54 fake kubectl models absence the way real kubectl behaves — `--ignore-not-found` present: success + empty; absent: `Error from server (NotFound)` + exit 1 — and gains `FAKE_GET_FAIL`/`FAKE_GET_FAIL_MSG` for transient-error injection and `FAKE_WARN_STDERR` for success-path stderr warnings. Six new bats tests pin the contract: an absent object still completes and stamps; a transient error on the adopt path, the KMT listing, or the pin path aborts before the version stamp; a deprecation warning does not break adoption or fake a foreign owner; and a failure whose message contains "not found" still aborts without stamping. - New `hack/kubernetes-nodes-unpin-hook.bats` tests the hook's embedded script behaviorally — renders the chart, extracts the Job's args, executes them against a fake kubectl — since error classification cannot be asserted from the manifest. Seven cases: absent objects no-op and succeed (native pool preserved), a transient named-get error fails the Job (the original script printed "unpin done" and exited 0 under the same injection), a "not found"-worded failure fails the Job, a failed KMT listing fails the Job, a success-path warning does not fake a pinned state (the `2>&1` variant issued three spurious annotate calls), keep-annotated objects are unpinned with the 6-hex hash anchor respected, and an existing-but-unpinned object is untouched. Verified locally: both migration-54 bats suites (14 + 3 tests), the hook suite (7 tests), and `helm unittest` for both charts (kubernetes-nodes 32 tests + 12 snapshots, kubernetes 117 tests) all pass. The slash-key deadlock was reproduced against the pre-fix script (exit 1 at the first pin read, no stamp) before the fix turned it into a warn-and-skip. ### 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. --> - [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: Walked the trigger map against the diff: a platform migration script, a hook script inside an existing chart template, and unit-test additions under `hack/`. No package added or renamed, no values/schema/API/README change, no `hack/*.mk` or make-target change, no node-prerequisite change — no trigger matches. ### Release note ```release-note fix(platform): the worker-pool adoption migration and the kubernetes-nodes pre-delete unpin hook fail closed on transient apiserver errors instead of misreading them as absent objects, which could otherwise let a platform upgrade prune a live worker pool or let a pool deletion leak its VMs ``` |
||
|
Timofei Larkin
|
616d96f6fc |
docs(kubernetes-nodes): align version description with skew guard
The version field's description said the pool "must match the parent cluster's version", but assertParentVersion deliberately enforces a different contract: a pool may lag the parent -- the normal state during a rolling upgrade, parent Kubernetes CR first, then each pool -- and only a pool AHEAD of the parent fails the render (a symmetric equality requirement would flip every pool to render-failure the moment the parent is bumped). A user following the description would believe the intended upgrade sequence leaves their pools unsupported. Reword the @param to state the actual contract and regenerate with cozyvalues-gen v1.6.0 (the CI-pinned version; 1.3.0 silently drops the @immutable rules) so schema, README, Go types and the RD all follow. Description-only: no defaults, enums or validations change, pinned by the generate-drift CI check and the existing parent_version_skew_test lag case the words now agree with. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
66c96b4329 |
fix(dashboard): stop requesting offline_access so logout can revoke sessions (#3447)
## What this PR does The dashboard and linstor-gui auth-proxies (oauth2-proxy) requested the `offline_access` scope, so Keycloak issued an **offline** refresh token. oauth2-proxy has no server-side session store: it keeps the refresh token in the `kc-access` cookie and silently refreshes it (`--cookie-refresh`). An offline token is decoupled from the SSO session by design, so logout (`end_session`) does not revoke it. As a result a copied `kc-access` cookie stayed usable for the whole offline-session lifetime (Keycloak default ~30 days) even after the user logged out — logout did not invalidate it. This drops `offline_access` from the proxy `--scope` in both packages, so Keycloak issues an **online** refresh token bound to the SSO session, which logout revokes. The `offline_access` optional client scope is also removed for cleanliness (with a note on why). Scope of the fix: - `packages/system/dashboard` — proxy `--scope` + optional client scope - `packages/system/linstor-gui` — proxy `--scope` + optional client scope + ingress auth unittest The public `kubernetes` OIDC clients (kubectl `oidc-login`) are intentionally left untouched: `offline_access` is legitimate there and those clients do not sit behind oauth2-proxy. Notes: - The keycloak-operator only *adds* client scopes, so removing the optional scope does not strip the realm-default `offline_access` from an already-created client. The proxy `--scope` change is what actually prevents issuance. - This prevents *new* offline tokens. Offline sessions already issued to existing users remain valid until the realm's offline-session idle timeout (~30d) or explicit revocation via the Keycloak Admin REST API / Account Console. - After this change dashboard session lifetime is governed by the online SSO session (`client.session.idle.timeout` / `client.session.max.lifespan`). ### Screenshots Not applicable — no UI change (auth-proxy OIDC scope only). ### Downstream repositories Walked the trigger map against the diff: the change touches only helm templates and a unittest under `packages/system/dashboard` and `packages/system/linstor-gui`. No downstream repository restates the auth-proxy OIDC scope. - [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: ### Release note ```release-note fix(dashboard): stop requesting the offline_access scope in the dashboard and linstor-gui auth-proxies so that logout revokes the session; a copied kc-access cookie is no longer refreshable after logout. Offline sessions already issued to existing users must be revoked manually via Keycloak. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Security Improvements** * Removed requests for offline access and persistent refresh tokens from OIDC authentication. * Sessions now rely on standard scopes, helping ensure logout invalidates authentication as expected. * **Tests** * Updated authentication checks to verify the revised OIDC scope configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
af2b40be9f |
docs(kubernetes): re-home talos-reconcile comments after split
Three template comments in the control-plane chart still described the
talos-reconcile Job as living here, but the worker split moved it to
the kubernetes-nodes chart (one run per pool release):
- talos-pki.yaml pointed at "the routing note in
talos-reconcile-job.yaml", a file this chart no longer contains.
- cluster.yaml said "the talos-reconcile main-phase Job re-patches
certSANs" as if this chart still runs it; it now also states the
consequence the kubernetes-nodes side already documents -- the
ClusterIP SAN arrives only through a pool and a pool-less cluster
keeps DNS SANs only, a best-effort extra that nothing depends on.
- cilium.yaml credited the /etc/hosts seeding to the same Job without
naming its new home; it also now notes why the entry is always
present wherever Cilium runs (workers only exist through pools).
Comments only ({{/* */}} blocks, stripped at render time); no manifest
output changes, pinned by the chart's unittest suite staying green.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
|
||
|
Timofei Larkin
|
730d006ce8 |
fix(platform): skip pinning names no object can have
The pin path composed "kubernetes-<cluster>-<group>" and read it back, but a group key that cannot form a valid object name (a slash, a space, an underscore...) provably never had objects applied under it -- the old chart's apply would have been rejected by the apiserver. Worse, kubectl rejects a name containing '/' client-side, deterministically, so pin_keep's fail-closed read classified it as a real failure and the migration exited 1 on every retry: a permanent fleet-wide pre-upgrade deadlock over a pool that cannot exist. Lean on the invariant instead of the read: pin_pool_objects statically checks the composed name against the DNS-1123 subdomain shape and skips with a note when it cannot name an object -- nothing to pin, no kubectl call to misread. Keys that are label-invalid but still valid object names (e.g. dotted) may have live objects and keep today's pin behavior; the pin-path transient-error test now uses a dotted key so it still exercises a genuine read. The fake kubectl reproduces real kubectl's client-side slash rejection, and a new bats case pins the skip: a "pool/a" key must warn, pin nothing, apply nothing, and still reach the version stamp. Red against the previous script, which deadlocked at the first pin read. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
a54d5ca4a5 |
fix(kubernetes-nodes): unpin absence via --ignore-not-found
Same two holes as the migration's read classification, in the unpin hook. Capturing stderr into $pol (2>&1) contaminated the success path: kubectl warns on successful reads, so an unannotated object read as "pinned" and drew a spurious annotate call -- one per named object. And the *"not found"* pattern classified a transient conversion-webhook failure as absence, letting the Job report success while the keep-annotated objects leaked past the uninstall. --ignore-not-found makes the whole classification unnecessary here: absence folds into the same empty-output no-op branch as "present but unannotated", which is exactly the semantics unpin() wants, and any non-zero exit is a real failure that set -e turns into a failed Job. The case/esac disappears; stderr stays on the Job log where it is visible instead of parsed. This also avoids tempfile juggling under the container's readOnlyRootFilesystem. The bats suite gains a previously missing failure-path case and regression tests for both holes, red against the previous commit: a failed KMT listing must fail the Job (FAKE_LIST_FAIL), a read failure whose message contains "not found" must fail the Job, and a success-path stderr warning must not fake a pinned state (FAKE_WARN_STDERR). Absence is now modeled the way real kubectl behaves under --ignore-not-found: success with empty output. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
bdadb3ff04 |
fix(platform): detect absence via --ignore-not-found
The stderr-based classification this branch introduced had two holes of its own. Capturing stderr into the data variable (2>&1) contaminates the SUCCESS path: kubectl prints warnings on successful reads (API deprecation headers -- deterministic for a deprecated GVK, and CAPI v1beta1 is one -- plus client-side throttling notices), so the warning glued onto the owner annotation would trip adopt_one's foreign-owner refusal on every tenant and permanently deadlock the pre-upgrade hook. And the *"not found"* pattern over-matches: a transient conversion-webhook outage fails a read with '... service "capi-webhook-service" not found', which the classifier would treat as absence -- skipping the keep pin and re-opening the exact fail-open prune hazard this branch exists to close. Detect absence with --ignore-not-found instead: a missing object is SUCCESS with empty output, so a non-zero exit can only be a real failure and aborts before the version stamp, with no stderr parsing at all. adopt_one reads -o json and extracts the owner annotation with jq (already a dependency), keeping absence distinct from present-but- unannotated; pin_keep reads -o name. list_kmts still needs stderr for the one legitimate classification -- the two deterministic missing-CRD messages -- so it captures stderr to a temp file, never into data, and no longer accepts NotFound-shaped text for a list. The fake kubectl honors --ignore-not-found the way real kubectl does, gains FAKE_WARN_STDERR (success-path stderr warnings) and FAKE_GET_FAIL_MSG (transient error with arbitrary text). Two new bats tests, each red against the previous commit: a deprecation warning must not break adoption or fake a foreign owner, and a failure whose message contains "not found" must still abort without stamping. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
1ca947e2fb |
fix(kubernetes-nodes): fail unpin reads closed
The pre-delete unpin hook's existence check swallowed every kubectl get failure (`... >/dev/null 2>&1 || return 0`), four lines below the comment declaring "Failures are NOT swallowed". kubectl exits 1 for NotFound and for a transient apiserver error alike, so a timeout during uninstall read as "object absent": the unpin was skipped, the Job succeeded, and Helm then skipped the still-keep-annotated MachineDeployment -- leaking the adopted pool's worker VMs into the tenant namespace, running and unmanaged, after the pool was deleted. The KMT-listing path was already hardened for exactly this class; the existence check was the remaining instance. Classify by stderr: NotFound (native pool, or already gone) stays a no-op; any other failure fails the Job, bounded by backoffLimit and activeDeadlineSeconds, with logs retained. The existence check and the annotation read fold into a single request. The embedded script's behavior cannot be asserted from the manifest, so a new bats suite renders the chart, extracts the Job's script, and executes it against a fake kubectl: NotFound no-ops and succeeds, a transient named-get error fails the Job (red against the previous script, which printed "unpin done" and exited 0), keep-annotated objects are unpinned with the 6-hex hash anchor respected, and an unpinned existing object is left untouched. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
f8258c8859 |
fix(platform): fail migration 54 reads closed
kubectl exits 1 for NotFound and for transient server failures (timeout, 429, etcd hiccup) alike, and migration 54 treated every non-zero read as "object absent". A transient error while reading a worker object would skip the keep pin and the ownership rewrite, let the migration complete and stamp version 55 (so it never re-runs), and the parent's control-plane-only upgrade would then prune the un-pinned live MachineDeployment -- deleting every running worker VM in the pool. The doubly-masked KubevirtMachineTemplate listings (2>/dev/null plus `|| true`) had the same shape: a failed list adopted or pinned zero KMTs, permanently stranding a preserved older-revision KMT still referenced by an in-flight MachineSet, since the child chart only re-emits KMTs already annotated with its own release name. The `cur=$(... || true)` owner read additionally bypassed the foreign-owner refusal guard. Classify reads by stderr, the migration-47 precedent: NotFound means genuinely absent and is skipped; anything else exits 1 so run-migrations.sh leaves CURRENT_VERSION at 54 and retries on the next upgrade. adopt_one and pin_keep fold the existence check into the annotation read (one request instead of two); KMT listings go through a capture-first list_kmts helper because `for x in $(...)` discards the substitution's exit status even under set -e, leaving `|| true` to cover only the empty-grep-match case, mirroring the unpin hook. The child-HR existence check stays deliberately tolerant: its fallback is an idempotent apply that itself fails closed. adopt_one now also stamps meta.helm.sh/release-namespace so an object whose Helm annotations were stripped out-of-band cannot fail the child's ownership validation. The fake kubectl now reports absent objects the way the real apiserver does (NotFound on stderr, exit 1) and gains FAKE_GET_FAIL to inject transient errors; four new bats tests pin the classification: NotFound still completes and stamps, while a transient error on the adopt path, the KMT listing, or the pin path aborts before the version stamp. Each red against the previous script. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
436743eb34 |
feat(workloadmonitor): bucket metrics endpoint override (#3443)
## What this PR does The cozystack-controller meters S3 bucket sizes for billing by querying SeaweedFS `SeaweedFS_s3_bucket_(size|physical_size)_bytes` metrics from a VictoriaMetrics vmselect it locates via the `namespace.cozystack.io/monitoring` label on the tenant namespace. That discovery hardcodes the assumption that the monitoring stack scraping SeaweedFS runs in the same cluster. Some deployments host SeaweedFS — API and metrics both — in a separate cluster, so bucket sizes must be fetched from a remote Prometheus-compatible endpoint that discovery can never produce. ### Configuration surface A single controller flag, `--seaweedfs-metrics-endpoint`, exposed as a chart value: ```yaml cozystackController: seaweedfsMetricsEndpoint: "https://vm.example.com/path/to/prometheus" ``` The value is an arbitrary base URL of a Prometheus-compatible query API — scheme, host, optional port, and an optional path prefix — and the controller appends `/api/v1/query` to it, normalizing slashes so a trailing slash or a path prefix never produces a doubled or dropped slash. Basic auth can be embedded as URL userinfo (`https://user:pass@vm.example.com/...`); net/http turns it into an `Authorization` header on every request. TLS uses the system trust roots; a custom CA or bearer-token support would be a follow-up flag if a deployment needs it. The flag is validated and normalized at startup (absolute URL, http/https, no query string or fragment), so a malformed value fails the rollout instead of failing queries at runtime. Cluster-wide scope is deliberate: where SeaweedFS and its metrics live is a property of the deployment, not of any tenant, and tenant monitoring stacks keep working and keep being discovered via the `namespace.cozystack.io/monitoring` label for everything else — the flag only redirects the bucket-size queries. Being a deployment argument, it is settable exclusively by platform administrators, so no new writable surface is exposed to tenants. Rejected alternatives: a WorkloadMonitor spec field (bucket monitors are rendered by the tenant-facing bucket chart, so the field would have to travel through tenant-writable values — an SSRF primitive — and it would be near-permanent CRD surface for a deployment-level fact) and per-namespace annotations (per-tenant granularity that nothing needs, plus a second admin-facing configuration mechanism to document and validate). ### Backwards compatibility Strictly opt-in: with the flag unset — the default — resolution walks the exact same label-discovery path and produces the same vmselect URL. No existing deployment changes behaviour. ### Billing safety: failures no longer collapse to zero This also fixes a latent billing bug in the existing path: any metrics query failure (endpoint down, non-200, malformed response) produced an empty result map, and the reconciler overwrote `Workload.status.resources` with it — silently wiping the recorded bucket sizes, which billing reads as an empty bucket. Now: - `queryAllBucketMetrics` returns errors instead of swallowing them into an empty map, so "the bucket is empty" and "the endpoint could not be queried" are distinguishable. - On failure — and per bucket, when a query succeeds but reports no series for a bucket that previously had a size — the reconciler retains the last known `s3-storage-bytes` and `s3-physical-storage-bytes` values instead of dropping them. - The failure is surfaced three ways: a `BucketMetricsUnavailable` warning event on the WorkloadMonitor, an error-level log line, and an error returned from `Reconcile`, which lands in controller-runtime's reconcile error metrics for alerting. Unit tests cover URL validation and path joining, flag-over-discovery precedence, userinfo-based basic auth, and both retention scenarios (query failure and missing series). ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff (controller Go code plus the cozystack-controller chart value and arg plumbing): none of the listed triggers match — no package added or renamed, no CRD, `ApplicationDefinition`, or platform values change, no `hack/` or layout change. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(workloadmonitor): a new --seaweedfs-metrics-endpoint controller flag (chart value cozystackController.seaweedfsMetricsEndpoint) points bucket size metering at an explicit Prometheus-compatible endpoint for deployments where SeaweedFS and its metrics live in a separate cluster; metrics query failures now retain the last known bucket sizes and surface an event and error instead of silently zeroing billing data ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an optional SeaweedFS metrics endpoint configuration. * Metrics endpoint URLs are validated and normalized before use. * Added support for endpoint overrides when discovering bucket metrics. * **Bug Fixes** * Preserved previously recorded bucket sizes when metrics are unavailable or incomplete. * Metrics failures now trigger retries and visible warning events without blocking other reconciliation. * Improved handling of invalid or unsuccessful metrics responses. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
c15b4bab6f |
test(kubernetes-nodes): assert unpin hook egress destination
The CiliumNetworkPolicy test verified only that the policy selects the unpin pod, not spec.egress, so an egress regression that left the uninstall hook unable to reach the apiserver would still pass. Assert the kube-apiserver egress entity. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
040ad1d20d |
fix(kubernetes-nodes): fail unpin hook on KMT discovery error, do not leak
The pre-delete unpin hook discovered this pool's hash-suffixed KubevirtMachineTemplates through `kubectl get ... 2>/dev/null | grep ... || true`, which turned an API or RBAC failure into an empty result: the Job then reported success while the keep-annotated templates stayed pinned and leaked on uninstall (Helm skips keep-annotated resources). That contradicts the unpin() contract right above it, where annotate failures are deliberately propagated. Split the list out of the pipe so its failure fails the Job (set -e, bounded by backoffLimit/activeDeadlineSeconds, logs retained), while a successful-but-empty listing still no-ops the loop. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
928df3c2d3 |
fix(platform): harden migration 54 against non-object and whitespace nodeGroups
The RFC-1123 key guard added in the previous round closed hostile map
keys, but migration 54 still trusted the shape of nodeGroups and of each
group value, and word-split keys on whitespace. Because it runs as the
platform pre-upgrade hook, either gap can deadlock the upgrade for every
tenant off one hand-mangled parent HelmRelease (kubectl edit bypasses the
aggregated API schema, the same vector the key guard addresses).
Shape: a non-object nodeGroups (array/string/number) crashes jq under
set -e (keys[] on an array yields indices, then .[$idx] fails); a
non-object group value aborts the value-mapping jq on `$grp | has(...)`.
Normalize both: skip the cluster for a non-object nodeGroups map, and
route a non-object group value through the same warn + pin + skip path as
an invalid name. {} / null still fall through to the implicit md0 default.
Whitespace: an unquoted `for group in $(... keys[])` word-splits (and
globs) a key like 'my pool' into fragments that each pass the RFC-1123
guard, fabricating garbage child releases. Iterate keys newline-safely so
the intact key reaches the guard. Fail-closed exit semantics are
preserved through the while pipeline.
Covered by three new bats fixtures in migration-54-adopt-path.bats, each
red against the current script and green with this change.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
51c34c236e |
fix(linstor-gui): stop requesting offline_access so logout can revoke sessions
Same offline-token-survives-logout issue as the dashboard: the linstor-gui auth-proxy requested offline_access, so a copied kc-access cookie stayed valid for the whole offline-session lifetime after logout. Drop offline_access from the proxy --scope and the optional client scope, and update the ingress auth unittest accordingly. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
99936a92f8 |
fix(dashboard): stop requesting offline_access so logout can revoke sessions
The dashboard auth-proxy (oauth2-proxy) requested the offline_access scope, so Keycloak issued an offline refresh token. oauth2-proxy stores that token in the kc-access cookie (no server-side session store) and silently refreshes it, and an offline token is decoupled from the SSO session: logout (end_session) does not revoke it. A copied cookie therefore stayed usable for the whole offline-session lifetime even after the user logged out. Drop offline_access from the proxy --scope so Keycloak issues an online refresh token bound to the SSO session, which logout revokes. Also remove the offline_access optional client scope for cleanliness. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Aleksei Sviridkin
|
a80f13405c |
fix(cozy-lib): converge the CA trust-anchor helper on the canonical name (#3408)
## What this PR does Ships `cozy-lib.tls.caCertSecret`, a render-time helper that emits a key-free trust-anchor Secret carrying only `ca.crt`. It converges the object name on `<release>.tenant-ca`, requires the whole value to be certificate blocks so a stray private-key header or trailing bytes are rejected, and coerces numeric scalars before the guard runs so a numeric value cannot slip past. No chart calls it yet. It is the render-time producer half of the trust-anchor contract; the controller half is #3407, now merged. The doc-comment on the helper shows the one-line call a chart uses, and says outright there is no caller yet, so nobody reads it as wired when it is not. Split out from #3299. Comments trimmed on review. ```release-note fix(cozy-lib): the CA trust-anchor helper emits the canonical `<release>.tenant-ca` name and rejects non-certificate input. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated generated TLS CA Secret naming to use the `<release>.tenant-ca` convention. - Added/ensured a tenant CA label is set alongside the existing tenant resource label. - Continued support for custom labels (caller labels still applied). - **Bug Fixes** - Strengthened CA certificate PEM validation to be fail-closed: requires non-empty, complete `BEGIN/END CERTIFICATE` blocks only. - Rejects private-key material and other malformed or improperly structured PEM input, including unexpected extra content. - **Tests** - Updated TLS CA certificate fixtures and assertions to cover additional negative cases and refined error expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Timofei Larkin
|
95e4faf68e |
feat(workloadmonitor): bucket metrics endpoint override
The controller discovers where to query SeaweedFS bucket size metrics via the namespace.cozystack.io/monitoring label, which assumes the monitoring stack that scrapes SeaweedFS runs in the same cluster. Some deployments host SeaweedFS and its metrics in a separate cluster, so add a --seaweedfs-metrics-endpoint flag to cozystack-controller (and a matching cozystackController.seaweedfsMetricsEndpoint chart value): an explicit base URL of a Prometheus-compatible query API, with scheme, host, optional port and path prefix; /api/v1/query is appended. Basic auth may be embedded as URL userinfo. The flag is validated and normalized at startup, so a malformed URL fails the rollout rather than the queries. Empty keeps label-based discovery unchanged. Bucket sizes feed billing, so metrics failures no longer collapse to "no size": previously any query failure yielded an empty result map and the reconciler overwrote Workload resources with it, silently wiping the recorded sizes. The reconciler now retains the last known s3-storage-bytes and s3-physical-storage-bytes values, emits a BucketMetricsUnavailable warning event on the WorkloadMonitor, and returns the error from Reconcile so it lands in the reconcile error metrics. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
2823b76802 |
feat(gateway): advertise HTTP/2 via ALPN on Gateway API listeners (#3398)
## What this PR does Browsers negotiate HTTP/2 exclusively through TLS ALPN: if the terminating listener does not advertise `h2`, every client silently falls back to HTTP/1.1, with no error surfaced anywhere. Cozystack's Gateway API path is served by Cilium's embedded Envoy, and Cilium ships with `enable-gateway-api-alpn=false` — so every Gateway listener advertised no ALPN protocols and all browser traffic through a Gateway ran over HTTP/1.1 (observable in browser dev tools as `http/1.1` on every request). The ingress-nginx path this replaces advertised `h2` out of the box, so the migration to Gateway API silently downgraded clients: head-of-line blocking and the ~6-connections-per-origin limit are back for anything served through a Gateway. This PR enables ALPN (`h2`, then `http/1.1`) on Gateway API listeners, default-on, with an opt-out at each management layer: - `packages/system/cilium/values.yaml` now defaults `gatewayAPI.enableAlpn: true`, which renders `enable-gateway-api-alpn: "true"` into the `cilium-config` ConfigMap. - The platform chart gains `gateway.http2` (default `true`) next to the existing `gateway.enabled`/`gateway.attachedNamespaces`. The `isp-full` and `isp-full-generic` bundles thread it as inline component values into the `cozystack.networking` Package CR (`components.cilium.values.cilium.gatewayAPI.enableAlpn`); inline HelmRelease values override the package `valuesFiles`, so an explicit `gateway.http2: false` actually reaches the dataplane rather than being shadowed by the new chart default. On `isp-hosted` the hosting-side CNI owns this setting and the flag is a no-op. - Tenant Kubernetes clusters (`packages/apps/kubernetes`) get the same fix: when the `gatewayAPI` addon is enabled, the tenant cilium HelmRelease now sets `enableAlpn: true`; individual clusters opt out via `addons.cilium.valuesOverride.cilium.gatewayAPI.enableAlpn=false`. ### Why default-on rather than opt-in HTTP/2 for browsers was the platform's behavior before the Gateway API migration, so keeping ALPN off would preserve a regression, not a status quo. Defaulting on is safe because ALPN is a negotiation: clients that cannot speak HTTP/2 simply keep negotiating `http/1.1`, exactly as before. Operators who need the old wire behavior can set `gateway.http2: false` platform-wide. ### Frontend vs backend hop This change affects the client↔gateway hop only. Gateway↔backend connections stay HTTP/1.1 unless a Service explicitly opts in per GEP-1911 by declaring `appProtocol: kubernetes.io/h2c` on its port. Cilium couples that backend-protocol support to the ALPN flag (`enable-gateway-api-app-protocol` flips to true together with it), but it only acts on Services that declare a recognized `appProtocol` value — and no cozystack-shipped Service does today (only plain `http`/`https` values exist in-tree, which Cilium ignores for protocol selection). So upstreams see no change from this PR alone; h2c/gRPC backends become possible to configure, not configured. The setting is cluster-wide Cilium agent config, so it covers the root Gateway and all tenant Gateways at once — Cilium offers no per-Gateway granularity here. Flipping it re-rolls the cilium DaemonSet on upgrade (`rollOutCiliumPods: true`), the same disruption profile as any other cilium config change. ### How it was verified - New helm-unittest suites: `packages/core/platform/tests/bundles_gateway_http2_wiring_test.yaml` asserts the Package CR carries `enableAlpn` true by default and false with `gateway.http2: false` for both cilium bundles; `packages/apps/kubernetes/tests/cilium_gateway_alpn_test.yaml` asserts the tenant HelmRelease default, the addon-off case, and the `valuesOverride` opt-out. Full suites for both charts pass (93 and 184 tests). - `helm template` of `packages/system/cilium` confirms the value is consumed, not just rendered: default produces `enable-gateway-api-alpn: "true"` (and `enable-gateway-api-app-protocol: "true"`) in `cilium-config`; the threaded `false` produces `"false"` for both. ### Screenshots Not a UI change. ### Downstream repositories The website's hand-written platform values table (`content/en/docs/next/operations/configuration/platform-package.md`) needs a row for `gateway.http2` — filed as cozystack/website#625. No other downstream repository restates anything in this diff (no app `values.schema.json`, ansible-consumed platform key, or hack/ tooling changed). - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/625 - [ ] [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: ### Release note ```release-note feat(gateway): Gateway API listeners now advertise HTTP/2 via TLS ALPN (h2, then http/1.1) by default, restoring the browser HTTP/2 support lost in the migration from ingress-nginx. Opt out platform-wide with gateway.http2=false, or per tenant Kubernetes cluster via addons.cilium.valuesOverride.cilium.gatewayAPI.enableAlpn=false. Backend (gateway-to-upstream) connections are unchanged and stay HTTP/1.1 unless a Service declares appProtocol kubernetes.io/h2c. ``` |
||
|
Aleksei Sviridkin
|
0ba79c0168 |
fix(cozystack-basics): gate the hostname VAP policies on the VAP API (#3409)
## What this PR does Wraps the route, gateway, and ingress hostname `ValidatingAdmissionPolicy` templates in a `.Capabilities.APIVersions.Has` gate so they render only where the VAP API is available. Without it, an operator-generated HelmRelease with drift detection off renders the policies out at first install on a cluster missing the API and never adds them back. Each template gains a sibling test asserting it renders zero documents when the API is absent. Install-robustness fix, split out from #3299. ```release-note fix(cozystack-basics): hostname ValidatingAdmissionPolicies render only where the VAP API is available, so a first install on a cluster without it no longer drops them permanently. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Hostname validation policies now render only when the Kubernetes admission policy API is available, preventing unsupported policy resources on older clusters. * **Tests** * Added/updated unit tests to explicitly simulate both missing and present admission policy API capabilities, ensuring templates render (or not) as expected. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
5cdeba7c41 |
feat(cozy-lib): align the CA trust-anchor helper with the canonical name
Rename the Secret cozy-lib.tls.caCertSecret renders from "<release>-ca-cert" to "<release>.tenant-ca", the same canonical, key-free name the CA-extraction controller publishes, so a chart that holds its CA PEM at render time and an operator whose CA is minted asynchronously converge on one object a tenant can name. "<release>-ca-cert" is not a free name — Percona Server for MongoDB already claims it with a key-bearing Secret. Harden the write guard: require a complete BEGIN/END CERTIFICATE block, keep failing closed on any PEM private-key header, and coerce a non-string caCert value before trimming so an unquoted numeric YAML scalar cannot panic the template. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
af397bb62c |
chore: ignore root-level Go binaries built from cmd/ (#3412)
## What this PR does Adds the root-level binary names that `go build ./cmd/<name>` (without `-o`) drops in the repository root to `.gitignore`, so `git add .` cannot accidentally commit an ~80 MB executable. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated repository settings to exclude generated Go executables from version control. * Added explanatory comments for the ignored build artifacts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
5a7e4c5391 |
feat(api)!: require the application release prefix to be dot-free (#3411)
## What this PR does Constrains `ApplicationDefinition.spec.application.release.prefix` to `^[a-z0-9-]*$`, which excludes the dot. Release names are `<prefix><app>`, and the tenant CA trust anchor is projected to `<release>.tenant-ca`; the dot is the separator that keeps that name unreachable by any engine, so a prefix carrying a dot would break the guarantee. Every in-tree prefix already conforms. Split out from #3299. ```release-note feat(api)!: `ApplicationDefinition.spec.application.release.prefix` must match `^[a-z0-9-]*$` (dot-free). A definition with a prefix containing a dot or any other character is rejected at admission; correct it before applying this CRD. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Added validation for release prefixes to allow only lowercase letters, numbers, and hyphens. * Updated guidance explaining how prefixes affect generated release names and tenant CA trust anchors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
6ecb36ffc7 |
feat(cozystack-controller): extract application CA into a key-free tenant Secret (#3407)
## What this PR does Adds an engine-agnostic controller that publishes a key-free `<release>.tenant-ca` Secret (only `ca.crt`) for a managed application, so a tenant can verify the application's TLS without ever seeing a private key. The source is declared, not guessed: a chart renders a namespaced `TenantProjection` sentinel (group `internal.cozystack.io`) naming the CA Secret to lift; the controller watches the sentinel, extracts `ca.crt`, and writes the projection owner-referenced to the sentinel, so garbage collection is native. The projection carries the `internal.cozystack.io/tenant-ca` label the lineage webhook turns into tenant visibility. Postgres is the first consumer. The trust boundary is RBAC: no tenant role grants any verb on `internal.cozystack.io`, so a tenant cannot forge a sentinel. A companion `ValidatingAdmissionPolicy` that pins the writer to helm-controller ships separately as defence in depth. This is the controller-and-CRD slice, split out from #3299 for focused review. It is the only piece required for the feature to work; the render-time helper, the writer policy, the capability-gate, and the release-prefix validation land as their own PRs. ### Downstream repositories Walked the trigger map against the diff. This adds an internal CRD and controller and a sentinel to the Postgres chart; no app is added or renamed and no `values.schema.json` changes, so the typed provider and Ansible are unaffected. The one to confirm is the Postgres README edit — if the website mirrors it, it needs a refresh. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: ### Release note ```release-note feat(cozystack-controller): publish a key-free per-application CA trust anchor (`<release>.tenant-ca`) that tenants read to verify managed-application TLS, declared through a new `TenantProjection` CRD. Postgres is the first consumer. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added tenant-facing CA certificate projections for PostgreSQL deployments. * Tenant secrets now provide only the CA certificate, excluding private keys. * Added support for configuring and monitoring tenant CA projections. * **Documentation** * Updated PostgreSQL TLS instructions to retrieve certificates through tenant secrets. * **Bug Fixes** * Improved protection against invalid certificates, secret collisions, and accidental exposure of private key material. * Added validation to ensure projected certificates remain synchronized with their sources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
8aeedde088 |
feat(backups): support PostgreSQL point-in-time recovery (PITR) (#3383)
## What this PR does Makes PostgreSQL point-in-time recovery (PITR) a genuinely supported, tested capability for the new `backups.cozystack.io` API. Fixes #2774. Was **stacked on #3313** (`fix/3300-cnpg-barman-plugin`) for the `recoveryTime` → CNPG `bootstrap.recovery.recoveryTarget.targetTime` plumbing; #3313 has since merged and this branch is rebased on `main`. This PR adds the reliability + verification layer #2774 asked for. ### Changes - **Precise failure reason for an unreachable target.** The restore deadline (`spec.options.restoreTimeoutSeconds`, default 30m) stays the sole authority for "this restore is stuck" — the client waits the full window either way. What changes is the *diagnosis*: once the deadline has elapsed, if `recoveryTime` was set and the recovery Cluster never went healthy, the CNPG driver reads the `full-recovery` pod logs and — when several distinct attempts exited with PostgreSQL's `FATAL: recovery ended before configured recovery target was reached` — fails the RestoreJob with `RecoveryConverged=False` / `Ready=False` (reason `RecoveryTargetUnreachable`) and an actionable message, instead of a bare generic timeout. Timing is unchanged; only the reason/message differ. (For a faster rejection, set a shorter `restoreTimeoutSeconds`.) Design points: - **We deliberately do NOT trip early off the FATAL.** A valid near-now target emits that same FATAL transiently — repeatedly, for many attempts, on a slow archiver — before it converges, so any earlier trip would wrongly reject a recoverable restore (observed twice in CI). The FATAL is used only to *explain* a failure the deadline has already declared. - **Health wins unconditionally** — the deadline/classification check precedes the healthy check, but a recovery that converges within the window reaches success and is never classified as failed (a live dev7 restore did exactly this). - **Counting distinct FATAL-ing attempts** (not pod failures) tolerates a one-off archive-lag miss on a near-now target and ignores transient recovery-pod crashes (a node blip / brief API-server unreachability never emits that FATAL). - Adds the `pods/log` RBAC grant the log read needs (distinct subresource from `pods`); a `Forbidden` is logged loudly and surfaced in the failure message rather than silently disabling the classification. - **e2e** (`examples/backups/postgres/run-all.sh`, wired into the Chainsaw `postgres-2-backup-roundtrip` suite): - Step 45 — write `before`, capture server timestamp (µs precision), write `after`, restore to that timestamp, assert `before` present and `after` absent. - Step 46 — a `recoveryTime` an hour ahead must fail with reason `RecoveryTargetUnreachable` once the deadline elapses (asserts the *reason*, so it proves the classification fired, not just that a timeout occurred). - Chainsaw budget raised 25m → 40m to cover three sequential restores + the post-deadline classification. - **Docs + example**: uncommented `45-restorejob-pitr.yaml`; a `docs/operations/backup-classes.md` PITR section (recoverable window, discovering earliest/latest restorable time from the backup catalog since `firstRecoverabilityPoint` is unreliable under the plugin, archive-lag tolerance, idempotency under GitOps). ### Verification - `go build` / `go vet` clean; unit tests pass (`logIndicatesRecoveryTargetUnreachable`, `recoveryUnreachableFromLogs`, `recoveryPodsToInspect`, nil-Clientset guard, a reconcile-level success-path test asserting `Succeeded` + `RecoveryConverged=True`, and — added per review — `TestReconcileCNPGRestore_DeadlineClassifiesRecoveryTargetUnreachable`, a reconcile-level test driving the deadline branch through all three sub-paths: FATAL found → reason `RecoveryTargetUnreachable`, FATAL absent → generic timeout, `Forbidden` log read → generic timeout + missing-`pods/log`-RBAC note. It fails if the call-site reason is reverted to the generic one). - Reviewed via adversarial branch-review; fixes include a critical missing-RBAC dead-on-arrival, a classification false-positive on transient recovery-pod crashes, and a sub-second e2e flake. - **dev7**: backup + WAL archiving + recovery convergence verified live on the #3313 build. The load-bearing assumption of the classification — CNPG **accumulates** failed `full-recovery` pods rather than reusing one — was confirmed (observed 7 simultaneous failed recovery pods). A fully-green live step-46 run was blocked by intermittent COSI bucket-provisioning flakiness on dev7; the negative path is covered by the CI Chainsaw e2e. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff. The only user-facing surface is the new `docs/operations/backup-classes.md` PITR section; the website keeps a hand-mirrored copy per docs version. - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: PITR docs mirror warranted (not yet opened; same pending mirror as #3313's backup-classes changes) - [x] No other downstream repository is affected by this change ```release-note feat(backups): PostgreSQL RestoreJob now supports point-in-time recovery (PITR). Set spec.options.recoveryTime (RFC3339) to restore to an exact instant. A recoveryTime past the latest archived WAL still runs to the restore deadline (spec.options.restoreTimeoutSeconds, default 30m), but the failure is now reported with the precise reason RecoveryTargetUnreachable and an actionable message instead of a generic timeout; set a shorter restoreTimeoutSeconds for a faster rejection. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added PostgreSQL point-in-time recovery (PITR) support via RFC3339 `recoveryTime`. * Introduced a PITR RestoreJob example, plus helper variables and an unreachable-target scenario. * **Bug Fixes** * Improved restore deadline handling with deterministic, log-based fail-fast classification for unreachable recovery targets, including clearer conditions. * **Documentation** * Added detailed PITR operational documentation (recovery windows, failure behavior, and GitOps/idempotent behavior). * Expanded README and example comments for PITR verification and failure expectations. * **Tests** * Extended unit tests and end-to-end chainsaw workflow to cover PITR success and unreachable failures. * **Chores** * Updated RBAC to allow pod log reads; refined cleanup and run scripts for additional RestoreJob artifacts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
267ea513ad |
fix(cozystack-basics): gate the hostname VAP policies on the VAP API
Render the route, gateway and ingress hostname ValidatingAdmissionPolicies only where the admissionregistration.k8s.io/v1/ValidatingAdmissionPolicy API is served, matching the deletion-protection guard, so a cluster without that API does not receive an unrenderable resource. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
myasnikovdaniil
|
18e459a1cb |
fix(e2e): improve failure diagnostics (#3421)
## What this PR does Improves E2E failure diagnostics tracked in #3368: - preserves scheduled, non-terminal NotReady pods before a pod IP is assigned - always captures load-balancer state while gating IP-dependent diagnostics - collects Talos dmesg, kubelet, and containerd logs per host node using explicit endpoint and node arguments - renders COSI Bucket catch output as YAML - makes pre-commit generation locking work from linked worktrees - adds regression coverage for dataplane and Talos collection Tenant-worker Talos logs remain outside this PR because the runner does not currently have a tenant talosconfig. Part of #3368. ## Testing - `hack/cozytest.sh hack/capture-dataplane.bats` - `hack/cozytest.sh hack/cozyreport-talos.bats` - `sh -n hack/e2e-capture-dataplane.sh` - `sh -n hack/cozyreport.sh` - `chainsaw test --no-cluster --config hack/e2e-chainsaw/.chainsaw.yaml --include-test-regex '^$' hack/e2e-chainsaw/bucket hack/e2e-chainsaw/harbor` - `pre-commit run --all-files` ### Screenshots Not applicable. ### 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: ### Release note ```release-note fix(e2e): improve diagnostic capture for dataplane, Talos, and COSI failures ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Improved dataplane diagnostics for scheduled pods that are NotReady, including cases without an assigned IP. - LoadBalancer checks now proceed even when no affected pods are found. - Standardized Talos node diagnostics collection for consistent kernel, kubelet, and containerd logs. - Improved reliability of generation hooks by using a git-common lockfile path for concurrent runs. - **Tests** - Added/expanded Bats coverage for dataplane pod filtering and Talos diagnostics argument correctness. - Improved end-to-end bucket test retrieval/formatting for clearer failure output. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
f077420fbe |
fix(http-cache,vpa): fix haproxy 3.3 backend name conflict and vpa-for-vpa OOM (#2654)
## What this PR does Fixes two independent issues found during cluster operations: **http-cache (HAProxy 3.3 breaking change)** HAProxy 3.3 no longer allows a frontend and backend to share the same name. The existing config had both `frontend http` and `backend http`, which causes haproxy to refuse to start on upgrade. Rename `backend http` → `backend cache` and update the `default_backend` directive accordingly. **vpa-for-vpa (OOMKilled recommender)** The VPA recommender deployed by `vpa-for-vpa` was hitting its 512Mi memory limit and being OOMKilled. Increase the limit to 1Gi to keep the recommender running stably. ### Release note ```release-note fix(http-cache): rename haproxy backend to avoid HAProxy 3.3 name conflict between frontend and backend fix(vpa): increase vpa-for-vpa recommender memory limit from 512Mi to 1Gi to prevent OOMKilled restarts ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Corrected HAProxy routing so HTTP traffic is directed to the cache backend. * Increased the VPA recommender’s memory limit to improve reliability under higher workloads. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
c503ae29d9 |
fix(backups): request S3 checksum only when required for barman-cloud (non-AWS S3 / Ceph RGW) (#3417)
## What this PR does Fixes CNPG **barman-cloud plugin** backups to non-AWS, S3-compatible object stores (Ceph RADOS Gateway, and some MinIO / Cloudflare R2 builds). The plugin's sidecar uploads via **boto3**. Since botocore ~1.36 (early 2025) the default `RequestChecksumCalculation` is `when_supported`, so a flexible checksum (CRC32) plus the corresponding `x-amz-content-sha256` handling is attached to every `PutObject`. AWS S3 accepts it, but several S3-compatible backends reject the request: ``` InvalidArgument: x-amz-content-sha256 must be UNSIGNED-PAYLOAD, STREAMING-AWS4-HMAC-SHA256-PAYLOAD or a valid sha256 value. ``` so every backup / WAL-archive upload fails against them and the `ScheduledBackup` never stores anything. This sets `spec.instanceSidecarConfiguration.env` on the rendered `ObjectStore`s to carry `AWS_REQUEST_CHECKSUM_CALCULATION=when_required` (botocore reads this env var from the sidecar's environment). `when_required` computes a checksum only for operations that mandate one; a plain `PutObject` then carries none, which both AWS S3 and the affected backends accept — a safe default everywhere. No API/CRD change, no new value, no new dependency. Applied to every barman-cloud `ObjectStore` Cozystack creates: - **chart-rendered** — `packages/system/keycloak/templates/db.yaml` (Keycloak system DB) and `packages/apps/postgres/templates/db.yaml` (postgres app — backup **and** bootstrap-recovery `ObjectStore`s), via a shared `cozy-lib` helper (`cozy-lib.barman.checksumSidecarConfiguration`); - **Go-constructed** — the platform-managed `useSystemBucket=true` `ObjectStore` SSA-applied by `internal/backupcontroller/cnpgstrategy_controller.go` (`barmanSidecarConfiguration`), whose default system bucket is SeaweedFS. Regression coverage: a Go unit test on the SSA-applied `ObjectStore`, plus helm-unittest assertions on `spec.instanceSidecarConfiguration.env` for all three rendered `ObjectStore`s (mutation-checked — reverting the change fails them). Mirrors the same fix already applied to the etcd path in `cozystack/etcd-operator#342` (released in `v0.5.3`) for the same Ceph RGW backend. **Verification:** reproduced end-to-end against a real Ceph RGW endpoint. With default botocore, `PutObject` fails with the `InvalidArgument` above; with `AWS_REQUEST_CHECKSUM_CALCULATION=when_required` in the environment it succeeds. Deployed the plugin + a throwaway CNPG cluster whose `ObjectStore` carried this env: continuous WAL archiving reported `ContinuousArchiving=True` and an on-demand `Backup` completed, with the base backup and WAL segments present in the bucket. `helm template` renders the env on all three `ObjectStore`s; the `packages/apps/postgres` unit-test suite still passes. ### Screenshots N/A — no UI changes. ### Downstream repositories <!-- Walked the trigger map in docs/agents/contributing.md against the diff, file by file. The diff touches only two Helm templates (ObjectStore renders); it adds no package, no values.yaml / values.schema.json field, no API/CRD change, no variant/component/asset change, no ApplicationDefinition change, and no developer-tooling change. None of the trigger-map entries match. --> - [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: ### Release note ```release-note fix(backups): CNPG barman-cloud backups now work against non-AWS S3-compatible object stores (Ceph RADOS Gateway, some MinIO/Cloudflare R2). The backup sidecar requests an S3 request checksum only when required, avoiding the "x-amz-content-sha256 must be UNSIGNED-PAYLOAD, ..." error that previously broke every backup and WAL-archive upload to those backends. No change for AWS S3. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved compatibility with S3-compatible, non-AWS storage backends for PostgreSQL and Keycloak backups and recovery. * Pin the barman-cloud sidecar to compute AWS request checksums only when required (`AWS_REQUEST_CHECKSUM_CALCULATION=when_required`) to prevent failures with strict S3 gateways. * **Tests** * Updated/added assertions in controller and Helm/YAML suites to verify the checksum pin is applied to generated and recovery ObjectStores (Postgres and Keycloak). <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
a2acfcde1a |
docs(release): add changelog for v1.6.0 (#3429)
## What this PR does Adds `docs/changelogs/v1.6.0.md` (667 lines), generated locally with the process in `docs/agents/changelog.md`. Normally `tags.yaml`'s `generate-changelog` job produces this on the stable tag push, but that job never ran for `v1.6.0` — see the note below. **Coverage boundary.** The changelog runs through #3386, i.e. the release period ending at the `v1.6.0-rc.4` cut. Roughly eight PRs merged to `main` after that cut are deliberately absent, because `v1.6.0` is a promotion rather than a rebuild: every shipped artifact is pinned to an rc.4 digest (`cozystack-packages@sha256:bf682087…`, `cozystack-operator@sha256:93b305f7…`), so that later code is in the tagged tree but not in anything users install. The changelog describes what shipped. **Why this is a manual PR.** `generate-changelog` and `update-website-docs` are jobs in `tags.yaml`, triggered by `push: tags`. The `v1.6.0` tag was pushed by `pull-requests-release.yaml`'s finalize job using the `cozystack-ci` app token, and no workflow run was created for that ref — every rc tag, pushed by `cut-prerelease.yaml` with the same app, did trigger it. `v1.6.0` is the first stable tag created through the promote flow, so this path had never run before. The cause is not yet established and needs a repro rather than a guess; the website docs versioning is still outstanding for the same reason. ### Screenshots N/A — documentation only. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, which is a single new file under `docs/changelogs/`. No trigger matches: the map covers package layout, release assets, CRDs, workflow behaviour and naming contracts, none of which a changelog file touches. The separate repository list in `docs/agents/changelog.md` was already consumed while generating this file — it feeds the notes, it is not a code coupling. - [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: ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added comprehensive release notes for version 1.6.0. * Documented breaking changes, upgrade prerequisites, manual upgrade steps, and post-upgrade expectations. * Highlighted major features, deprecations, security updates, bug fixes, operational notes, and component version updates. * Included configuration, CI, development, and related-project changelog updates. * Added contributor acknowledgements and a link to the full changelog. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
206957610e |
test(platform): cover implicit-md0 migration path and sync jq comment
Both existing adopt-values cases feed an explicit md0, leaving the DEFAULT_MD0
materialisation (empty nodeGroups) uncovered. Add a case asserting a parent HR
with no nodeGroups yields the child HR kubernetes-nodes-<cluster>-md0 with
minReplicas 0 and the ingress-nginx role. Also correct the header comment: the
jq picks ["version","talos"] and narrows images to {kubectl} separately, not
["version","talos","images"].
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
d90cefbf8d |
chore(kubernetes): drop orphaned cpuToMillicores doc comment
The PR removed the kubernetes.cpuToMillicores helper but left its documentation comment dangling at the end of _resources.tpl. Delete it. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
ccd93eacff |
docs(storage-immutability): record kubernetes-nodes storageClass/cluster as immutable
The doc still claimed the kubernetes-nodes per-pool storageClass is intentionally not immutable and undefaulted. This branch annotated it immutable (self == oldSelf XValidation, @immutable in values.yaml) and it defaults to replicated, and the cluster field is immutable too. Rewrite the paragraph to describe both fields, add kubernetes-nodes to the covered-apps list, and drop the now-stale top-level-only caveat on the kubernetes entry. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
c8ba404646 |
fix(platform): guard migration 54 against RFC-1123-invalid pool names
The 53-char overflow branch exists because exiting the migration deadlocks the platform pre-upgrade hook for every tenant. The equivalent deadlock via name validity was unguarded: the old nodeGroups map key was never schema-constrained (map[string]NodeGroup, no propertyNames pattern), so a stored parent HR can carry a key like 'My_Pool' whose own render already failed at apply time. The migration then built child_name=kubernetes-nodes-<cluster>-My_Pool, kubectl apply rejected the RFC-1123-invalid HelmRelease name, the script exited 1 under set -e, and every subsequent platform upgrade re-ran and re-failed. Validate child_name against the RFC-1123 label pattern alongside the length check and route failures through the same warn + pin + skip path. Factor the pin-and-skip logic shared by both guards into pin_pool_objects. Cover the new branch in migration-54-adopt-path.bats (the fake kubectl apply now rejects an invalid metadata.name, so the case fails without the guard). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
a1ee339b5a |
fix(kubernetes-nodes): reap the pre-delete unpin hook Job after it finishes
The unpin Job carried only helm.sh/hook-delete-policy: before-hook-creation, and its comment claimed a successful run is torn down with the release. Helm does not track hook resources as release members, so after helm uninstall the succeeded Job and its pod remained in the tenant namespace indefinitely, one per deleted pool name per namespace — before-hook-creation only fires on a next pre-delete of a same-named release, which never comes for a deleted pool. Add spec.ttlSecondsAfterFinished: 86400 so a finished Job is garbage-collected (a failed run still keeps its logs for a day), and correct the comment. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
71a8747d02 |
fix(kubernetes-nodes): make parent-version skew guard directional
assertParentVersion failed the pool render whenever the pool minor differed from the parent KamajiControlPlane minor in either direction. A worker minor lagging the control plane is supported (n-3 upstream) and is the normal state during a rolling upgrade, where the parent Kubernetes CR is bumped first and each pool follows. The symmetric check flipped every pool HelmRelease of a cluster to render-failure the moment the parent was bumped, blocking all pool operations until each pool version was hand-edited. Fail only when the pool minor is ahead of the parent minor (semverCompare on normalized major.minor.0), matching the stated rationale, and cover all four directions (ahead / lag / equal / no parent) with a helm-unittest suite driven by a mocked KamajiControlPlane. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
myasnikovdaniil
|
764e30418d |
chore(release): promote v1.6.0-rc.4 -> v1.6.0 (#3425)
Promotes `v1.6.0-rc.4` to stable `v1.6.0` — no rebuild. The tree pins the rc's images by digest. On merge, pull-requests-release.yaml creates the write-once `v1.6.0` tag at the merge commit, retags those digests to `v1.6.0` (+`:latest` when this is the newest stable), publishes the stable cozy-installer chart, and publishes the release — so the stable artifacts are bit-for-bit the rc that passed e2e. The `full-e2e` label forces the full suite on this PR. Do NOT squash-merge (decision B): the stable tag must attach to a real merge commit.v1.6.0 api/apps/v1alpha1/v1.6.0 |
||
|
myasnikovdaniil
|
d3815a4ce6 |
Merge branch 'main' into release-1.6.0
Signed-off-by: myasnikovdaniil <60174387+myasnikovdaniil@users.noreply.github.com> |
||
|
cozystack-ci[bot]
|
235d38dae1 |
Prepare release v1.6.0 (promoted from v1.6.0-rc.4)
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
myasnikovdaniil
|
706c3f06fc |
fix(release): reuse only an OPEN promote PR, not one in any state (#3424)
## What this PR does The promote step guarded PR creation with `gh pr view <branch>`, which resolves a pull request by head branch **in any state**. A closed one satisfied the guard, so creation was skipped while the step still exited 0 — a promotion that reports success and leaves nothing to merge. Same silent-skip shape as the enumeration bug fixed in #3404: the run is green, the artifact is missing, and nothing says so. `STABLE_BRANCH` is derived from the stable version (`release-${version}`), not from the rc number, so every rc promoted to that version shares it. Once any promote PR for a version has been closed, both re-dispatch paths this workflow documents as supported are wedged: re-dispatching the same rc, and promoting a newer rc to the same version. That second one is named in the workflow's own header as the reason a leftover draft is tolerated — the tolerance was implemented for the release draft and missed for the PR. Neither path recovers by reopening the old PR, because the step's own `git checkout -B` plus force-push makes that PR's head unreachable, and GitHub then refuses: ``` 422 Validation Failed state cannot be changed. The release-1.6.0 branch was force-pushed or recreated. ``` The fix asks the question the guard meant to ask — is there an **open** PR for this head — and creates one otherwise. **Hit live.** Promoting `v1.6.0-rc.4` after #3397 had been closed: [run 29917852639](https://github.com/cozystack/cozystack/actions/runs/29917852639) went green, logged `PR already open for release-1.6.0`, and opened no PR. This currently blocks the v1.6.0 release. **Verified in both directions** against live data, so the change is not vacuous: | head branch | PR state | old guard | new guard | | --- | --- | --- | --- | | `release-1.6.0` | #3397 closed | `TRUE` → skip (bug) | `FALSE` → create ✅ | | `chore/gitignore` | #3412 open | `TRUE` → skip | `TRUE` → skip ✅ | `actionlint` exits 0 and `zizmor` reports no findings. The guard reads `${STABLE_BRANCH}` as a shell variable from the step's `env:` block rather than as a `${{ }}` expansion inside `run:`, so no expression-injection surface is added. ### Screenshots N/A — no UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, which is one file: `.github/workflows/promote-rc.yaml`. The only workflow-related trigger in the map is cozystack/ccp on "change release-prep behaviour in `.github/workflows/tags.yaml`", which this does not touch. This change also restores the documented behaviour of `promote-rc.yaml` rather than altering its contract, so nothing downstream sees a result different from what the docs already promise. - [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: ### Release note ```release-note fix(release): promoting a release candidate now opens the promotion pull request even when an earlier promotion attempt for the same version was abandoned. Previously the workflow mistook a closed pull request for an open one, skipped creating a new one, and reported success with nothing left to merge. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved release PR creation checks so closed or previously merged pull requests no longer prevent new release PRs from being opened. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
6a41d65f16 |
test(e2e): avoid ripgrep dependency in diagnostics tests
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
ef2fa628f5 |
fix(ci): make pre-commit locking work in worktrees
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
4e0cfd994d |
test(e2e): render COSI catch output as YAML
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
ad1adbf785 |
test(e2e): fix diagnostic collector blind spots
Assisted-By: GPT-5 <noreply@openai.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
55195ded0c |
test(e2e): capture previous container logs and stop killing the snapshot (#3346)
## What this PR does Root-causing a recent E2E failure was nearly blocked because the diagnostics captured on failure had collected almost nothing. Two independent mechanisms were responsible, and both had been silently in place for a long time. **The failing container's previous logs were never captured.** The pod had crash-looped, so the decisive evidence lived in the first container instance, reachable only through `kubectl logs --previous`. Chainsaw's `podLogs` action has no `previous` field, so this could not be expressed per-suite; the conclusion had to be reconstructed from indirect evidence instead. A new capture runs from the global catch and from the BATS failure hook, selecting containers at runtime by `restartCount > 0` across both init and regular containers, so a healthy run stays quiet and future suites are covered without per-suite wiring. **The catch was structurally guaranteed to be killed.** A Chainsaw `script` op is bounded by `timeouts.exec`, which is 5m, while the catch's own inner budgets summed to roughly 960s. It reported `SCRIPT | ERROR | signal: killed` and produced nothing — and its second leg, the data-plane capture, had therefore never run at all. The kill signature was itself misleading: `timeout` exits 124 and cannot produce that message, which comes from Go's `exec.Cmd` killing the process group. Each catch op now carries an explicit timeout above the budgets it contains, so the inner one fires and leaves a partial, usable artifact instead of losing everything. **The snapshot was truncated at 60 seconds.** `crust-gather --duration` defaults to 60s and was never passed, so every snapshot ever taken was cut short, with nothing in the artifact saying so. The API discovery it performs first is not covered by that budget at all, which is where the outer wall-clock was going on a cluster with a degraded aggregated APIService — precisely the state a failed run is in. Both budgets are now set explicitly, and the collector's output goes to a log inside the snapshot with a completeness verdict, so "is this snapshot complete?" is answerable from the artifact. Failure paths are reported rather than swallowed: an unreachable API says so instead of implying nothing had restarted, a truncated capture is kept and marked, and every dropped item is logged, since silent truncation reads as full coverage. ### What is not verified here The mechanism claims are read from the pinned versions of Chainsaw and crust-gather. Everything cluster-facing is not verified by this PR: the go-template's output shape against a live API server, `--previous` returning the first instance for a real crash-looper, and whether the new collection budgets suffice on a degraded cluster. The bats suite covers the selection helpers only, and those cases are mutation-tested. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Diagnostics** * Improved end-to-end failure evidence by capturing “previous instance” logs for crash-looping pods (including restarted init containers) and prioritizing relevant namespaces. * Added time-bounded diagnostics collection with explicit outer timeouts; collector output is saved to per-snapshot log files. * Classifies collector outcomes as complete, timeout-truncated, or failed, while preserving the original test failure signal. * **Documentation** * Expanded E2E failure-diagnostics guidance with new timeout and evidence-capture conventions. * **Tests** * Added unit-style tests for previous-log selection, prioritization, row limiting, and deterministic log artifact naming. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov Daniil
|
baf0158d81 |
fix(release): reuse only an OPEN promote PR, not one in any state
The promote step guarded PR creation with `gh pr view <branch>`, which resolves a pull request by head branch in any state. A closed one satisfied the guard, so creation was skipped while the step still exited 0 — a promotion that reports success and leaves nothing to merge, the same silent-skip shape as the enumeration bug fixed in #3404. STABLE_BRANCH is derived from the stable version, so every rc promoted to that version shares it. Both re-dispatch paths this workflow documents as supported are therefore wedged once any promote PR for that version has been closed: re-dispatching the same rc, and promoting a newer rc to the same version. Neither recovers by reopening the old PR, because the step's own force-push makes its head unreachable and GitHub refuses with "state cannot be changed. The <branch> branch was force-pushed or recreated." Hit live while promoting v1.6.0-rc.4 after #3397 had been closed: the run went green and opened no PR. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Timofei Larkin
|
d5806eb1a0 |
feat(kubernetes-nodes): split worker node pool into its own app (Phase 2, additive) (#3314)
## What this PR does Phase 2 of the Kubernetes-app split ([cozystack/community#8](https://github.com/cozystack/community/pull/8)) — the **additive** half. Introduces `kubernetes-nodes`: a per-pool application that renders the worker node-pool objects (`KubevirtMachineTemplate`, `MachineDeployment`, `MachineHealthCheck`, worker `WorkloadMonitor`, and the `talos-reconcile` Job + RBAC) extracted from the monolithic `kubernetes` chart. A `KubernetesNodes` CR named `<cluster>-<pool>` attaches to a parent `Kubernetes` cluster in the same namespace by name convention (`kubernetes-<cluster>`), mirroring the `vm-instance`/`vm-disk` precedent. This PR is additive: the parent `kubernetes` chart gets exactly one scoping fix (its old-KMT preservation block is limited to KMTs owned by its own Helm release) so that attaching a `kubernetes-nodes` pool to a live cluster cannot break the parent's next upgrade. Existing clusters keep provisioning workers via `nodeGroups`. Removing `nodeGroups` from the parent and adopting existing pools into `kubernetes-nodes` releases is a follow-up **breaking** PR (Phase 2b). Key properties: - Pool objects render **byte-identical** to the parent for the same group (golden-parity check in `tests/render-parity.sh`), so the follow-up adoption never churns live worker VMs — the content-hashed `KubevirtMachineTemplate` name is preserved. - cluster-autoscaler (kept in the parent chart) discovers the pool's `MachineDeployment`s cross-release via the unchanged `cluster.x-k8s.io/cluster-name` label, namespace, and min/max annotations. - Registered as a dedicated `cozystack.kubernetes-nodes-application` PackageSource + `kubernetes-nodes-rd` ApplicationDefinition, wired into the iaas bundle (mirrors `vm-instance`/`vm-disk`). ### Downstream repositories Left unticked deliberately — this is a draft. A new user-facing `KubernetesNodes` API is added, so cozystack/website (docs) and cozystack/terraform-provider-cozystack likely need follow-ups; the trigger-map walk will be completed before this leaves draft. - [ ] 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: ```release-note Add the `kubernetes-nodes` application: manage the worker node pools of a managed Kubernetes cluster as independent resources, one HelmRelease per pool. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a new Kubernetes worker node pool offering with configurable sizing (CPU/memory/instance, disk), roles, storage class, GPUs, and Talos worker image settings. * Introduced a dedicated Helm package, dashboard/resource-map access, and Flux application wiring for installing/managing pools. * **Bug Fixes** * Improved safeguards against pool naming/ownership conflicts and tightened upgrade behavior for reusing previously created templates. * **Documentation** * Added full node pool configuration docs and operational notes (including supported Kubernetes versions and reconciliation expectations). * **Tests** * Added node pool rendering/validation tests plus golden parity checks between chart layouts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey KolkovandClaude Opus 4.8
|
80a03b1e09 |
test(backups): assert the barman-cloud sidecar checksum env on rendered ObjectStores
helm-unittest coverage for the S3 request-checksum fix (was mutation-negative before): assert spec.instanceSidecarConfiguration.env pins AWS_REQUEST_CHECKSUM_CALCULATION=when_required on all rendered barman-cloud ObjectStores — postgres backup + bootstrap-recovery (packages/apps/postgres/tests/backup_storage_test.yaml) and the Keycloak DB (new packages/system/keycloak/tests/db_backup_test.yaml). Verified by mutation: reverting the template change now fails these assertions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> |
||
|
Myasnikov Daniil
|
284a17ba1f |
fix(release): unblock the v1.6.0 promote on the rc.4 staging tree
Promoting this branch fails in two independent ways, both landing after finalize has already created the write-once stable tag, published the GitHub release and moved :latest. chBackupClientImage had no producer and froze at v1.4.0-rc.2 while .migrations.image advanced to v1.6.0-rc.4, leaving platform-migrations pinned at two digests. promote-retag.sh copies every collected <repo>@<digest> to <repo>:<stable>, so both compete for one destination tag: the lower-sorting v1.4.0-rc.2 digest wins it and the second copy trips the write-once guard, aborting the retag with five repositories still untagged and the installer chart unpublished. Align it to the digest this branch already carries for the migrations image. Both come from the same tree, so the runner keeps the curl, jq and ca-certificates the Altinity strategy Pod needs, and that Pod overrides the ENTRYPOINT with its own command. promote-rc.yaml on main now calls hack/promote-rewrite-tags.sh, which does not exist here, so the dispatch would fail outright. Vendor it and hack/lib/image-refs.sh verbatim from main (identical blobs). That also fixes the rewrite's coverage: the old inline glob enumerated 155 files and missed 10 rc-bearing ones, which is why the earlier promote left 11 references reading v1.6.0-rc.4. The shared enumeration covers all 168 and fails the promotion if any rc reference survives. Both fixes are on main via #3405 and #3404; this carries them onto the rc.4 staging tree so v1.6.0 ships the bytes rc.4 tested. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
d0c0f399f8 |
fix(ci): cover every image-ref storage shape in promote, retag and mirror (#3404)
## The bug Promotion rewrites the rc version substring to the stable version across the vendored image references. That rewrite lived inline in `promote-rc.yaml` and globbed the depth-2 package `values.yaml` plus `packages/apps/kubernetes/images/*.tag` **only**, on the premise that the kubernetes app was the only one whose `.tag` files carry the cozystack version. Nine other `.tag` files do, and `system/multus` seds its reference straight into a vendored upstream daemonset manifest. #3397 (promote `v1.6.0-rc.4` → `v1.6.0`) is therefore staged with **33 of 55 first-party references still reading `v1.6.0-rc.4`**. `hack/promote-retag.sh` and `hack/nightly-mirror.sh` had the same blind spot from the other direction — both scanned the depth-2 `values.yaml` alone, so those images were never retagged to the stable version (**30 references selected before, 42 after**) and never mirrored to the public registry for a nightly. A retag miss leaves an image short of a tag, since the digest still resolves inside one registry. A mirror miss is worse: the host rewrite walks the same file list, so a reference that is neither mirrored nor rewritten leaves the published tree pointing at the private build registry. `v1.6.0` is the first release to go through promotion — every earlier release was a full rebuild from the tag, which stamped every file uniformly. The glob has always been wrong; the old path just made it unobservable. ## Why it could not be caught before The rewrite was workflow-inline, so there was nothing to unit test. The only way to observe the miss was to cut a release. It is a script now — `hack/promote-rewrite-tags.sh` — and `hack/promote-rewrite-tags_test.bats` round-trips it against the real tree under `make unit-tests`, with no cluster, registry or release involved. ## The fix The three call sites each carried their own idea of where a reference can live, which is how they drifted apart. That knowledge moves into `hack/lib/image-refs.sh` as the single enumeration — `image_ref_files()` for the files, `collect_image_refs()` for the references inside them — and all three source it, so a new storage location is declared once and reaches every consumer. The rewrite also asserts its own postcondition: it scans **wider** than it rewrites and fails the promotion when a reference survives in a location the enumeration does not know about, rather than shipping it quietly. The wide scan is shape-filtered to image-reference position, because `X.Y.Z-rc.N` is an ordinary version string that other things legitimately contain — `kubevirt-csi-driver/go.sum` pins `github.com/golang/protobuf v1.4.0-rc.2`, and `v1.4.0-rc.2` is a cozystack rc that was actually cut. Narrowing it to `${RC}@sha256:` instead would have been wrong: `system/cilium/values.yaml` keeps `tag:` and `digest:` under separate keys. `docs/agents/image-refs.md` records the contract — three tag classes, three storage shapes, the invariants, and which consumer does what. ## Not a bug: kamaji `cluster-api-control-plane-provider-kamaji` stays at `v0.19.0-cozystack.0` across the promotion. It is a first-party rebuild of an upstream component, versioned by that component rather than by the cozystack version line, so no rewrite is owed. A test pins this so a future fix here cannot over-reach. ## Documented known gaps, deliberately not fixed here Both are pre-existing and in the **host** dimension; the version dimension this PR owns is complete, because the version always lives in a `tag` key regardless of where the host sits. - **The nightly host rewrite cannot reach a split host.** It is a literal `<src-registry>/` substring replace, so `keycloak-operator` (host in a sibling `registry:` key) and `kubeovn` (`global.registry.address`, no trailing slash) are mirrored but keep pointing at the build registry in the published tree. Fixing it needs structure-aware rewriting. - **A reference inside a gzip is invisible.** `capi-providers-cpprovider` ships `files/components.gz`, and the promote postcondition cannot catch this class at all since `grep -rIl` skips binaries. Nothing is owed today — kamaji is component-versioned and the image is still mirrored and retagged via its `.tag` — so the residual is that a nightly's kamaji ConfigMap keeps the build-registry host. ## Testing - 17 tests in the new suite; `make bats-unit-tests` green (312 tests, exit 0); `shellcheck -x` clean; `zizmor` clean on the workflow. - Verified against four real rc versions (`1.4.0-rc.2`, `1.4.0-rc.4`, `1.5.0-rc.2`, `1.6.0-rc.4`) in both directions — no false positives, no false negatives. - Every test is mutation-proven. Reverting the enumeration to the old glob, deleting any of the five YAML shapes, corrupting emitted digests, reverting extras to parse-only, dropping the unreadable-file guard, broadening shape 4 beyond `global.images`, or planting an unenumerated first-party reference in any storage shape each fail a named test. ## Reviewer note, out of scope `platform-migrations` is pinned at two different digests — `core/platform/values.yaml` at `v1.5.0` and `backupstrategy-controller/values.yaml` at `v1.4.0-rc.2` (an unowned pin; no Makefile writes that key, tracked in #3143). A `promote-retag.sh` dry-run emits **two copies to the same destination tag**, so the write-once guard will fail the promotion. This is independent of this PR and needs a decision about the backup-client image version, so it is left alone here — but it blocks the 1.6.0 promote and wants fixing before the re-cut. ## Release note ```release-note Promotion, retagging and nightly mirroring now cover every image reference the build stamps, including those stored in `images/*.tag` files and in vendored manifests. Previously a promoted release could ship images still tagged with the release-candidate version, and those images received no stable tag in the registry. ``` Fixes the promote half of #3143. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Image promotion and nightly mirroring now detect digest-pinned image references across values files, standalone tag files, and supported templates. - Stable release preparation supports RC→stable tag rewriting via a dedicated promotion script, with broader verification to ensure no RC references remain. - **Bug Fixes** - Improved completeness of ref discovery and scanning, including previously missed template/tag locations. - **Documentation** - Added and linked guidance documenting image-reference invariants and how tooling enumerates supported storage shapes. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |