mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-07 20:43:54 +09:00
b2d00af49517e05348cc1681021ebd2ca46800a3
4576
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Aleksei Sviridkin
|
b2d00af495 |
[kubernetes] Add CPU/memory limits to csi-resizer sidecar (#2614)
## Summary - The `csi-resizer` container in `packages/apps/kubernetes/templates/csi/deploy.yaml` was the only sidecar in the `kcsi-controller` Deployment without `resources.limits`, which triggers a "no CPU limit" conformance warning. - Aligned it with the other six CSI sidecars in the same Deployment: `limits` `cpu: 512m` / `memory: 512Mi` and `requests` `cpu: 125m` / `memory: 128Mi`. - No other files touched; vendored charts and other CSI drivers (`nfs-driver`, `piraeus-operator`) are out of scope for this PR. ## Test plan - [ ] `make -C packages/apps/kubernetes show NAME=test NAMESPACE=test` renders the Deployment with the new `limits` block - [ ] Re-run the conformance/lint tool that originally flagged the warning — `csi-resizer has no CPU limit` should be gone - [ ] CI passes ### Release note \`\`\`release-note Set CPU and memory limits on the csi-resizer sidecar of the kubevirt-csi controller to align with the other CSI sidecars and clear the "no CPU limit" conformance warning. \`\`\` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated resource allocation for the CSI resizer component, increasing CPU and memory requests to improve performance and reliability. [](https://app.coderabbit.ai/change-stack/cozystack/cozystack/pull/2614) <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Matthieu
|
50c4080ba3 |
[kubernetes] Add CPU/memory limits to csi-resizer sidecar
The csi-resizer container was the only sidecar in the kcsi-controller Deployment without resources.limits, triggering a 'no CPU limit' conformance warning. Align it with the other six CSI sidecars in the same Deployment (limits 512m/512Mi, requests 125m/128Mi) so capacity planning and audits stay uniform. Signed-off-by: Matthieu <matthieu@hidora.com> |
||
|
Aleksei Sviridkin
|
58d61d5652 |
fix(ci): build api-gate detector from base tip when merge base predates it (#3193)
## What this PR does The `API Review Gate` required check builds the `cmd/api-gate` detector from the PR's merge-base checkout so a PR cannot neuter the detector by editing its own code. When a PR's merge base predates the commit that introduced the gate (`fa871249c`), `cmd/api-gate` is absent from both the merge base and the equally old head, so the head fallback also fails to build and the check errors out before evaluating any API content. This red-blocks every open PR whose merge base is older than the gate, and neither a re-run nor an approving review clears it. This adds the base branch tip (`origin/<base>`) as an intermediate build source: prefer the merge-base checkout, fall back to the base tip when the merge base lacks the tool, and bootstrap from the head only while the tool exists nowhere on the base branch yet. The base tip is a protected, non-PR-authored branch, so it preserves the "analyze the head as data, not code" property. The API surface is still diffed against the merge base; only where the detector binary is compiled changes. This unblocks the pre-gate backlog without forcing every affected PR to rebase. The selection is kept inline in the workflow rather than in a helper script: a helper would be read from the untrusted PR head, which both reintroduces the failure for heads that predate the helper and lets a PR steer the selection away from the trusted build. Closes #3192 ### Screenshots N/A — CI-only change, no user-facing UI. ### Release note ```release-note fix(ci): API Review Gate now builds its detector from the base branch tip when the PR merge base predates the gate, so pre-gate PRs stop failing the check without needing a rebase. ``` |
||
|
Aleksei Sviridkin
|
361748ea6a |
fix(ci): build api-gate detector from base tip when merge base predates it
The API Review Gate builds cmd/api-gate from the PR's merge-base checkout so a PR cannot neuter the detector by editing its own code. When the merge base predates the commit that introduced the gate, cmd/api-gate is absent from both the merge base and the equally old head, so the head fallback also fails to build and the required check errors out before evaluating any API content. This blocks every open PR whose merge base is older than the gate, and neither a re-run nor an approving review clears it. Add the base branch tip as an intermediate build source: prefer the merge-base checkout, fall back to the base tip when the merge base lacks the tool, and bootstrap from head only while the tool exists nowhere on the base branch yet. The base tip is a protected branch, not PR-authored, so it preserves the "analyze head as data, not code" property. The API surface is still diffed against the merge base; only where the detector binary is compiled changes. This unblocks the backlog without forcing every affected PR to rebase. The selection stays inline in the workflow rather than in a helper script: a helper would be read from the untrusted PR head, which both reintroduces the failure for heads that predate the helper and lets a PR steer the selection away from the trusted build. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
IvanHunters
|
1baf126b1a |
fix(tenant): inherit full ancestor label chain from parent namespace (#2912)
## Summary
`packages/apps/tenant/templates/namespace.yaml` labels every tenant
namespace
with `tenant.cozystack.io/<ancestor>` for each of its ancestors. These
labels
drive the `<ancestor>-egress` `CiliumClusterwideNetworkPolicy`
(`networkpolicy.yaml`): an ancestor tenant's pods may reach a namespace
only if
that namespace carries `tenant.cozystack.io/<ancestor>`.
The chain was derived by splitting `.Release.Namespace`, which only ever
reached
one level up. A direct child of `tenant-root` got `tenant-root` (its
parent
namespace *is* `tenant-root`), but a grandchild — `tenant-ktj-htdev`
created
under `tenant-ktj` — got only `tenant-ktj` and dropped `tenant-root`
entirely.
Without `tenant.cozystack.io/tenant-root` on a nested namespace, the
root
ingress controller cannot reach the proxy service in a nested
`kubernetes`
tenant, surfacing as `upstream timed out (110) while connecting to
upstream` in
the nginx logs.
## Fix
Derive the ancestor chain deterministically from the namespace name
instead of
from any cluster lookup. Every tenant descends from `tenant-root`, so
that label
is emitted unconditionally; the remaining ancestors are encoded in the
namespace
name itself, because `tenant.name` builds each child as
`<parent-namespace>-<word>` — so every progressive dash-prefix of the
name is a
real ancestor namespace (`tenant-ktj-htdev` yields `tenant-ktj` and
`tenant-ktj-htdev`). The logic lives in a new
`tenant.ancestorTenantLabels`
helper:
```yaml
labels:
{{- include "tenant.ancestorTenantLabels" $tenantName | nindent 4 }}
```
This needs no cluster state, renders identically offline (so it is
unit-testable), and converges on every reconcile regardless of the order
in
which parent and child `HelmRelease`s reconcile — unlike a
parent-namespace
`lookup`, which helm-controller does not re-render on and which is
ordering-dependent. The `ownerReference` `lookup` is kept but made
non-fatal:
during a real reconcile the parent namespace always exists (the tenant
`HelmRelease` and its `cozystack-values` Secret live in it), so the
`ownerReference` is always emitted; when the lookup is empty (offline /
unit
test) it is simply omitted so the labels still render.
Migration `49` backfills the labels on existing clusters so they heal on
upgrade
regardless of per-tenant `HelmRelease` state (e.g. suspended or failed
releases), and `migrations.targetVersion` is bumped to `50` so the
runner
iterates it. The migration derives the same chain from each namespace
name,
is idempotent (`kubectl label --overwrite`) and best-effort
(per-namespace
failures are tolerated so a transient apiserver error cannot abort the
upgrade).
## Test plan
- [x] `helm template packages/apps/tenant` at depth 1/2/3 — emits
exactly the
expected ancestor set at each depth: depth-1 `{tenant-root,
tenant-ktj}`,
depth-2 adds `tenant-ktj-htdev`, depth-3 adds `tenant-ktj-htdev-foo`. No
missing, no stray labels.
- [x] `helm unittest packages/apps/tenant` — new
`tests/namespace_ancestor_labels_test.yaml` pins the chain at depths
1/2/3
plus a negative assertion; whole suite green.
- [x] `bats hack/cozystack-version-stamp.bats` — migration `49` stamps
the
version through the shared `stamp_cozystack_version` helper (all 10
tests
pass, including the guard that migrations >= 42 must use the helper).
- [x] Migration `49` ancestor derivation verified under `/bin/sh` to
match the
helm output byte-for-byte, and dry-runs cleanly under `set -euo
pipefail`.
- [ ] Live cluster: create a 3-level tenant (`root -> ktj -> htdev`),
verify
`tenant-ktj-htdev` has all three `tenant.cozystack.io/*` labels.
- [ ] Live cluster: create a `kubernetes` tenant under `htdev` with
`addons.ingressNginx.hosts`, verify an external HTTP request reaches the
tenant ingress controller via the root-ingress proxy (no `upstream timed
out`).
## Closes
- Closes #2810
- Closes #2171
```release-note
fix(tenant): tenant namespaces now carry the full `tenant.cozystack.io/<ancestor>` label chain (including `tenant-root`) at any nesting depth, restoring root-ingress reachability to nested tenants; a migration backfills the labels on existing clusters
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved tenant namespace label inheritance across multi-level tenant
hierarchies, ensuring all ancestor tenant labels are consistently
applied.
* **New Features**
* Standardized ancestor label generation to reliably include the full
tenant chain for derived tenant namespaces.
* **Tests**
* Added regression coverage for ancestor label presence and absence at
multiple hierarchy depths.
* **Chores**
* Added a migration to backfill missing ancestor labels for existing
tenant namespaces during platform upgrades.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
myasnikovdaniil
|
c5ff2178e7 |
docs(release): add changelog for v1.4.5 (#3083)
This PR adds the changelog for release `v1.4.5`.
✅ Changelog has been automatically generated in
`docs/changelogs/v1.4.5.md`.
|
||
|
myasnikovdaniil
|
6f39beffb6 |
docs(release): add changelog for v1.5.2 (#3162)
This PR adds the changelog for release `v1.5.2`.
✅ Changelog has been automatically generated in
`docs/changelogs/v1.5.2.md`.
|
||
|
cozystack-ci[bot]
|
ee29392584 |
docs: add changelog for v1.5.2
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
cozystack-ci[bot]
|
a8220fa21f |
docs: add changelog for v1.4.5
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |
||
|
myasnikovdaniil
|
1e5f53881f |
fix(kamaji): backport datastore unused-deletion deadlock fix (clastix/kamaji#1122) (#3146)
## What Backports the upstream Kamaji fix [clastix/kamaji#1122](https://github.com/clastix/kamaji/pull/1122) (which fixes clastix/kamaji#1115) as a source patch on top of the pinned `26.3.6-edge` build. No version bump, no CRD/RBAC changes. This lands the fix on `main`; the `backport` / `backport-previous` labels fan it out to `release-1.5` and `release-1.4` via the backport automation. Supersedes the per-release PRs #3021 (release-1.5) and #3020 (release-1.4). ## Why Deleting a Tenant whose etcd `DataStore` has **0 referencing `TenantControlPlane`s** — e.g. a tenant deleted before any Kubernetes app / TCP is ever created in it — leaves the `DataStore` stuck `Terminating` with finalizer `kamaji.clastix.io/TenantControlPlane`. That hangs `helm uninstall <etcd>` (which waits by default), freezes the etcd HelmRelease at `status=uninstalling`, and wedges the tenant namespace in `Terminating` indefinitely. Tracked in #3018. Root cause: in `26.3.x-edge`, DataStore deletion drives an `AllowedDeletion` condition that only transitions `False → True`. With 0 referencing TCPs at deletion time the condition is never initialized, so the reconcile falls through to logging `DataStore can be safely deleted` and the deferred finalizer-removal (gated on `IsStatusConditionTrue`) never runs — with no requeue to retry. The pinned `26.3.6-edge` predates the upstream fix (merged 2026-04-14, first released in `26.4.4-edge`). ## Change Adds `packages/system/kamaji/images/kamaji/patches/fix-datastore-unused-deletion.diff`, applied by the existing `RUN git apply /patches/*.diff` step in the kamaji image build. The one-line guard adds the missing 0-TCP path: ```go - if meta.IsStatusConditionFalse(..., DataStoreConditionAllowedDeletionType) { + if meta.IsStatusConditionFalse(..., DataStoreConditionAllowedDeletionType) || len(tcpList.Items) == 0 { ``` ## Test - `git apply --check` confirms the patch applies cleanly to the `26.3.6-edge` source pinned on `main`; after applying, the guard is present at `controllers/datastore_controller.go`. - Reproduced the deadlock on a v1.4.x cluster (tenant deleted before any TCP existed) and confirmed the manual unstick; this patch removes the need for it. Workaround for an already-stuck DataStore (0 TCP refs): `kubectl patch datastore <name> --type=merge -p '{"metadata":{"finalizers":null}}'`. Relates to #2885 (full kamaji bump for CVEs — supersedes this patch once it lands). ```release-note Fix tenant namespace stuck `Terminating` when an etcd DataStore with no referencing TenantControlPlanes is deleted (Kamaji datastore unused-deletion deadlock). ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Data stores are now recognized as unused even when no matching tenant control plane resources are found, improving deletion handling in that case. * This prevents inactive data stores from being left behind when there are no associated matches. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
5b261b9daf |
fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled (#3130)
## What this PR does
The `victoria-metrics-operator` HelmRelease in the `kubernetes` app
chart is rendered when `addons.monitoringAgents.enabled` is true, but it
listed `cert-manager-crds` in its `dependsOn` **unconditionally**. The
`cert-manager-crds` HelmRelease is only created when
`addons.certManager.enabled` is true. These are two independent toggles.
### The valid combination that triggers it
`addons.monitoringAgents.enabled: true` + `addons.certManager.enabled:
false` (both default-able, both schema-valid). In this configuration
vmop renders but the dependency it points at never does.
### Before / after
Before:
```yaml
dependsOn:
- name: {{ .Release.Name }}-cilium
namespace: {{ .Release.Namespace }}
- name: {{ .Release.Name }}-cert-manager-crds # always present
namespace: {{ .Release.Namespace }}
```
After:
```yaml
dependsOn:
- name: {{ .Release.Name }}-cilium
namespace: {{ .Release.Namespace }}
{{- if .Values.addons.certManager.enabled }}
- name: {{ .Release.Name }}-cert-manager-crds
namespace: {{ .Release.Namespace }}
{{- end }}
```
### The cascade
With cert-manager disabled, Flux reports on the vmop HelmRelease:
`unable to get '...cert-manager-crds' dependency: helmreleases... not
found`. Because vmop never goes Ready, the dependency chain fails behind
it: `monitoring-agents` (dependsOn `cozy-victoria-metrics-operator`) →
not Ready → `vertical-pod-autoscaler` (dependsOn `monitoring-agents`) →
not Ready.
### Why gating (not unconditional CRDs) is correct
vmop does not require cert-manager. The vendored vmop chart
(`packages/system/victoria-metrics-operator`) defaults
`admissionWebhooks.certManager.enabled` to `false`, and the cozystack
wrapper does not override it. With that default the chart self-generates
the webhook CA/cert (`genCA`/`genSignedCert` in `_helpers.tpl`); the
cert-manager Issuer/Certificate path is fully gated behind
`certManager.enabled`. So the dependency only belongs there when
cert-manager is actually enabled.
Principle: a `dependsOn` must never reference a HelmRelease that isn't
created under the same value combination.
### How it was verified
`helm template` of the template with a CI values fixture:
`certManager.enabled=false` → cilium only:
```yaml
dependsOn:
- name: test-cilium
namespace: tenant-root
```
`certManager.enabled=true` → cilium + cert-manager-crds:
```yaml
dependsOn:
- name: test-cilium
namespace: tenant-root
- name: test-cert-manager-crds
namespace: tenant-root
```
A new `helm unittest` suite
(`tests/victoria-metrics-operator_test.yaml`) asserts the `dependsOn`
for both toggle states. Full package suite passes (13 suites / 125
tests).
### Release note
```release-note
fix(kubernetes): enabling monitoring agents without cert-manager no longer blocks the victoria-metrics-operator HelmRelease (and the monitoring-agents / vertical-pod-autoscaler chain behind it) on a non-existent cert-manager-crds dependency
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
## Summary by CodeRabbit
* **New Features**
* Added automated template test coverage for the Victoria Metrics
Operator HelmRelease.
* Validates rendered release name and dependency behavior across key
configuration scenarios.
* **Bug Fixes**
* Updated the HelmRelease dependency wiring so the cert-manager CRDs
dependency is included only when cert-manager is enabled.
* Preserved existing monitoring and outer conditional behavior while
refining dependency rendering.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Myasnikov Daniil
|
47638f8569 |
fix(kamaji): backport datastore unused-deletion deadlock fix (clastix/kamaji#1122)
Deleting a Tenant whose etcd DataStore has 0 referencing TenantControlPlanes left the DataStore stuck Terminating (finalizer kamaji.clastix.io/TenantControlPlane), hanging the etcd helm uninstall and wedging the tenant namespace. Backports clastix/kamaji#1122 (fixes clastix/kamaji#1115) as a source patch on the pinned 26.3.6-edge build. Refs #3018. Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
myasnikovdaniil
|
dad78bd57e |
fix(ci): nest backport conflict_resolution under experimental input (#3155)
## What this PR does The Automatic Backport workflow passed `conflict_resolution: draft_commit_conflicts` as a **top-level** input to `korthout/backport-action`, but the action only reads `conflict_resolution` from inside its `experimental` JSON input. Passed top-level it was silently ignored — the action run logs warn `Unexpected input(s) 'conflict_resolution'` — so it fell back to the `experimental` default of `fail`. On a cherry-pick conflict the action then created **no** pull request and only left a "Backport failed" comment on the source PR (this bit #3121 and #3119). This moves the key into the `experimental` input, so a conflicting backport instead opens a **draft PR with the first conflict committed** for maintainers to resolve in place. No version bump: the pinned `v3.2.1` already supports `draft_commit_conflicts` — the bug was purely the input placement. ### Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Backport pull requests now handle conflicts more reliably and open as draft PRs when conflicts are detected. * Improved configuration handling so the expected conflict behavior is applied consistently during backport automation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
IvanHunters
|
59964043e3 |
fix(platform): migration 49 backfills tenant ancestor labels
The tenant chart fix makes new and re-reconciled tenant namespaces self-compute their full tenant.cozystack.io/<ancestor> chain, but existing namespaces whose HelmRelease does not promptly re-reconcile (suspended or failed releases) keep the incomplete labels — and with them the broken <ancestor>-egress reachability that surfaces as upstream timeouts to nested tenants. Backfill the labels directly so clusters heal on upgrade regardless of per-tenant HelmRelease state. Bump targetVersion to 50 so run-migrations.sh iterates the new script (seq CURRENT..49); the missing-file guard otherwise never fires. Best-effort and idempotent: kubectl label --overwrite adds only the listed labels, and per-namespace failures are tolerated so a transient apiserver error cannot abort the platform upgrade. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1ac9398a4b |
fix(tenant): emit full ancestor label chain on tenant namespaces
The tenant.cozystack.io/<ancestor> namespace labels drive the <ancestor>-egress CiliumClusterwideNetworkPolicy, which lets an ancestor tenant reach a descendant namespace only when the descendant carries tenant.cozystack.io/<ancestor>. namespace.yaml derived that chain by splitting .Release.Namespace, which only reached one level up: a direct child of tenant-root got tenant-root, but a grandchild (tenant-ktj-htdev under tenant-ktj) got only tenant-ktj and dropped tenant-root entirely. The root ingress controller in tenant-root then could not reach nested tenants, surfacing as upstream timeouts. Derive the chain deterministically from the namespace name instead. Every tenant descends from tenant-root, and tenant.name builds each child as <parent-namespace>-<word>, so every progressive dash-prefix of the name is a real ancestor namespace. This needs no cluster lookup, renders offline (so it is unit-testable), and converges on every reconcile regardless of parent/child HelmRelease ordering. The ownerReference lookup is made non-fatal so the template still renders when the parent namespace lookup is empty; in a real reconcile the parent always exists and the ownerReference is always emitted. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
2b1d3e904d |
fix(kubernetes): gate vmop cert-manager-crds dependency on certManager.enabled
The victoria-metrics-operator HelmRelease is gated on
monitoringAgents.enabled but listed cert-manager-crds in dependsOn
unconditionally, while the cert-manager-crds HelmRelease is only
created when certManager.enabled is true. The valid combination
monitoringAgents.enabled=true + certManager.enabled=false therefore
left vmop blocked on a non-existent dependency
("...cert-manager-crds ... not found"), cascading to monitoring-agents
and vertical-pod-autoscaler. Gate the dependency on the same condition
that creates it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
scooby87
|
71ca2b1bae |
fix(mariadb): re-guard replication so single-replica passes the operator webhook (#3180)
## What this PR does Fixes a regression where **single-replica MariaDB can no longer be created**. [PR #2279](https://github.com/cozystack/cozystack/pull/2279) made `replication.enabled: true` unconditional in the MariaDB CR so the operator would always create `<release>-primary` / `<release>-secondary` Services (expected by the dashboard RBAC and the ApplicationDefinition). But the vendored mariadb-operator's validating webhook (`vmariadb-v1alpha1.kb.io`) **rejects `replication.enabled: true` when `spec.replicas == 1`** — replication needs a primary + at least one replica. As a result, single-replica installs never became Ready (the HelmRelease failed at admission). The webhook rule lives in the operator's Go code (not in the vendored CRD — there is no CEL rule tying replication to replica count), so the failing path is reproduced by an e2e test rather than a schema assertion. ### Fix — re-guard replication, teach consumers about the bare service The operator provisions a bare `<release>` Service in **both** topologies; `-primary`/`-secondary` exist **only** when replication is on (`replicas>1`). Instead of forcing replication for everyone, this PR lets replication track the replica count and makes the service-name consumers handle the single-replica bare service: - **`mariadb.yaml`** — gate `replication` on `replicas>1`; restore single-replica external access via `service.type: LoadBalancer`, and keep `primaryService: LoadBalancer` for `replicas>1` only. (Effectively reverts #2279's two hunks.) - **`backup-cronjob.yaml`** — single-replica dumps target the bare service (no `-secondary` exists without replication); `replicas>1` still offloads to `-secondary`. - **`dashboard-resourcemap.yaml`** — always grant RBAC on the bare service, plus `-primary`/`-secondary` when `replicas>1`, matching what the ApplicationDefinition lists. - **`mariadb-rd`** — list the bare service in `services.include` so single-replica installs are visible in the dashboard. - **`hack/e2e-apps/mariadb.bats`** — new `Create single-replica MariaDB` test that reproduces the webhook rejection and guards against regression. Verified: `helm template` at `replicas=1`/`2` renders the intended CR/RBAC/backup host; `helm unittest` passes; `update-crd.sh` keeps the RD stable; `select-e2e.sh` maps the change to the mariadb suite. ### Release note ```release-note fix(mariadb): single-replica instances can be created again — replication is now enabled only for replicas>1 (matching the operator webhook), while the dashboard, RBAC, and backups correctly target the bare service for single-replica setups ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an end-to-end test covering MariaDB deployments with a single replica, including validation of the main service, endpoints, StatefulSet replica count, and metrics components. * **Bug Fixes** * Updated MariaDB chart behavior to correctly handle single-replica vs multi-replica configurations for external access, replication settings, and backup host selection. * Refined dashboard RBAC and secret inclusion so access applies to the main MariaDB service in addition to replica-specific services. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
41fd08e2ba |
feat(keycloak): enable UPDATE_EMAIL required action in cozy realm (#3175)
## What this PR does The portal user-settings page lets a user change their email via a Keycloak `kc_action=UPDATE_EMAIL` redirect. That flow only works when the **UPDATE_EMAIL required action is enabled at the realm level** — and on a fresh realm it is registered but **disabled by default**. `update-email` is a supported, default-on feature since Keycloak 26.4 (keycloak/keycloak#39722), so the provider is already loaded on the 26.x server this repo ships — **no `KC_FEATURES` change and no Keycloak restart are needed**. The `edp-keycloak-operator` has no CRD field for required actions, so this adds a small one-shot, idempotent `post-install`/`post-upgrade` Job to `keycloak-configure` that enables it via `kcadm.sh`, waiting for the operator to reconcile the realm first. Verified against a live Keycloak 26.5.2: the `UPDATE_EMAIL` action goes from `enabled=false` to `enabled=true`, and re-running the Job is a no-op. ### Release note ```release-note feat(keycloak): enable the UPDATE_EMAIL required action in the cozy realm so users can change their email from the portal ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Keycloak now automatically enables the email update action during install and upgrade, helping users change their email address in the `cozy` realm. * The configuration now includes the required Keycloak container image for this setup task. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
e4c6802863 |
fix(cozy-lib,harbor,mariadb): digest-pin cleanup-hook kubectl image and route it through cozy-lib.images-registry (#3171)
## What this PR does Fixes #3088. The post-delete cleanup Jobs in the **harbor** and **mariadb** charts hardcoded the cleanup image as `docker.io/clastix/kubectl:v1.32` — a moving Docker Hub tag. This: - **broke reproducibility / was a supply-chain risk** — `v1.32` is mutable and can be re-pushed to a different image; - **could not be satisfied on air-gapped / mirrored clusters** — the hardcoded `docker.io` bypassed the cluster images registry, and it failed at the worst moment: during **uninstall**, when the `post-delete` hook runs and a stuck Job stalls release teardown. ### Changes - **New canonical helper `cozy-lib.image`** (`packages/library/cozy-lib/templates/_cozyconfig.tpl`). Given a registry-relative image path and the global context, it prefixes `cozy-lib.images-registry` when set (`<registry>/<image>`) and returns the image unchanged when empty — **with no leading `/`**, so standard installs still resolve from the default registry. This establishes the previously-undefined "how to reference a cluster image" pattern (`images-registry` was defined but never consumed). - **Digest-pinned image** in both cleanup hooks: `clastix/kubectl:v1.32@sha256:b9ef7d8dbe65bcc81a46c09b8dc7543103055021c4f43287bf59e92a8f4fe05c` (the immutable multi-arch OCI index for the `v1.32` tag; the same digest already pinned for `postgres-operator`'s webhook hook). - **Renovate keeps the pin fresh.** The repo disables the helm-values manager (`enabledManagers`), and the ref is assembled at render time by the helper rather than being a Dockerfile / go.mod dependency, so the built-in managers can't see it. A `custom.regex` manager over the hook templates tracks `clastix/kubectl` by tag+digest — mirroring the existing `extra/etcd` manager — so the supply-chain pin doesn't silently age. - **Hardened `cozy-lib.images-registry`** against a nil `.Values._cluster` so the helper never crashes the render — relevant precisely because this hook renders during teardown. - **Helm unit tests** in both charts assert the rendered image for three cases: empty registry (default), registry set, and the no-leading-slash edge case. Assertions match by **pattern** (routed + digest-pinned + no leading slash) rather than an exact digest, so a Renovate bump keeps them green. ### Verification - `helm unittest` passes for both charts (harbor: 16, mariadb: 10). - Real `helm template` renders confirmed for both charts: - empty registry → `clastix/kubectl:v1.32@sha256:b9ef…` - `images-registry=registry.internal:5000` → `registry.internal:5000/clastix/kubectl:v1.32@sha256:b9ef…` - The Renovate `matchStrings` regex verified to capture `currentValue=v1.32` and the digest from both hook templates. ### Note on the wider footprint The same `docker.io/clastix/kubectl:v1.32` hardcode is already merged on `main` in several other cleanup hooks (bucket, qdrant, gateway, seaweedfs, kubernetes, tenant, dashboard, keycloak-configure) and appears in the open PRs #3170 (etcd), #3094 (monitoring), #3072 (clickhouse). This PR fixes harbor + mariadb per the issue and establishes the `cozy-lib.image` + Renovate pattern; migrating the remaining hooks is a follow-up. ### Release note ```release-note fix(cozy-lib,harbor,mariadb): digest-pin the harbor and mariadb post-delete cleanup-hook kubectl image and route it through the cluster images registry (`cozy-lib.images-registry`) via the new `cozy-lib.image` helper, so mirrored / air-gapped installs can resolve it and uninstall no longer depends on a moving Docker Hub tag; a custom Renovate manager keeps the digest fresh ``` 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated post-delete cleanup hook Jobs to use digest-pinned container images for more secure, repeatable execution. * Improved image reference rendering so cleanup hooks work correctly with or without an image registry configured, avoiding invalid leading/trailing slash issues. * **Tests** * Extended cleanup hook rendering tests to validate digest pinning and correct registry prefix behavior, including edge cases. * **Chores** * Enhanced automated dependency detection to treat digest-pinned hook image references as stable. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
6bf836db30 |
fix(dashboard): show LoadBalancer external IP on the app Services tab (#3172)
## What this PR does For an application exposed with `external: true` (which creates a `type: LoadBalancer` Service), the app detail → **Services** tab showed the service Type and Cluster IP but not the external IP assigned by MetalLB — so there was no way to learn the connection address from the per-app view. The pre-1.4.0 dashboard showed this, so it read as a regression (reported in #2789). Root cause: `ServicesTab.tsx` queried Services with a spec-only local type (`{ type, clusterIP, ports }`) and never read `status.loadBalancer.ingress[]`. The data was already reachable — `ExternalIpsPage.tsx` renders it — so this was a wiring gap in the per-app tab, not missing data. Changes: - Add a `ServiceStatus` type and extend the `useK8sList` query to `K8sResource<ServiceSpec, ServiceStatus>`. - Add an **External IP** column: `ip ?? hostname ?? "Pending"` for `LoadBalancer` services, `—` otherwise — mirroring `ExternalIpsPage`. - Add a co-located regression test (`ServicesTab.test.tsx`) covering the IP, hostname→Pending fallback, and the ClusterIP no-external case. Verified: `pnpm typecheck` clean, `pnpm test` green (320 tests, incl. 3 new), `eslint` clean on the changed files. Fixes #3111 ### Screenshots <img width="1220" height="572" alt="Снимок экрана 2026-07-02 в 14 12 11" src="https://github.com/user-attachments/assets/82cd1da8-ff81-4b95-a6bf-18729b6c85c7" /> ### Release note ```release-note fix(dashboard): show the external LoadBalancer IP on the per-app Services tab ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an **External IP** column to the service details view. * Load balancer services now show the external IP or hostname when available. * **Bug Fixes** * Fixed the external IP display for load balancer services so it no longer misses valid IPs. * Improved fallback behavior by showing **Pending** when no external endpoint is assigned. * Non-load balancer services now show an em dash instead of misleading external IP text. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
a16268152d |
fix(etcd): clean up orphaned data PVCs and TLS secrets on tenant module delete (#3090) (#3170)
## What this PR does When the `etcd` tenant module is disabled (`etcd: false`), the etcd HelmRelease is pruned but two classes of resources leak into the tenant namespace (reported in #3090): - **Data PVCs** — the etcd-operator StatefulSet `volumeClaimTemplate` PVCs `data-etcd-<ordinal>` are never garbage-collected on `EtcdCluster` deletion. - **TLS secrets** — the five `etcd-*-tls` secrets survive the uninstall: - `etcd-ca-tls` / `etcd-client-tls` are created by this chart as `pre-install` hooks with `helm.sh/resource-policy: keep`, so Helm deliberately keeps them; - `etcd-peer-ca-tls` / `etcd-server-tls` / `etcd-peer-tls` are populated by cert-manager from `Certificate` resources and carry no owner reference back to the chart. This adds a `post-delete` cleanup hook (`packages/extra/etcd/templates/hooks/cleanup.yaml`), modeled on the accepted seaweedfs (#3092) and monitoring (#3091) sibling fixes, that: - deletes the data PVCs by name, one per replica (`data-etcd-0 … data-etcd-<replicas-1>`); - deletes the five `etcd-*-tls` secrets by name. `check-release-name` pins the release name to `etcd`, so every target name is deterministic and no sibling workload shares the `data-etcd-*` prefix in a tenant namespace — deletion by name is safe. cert-manager TLS secrets carry only the cluster-wide `controller.cert-manager.io/fao` label, so the RBAC grants `delete` by explicit `resourceNames` rather than an unsafe label sweep. The cleanup Job is hardened: non-root, read-only root fs, dropped capabilities, `seccompProfile: RuntimeDefault`, resource requests/limits, `activeDeadlineSeconds`. Deletes use `--ignore-not-found` and failures are logged but never block teardown. A Helm unit-test suite (`tests/cleanup_test.yaml`) asserts the hook metadata/RBAC/hardening and that the rendered commands target exactly the reported PVCs and secrets, including scaling the PVC list to the configured replica count. > [!NOTE] > The **stuck HelmRelease finalizer** noted in #3090 is the platform-wide sharded-helm-controller side-bug (tracked separately); it affects every `chartRef`-based tenant module identically, not just etcd. This PR fixes the PVC and secret orphans the etcd chart itself owns, matching the scope of the merged seaweedfs/monitoring fixes. > [!WARNING] > **Disabling the etcd module now permanently deletes its data PVCs and the etcd data they hold.** This is intentional and fixes the storage leak. Back up before disabling if the data matters. Fixes #3090. ### Release note ```release-note fix(etcd): garbage-collect the orphaned etcd data PVCs (data-etcd-*) and the five etcd-*-tls secrets left behind when a tenant's etcd module is disabled. WARNING: disabling etcd now permanently removes its data PVCs and the data they hold. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a Helm post-delete cleanup hook for etcd that removes per-replica etcd PVCs and the related TLS secrets when a release is deleted. * Cleanup permissions and targets now scale with replica count, including safe handling when replicas are set to zero. * **Tests** * Added Helm unit tests validating hook annotations/order, scoped RBAC, secure job hardening, and the exact deletion behavior for PVCs/secrets (including higher-replica scenarios). * **Chores** * Marked the `test` make target as phony for the etcd package. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Алексей Артамонов
|
f8e4222b7d |
test(mariadb): fix operator MariaDB CR name in single-replica failure dump
The failure-path dump queried mariadbs.k8s.mariadb.com with $name, but the
operator CR is named after the Helm release ({{ .Release.Name }} = mariadb-$name),
so the dump would hit NotFound instead of showing the rejected CR.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com>
|
||
|
Алексей Артамонов
|
a76c5929fd |
fix(cozy-lib,harbor,mariadb): digest-pin cleanup-hook kubectl image and route it through cozy-lib.images-registry
The post-delete cleanup Jobs in the harbor and mariadb charts hardcoded `docker.io/clastix/kubectl:v1.32` — a moving tag pulled directly from Docker Hub. This broke build reproducibility (the tag can be re-pushed), was a supply-chain risk, and could not be satisfied on air-gapped / mirrored clusters, where it failed at the worst moment: during uninstall, when the post-delete hook runs and stalls release teardown. Add a canonical `cozy-lib.image` helper that prefixes a registry-relative image path with `cozy-lib.images-registry` when set and returns it unchanged (no leading "/") when empty, so standard installs resolve from the default registry and mirrored installs resolve internally. Consume it from both cleanup hooks with the image digest-pinned to the immutable multi-arch index sha256:b9ef7d8dbe65bcc81a46c09b8dc7543103055021c4f43287bf59e92a8f4fe05c (the same digest already pinned for postgres-operator's webhook hook). Keep the pin fresh with a custom Renovate manager: the repo disables the helm-values manager (enabledManagers), and the ref is built at render time by the helper rather than being a Dockerfile/go.mod dependency, so the built-in managers cannot see it. A regex manager over the hook templates tracks clastix/kubectl by tag+digest — mirroring the existing extra/etcd manager — so the supply-chain pin does not silently age. Also harden `cozy-lib.images-registry` against a nil .Values._cluster so the helper never crashes the render — relevant precisely because the hook renders during teardown. Helm unit tests assert the rendered image (routed, digest-pinned, no leading slash) by pattern rather than an exact digest, so a Renovate bump keeps them green. Closes #3088 Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Andrei Kvapil
|
caa6607f4d |
feat(keycloak): add optional KMS-encrypting database proxy (#3003)
## What this PR does Adds an optional, flag-gated **column-level PII encryption** feature to the `keycloak` system package. When `encryption.enabled=true`, a `keycloak-kms-proxy` Deployment + Service is placed on the wire between the Keycloak StatefulSet and its CloudNativePG database, and Keycloak's `KC_DB_URL_HOST`/`KC_DB_URL_PORT` are repointed at the proxy. The proxy transparently encrypts Keycloak PII columns (username, email, first/last name, `pii-*` attributes, credential secrets) before they reach PostgreSQL and decrypts them on reads; the data-encryption key is wrapped by a KMS and never lives inside the database. The proxy lives in its own repository: https://github.com/cozystack/keycloak-kms-proxy (image `ghcr.io/cozystack/keycloak-kms-proxy:v0.1.0`). **KMS backend** is selectable via `encryption.kms.backend`: - `static` (default): a self-generated 32-byte KEK kept in a Secret — works on any cluster, suitable for evaluation. - `vault-transit`: the KEK lives in HashiCorp Vault (recommended for production; supports KEK rotation). The feature is **off by default**, so existing installs are unaffected — `KC_DB_URL_HOST` keeps sourcing from the CNPG `keycloak-db-app` secret. ### Changes - `values.yaml`: new `encryption` block. - `templates/proxy.yaml` (new): gated Deployment + Service (+ self-generated KEK Secret for the static backend), hardened pod (nonroot, read-only rootfs, dropped caps, seccomp). - `templates/sts.yaml`: conditional DB-endpoint override when encryption is enabled. - `tests/encryption_test.yaml` (new): helm-unittest covering on/off and both KMS backends. `helm unittest .` → 11/11 pass. ### Follow-ups (not in this draft) - Backend TLS re-origination to the CNPG endpoint + CA validation. - Liquibase bootstrap-window handling (`KKP_LENIENT`). - Offline backfill Job for migrating existing data. - HA via a shared DEK set (currently single replica). ### Screenshots N/A — no UI changes. ### Release note ```release-note feat(keycloak): add optional KMS-encrypting database proxy for column-level PII encryption (off by default; static KEK or Vault Transit) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added optional transparent encryption for Keycloak database sensitive fields * Users can configure encryption scope and select key management backend options * **Tests** * Added comprehensive test coverage validating encryption feature across multiple configuration scenarios <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Алексей АртамоновandClaude Opus 4.8
|
9472bb3412 |
fix(etcd): clamp cleanup PVC resourceNames so replicas:0 cannot widen RBAC
Address CodeRabbit review on #3170: the PVC cleanup Role generates its resourceNames from .Values.replicas. values.schema.json sets no minimum, so an explicit replicas: 0 rendered an empty resourceNames list — and an empty resourceNames in an RBAC rule matches every PVC in the namespace (a wildcard), silently undoing the least-privilege scoping the previous commit added. Clamp the count to max(1, replicas) via a shared $replicaCount used by both the Role resourceNames range and the delete command, so at least data-etcd-0 is always listed (it exists for any running cluster and the delete uses --ignore-not-found). Add a regression test asserting replicas: 0 still yields a name-scoped Role and a single-name delete rather than a namespace-wide grant. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Алексей АртамоновandClaude Opus 4.8
|
4a06df92e7 |
fix(etcd): scope cleanup Role to the data-etcd PVC names and assert hook metadata
Address CodeRabbit review on #3170: - Tighten the cleanup Role's PVC rule from a namespace-wide get/list/delete to get/delete by explicit resourceNames (data-etcd-<ordinal>, one per replica), mirroring the least-privilege pattern already used for the TLS secrets. The Job only ever deletes those deterministic names, so list is unnecessary and a regression can no longer reach a sibling workload's PVC. - Extend the Role and RoleBinding unit tests to assert helm.sh/hook and helm.sh/hook-delete-policy (not just the weight), so the RBAC cannot regress into normal release resources, and assert the PVC resourceNames. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Алексей АртамоновandClaude Opus 4.8
|
a92f060f63 |
fix(etcd): clean up orphaned data PVCs and TLS secrets on tenant module delete (#3090)
When the etcd tenant module is disabled (etcd: false), the etcd-operator StatefulSet data PVCs (data-etcd-<ordinal>) are never garbage-collected, and the five etcd-*-tls secrets are left orphaned in the tenant namespace: etcd-ca-tls / etcd-client-tls are pre-install hooks this chart marks with helm.sh/resource-policy: keep, and etcd-peer-ca-tls / etcd-server-tls / etcd-peer-tls are cert-manager-issued secrets with no owner reference back to the chart. None are reclaimed by the Helm uninstall. Add a post-delete cleanup Job (modeled on the seaweedfs/monitoring sibling fixes) that deletes the data PVCs by name (one per replica) and the five TLS secrets by name. check-release-name pins the release to "etcd", so every target name is deterministic and no sibling workload shares the data-etcd-* prefix. The Job runs non-root with a read-only root fs, dropped capabilities and seccompProfile RuntimeDefault; its RBAC is scoped to PVC get/list/delete and the five secrets by resourceNames. Deletes use --ignore-not-found and failures are logged but never block teardown. The stuck-HelmRelease finalizer noted in the issue is the platform-wide sharded-helm-controller side-bug tracked separately; this change addresses the PVC and secret orphans the etcd chart owns. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Алексей Артамонов
|
c7e8f4d975 |
fix(mariadb): re-guard replication so single-replica passes the operator webhook
PR #2279 made `replication.enabled: true` unconditional so the operator would always create -primary/-secondary services. But the mariadb-operator validating webhook (vmariadb-v1alpha1.kb.io) rejects replication when spec.replicas == 1, so single-replica MariaDB could no longer be created — the HelmRelease never became Ready. Re-guard replication on replicas>1 and teach the service-name consumers about the bare <release> service the operator provisions in both topologies: - mariadb.yaml: gate `replication` on replicas>1; restore single-replica external access via `service.type: LoadBalancer` and keep `primaryService` LoadBalancer for replicas>1 only. - backup-cronjob.yaml: route single-replica dumps to the bare service (no -secondary exists without replication). - dashboard-resourcemap.yaml: always grant RBAC on the bare service, plus -primary/-secondary when replicas>1, matching the ApplicationDefinition. - mariadb-rd: list the bare service in services.include so single-replica installs are visible in the dashboard. Adds a single-replica e2e test that reproduces the webhook rejection and guards against regression. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
scooby87
|
fdbe6b299c |
fix(clickhouse)!: clean up orphaned keeper and data PVCs on delete (#3057) (#3072)
## What this PR does After deleting a `ClickHouse` app CR, data PVCs were left orphaned in the tenant namespace and consumed storage permanently: - the keeper PVC `default-chk-<release>-keeper-cluster1-0-0-0`, backing a `ClickHouseKeeperInstallation` reconciled by the clickhouse-keeper-operator; - the main ClickHouse data / log PVCs. The keeper operator does not delete data PVCs when the CR is removed (data-safety default) and the CRD exposes no `persistentVolumeClaimRetentionPolicy` equivalent, so the volumes leaked. This adds a `post-delete` cleanup hook (modeled on the existing MariaDB hook at `packages/apps/mariadb/templates/hooks/cleanup-pvc.yaml`) that deletes every PVC carrying the release label `app.kubernetes.io/instance=<release>`. It also adds that label to the keeper `volumeClaimTemplates` in `chkeeper.yaml`: the keeper PVC previously had **no** release label (the chart set labels only on the keeper pod template, not the PVC), so the operator never stamped one and a single selector would have missed it — leaving the reported leak unfixed. ### Existing releases (label backfill migration) The altinity clickhouse-keeper-operator only stamps `volumeClaimTemplates.metadata.labels` onto PVCs it creates **after** the chart change; it never re-labels PVCs that already exist. So for releases installed before this fix, the keeper PVC stays unlabeled and the cleanup selector would still miss it. To close #3057 for existing customers too, this PR adds migration `48` (`48 -> 49`, `packages/core/platform/images/migrations/migrations/48`) that back-fills `app.kubernetes.io/instance=<release>` onto pre-existing keeper PVCs, and bumps `migrations.targetVersion` to `49`. The migration: - routes its version stamp through the shared `lib/cozystack-version.sh` helper (`stamp_cozystack_version 49`), per the `cozystack-version-stamp.bats` convention; - no-ops when the keeper CRD is absent, and tolerates a transient apiserver error on the keeper list (`|| true`) so it cannot abort a platform-wide upgrade — the backfill is best-effort (worst case is the pre-existing leak, no worse than today); - is idempotent (skips PVCs already labeled). The main ClickHouse data/log PVCs have carried `app.kubernetes.io/instance` since 2025-06-03 (well before the migration era), so they need no backfill. ### Hook failure handling The cleanup hook does **not** mask failures: `kubectl delete` failing makes the Job exit non-zero. The **Job** uses `helm.sh/hook-delete-policy: before-hook-creation` only (no `hook-succeeded`) so a failed cleanup Job and its logs survive for debugging; the **ServiceAccount/Role/RoleBinding** keep `before-hook-creation,hook-succeeded` so the RBAC artifacts are reclaimed after a successful uninstall instead of lingering in the namespace. ### Scope The hook intentionally reclaims **only PVCs** of the release. Other release state (Secrets, ConfigMaps) is owned/garbage-collected through the normal Helm uninstall path and is out of scope here. > [!WARNING] > **This is destructive — deleting a ClickHouse app now permanently deletes its data.** > The hook removes **all** PVCs of the release: the keeper PVC **and** the main > ClickHouse data and log PVCs. After deletion nothing is left behind to recover > data from. This is intentional and matches the existing MariaDB cleanup-hook > behaviour and the general cozystack model (deleting a managed app reclaims its > storage). Back up before deleting. Fixes #3057. ### Release note ```release-note fix(clickhouse): garbage-collect the keeper and data/log PVCs when a ClickHouse app is deleted, so no orphaned volumes are left behind. A migration back-fills the release label onto keeper PVCs from releases installed before this fix so they are cleaned up too. WARNING: deleting a ClickHouse app now permanently removes all of its PVCs (keeper + data + logs) and the data they hold. ``` |
||
|
Aleksei Sviridkin
|
9aa045fc30 |
feat(ci): gate sizeable API changes behind API-owner review (#3167)
## What this PR does Adds a CI gate that requires an approving review from a designated API owner (`lllamnyp` or `kvaps`) when a pull request makes a **sizeable** change to the Cozystack API surface. Sizeable means one of: 1. A new API group. 2. A new resource. 3. A breaking change to an existing resource's schema. 4. Removal of a resource or an entire API group. Detection is fully deterministic — no LLM or network calls, so it runs on every PR. ### How it works - **`cmd/api-gate` + `internal/apigate`** — a small Go tool that loads the API surface from two checkouts (merge base and PR head) and classifies the delta. Every resource's schema is normalized to a parsed OpenAPIv3Schema; the tool only unwraps it from its two container formats and reads identity (group/kind/plural) from the wrapper. | Surface | Files scanned | Schema source | | --- | --- | --- | | Apps (`apps.cozystack.io`) | `packages/system/*-rd/cozyrds/*.yaml` | `spec.application.openAPISchema` | | Typed groups (`cozystack.io`, `backups.*`, `network.*`, `gateway.*`) | every `*.cozystack.io` CRD discovered under a `crds/` or `definitions/` directory in `packages/` and `internal/` | served `spec.versions[].schema.openAPIV3Schema` | | Static Go-backed (`core.*`, `sdn.*`) | `pkg/apiserver/apiserver.go` storage registrations | none — new/removed group / resource only | CRDs are found by location + content rather than a fixed file list, so a first-party CRD that moves (e.g. into a chart's `crds/` dir) stays covered. Only `served` CRD versions count as live API surface, so the standard deprecation step (`served: true → false`) registers as a removed version. An empty snapshot is treated as an error, never a silent pass. - **Breaking classification is semantic, not textual.** Additive changes (new optional field, new enum value, widened type, relaxed or removed constraint, description/default edits) pass. Flagged as breaking: removed fields, type narrowing (including adding a type to a previously-unconstrained field), enum-value removal, newly-required fields, added/tightened `pattern`/min/max/length constraints, `additionalProperties` restricted to `false`, added `x-kubernetes-validations` (CEL) rules, and removed served versions. - **`.github/workflows/api-review-gate.yaml`** builds the tool, diffs the PR head against its true **merge base**, and — when the change is sizeable — requires an `APPROVED` review from an API owner. Only each reviewer's *latest* review *on the current head commit* counts, so a stale or superseded approval no longer satisfies the gate. It re-runs on `pull_request_review` so an approval flips the check without a new push. The token is scoped to the approval step alone (`persist-credentials: false` on checkout) so PR-controlled build steps can't read it. The allowlist lives in one place (`API_OWNERS`). The job runs on every PR (no path filter) so it can safely be marked a required check. ### Follow-up needed after merge Mark **"Require API owner review for sizeable API changes"** as a required status check in `main` branch protection for the gate to block merges. ### Tests Table-driven unit tests cover the schema-diff rules, the classifier (new group / new resource / breaking, including the double-count and pure-removal edge cases), all three parsers, and an on-disk end-to-end snapshot test. Verified against the real repo tree: identical trees pass; injected new-group/new-resource/breaking changes are all caught; an additive-only edit passes. ### Release note ```release-note feat(ci): require a review from an API owner (lllamnyp or kvaps) when a PR introduces a new API group, a new resource, or a breaking change to an existing API schema ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an API review gate workflow that flags “sizeable” API changes and requires at least one approving API-owner review. * Introduced an API-gate CLI to compare API snapshots, classify changes (new/removed resources and groups, plus breaking schema updates), and generate readable reports. * Enhanced snapshot parsing and deterministic reporting across multiple input sources. * **Tests** * Added comprehensive unit tests covering parsing, snapshot loading, schema diff detection, and classification/report output behavior. * **Chores** * Updated API gate ownership rules for the new workflow and related tooling. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
9e89ae84ac |
fix(kubernetes): default worker disk storageClass to the application storageClass (#3129)
## What this PR does
Worker node VMs in a tenant Kubernetes cluster set `evictionStrategy:
External`, so they live-migrate. Live migration requires the worker
system DataVolume to be ReadWriteMany, and linstor-csi only grants RWX
on a DRBD-backed StorageClass. When a node group leaves `storageClass`
empty, the worker DataVolume template previously omitted
`storageClassName` and fell through to the management cluster's default
StorageClass — which is operator-configured and not guaranteed to be
DRBD-backed. On any cluster whose default StorageClass is not a
replicated/DRBD class, the worker disk lands on a non-RWX class and live
migration breaks.
This changes the empty-`storageClass` fallback from the management
cluster default to the application-level `storageClass` (default
`replicated`, a DRBD/LINSTOR class): `{{- with .group.storageClass |
default $.Values.storageClass }}`. A per-node-group `storageClass` still
wins when set; when both the node group and the application
`storageClass` are empty, `storageClassName` is omitted so the cluster
default still applies (explicit opt-out preserved).
Doc and codegen surfaces (`types.go`, `values.yaml`,
`values.schema.json`, `README.md`, and the generated `kubernetes-rd`
ResourceDefinition) are regenerated in lockstep with the new field
description. Chart unit tests cover the full contract: fallback to the
application class, a custom application class, per-node-group override
winning, and the both-empty omit path.
**Upgrade impact — one-time worker-pool roll.** The
`KubevirtMachineTemplate` name embeds a hash of the rendered worker
template body, and the MachineDeployment references that name. On the
shipped default (`md0` leaves `storageClass` empty) the body changes
from "no `storageClassName`" to `storageClassName: replicated`, so on
upgrade the template is renamed and Cluster API performs a one-time
rolling replacement of the worker pool — for every existing tenant
cluster, including those whose default StorageClass already was
`replicated` (the roll is driven by the body hash, not by whether the
effective class differs). This is the same class of roll any
worker-template change triggers and is unavoidable with this approach.
Two test pins document and guard the hash so future body changes are
deliberate.
### Release note
```release-note
fix(kubernetes): worker node disks now fall back to the application-level storageClass (a replicated/DRBD class) instead of the management-cluster default when a node group leaves storageClass empty, so live migration of worker VMs gets the ReadWriteMany volume it requires. Upgrading rolls each tenant cluster's worker pool once.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved worker disk `storageClassName` fallback for empty
per-node-group values: it now reliably uses the app-level
replicated/DRBD StorageClass (rather than relying on the
management-cluster default).
* Ensured KubeVirt data volume templates and rendering behavior are
consistent across single- and multi-node-group setups.
* **Documentation**
* Refined `storageClass`/RWX/DRBD guidance across configuration docs and
schema descriptions, including the intended non-immutable behavior
(optional/undefaulted).
* **Tests**
* Updated template test expectations to cover the new fallback vs
omission cases, including naming assertions for rendered templates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Andrei Kvapil
|
83bd83062e |
feat(keycloak): enable UPDATE_EMAIL required action in cozy realm
The portal user-settings page changes a user's email via a Keycloak kc_action=UPDATE_EMAIL redirect, which only works when the UPDATE_EMAIL required action is enabled at the realm level. On a fresh realm the action is registered (update-email is a supported, default-on feature since Keycloak 26.4) but disabled by default. The edp-keycloak-operator has no CRD field for required actions, so add a one-shot, idempotent post-install/post-upgrade Job to keycloak-configure that enables it via kcadm.sh, waiting for the operator to reconcile the realm first. Assisted-By: Claude Signed-off-by: Andrei Kvapil <kvapss@gmail.com> |
||
|
Алексей Артамонов
|
bf75d32a1a |
fix(dashboard): show LoadBalancer external IP on the app Services tab
The per-app Services tab queried Services with a spec-only type, so it never read status.loadBalancer.ingress and showed only the cluster-internal IP for external: true (type: LoadBalancer) services. This regressed the pre-1.4.0 behaviour (cozystack/cozystack#2789). Add a ServiceStatus type to the useK8sList query and surface an "External IP" column (ip / hostname / Pending for LoadBalancer, — otherwise), mirroring what ExternalIpsPage already does. Fixes #3111 Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Алексей Артамонов
|
7faa32508d |
fix(clickhouse): reclaim cleanup hook RBAC on successful uninstall
Address review [MINOR] on #3072: keep the cleanup Job on before-hook-creation (so a failed Job and its logs survive for post-mortem), but restore before-hook-creation,hook-succeeded on the ServiceAccount/Role/RoleBinding so those artifacts do not linger in the namespace after a successful uninstall. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Алексей Артамонов
|
2e65ce108b |
fix(clickhouse): guard migration when keeper CRD absent and quote hook scalars
Address CodeRabbit review on #3072: - Migration 45 now skips the backfill when the clickhousekeeperinstallations CRD is not installed. The migration runs platform-wide via targetVersion, so on clusters that never deployed ClickHouse the kubectl get would otherwise fail under set -euo pipefail and abort the whole upgrade. - Quote the templated name/label/serviceAccountName scalars in the cleanup hook so the source parses under YAMLlint; rendered output is unchanged. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Алексей Артамонов
|
a2ae1621af |
fix(clickhouse): surface cleanup hook failures and backfill legacy keeper PVC labels
Address review on #3072: - Cleanup hook no longer masks kubectl delete failures: the command now fails (exit 1) when PVC deletion fails instead of swallowing the error with || echo. The hook-delete-policy drops hook-succeeded (keeps only before-hook-creation) so a failed cleanup Job survives for inspection. - Add migration 45 (45 -> 46) that backfills app.kubernetes.io/instance onto keeper PVCs created before this change; the altinity keeper operator never re-stamps volumeClaimTemplate labels onto existing PVCs, so without this the cleanup selector misses pre-upgrade keeper PVCs and #3057 persists for existing releases. Bump migrations.targetVersion to 46. - Unify helm.sh/hook-weight key quoting across the four hook manifests. - Extend the unittest to assert the new hook-delete-policy and exit-on-failure. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Алексей Артамонов <alexeyartamonov1987@gmail.com> |
||
|
Алексей Артамонов
|
2c8f6b8ca1 |
fix(clickhouse)!: clean up orphaned keeper and data PVCs on delete (#3057)
After deleting a ClickHouse app CR, the keeper PVC and the main data/log PVCs were left orphaned in the tenant namespace, consuming storage permanently. The clickhouse-keeper-operator does not delete data PVCs on CR removal (data-safety default) and the CRD exposes no persistentVolumeClaimRetentionPolicy equivalent. Add a post-delete cleanup hook (modeled on the MariaDB hook) that deletes every PVC carrying the release label app.kubernetes.io/instance=<release>. Also stamp that label on the keeper volumeClaimTemplates in chkeeper.yaml: the keeper PVC previously had no release label (the chart labelled only the keeper pod template, not the PVC), so a single selector would have missed the very PVC the issue reports. BREAKING CHANGE: deleting a ClickHouse app now permanently removes all of its PVCs (keeper + data + logs) and the data they hold. Nothing is left behind to recover from. This matches the existing MariaDB cleanup-hook behaviour and the general cozystack model where deleting a managed app reclaims its storage. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
4783adddf7 |
fix(seaweedfs)!: clean up orphaned data PVCs and cert/db secrets on tenant module delete (#3092) (#3095)
## What this PR does
When the `seaweedfs` tenant module is disabled, the SeaweedFS
StatefulSet data PVCs (`data1-<release>-system-volume-*`) and a set of
certificate/credential secrets were left orphaned in the tenant
namespace.
This adds a `post-delete` cleanup hook
(`packages/extra/seaweedfs/templates/hooks/cleanup.yaml`) that:
- deletes the volume PVCs by their release-scoped labels
(`app.kubernetes.io/name=seaweedfs,
app.kubernetes.io/instance=<release>-system,
app.kubernetes.io/component=volume`);
- deletes the orphaned secrets by name (`<release>-s3-ingress-tls`,
`<release>-system-{admin,ca,client,filer,master,volume,worker}-cert`,
`<release>-system-db-secret`) — cert-manager TLS secrets carry only the
cluster-wide `controller.cert-manager.io/fao=true` label, so they are
removed by name rather than by an unsafe label sweep.
All names are templated from `.Release.Name` (the chart renders the
StatefulSet/cert HelmRelease as `<release>-system`). CNPG PostgreSQL
PVCs (`<release>-db-*`) are intentionally **not** targeted — CNPG cleans
those itself.
The cleanup Job is hardened: non-root, read-only root fs, dropped
capabilities, `seccompProfile: RuntimeDefault`, resource
requests/limits; RBAC scoped to PVC get/list/delete and the secrets by
`resourceNames`. Failures are logged but never block teardown.
> [!WARNING]
> **Disabling the seaweedfs module now permanently deletes its volume
data PVCs and the object data they hold.** This is intentional and fixes
the storage leak. Back up before disabling if the data matters.
Fixes #3092.
### Release note
```release-note
fix(seaweedfs): garbage-collect the orphaned SeaweedFS volume PVCs and the certificate/db secrets left behind when a tenant's seaweedfs module is disabled. WARNING: disabling seaweedfs now permanently removes its volume PVCs and the data they hold.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added a Helm post-delete cleanup hook for SeaweedFS to remove orphaned
PersistentVolumeClaims and related certificate/database Secrets.
* **Bug Fixes**
* Cleanup deletions now ignore missing resources to reduce hook
failures.
* **Tests**
* Added a Helm/Kubernetes test suite validating the cleanup hook
behavior when the module is disabled.
* **Chores**
* Marked the SeaweedFS Makefile `test` target as phony.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
||
|
Алексей Артамонов
|
fff2e7716f |
fix(seaweedfs)!: clean up orphaned data PVCs and cert/db secrets on tenant module delete (#3092)
When the seaweedfs tenant module is disabled, the StatefulSet volume PVCs and a set of certificate/credential secrets were left orphaned. Add a post-delete cleanup hook that deletes the volume PVCs by their release-scoped labels and the secrets by name (all templated from .Release.Name). CNPG PVCs are not targeted (CNPG cleans them). The cleanup Job is hardened (non-root, read-only root fs, dropped caps, resource limits); RBAC scoped via resourceNames; failures are logged, never blocking teardown. The PVC selector matches on name + instance only, without an app.kubernetes.io/component clause, so volume PVCs from custom storage pools and MultiZone topologies (component volume-<pool>/volume-<zone>[-<pool>]) are cleaned up too, not just the default pool. Both deletes use --wait=false and the Job has activeDeadlineSeconds so a finalizer-stuck resource cannot hang the post-delete hook. NOTE: disabling seaweedfs now permanently removes its volume PVCs and the data they hold. BREAKING CHANGE: disabling the seaweedfs tenant module now permanently deletes its volume PVCs and the object data they hold. Back up before disabling if the data matters. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> |
||
|
Timofei Larkin
|
dc615664a3 |
fix(ci): flag constraints added to open maps/arrays
childSchemas now recurses into items/additionalProperties whenever head defines a schema, diffing against an empty base when base had none. This catches constraining a previously-unconstrained array element or map value (base absent/true -> schema), while still treating additionalProperties:false -> schema and schema removal as safe relaxations. Also tighten the first-party group check to cozystack.io and its dot-delimited subdomains so a lookalike like fakecozystack.io is not gated. Both covered by tests. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
f3e55d2be4 |
fix(ci): harden API gate against tampering and schema gaps
Scope the gate's own files (cmd/api-gate, internal/apigate, the workflow) to the API owners in CODEOWNERS, and compile the detector from the trusted base checkout when present (head fallback only during bootstrap), so a PR cannot neuter the gate by editing its own detector code. Detect two more breaking vectors: removing nullable:true (rejects null) and adding a oneOf/allOf/not composition constraint. Both covered by regression tests. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
dec840e938 |
fix(ci): address review of the API gate
Fix schema-diff correctness: guard nil head on removed child schemas, flag adding a type to a previously-unconstrained field, stop reporting a head becoming unconstrained as narrowing, detect additionalProperties:false, and only recurse items/additionalProperties when both sides carry a schema. Gate resource and group removal as new categories. Filter CRD versions to served ones so the deprecation flow surfaces as a removed version, and flag added x-kubernetes-validations (CEL) rules. Discover first-party cozystack.io CRDs by walking crds/ and definitions/ dirs so a relocated CRD (e.g. cozystack-scheduler) is covered, and error on an empty snapshot. Surface unmapped apiserver storage prefixes and allow hyphens in plurals; drop Markdown from text-mode reports. Workflow: diff against the true merge base, count only each reviewer's latest review on the current head commit, and scope the token to the approval step with persist-credentials:false. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Andrei KvapilandClaude
|
5d36e13d06 |
refactor(keycloak): drop the generic db.* connection override
Remove the db.host/db.port/db.urlProperties override and the extraVolumes/extraVolumeMounts plumbing: they are no longer needed now that the database endpoint is handled natively. Keycloak's DB host/port come straight from the keycloak-db-app secret again, except when encryption is enabled, where the keycloak-kms-proxy owns the repointing. Theme and admin-console hostname handling are unaffected. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei KvapilandClaude
|
d459ad800a |
test(keycloak): cover encryption misconfiguration failures
Add render-time negative-path coverage for the encryption proxy: an unsupported kms.backend and a vault-transit backend missing address, keyName, or tokenSecretName must all fail the template render rather than silently producing a broken Deployment. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei KvapilandClaude
|
956759388f |
docs(keycloak): document encryption operational caveats and cap proxy memory
Address review feedback on the encryption values block: - Spell out the operational caveats an operator must understand before flipping encryption on: the single-replica proxy turns HA Keycloak into a single point of failure; existing data is not migrated automatically so enabling (or later disabling) on a populated database breaks logins until the offline backfill runs; and a lost static KEK is permanent data loss, so the KEK Secret must be backed up. - Add a memory limit to the proxy resources per the repository style guide. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei KvapilandClaude
|
e97c918829 |
fix(keycloak): fail fast on invalid KMS backend and harden proxy template
Address review feedback on the encryption proxy template: - Validate encryption.kms.backend at render time and fail with a clear message on any value other than static/vault-transit. Both the KEK Secret and the env wiring now key off the same validated $backend, so a typo can no longer render a Deployment that mounts a KEK Secret that was never created (which would silently break the whole IdP). - Guard the static KEK lookup against a Secret that exists without a usable 'kek' key, preserving the freshly generated KEK instead of nil/empty. - Default kms.vault to an empty dict and require address/keyName at render time so an incomplete Vault config fails with an actionable message rather than a generic nil dereference. - Publish the metrics port (9090) on the proxy Service so it can be scraped. - Clarify the backend-TLS note: the proxy negotiates TLS in-band; CNPG CA pinning is the remaining follow-up. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrei Kvapil
|
f5b078d8c7 |
feat(keycloak): add optional KMS-encrypting DB proxy
Add an opt-in `encryption` flag to the keycloak system package. When encryption.enabled=true, a keycloak-kms-proxy Deployment + Service is placed on the wire between Keycloak and its CloudNativePG database, and Keycloak's KC_DB_URL_HOST/PORT are repointed at the proxy. The proxy transparently encrypts PII columns (username, email, first/last name, PII attributes, credentials) before they reach PostgreSQL and decrypts them on reads; the data-encryption key is wrapped by a KMS and never lives inside the database. The KMS backend is selectable: a self-generated static KEK (default, works on any cluster, suitable for evaluation) or HashiCorp Vault Transit (recommended for production, supports KEK rotation). The feature is off by default, so existing installs are unaffected. Skeleton/draft scope: backend TLS re-origination to CNPG, the Liquibase bootstrap window (KKP_LENIENT), an offline backfill Job for pre-existing data, and HA via a shared DEK set are tracked as follow-ups. Proxy project: https://github.com/cozystack/keycloak-kms-proxy Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Timofei Larkin
|
fa871249c6 |
feat(ci): gate sizeable API changes behind API-owner review
Add a deterministic api-gate tool (cmd/api-gate, internal/apigate) and a workflow that require an approving review from an API owner (lllamnyp or kvaps) when a PR introduces a new API group, a new resource, or a breaking schema change to an existing resource. Detection diffs the checked-in OpenAPIv3 schemas (apps cozyrds + CRD manifests) and the apiserver storage registrations; no network or LLM calls. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Aleksei Sviridkin
|
a31a73cb87 |
fix(tenant): gate VM-resource HelmReleases on victoria-metrics-operator readiness (#3141)
## What this PR does The tenant `etcd`, `ingress` and `monitoring` HelmReleases create VictoriaMetrics custom resources — `VMPodScrape` directly in `etcd` and `ingress`, and `VMCluster` / `VMAlert` / `VLCluster` / `VMAlertmanager` / `VMServiceScrape` via the `monitoring-system` child that `monitoring` creates. All of these are intercepted by the `victoria-metrics-operator` validating webhook, which is configured with `failurePolicy: Fail`. On a cold install the operator pod can be briefly unavailable on `:9443` (for example a transient API-server blip during startup leaves the webhook backend not yet serving). In that window the tenant releases reconcile, the webhook rejects their VM* resources with `connection refused`, and the releases churn through failed-install → rollback → retry. They carry `install.remediation.retries: -1` and recover eventually, but the churn can outlast a cold-install readiness deadline and surfaces as spurious install failures. This adds a Flux `spec.dependsOn` from each of those three tenant releases to the `victoria-metrics-operator` HelmRelease in `cozy-victoria-metrics-operator`, so they wait for the operator to be Ready before creating any VM* resource. The system monitoring components already gate on the operator the same way (a variant-level `dependsOn` in the platform package sources); this brings the tenant releases to parity. `failurePolicy: Fail` is preserved — validation is unchanged. The tenant releases are sharded (`sharding.fluxcd.io/key`) while the operator release is not. helm-controller resolves `dependsOn` through an uncached API read that bypasses the shard's watch-label-selector cache (since v1.1.0, PR fluxcd/helm-controller#1070), so the cross-shard reference is visible to the tenant shard's controller; this repo ships helm-controller v1.5.0. A helm-unittest suite pins the gate on all three releases and asserts that the tenant releases creating no VM* resources (`seaweedfs`, `gateway`) do not carry it, documenting the intentional narrowness of the gate. ### Release note ```release-note fix(tenant): gate tenant etcd, ingress and monitoring HelmReleases on victoria-metrics-operator readiness so they no longer fail to install when the operator's admission webhook is briefly unavailable on a cold install ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved tenant installation reliability by adding readiness gating: etcd, ingress, and monitoring Helm releases now wait for the Victoria Metrics operator to become Ready before applying dependent resources. * Prevents install/rollback failures caused by temporary admission webhook unavailability during cold starts. * **Tests** * Added Helm-unittest coverage to confirm the dependency gate is present only for templates that create VM* resources (and omitted for templates that don’t). <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
03435dcc50 |
fix(flux): tune source-watcher concurrency + CPU to unblock platform install under load (#3163)
## What this PR does `source-watcher` is the Flux controller that turns each `PackageSource` into an `ExternalArtifact` (one `ArtifactGenerator` per source, ~100 builds on the management cluster during install). The `cozystack-platform` HelmRelease blocks until all of those artifacts are Ready, so source-watcher's throughput sits on the install's critical path. It was the only Flux controller left without concurrency/CPU tuning. In the bootstrap all-in-one pod it ran on the binary default (`--concurrent=10`) with the shared 100m CPU request; in the production `FluxInstance` it was absent from the reconciler patch that source-, kustomize- and helm-controller already receive. Under load its per-artifact builds finish at staggered times, leaving a random tail of not-yet-Ready artifacts that times out the platform install. A failed run's diagnostics show the shape of this: the `cozystack-platform` HelmRelease timed out with `timeout waiting for: PackageSource/… status: 'InProgress'` (a run-to-run varying tail — e.g. `bucket-application`, `linstor-gui`, `capi-provider-core`), while its ~200 ExternalArtifacts reached Ready in a staggered trickle spread over ~60 seconds rather than together. That staggered completion of independent per-artifact builds is the signature of a throughput-limited builder, not a single all-or-nothing pull. This raises source-watcher to `--concurrent=20` in both install paths: - **Bootstrap all-in-one pod** (`internal/fluxinstall/manifests/fluxcd.yaml`, regenerated from `flux-aio.cue` + the `flux-aio` Makefile): `--concurrent=20` plus a 500m CPU request. The AIO pod sets no CPU limits, so the request is what drives source-watcher's proportional CPU share when the node is saturated. - **Production `FluxInstance`** (`packages/system/fluxcd/values.yaml`): source-watcher is added to the shared reconciler patch, so it now gets the same `--concurrent=20`, `--requeue-dependency=5s` and 2000m/2048Mi limits as the other tuned controllers. The `flux-aio` module exposes a per-controller resources override for the watcher (set via CUE) but no args override, so the bootstrap `--concurrent` bump is a guarded post-render patch in the Makefile; the embedded manifest is the product of that pipeline. source-watcher's support for both `--concurrent` and `--requeue-dependency` was confirmed against its `cmd/main.go`, and the production `op: replace` on `resources/limits` is safe because the distribution ships source-watcher with a `resources.limits` block (same shape as source-controller). Optional follow-up, intentionally left out to keep this focused on the root cause: if the artifact-build tail still brushes the 10-minute platform HelmRelease timeout under load, the installer's HelmRelease install/upgrade timeout values give a safety margin. ### Release note ```release-note fix(flux): tune source-watcher concurrency and CPU so the platform install does not time out under load ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Increased the source watcher’s concurrency to improve throughput and better handle larger workloads. * Added higher CPU and memory settings for the watcher to support more stable operation under load. * **Bug Fixes** * Updated deployment patching so the watcher receives the same configuration adjustments as other controllers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
