mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 16:47:20 +09:00
648eb42cae3f12bd367102048fa89089fc40407a
382
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Myasnikov Daniil
|
8355f3e36f |
docs(seaweedfs): correct the stale "D-wedged is adopted" comments
The naming guard now refuses every two-generation state, but two comments in extra/seaweedfs still described a D-wedged duplicate (zero ready replicas) as safe to adopt and rendering through. readyReplicas is a snapshot, not proof the duplicate never served, so it cannot be told apart from D-split and the guard refuses it too. Align the state summary in the seaweedfs template comment and the test header with the presence-only refusal behaviour. No classifier change -- comments only. Address review feedback from coderabbitai on packages/extra/seaweedfs/tests/fullname_override_test.yaml. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
a099b7e086 |
fix(seaweedfs): fail the cleanup Job on a database-owner lookup error
The post-delete cleanup hook read Cluster/seaweedfs-db's Helm owner with `|| true`, so a Forbidden, a timeout or any API error collapsed to an empty owner, took the "nothing to reclaim" branch, and let the Job complete -- permanently orphaning the keep-protected Cluster the hook exists to reclaim, with nothing else that will ever collect it. Only a genuine NotFound now counts as absence; every other error prints the failure and exits non-zero so the Job retries. This matches the fail-closed handling the delete path already had. Add a regression assertion that the ownership lookup no longer swallows failures. Address review feedback from coderabbitai on packages/extra/seaweedfs/templates/hooks/cleanup.yaml. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
b16a51e60e |
test(seaweedfs): pin the naming guard's MultiZone zone-component classification
MultiZone (and Simple-with-pools) tenants render no plain `volume` component — extra/seaweedfs sets volume.enabled=false and the vendored chart emits one component per zone key with suffix `volume-<key>`, truncated at (62 - len(suffix)), which is SHORTER than the 56-char cut the guard's renamedVolumePrefix replays. The final review pass flagged this divergence as a guard bypass. For the supported instance the prefixes cannot diverge: the tenant module hardcodes the name `seaweedfs`, the fullname is 16 chars, and nothing truncates until a zone/pool key of ~40+ characters. Three new cases pin that supported path — legacy-only zones adopt, release-named-only zones refuse as class S, both generations refuse — since the suite previously had no zone shapes at all and the zone objects are the ONLY evidence on a MultiZone tenant. The divergence beyond that boundary (long instance names or absurd keys) is an accepted, documented limit, not a fix: decided in the 1.6 triage, recorded in _naming.tpl (both copies, kept in lockstep) and the runbook's Scope section. Instance names are fixed by the tenant module, so absurd keys are the only reachable trigger; revisit if instance naming is ever opened up. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
601f164c5f |
fix(seaweedfs): stop the naming guard guessing which volume generation holds the data
The guard told a class-S tenant from a D tenant by comparing PVC creationTimestamps,
on the premise that "PVCs are never recreated in place, so the older generation is
where the data was born". The premise is false, and this runbook is the counter-
example: Step 2's re-bind DELETES each release-named claim and recreates it under
the chart name against the same PV.
So a tenant interrupted part-way through Step 2 — after volume-0, before volume-1 —
has data1-seaweedfs-volume-0 created SECONDS ago holding real data, beside
data1-seaweedfs-system-volume-1 with the original timestamp also holding real data.
That is the exact inversion of the rule. Reproduced against the old template:
keeps its data on the release-named volume PVCs (created 2026-06-20T07:01:59Z,
OLDER than the chart-named data1-seaweedfs-volume-* claims created
2026-07-17T09:30:00Z) ... Remove the empty chart-named set ... (Step 2a, then Step 2)
Step 2a then runs `grep -E '^data1-seaweedfs-volume' | xargs kubectl delete pvc` —
deleting the claim Step 2 had just re-bound — and Step 2 had already restored the
PV's original reclaim policy, so on any Delete-policy StorageClass the volume and
the bytes go with it. The guard's own recovery path was a data-loss instruction.
Two more holes in the same classifier:
- The S-damaged branch was gated on $systemPVC, which requires the PVC name to
contain "seaweedfs". seaweedfs.componentName truncates the fullname to 56 chars
before appending -volume, so an instance name >= ~40 chars drops the chart name
off the PVC name — the very case fullname_override_named_test.yaml exists for.
Probed with `archive-of-quarterly-financial-statements-x1`: damaged + renamed set
Ready fell through to the D-split branch ("is running two live SeaweedFS sets ...
Stop the split"), sending the operator to quiesce the set holding ALL the data —
the exact misclassification the parent commit exists to remove. Damaged + not
Ready fired no branch at all and rendered through unguarded.
- readyReplicas == 0 was read as proof the duplicate never served. It is a
snapshot: a duplicate that crashed, was scaled down, or lost readiness AFTER
serving writes reads identically, and adoption strands whatever it wrote.
There is no sound signal here. Claim timestamps are mutable, StatefulSets are
recreated by the adoption hook, and liveness is a snapshot — nothing durable in the
object graph distinguishes "empty set a bad upgrade created" from "the data,
mid-re-bind". Rather than add a fourth heuristic, the guard now decides only what
is decidable:
legacy generation only (or nothing) -> render; adoption is correct
release-named generation only -> refuse, class S (Step 2)
BOTH generations -> refuse; an operator classifies
Both generation flags now OR the PVC and StatefulSet evidence, so the truncated
long-name tenant is seen via its label-matched StatefulSet.
The cost is bounded and lands only where it should. A duplicate exists only on a
tenant that passed through the broken 4.31 rename in 1.5.x — already damaged,
already needing an operator. A 1.4.x tenant upgrading straight to 1.6 never
renames, so it has one generation and is untouched. Confirmed on the stand:
tenant-l (1.4.5-era, never upgraded) is legacy-only and dry-runs clean; the four
both-generation tenants all already need an operator. What is lost is D-wedged
auto-adoption, which was resting on the readyReplicas assumption above.
Classification moves to the runbook, where a human has signals a template does not:
Step 1 now reads the ages of the PVs (durable — they survive the re-bind) via
.spec.volumeName instead of the claims, reports MIXED instead of picking, and walks
the operator through interrupted-Step-2 first, then strict age separation, then
verifying emptiness directly, then escalating rather than guessing. Step 2 stashes
the PV's original reclaim policy ON THE PV and leaves it Retain until Step 5
verifies the tenant, so a mis-step during recovery is survivable. Step 2a grows a
precondition that refuses unless every chart-named claim is bound to a strictly
newer PV.
Tests: 7 of 14 in the enforcing suite fail against the old template, including the
interrupted-Step-2 tenant and both truncated-long-name cases. hack/seaweedfs-guard-
parity.bats pins the two copies (system/ enforcing, extra/ sibling) against silent
drift — they cannot share a library, neither chart depends on cozy-lib, and "keep
these in sync" as a comment is the failure class this whole branch is about; it
also pins that creationTimestamp and readyReplicas never come back.
`make test` in both charts now runs helm template --is-upgrade and greps for the
refusal. The fail-closed canary — the headline safety property — had ZERO automated
coverage: guard_fail_closed_test.yaml passes with AND without the fix, because
helm-unittest 1.0.3 ignores release.isUpgrade. Verified the new target fails when
the canary is removed.
Review round 3 — the refusal was right; the justification was false and the
procedure that replaced the chart's judgement was broken on both axes.
THE COMMENT WAS WRONG. It said "nothing durable in the object graph distinguishes
the empty generation from the data mid-re-bind". A durable signal does exist, and
the reviewers were right to call that out: the Helm release history. Verified on
the stand — sh.helm.release.v1.<name>-system.v1 records the scheme the tenant was
BORN under (tenant-root/l/dsplit/named rev 1 render seaweedfs-master => pre-4.31;
tenant-fresh renders seaweedfs-system-master => 1.5.x), and info.first_deployed is
on every retained revision, so anchoring PV creationTimestamps against it survives
history pruning and the Step 2 re-bind (tenant-root: chart-named +10s vs
release-named +2084s; tenant-fresh: +5225s vs +6s).
BUT IT DOES NOT UNLOCK AUTO-ADOPTION, and that is the substantive disagreement
with the round-3 direction. Birth order answers "which generation is ORIGINAL".
Adoption needs "is the OTHER one EMPTY". Those come apart exactly where it
matters: D-wedged (duplicate never scheduled, safe to adopt) and D-split
(duplicate served writes, adopting strands them) are BOTH born pre-4.31 and are
therefore identical on every durable signal. Verified: tenant-root and
tenant-dsplit have the same rev-1 scheme and the same first_deployed deltas, yet
one duplicate served and the other did not. The only thing that ever separated
them was readyReplicas, which is a snapshot and not evidence. So the render keeps
refusing on presence, and the comment now says why — naming the signal, so nobody
re-derives the broken one.
The signal IS used where it belongs, in the audit.
- naming-guard.yaml / seaweedfs.yaml: match the release-named generation by
RECONSTRUCTING the name 4.31 gives this release's volume component
(templates/_naming.tpl, replaying seaweedfs.fullname + componentName), tested
FIRST because the prefixes nest. Fixes a blocker: an instance legitimately named
`seaweedfs-volume` renders seaweedfs-volume-system-volume and
data1-seaweedfs-volume-system-volume-0, which BOTH satisfy the chart-named
prefixes — a prefix-only guard read its live storage as legacy and would render
onto the empty chart-named claims, stranding the real generation. The parity
bats passed straight through it, because parity pins sameness, not correctness.
Reconstruction also removes the name-match blind spot at source: a truncated long
instance name now matches by PVC, not only by StatefulSet.
- hack/seaweedfs-naming-audit.sh + .bats: the classifier is a tested script now.
Two revisions of it shipped as an untested snippet inside the runbook and both
were wrong in ways that routed a live tenant into the step that strands its data:
a selector `^data1-(.*seaweedfs.*)-volume` that also matched the chart-named
claims, so the release-named range spanned BOTH generations, every tenant read as
"overlapping / mid-rebind", and a genuine S-damaged tenant was routed AWAY from
Step 2a into Step 2 — which deletes its data claim, fails to recreate it, then
deletes its StatefulSets; and a `grep seaweedfs` name match that reported
"L — nothing to do" for a long-named tenant the chart refuses. Both are covered.
The audit reports L/S/MIXED matching the guard, names the original generation
from both durable signals, detects MID-REBIND (both generations at
first_deployed), and states plainly that "original" is not "empty" and that
emptiness must be established from the duplicate's volume files before deleting.
A bats case renders _naming.tpl through helm and compares it to the script's
reconstruction, so the two implementations cannot drift.
- Runbook: Step 1 calls the script. Steps 3/4 were circular — Step 3 ended in an
upgrade the presence-gated guard refuses, and Step 4 told the operator to clear
duplicates AFTER an upgrade that cannot run until they are gone. Step 3 now
removes the duplicate BEFORE the upgrade and re-runs the audit to prove it; Step 4
starts from "every tenant reads L or S". D-wedged no longer claims to need
nothing. Step 3's "enumerate fids that resolve only on the duplicate" was not an
executable procedure and is now an explicit escalation: two masters allocate
volume IDs from independent sequences into one shared seaweedfs-db, so there is
no supported tool that reconciles two volume-ID spaces against one metadata
store. Step 2's re-bind restores the cleanup hook's labels on the recreated
claims, without which a recovered tenant leaks its volumes on a later delete.
Fixture corrected: data1-...-x1-system-volume-0 is not a name the chart renders;
componentName gives ...-x1-system-seaw-volume-0.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
47e24f01d1 |
fix(platform): hand seaweedfs-db over for every instance name, not just the default
Migration 43 performs the v1.5.0 db-split hand-over: it re-owns Cluster/seaweedfs-db from <name>-system to the new <name>-db release and stamps helm.sh/resource-policy: keep, so the <name>-system upgrade — whose post-split chart no longer renders the Cluster — does not prune it as a removed resource. It compared the owning release name against the literal "seaweedfs-system". SeaweedFS is a user-creatable kind, so an instance named `foo` is owned by `foo-system` and was silently skipped: no re-own, no keep. The <name>-system upgrade then deleted the Cluster, CNPG took its PVC with it, and the filer metadata — the index for every object in that tenant's S3 — was destroyed. Data loss, not an outage: the volume PVCs still hold the bytes, nothing can find them. Verified live on the upgrade stand, which is a natural control group. All five tenants sit in the SAME failed-upgrade loop (<name>-system last-deployed is rev 1, a pre-split revision whose manifest still contains the Cluster, so every retry recomputes the same deletion): tenant-root/dsplit/l instance `seaweedfs` owner=seaweedfs-db keep Cluster alive tenant-named instance `foo` no Cluster at all Cluster GONE The only difference is that migration 43 fired for the default-named instances and skipped `foo`. That also answers whether `keep` is sufficient: three tenants survive the identical prune loop with it. Correction to the reported mechanism: this is the 1.4->1.5 db split (PR #2601, v1.5.0), not a 1.5->1.6 change — v1.5.3 and main are identical on every db-split file. And the split does protect the object; it just only protected the instances someone happened to name `seaweedfs`. - lib/seaweedfs-db-adopt.sh: shared hand-over matching the `-system` SUFFIX, so `foo-system` -> `foo-db` exactly as `seaweedfs-system` -> `seaweedfs-db`. Idempotent, and refuses a release named literally `-system` rather than annotating an owner of `-db` that no release would claim. - Migration 43 sources it: covers clusters upgrading from before 43 (1.4.x). - Migration 53 (new) re-runs it: migrations never re-run, so any cluster already at >= 44 ran the hardcoded version and is still exposed. As a pre-upgrade hook it lands before <name>-system re-renders, closing the window on THIS upgrade. This is the one that matters for 1.5.x -> 1.6. - targetVersion 53 -> 54. Tests fail on unfixed code: the non-default-name case produces no ANNOTATE at all against the hardcoded 43, while the default-name case passes — which is precisely the bug's shape. They drive the real migration scripts against a fake kubectl, following hack/migration-50-etcd-adopt.bats. LIMIT, stated plainly: a Cluster that is already deleted cannot be recovered by either migration — there is nothing left to re-annotate and the PVC went with it. Such a tenant needs seaweedfs-db restored from a backup. Runbook Step 0 adds the ownership audit that distinguishes at-risk from already-lost, and records that <name>-db can report Ready while its Cluster is gone (it rendered fine; a later <name>-system prune removed it and Flux has not re-checked) — so the audit trusts `kubectl get cluster`, not the HelmRelease status. Review round 2 — two ways this still lost the database: - OWNERSHIP IS NOT SAFETY. The helper skipped any Cluster already owned by <name>-db, treating ownership as proof the hand-over was done. It is not: where the hand-over was skipped, <name>-system prunes the Cluster and <name>-db RECREATES it under its own ownership with NO keep, while <name>-system's prune baseline still lists it — so the next reconcile deletes it again. That is the observed delete/recreate loop, and the skip walked straight past it. Verified on the stand: tenant-l and tenant-root are seaweedfs-db-owned AND their <name>-system deployed revision (rev 1) still contains the Cluster, so only keep saves them; tenant-fresh is seaweedfs-db-owned without keep but was installed after the split, so its baseline never had one. Telling those apart needs the release's deployed manifest, which this script cannot read cheaply or reliably. The costs are asymmetric — a needless keep leaves an orphan on delete (now reclaimed by the cleanup hook), a missing one loses the database — so keep is stamped on every Cluster owned by either side of the split. - FAIL OPEN, THEN STAMP. `set -euo pipefail` does not abort on a failing command substitution in a `for` word-list: the loop ran zero times, the script continued, and stamp_cozystack_version ran anyway. Migrations never re-run, so one throttle, RBAC hiccup, or not-yet-established CNPG apiservice behind the pre-upgrade hook permanently left every at-risk tenant exposed with no later migration to catch them. Reproduced: against the old helper a failing fleet scan still logged STAMP 54. Now every kubectl failure is fatal EXCEPT the two that genuinely mean "nothing to do" — the CNPG resource type not being served (the fail-open that IS load-bearing: a cluster without CNPG must still upgrade) and a Cluster vanishing between scan and read. An unreadable annotation is no longer indistinguishable from an absent one, and the unowned case now warns instead of passing silently. - ORPHAN ON DELETE. `keep` is permanent (removal deferred to the 1.7 batch migrations) and also survives the <name>-db release's own uninstall, so `kubectl delete seaweedfs <name>` left the CNPG Cluster, its Postgres pods and its PVCs behind — the cleanup hook selects app.kubernetes.io/instance=<name>-system, which the db PVCs (cnpg.io/cluster=seaweedfs-db) do not carry. Migration 53 widened that from pre-1.5 default-named tenants to every instance name, so the hook now deletes the Cluster explicitly; its PVCs carry ownerReferences to it and follow. - Tests assert the stamped VERSION (44 / 54), not a bare "STAMP": a wrong number would loop run-migrations.sh forever and the old assertion could not tell. Review round 3 — the reclaim introduced a data-loss path of its own: - DELETING ONE APP DELETED ANOTHER APP'S DATABASE. The db chart hardcodes the Cluster name `seaweedfs-db` for every instance, and the reclaim was scoped by that name in the namespace rather than by what the release owns. templates/seaweedfs-db.yaml gates the <name>-db HelmRelease on `topology != "Client"`; the cleanup hook had no such gate. Verified by render: `helm template s3-remote . --set topology=Client` emits ZERO s3-remote-db HelmReleases yet still emitted the Role granting delete on clusters/seaweedfs-db AND `kubectl delete cluster.postgresql.cnpg.io -n tenant-root seaweedfs-db`. So in a namespace running a server instance plus a Client, deleting the CLIENT destroyed the SERVER's filer metadata — CNPG takes the PVC via ownerReferences — for an app the operator never touched. Reachable the same way via a second Simple instance, whose <name>-db loses Helm's ownership check but which still installs and still carries the hook. main's hook was instance-scoped; this was new, and mine. Now gated exactly as seaweedfs-db.yaml is, and the Job re-checks meta.helm.sh/release-name == <release>-db at run time before deleting. The gate stops the permission being granted where it can never be legitimate; the runtime check stops the delete where it can. - `|| echo` turned any API/authz/admission failure into exit zero, so backoffLimit never retried and hook-delete-policy removed the only cleanup Job while the keep-protected Cluster stayed orphaned. A failed reclaim now fails the Job. - TEST SHELL. The migrations run under /bin/sh = busybox ash (the image is FROM alpine, run-migrations.sh is #!/bin/sh), but the tests drove them with bash — so the fail-closed guarantees were asserted in a shell that never runs them, which matters more than usual given the bug they cover was a fail-open. The scripts are now invoked via `sh` and the fake kubectl is POSIX sh. Verified directly against the production base image (busybox 1.37.0 / alpine:3.24): `set -o pipefail` is supported, every script passes `ash -n`, and the failing-fleet-scan path exits non-zero without stamping under real ash. The recipe is in the bats header. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
8499b5f4a1 |
fix(seaweedfs): move the naming-migration guard into the render that actually renames
The class-S guard from #3282 lives in extra/seaweedfs, but that render is not in the path of a platform upgrade: the <name>-system HelmRelease pulls system/seaweedfs from a platform-managed ExternalArtifact, so bumping the platform upgrades the release directly from the new artifact and applies fullnameOverride: seaweedfs without extra/seaweedfs ever re-rendering. On a tenant installed fresh on 1.5.x that stood up an empty chart-named set beside the live release-named data, deleted the renamed s3 Service, and left S3 readable only because the empty set's pods cannot start. Worse, the damage flips the guard's own classification: creating the chart-named artifacts makes $legacyData true, so the guard then reports D-split and sends the operator to runbook Step 3 — which quiesces the renamed set, i.e. the one holding ALL the data. - Add the ENFORCING guard to system/seaweedfs (templates/naming-guard.yaml): the only render guaranteed to sit between a platform upgrade and the tenant's workloads. It also refuses to render if the fullnameOverride pin is ever lifted. - Tell S from D by PVC AGE when both generations exist: PVCs are never recreated in place (StatefulSets are, by the adoption hook), so the older volume-PVC generation is where the data was born. Renamed-older ⇒ class S (damaged variant, new Step 2a), never D-split. - Fail closed: the release namespace is a lookup canary; an upgrade that cannot see the cluster refuses instead of classifying blind. Client-side installs (CI lint/unittest) still render — they never touch live data. - Mirror the same classification in extra/seaweedfs so the operator-facing SeaweedFS application reports the correct class. - Tests now model the ExternalArtifact upgrade path in the chart it actually runs through (the old ones mocked lookups only in extra/seaweedfs, which is why this shipped green), including the damaged-S tenant observed live. - Runbook: add S-damaged audit classification (PVC age), Step 2a cleanup of the empty chart-named set, and the D-wedged anti-affinity stall workaround observed on 3-node clusters. helm-unittest 1.0.3 ignores release.isUpgrade, so the canary refusal is verified with 'helm template --is-upgrade' instead of a unit test. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
055c8c755c |
chore(packages): drop duplicate test targets
Three package Makefiles declared an identical `test` target twice, so every invocation printed "overriding commands for target 'test'" and "ignoring old commands". Make names the lines in that warning: it was already running the second recipe and discarding the first, so the first declaration was dead in each case and removing it changes nothing but the noise. Which copy is dead is not the same in all three, so the edit is not mechanical. In monitoring the `.PHONY: test` line sits above the dead recipe rather than the live one; `.PHONY` applies to the target name wherever it is declared, so the declaration stays and only the recipe under it goes. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
myasnikovdaniil
|
4a00b502d7 |
fix(seaweedfs): adopt legacy workloads in place across the 4.31 rename (#3282)
## What this PR does Fixes a v1.5.0 upgrade regression where a tenant ends up with **two** SeaweedFS workload sets. **Root cause.** v1.5.0 bumped the vendored SeaweedFS chart `4.0.405 → 4.31.0`. Upstream changed resource naming: before 4.31 the chart named workloads after the **chart** (`seaweedfs-*`, ignoring the release name), and 4.31 names them after the **release**. The data-plane release is `<name>-system`, so every StatefulSet wanted to become `seaweedfs-system-*`. StatefulSet names are immutable, so Helm could not rename in place — it stood up a second, empty set beside the running one while the data stayed on the original `data1-seaweedfs-volume-*` PVCs. Where the node count let both sets run, they shared one `seaweedfs-s3` Service (identical pod labels) and one `seaweedfs-db` filer metadata database, so the empty set answered reads and the two master sets mixed volume IDs from independent sequences into one metadata table. **Fix.** Pin `fullnameOverride` in `system/seaweedfs` values so the rendered names stop following the release name, which is the convention already used by `cozy-proxy`, `flux-operator`, `grafana-operator`, `linstor-scheduler` and `victoria-metrics-operator`. The names go back to what every pre-1.5 cluster already runs, so the upgrade adopts the running workloads **and their volumes** in place. No data migration, no platform migration. **The one case that cannot be adopted.** A tenant installed *fresh* on 1.5.x wrote its data under the renamed workloads, so its volumes are not the chart-named ones. Pinning the chart name there would rename the workloads *away* from that data and bring up an empty cluster beside it. Helm cannot move data between PVCs, so `extra/seaweedfs` refuses to render for such a tenant and points at the recovery runbook, which re-binds the existing PVs onto the adopted PVC names (no copying — the claim is renamed, the volume is not touched). The guard is fail-closed: it fires only when renamed volumes are present and no adopted ones are. Two naming subtleties the guard has to respect, both verified by rendering the shipped 1.5 chart: - Before 4.31 the chart ignored the release name, so an instance running under a non-default name (`SeaweedFS` is a user-creatable kind) *also* stored its data on `data1-seaweedfs-volume-*`. Such a tenant is adopted, not blocked. - On 4.31 the fullname helper appends the chart name when the release name does not contain it, and the component helper truncates to 56 characters. A fresh 1.5.x instance named `archive` therefore wrote to `data1-archive-system-seaweedfs-volume-*`, and a long name loses the chart name from its tail entirely. The guard matches volumes by shape and by the chart labels on the StatefulSet instead of reconstructing those names, so both are caught. **On the duplicate set.** Once the fix reconciles, the `seaweedfs-system-*` StatefulSets and Deployments are no longer part of the release and Helm removes them on its own. What it cannot remove — PVCs it did not template, and cert-manager Secrets that carry no owner reference — is inert, and the runbook clears it explicitly. A pre-upgrade migration was considered and dropped: it would duplicate what Helm already does, while its only unique action (deleting PVCs) cannot distinguish "the duplicate never served" from "it served and is currently down", and any transient delete failure under `set -e` would fail the platform's pre-upgrade hook and block delivery of this very fix. ## Testing `helm unittest` — `system/seaweedfs` 21 tests, `extra/seaweedfs` 17 tests. The guard's `lookup` branches are covered by mocking the Kubernetes provider (the pattern already used in `core/platform` and `system/cozystack-api`): adopted-only, renamed-only, both sets, zoned volumes, a non-default instance name, a truncated long name, and an unrelated `data1-*-volume-*` PVC that must not trip the guard. The s3-consumer suite now re-creates the name/fullname divergence explicitly, so it keeps guarding that regression regardless of the shipped default. The remaining regression test for this class of bug is an upgrade E2E (seed a pre-4.31 named set, upgrade, assert a single set with data intact) — no fresh-install E2E or `helm template` can reproduce an upgrade over pre-existing state. That belongs in the chainsaw upgrade lane and is a follow-up. ### Screenshots N/A — no UI changes. ### Release note ```release-note fix(seaweedfs): adopt existing SeaweedFS workloads and volumes in place when upgrading past the 4.31 chart rename, instead of standing up a second, empty cluster beside them ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an audit & recovery runbook for SeaweedFS 4.31 rename impacts. * **Bug Fixes** * Stabilized SeaweedFS workload/PVC naming across upgrades, including non-default instance names. * Added preflight safeguards to prevent upgrade scenarios that could split or wedge existing data. * Updated post-delete cleanup to remove secrets from both legacy and current naming schemes. * **Tests** * Expanded Helm unit coverage for naming guards, cleanup behavior, and S3 service name consumers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrei Kvapil
|
fbbe4af7df |
refactor(network): drop ExposureClass/ServiceExposure for native loadBalancerClass (#3218)
## What Removes the `network.cozystack.io` API group — `ExposureClass` + `ServiceExposure` (#3081) — and its reconciler, in favor of native `Service` `type: LoadBalancer` + `loadBalancerClass`. ## Why Full rationale and discussion in #3164. For **managed applications** the chart already owns the Service, so: - `type: LoadBalancer` + `loadBalancerClass` + an admin-provisioned pool covers vendor-neutral external exposure without a dedicated API group and controller; - the `externalIPs` (node-IP) default never went through `ServiceExposure` (its backend is a no-op) and is left unchanged; - the only current consumer, the host ingress, keeps rendering `type: LoadBalancer` and just drops the `ServiceExposure`; the admin provisions the address pool / announcer out of band, as the install docs already assume. `ServiceExposure`'s one unique feature — dynamic per-namespace scoping of a shared VIP pool — is a convenience the managed-app path does not need, and it does not justify a second tenant-networking API group next to `sdn.cozystack.io`. ## Changes - Delete `api/network/v1alpha1` and `internal/controller/serviceexposure` (+ CRD definitions, e2e bats, controller registration, RBAC). - Host ingress: drop the `ServiceExposure` template; add an optional `publishing.loadBalancerClass` that sets `spec.loadBalancerClass` on the host-ingress LoadBalancer Service. - The `externalIPs` path and default behavior are unchanged. ## Verification - `go build ./...` — green. - `helm unittest` — `core/platform` 72/72, `extra/ingress` 25/25 (incl. host-vs-tenant `loadBalancerClass` scoping and the immutable-field migration guard). ## Migration None required. `network.cozystack.io` (#3081) has never shipped in a release — it is on `main` only (not backported to `release-1.5`; absent from v1.4.x / v1.5.x), so no upgrading cluster has these CRDs and this removes it before it is ever released. Clusters that track `main` and explicitly set `publishing.exposureClass` (opt-in, only since 2026-06-30) should provision their MetalLB / Cilium address pool themselves and set `publishing.loadBalancerClass`; any orphaned `cozystack-<class>` pool can be deleted by hand. Default clusters (externalIPs path) are unaffected. ## Status **Draft / RFC for the #3164 discussion** — this removes an API group that landed only days ago, so nothing here should merge before the design call. cc @lexfrei (author of #3081) and @lllamnyp (#3164). Deferred: when Kubernetes actually removes `Service.spec.externalIPs`, the node-IP default migrates via a MetalLB `IPAddressPool` with no `L2Advertisement` (allocate-but-don't-announce) — a separate change. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for configuring the ingress host Service with a native `loadBalancerClass` value. * Expanded chart tests to verify class wiring and immutability behavior across install and upgrade scenarios. * **Bug Fixes** * Removed the legacy exposure-class/service-exposure flow and related controller behavior. * Updated templates and CRD packaging so the platform no longer renders or depends on those deprecated resources. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
f6a03e93f8 |
fix(etcd): complete v1alpha2 transition for in-cluster 1.5→1.6 upgrades (#3270)
Consolidates the etcd `v1alpha2` transition fix for **in-cluster 1.5 → 1.6 upgrades** into a single PR, rebased on current `main`. Supersedes #3265 and #3261 — their commits are carried here (authorship preserved), so those PRs can be closed once this lands. Fresh installs use the new shapes directly and are unaffected, so fresh-install CI does not catch these; the 1.5 → 1.6 e2e upgrade path is the authoritative regression guard. ## What this PR does ### 1. Keep the legacy `etcd-headless` Service alive during adoption — `packages/extra/etcd` `etcd-migrate` adopts legacy clusters **in place**: the Pods keep their original `spec.subdomain: etcd-headless` and are dialed at `etcd-<i>.etcd-headless.<ns>.svc` until they roll onto the operator's native `<member>.etcd.<ns>.svc` domain. The v1alpha2 operator only creates the native `etcd` Service and the legacy `etcd-headless` Service is pruned, so those per-pod names stop resolving (`no such host`), `MemberList` fails, and `status.readyMembers` never populates — the `EtcdCluster` never goes `Ready` even though etcd is healthy and in quorum. We ship a chart-managed transitional headless `etcd-headless` Service (selector mirrors the operator's native `etcd` Service via `etcd-operator.cozystack.io/cluster`, `publishNotReadyAddresses: true`) — the DNS counterpart of the legacy `*.etcd-headless.<ns>.svc` SAN already kept for this window. Removable together with that SAN once members roll onto the native subdomain. ### 2. Survive the immutable controller-Deployment selector on upgrade — `packages/system/etcd-operator` (#3242) #2859 replaced the upstream etcd-operator chart with the cozystack-authored one, changing `Deployment.spec.selector.matchLabels`. `spec.selector` is immutable, so `helm upgrade` cannot patch the existing Deployment and the whole HelmRelease upgrade fails (`field is immutable`). A **pre-upgrade hook** (`templates/pre-upgrade-selector-fix.yaml`: ServiceAccount + Role + RoleBinding + Job) deletes the Deployment **only** when its live selector is the pre-1.6 one, so Helm recreates it cleanly. No-op when the selector already matches, never runs on fresh install. ### 3. Raise the operator's memory cold-start floor — `packages/system/etcd-operator` Steady-state working set is ~250Mi; the static `limits.memory: 128Mi` OOMKills a Pod that starts before the VPA admission webhook rewrites it. Raise the floor to `256Mi` (and VPA `minAllowed` to match) as defense in depth so the operator never depends on VPA timing to avoid crashing. ### 4. Make migration 50 (etcd adoption) robust in-cluster — `packages/core/platform/images/migrations/migrations/50` (was #3261) Exact server/peer cert-SAN match, Secret-gated wait on the adoption Secret, and in-cluster (IPv6-safe) kubeconfig handling, so the migration script drives the adoption reliably from inside the cluster. Covered by `hack/migration-50-etcd-adopt.bats`. ### 5. Hardening / review fixes (this PR's original scope) - **Hook `runAsUser: 65532`.** `pre-upgrade-selector-fix.yaml` set `runAsNonRoot: true` but no numeric `runAsUser`; `clastix/kubectl`'s image user is the non-numeric name `nonroot`, which the kubelet cannot verify against `runAsNonRoot` — so the hook Pod fails admission and silently blocks the very upgrade it exists to unblock. Adds `runAsUser: 65532`, matching the postgres-operator webhook-ready hook that runs the same image. - **Digest-pin the kubectl image.** The values comment claimed digest-pinning but shipped a floating `v1.32` tag. Reuses the digest postgres-operator vendors, adds the `renovate` annotation, and templates `repo:tag@digest`. - **Tests.** `etcd-operator/tests/selector-fix-hook_test.yaml` (hook wiring, weight ordering, namespaced least-privilege RBAC, numeric-non-root security context, digest-pinned image), `etcd-operator/tests/deployment_test.yaml` (256Mi cold-start floor), `extra/etcd/tests/etcd-cluster_test.yaml` (transitional `etcd-headless` Service). Each assertion was mutation-tested. ### 6. Derive the default S3 endpoint from the provisioned bucket — `packages/system/backupstrategy-controller` Derive the default S3 endpoint (and per-driver scheme / TLS / `secure_connection`) from the provisioned bucket Secret instead of requiring it to be hand-set, so etcd (and other) backup strategies get a working endpoint by default. Docs in `docs/operations/backup-classes.md`; covered by `tests/endpoint_form_test.yaml`. ### Verification - `helm unittest` green on current `main`: etcd-operator **18/18**, extra/etcd **18/18**, backupstrategy-controller **11/11**. - Behaviours 1 & 2 were reproduced and confirmed live on a 1.5.2 → 1.6.0-rc.1 adoption (3-node cluster) — recreating the `etcd-headless` Service took the adopted cluster to `readyMembers=3 / Available=True`. ```release-note fix(etcd): complete the v1alpha2 transition on in-cluster 1.5→1.6 upgrades — keep the legacy etcd-headless Service alive so adopted members stay resolvable, delete the pre-1.6 operator Deployment via a pre-upgrade hook to get past the immutable selector, raise the operator's memory floor so it does not OOM before the VPA scales it, and make the etcd adoption migration robust in-cluster. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved backup storage endpoint handling across supported backup drivers, including provisioned and external S3 storage. * Added compatibility support for legacy etcd pod discovery during migration. * Added an automated upgrade safeguard for etcd operator deployments. * **Bug Fixes** * Improved certificate SAN detection and etcd migration authentication. * Increased the etcd operator’s minimum startup memory to prevent early restarts. * **Documentation** * Clarified backup endpoint, TLS, and driver-specific behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
ca4f99411f |
fix(etcd): harden v1alpha2 transition — hook runAsUser, digest-pin kubectl, tests
Addresses review on #3265: - pre-upgrade-selector-fix hook: add runAsUser: 65532. clastix/kubectl's image user is the non-numeric name `nonroot`, which the kubelet cannot verify against runAsNonRoot: true — so the Pod fails admission and the hook never runs, silently blocking the very 1.5->1.6 upgrade it exists to unblock. Matches postgres-operator's webhook-ready hook (same image). - Digest-pin the kubectl image (the comment already claimed digest-pinning but shipped a floating v1.32 tag). Reuse the digest postgres-operator already vendors, add the renovate annotation, and template repo:tag@digest. - Tests (both charts have CI helm-unittest suites, none previously covered these): - etcd-operator/tests/selector-fix-hook_test.yaml: hook wiring, namespaced least-privilege RBAC, numeric-non-root securityContext (guards the runAsUser regression above), digest-pinned image. - etcd-operator/tests/deployment_test.yaml: assert the 256Mi cold-start memory floor so it can't silently drop back to an OOMKilling value. - extra/etcd/tests/etcd-cluster_test.yaml: assert the transitional etcd-headless Service (headless, publishNotReadyAddresses, member selector, client/peer ports). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
d51d6914d7 |
fix(etcd): keep legacy etcd-headless Service alive during v1alpha2 adoption
etcd-migrate adopts legacy clusters IN PLACE, so the Pods keep their original spec.subdomain: etcd-headless and are dialed at etcd-<i>.etcd-headless.<ns>.svc until they eventually roll onto the operator's native <member>.etcd.<ns>.svc domain. The v1alpha2 operator only creates the native `etcd` Service, and the legacy `etcd-headless` Service is pruned during the transition, so those names stop resolving (no such host), the operator's MemberList fails, and status.readyMembers never populates -- the EtcdCluster never goes Ready even though the etcd processes are healthy and in quorum. Ship a chart-managed transitional headless `etcd-headless` Service (selector mirrors the operator's native `etcd` Service via etcd-operator.cozystack.io/cluster, publishNotReadyAddresses: true). This is the DNS counterpart of the legacy *.etcd-headless.<ns>.svc wildcard already kept in the server/peer cert SANs for the same transition window -- the TLS half of the compat was done, the DNS half was missing. Safe to remove, together with that SAN, once the members have rolled onto the native `etcd` subdomain. Verified live: recreating this Service on an adopted 3-node cluster restored per-pod DNS and the cluster went readyMembers=3 / Available=True. Refs: #3243 Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
IvanHunters
|
c31a9f8ec3 |
fix(monitoring): promote oidc.users[].role to required + regenerate schema
The `OIDCUser` typedef in `packages/extra/monitoring/values.yaml`
declared only `@field {string} email`; `cozyvalues-gen` regenerated
`values.schema.json`, `README.md`, and the mirrored `openAPISchema`
on the `Monitoring` ApplicationDefinition WITHOUT a `role` field.
Consequences:
- The `@enum OIDCRole` declaration was orphan — referenced from
nowhere, so the schema's items object had `required: ["email"]`
and one property.
- A dashboard-driven CR would offer a form without any role picker;
a schema-conformant `{email}` submission would render into the
users-Job with `role: null`, `jq --raw-output` yields the string
`"null"`, the `POST /api/orgs/1/users` body carries
`"role":"null"`, Grafana returns HTTP 400, and the post-install
hook exhausts `backoffLimit: 6` — HelmRelease never becomes Ready.
- Unit + e2e tests all hardcoded `role: Admin` / `role: Viewer`, so
the contract mismatch never surfaced in CI.
Fix:
- Add `@field {OIDCRole} role - ...` to the `OIDCUser` typedef in
`packages/extra/monitoring/values.yaml` so cozyvalues-gen materialises
the field with enum + required.
- Regenerate `values.schema.json`, `README.md`, and mirrored
`openAPISchema` via `make generate`.
- Add `monitoring.oidc.assertUsersRoleShape` template-level guard
(same shape as `assertUsersEmailShape`) so a stale-schema CR or a
direct-helm invocation fails at render time with a legible error
instead of at hook execution with a Grafana 400.
- Consolidate all `spec.oidc.users` guards into a single
`monitoring.oidc.assertUsersPrologue` include and call it once from
`oidc-users-job.yaml`, ordering the incompat check
(`assertCustomSecretRefUsersEmpty`) before the per-entry role /
email regexes so operators see the more actionable message first.
Drop the duplicate include from `grafana.yaml` (Job template renders
in every mode anyway).
- Two new helm-unittest cases lock the regression:
`oidc.users entry missing role fails the render` and
`oidc.users entry with unknown role fails the render`.
- Point the previous `secretRef + users` incompat unit test at the
users-Job template since that's where the prologue now fires.
- `docs/oidc-grafana.md`: correct the failed-hook post-mortem window
claim — with `ttlSecondsAfterFinished: 3600` the Kubernetes TTL
controller reaps the Job + Pod 1h after the terminal condition,
not "until the next helm-upgrade".
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
86e54f1296 |
docs(monitoring): align OIDC docs with the actual mode-agnostic contract
Previous docs claimed "the chart forces skip_org_role_sync = true regardless of mode" — a promise the code did not keep in CustomConfig mode. The chart is now fixed; sync the docs so the guarantee reads truthfully and the incompatibility surface is explicit: - `docs/oidc-grafana.md`: rewrite the CustomConfig section so both chart-forced settings are named explicitly, the `merge` semantics are documented (operator can't override), and the `secretRef` + `users` incompatibility is called out with an explicit failure path. - `system/monitoring/values.yaml`: expand the `config`/`secretRef` comments so operators reading the values doc see the merge behavior and the secretRef limitation before they hit the render-time fail. Add `enabled: "true"` to the "typical keys" list — Grafana defaults `enabled: false` and an inline map without it is inert. - `extra/monitoring`: regenerated values.schema.json + README.md via `make generate` (cozyvalues-gen + hack/update-crd.sh) to pick up the `oidc.users[].email` doc-string touch; mirrored openAPISchema on the ApplicationDefinition follows. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
26e5f06d27 |
fix(monitoring): chart-force skip_org_role_sync + email lookup in every OIDC mode
The chart previously injected the two settings the users-Job's
contract depends on — `skip_org_role_sync = true` and
`oauth_allow_insecure_email_lookup = true` — only inside the
`mode: System` branch of `grafana.yaml`. `values.yaml` and
`docs/oidc-grafana.md` both promised the settings would be forced
regardless of mode, but under `mode: CustomConfig` the operator's
inline `auth.generic_oauth` map was written verbatim, both
settings were absent, and the users-Job's role assignments were
silently stranded (no lookup by email → new Grafana account minted
at login) or transiently overwritten (no skip_org_role_sync → any
`role_attribute_path` or Grafana's `auto_assign_org_role: Viewer`
overwrote the Job's role on each login).
Fix: merge a `$chartForcedOidc` dict on top of both the generated
System block and the operator's inline map (`merge` wins for
overlapping keys, so the operator can NOT override the two
settings). CustomConfig secretRef mode cannot receive the merge
because the ini fragment is authoritative — the chart now hard-fails
render when both `customConfig.secretRef.name` and non-empty
`spec.oidc.users` are set, forcing the operator to either switch to
inline config or manage authorization themselves inside the ini.
Also:
- Align the inner HR `timeout` on `packages/extra/monitoring/templates/helmrelease.yaml`
from 10m to 15m so it matches the outer cozyrds
`release.cozystack.io/helm-install-timeout: "15m"` and does not
fail while the users-Job is still inside its
`activeDeadlineSeconds: 900`.
- Route the users-Job image through `cozy-lib.image` so
mirrored / air-gapped installations resolve it (matches the
pattern in `packages/apps/mariadb/templates/hooks/cleanup-pvc.yaml`).
- Add `spec.ttlSecondsAfterFinished: 3600` on the users-Job so a
succeeded Job + Pod are garbage-collected on the next reconcile
interval rather than lingering until the next hook activation
(which never fires on release uninstall).
- Harden the users-Job shell:
* emails read NUL-terminated (`jq --raw-output0`) so an address
containing whitespace cannot corrupt the `for email in ...`
loop's field splitting;
* `/api/users/lookup?loginOrEmail=<email>` URL-encoded via
`jq @uri` so `&`/`=`/`?` in the local part parse correctly on
the Grafana side;
* prune loop switched from pipe-delimited fields to TSV inside
NUL-terminated records, with `cut -f` for extraction.
- Add a template-level assertion (`monitoring.oidc.assertUsersEmailShape`)
rejecting emails that do not match a conservative regex —
cheaper failure mode than a 400 from Grafana.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
0a65dd04a1 |
feat(monitoring): expose spec.oidc.users on the Monitoring CR
Extends the tenant-facing `Monitoring` CR with the new
`spec.oidc.users: [{email, role: Admin|Editor|Viewer}]` field
consumed by the users-Job introduced in the previous commit. Works
for both `System` and `CustomConfig` modes; ignored when
`mode: None`.
Regenerated `packages/extra/monitoring/values.schema.json`,
`README.md`, and the mirrored `openAPISchema` on the
`monitoring-rd` ApplicationDefinition through the standard
`make generate` (cozyvalues-gen + hack/update-crd.sh) pipeline.
Also refreshed the stale annotation comment on the CR that still
claimed the chart provisions three KeycloakRealmGroups.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
Ivan Okhotnikov
|
1679c84bb0 |
chore(monitoring): regenerate extra/monitoring README
Column widths in the OIDC table contract by one character once the tenant-facing `grafanaAdmin` row is gone. Regenerating the table via `make generate` (cozyvalues-gen) so the pre-commit `make generate` diff check in CI stays clean. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
e05ca91ff0 |
fix(monitoring): keep OIDC grafanaAdmin as a platform-only lever
The tenant-facing `Monitoring` CR exposed `spec.oidc.grafanaAdmin` via `packages/extra/monitoring/values.schema.json`, which the ApplicationDefinition mirrored into `openAPISchema`. A tenant with create/patch on `Monitoring` in their own namespace could set the flag on their release, promoting members of their `<clientId>-admin` Keycloak group to server-level `GrafanaAdmin` instead of the intended org-level Admin. Drop the field from the tenant surface. The platform bundle still flips it on for the `monitoring-system` component through `Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin` (the platform Grafana chart continues to read the value verbatim in `packages/system/monitoring/values.yaml`); tenant Grafana releases inherit the chart default `false` and stay at org-level Admin. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
ffeb8e5e4f |
fix(monitoring): close self-review blockers for the OIDC PR
Address the two blocking-class findings from the cozy-review pass on
this PR:
(1) Missing PackageSource dependency on cozystack.keycloak-operator
(MAJOR cold-install race).
Without this edge, the monitoring chart can reconcile before the
v1.edp.epam.com CRDs are registered on a fresh cluster — the
`.Capabilities.APIVersions.Has` guard in
templates/grafana/oidc-keycloak.yaml then silently drops all
Keycloak-side objects for mode: System, and Grafana's
auth.generic_oauth block points at a client Keycloak never
provisions. Add the ordering edge, plus a
sources_monitoring_application_dependson_test.yaml regression
guard so a future edit cannot drop it silently.
(2) allow_assign_grafana_admin driven by `.Release.Name ==
monitoring-system` (MINOR fragile coupling).
Replace the release-name sniff with an explicit
`oidc.grafanaAdmin: false` values field, and flip it on for the
platform Grafana release via
Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin
in packages/core/platform/templates/bundles/system.yaml —
mirroring the pattern monitoring-agents already uses for
per-component overrides. Tenant Grafana releases inherit the
chart default false and stay at org-level Admin.
Two new helm-unittest cases in packages/system/monitoring/tests/
lock the values-driven behaviour on both sides
(grafanaAdmin=true → Admin uplift, default → tenant stays at org
Admin), plus bundles_monitoring_grafana_admin_test.yaml in
packages/core/platform/tests/ regression-guards the Package's
component override so the platform release keeps its uplift on
future edits.
Also document the mode-toggle group-membership loss in
docs/oidc-grafana.md — mode: System → CustomConfig/None deletes the
three chart-owned KeycloakRealmGroups; membership is not restored on
toggle-back. Not a bug, but the operator UX cost of the toggle
deserves a bullet in Failure modes.
Regenerated artifacts (cozyvalues-gen + hack/update-crd.sh):
packages/extra/monitoring/values.schema.json
packages/extra/monitoring/README.md
packages/system/monitoring-rd/cozyrds/monitoring.yaml
Test suites still green: packages/system/monitoring 23/23,
packages/core/platform 81/81.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
|
||
|
Ivan Okhotnikov
|
4fe85d039c |
chore(monitoring): move OIDC schema source to packages/extra + regenerate
The Monitoring public API surface (openAPISchema in monitoring-rd
cozyrds) is generated from packages/extra/monitoring/values.yaml by
cozyvalues-gen + hack/update-crd.sh — the foundation commit
(186239044) instead wrote it by hand in
packages/system/monitoring-rd/cozyrds/monitoring.yaml, which the
pre-commit hook then reverts against every push.
Move the oidc annotations into packages/extra/monitoring/values.yaml
(matching the pattern used by the rest of the top-level fields) and
run `make generate` in that directory. That regenerates:
- packages/extra/monitoring/values.schema.json
- packages/extra/monitoring/README.md
- packages/system/monitoring-rd/cozyrds/monitoring.yaml
(openAPISchema + keysOrder, both including the new oidc keys)
No behaviour change — the values.yaml under packages/system/monitoring
still carries the same oidc defaults so `helm template` works
stand-alone, and helm-unittest keeps passing (22/22).
Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
|
||
|
Aleksei SviridkinandMyasnikov Daniil
|
72db724d5f |
fix(seaweedfs): delete certificate secrets of both naming schemes on module delete
The post-delete hook deleted only the secrets named after the renamed scheme, so a tenant running the adopted chart-based names leaked its cert and db secrets on delete. List both schemes, and derive the renamed name from the chart's fullname helper so an instance running under a non-default name (whose secrets are <name>-system-seaweedfs-*) is cleaned up too. Co-authored-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
052ce2e4a3 |
fix(seaweedfs): refuse to rename a running SeaweedFS away from its data
With the chart-based names pinned back, an upgrade adopts the running workloads and their volumes in place. Before 4.31 the chart ignored the release name, so this holds for an instance running under any name: its data sits on the chart-named data1-seaweedfs-volume-* PVCs either way. Two states cannot be adopted and stop the render instead, each pointing the operator at the matching recovery step: - Fresh on 1.5.x: only the renamed volumes exist, so adopting the chart-based name would rename the workloads away from that data and bring up an empty cluster. Helm cannot move data between PVCs, so the operator is sent to the PV re-bind procedure. - D-split: both sets exist AND the renamed volume servers are live, so they may hold objects written through the split endpoint. Adopting would strand them, so the operator is sent to reconcile the split first. A duplicate that never served (zero ready replicas) is safe to adopt and renders through. The guard mirrors the chart's fullname helper to recognise what 4.31 named this instance (the helper appends the chart name when the release name does not contain it, then truncates), and matches the renamed volumes by shape plus the chart labels on the StatefulSet rather than reconstructing the exact name. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
249f7b5a5e |
feat(platform, ingress): propagate operator wildcard certificate to per-tenant termination points (#2990)
## What this PR does The operator-provided wildcard certificate (`publishing.certificates.wildcardSecretName`) currently serves the root tenant only: everything is same-namespace, so the root ingress controller and Gateway read the operator's TLS Secret directly. Child tenants run their own ingress controller / Gateway in their own namespace under a default-deny policy and cannot read that Secret across namespaces, so they fall back to per-host ACME. This propagates the operator wildcard to per-tenant termination points by replicating the Secret into each tenant namespace that owns one, keeping the root-MVP invariant intact: the wildcard is deliberately distributed to tenant namespaces, but no tenant RBAC is widened — every consumer reads only its own-namespace copy. A new controller in `cozystack-controller` does the replication with no extra operator input. It reads the same platform values channel the consumers read (`cozy-system/cozystack-values`): the wildcard Secret name from `_cluster.wildcard-secret-name` and the publishing namespace from `_cluster.expose-ingress`. It mirrors that Secret into every tenant namespace whose `namespace.cozystack.io/ingress` or `namespace.cozystack.io/gateway` label equals its own name — i.e. the namespace runs its own controller / Gateway. Because the source is derived from the same value that makes the consumers reference it, the replica is created whenever the consumers expect it — no manual labelling and no upgrade hazard where a child controller references a Secret that nothing creates. Replicas are refreshed on rotation and pruned when a namespace stops terminating TLS. Teardown of every replica happens on exactly one trigger: clearing `wildcardSecretName` (explicit disable). A source that is merely absent or mistyped, or a transiently-missing values channel, leaves existing replicas in place, so a brief gap, a delete+recreate rotation, or a misconfigured publishing namespace never drops tenant TLS. The platform controller's own ServiceAccount gains Secret writes; tenant RBAC is untouched. The controller does not cache every Secret in the cluster: the manager's Secret informer is scoped to managed replicas and the values channel only, and the operator source / foreign collisions are read through the uncached APIReader. The source is not watched (its name is dynamic), so an in-place rotation is picked up by a bounded periodic resync while the publishing tenant serves the source with no lag. On the consumer side, each tenant ingress controller now sets `--default-ssl-certificate` to the wildcard Secret in its own namespace (`.Release.Namespace/<name>`), so a child tenant serves the replica instead of minting per-host ACME. The Gateway path already renders `certMode: existingSecret` with a same-namespace `wildcardSecretRef` for every tenant, so it consumes the replica with no chart change. ### Why not clustersecret-operator The issue proposed the shipped `clustersecret-operator`. It materializes inline template data into selected namespaces; it cannot mirror an existing source Secret, and it is not installed in any default bundle. Driving it would mean embedding the certificate and private key as literal data in a cluster-scoped object rendered from a Helm `lookup` — that broadcasts key material the design keeps off any shared channel, and a `lookup` is invisible to the Flux helm-controller digest so it would not re-render on rotation. Its namespace selector also cannot express "the namespace whose owner label equals its own name", so it would over-replicate the key into every tenant namespace. A small dedicated reconciler targets exactly the termination namespaces and keeps the key off the values channel. ### Coverage caveat Replication delivers the certificate bytes, not SAN coverage: a single `*.<root-host>` does not match `*.<child-apex>`. Operators serving child-tenant subdomains supply a certificate whose SANs cover those apexes; the docs spell this out. The replica carries the wildcard private key into each terminating tenant namespace — the same exposure as the per-host ACME Secret it replaces, now shared across tenants. ### Tests - Go fake-client tests for the reconciler: termination-only replication (ingress-owning and gateway-owning namespaces only, never inheriting ones, never over the source), copy markers, rotation, prune on namespace demotion, the non-destructive source-deletion contract, replica self-heal after out-of-band deletion, the publishing-namespace self-replication guard, disable-via-config teardown, source rename, foreign-Secret collision safety (no overwrite, no requeue, does not block siblings), transient-error requeue, non-TLS source ignored, absent-channel keeps replicas, publishing-namespace mismatch keeps and requeues, the cache-scope pin, the enqueue-wiring map, an `ownsTerminationPoint` table, and the active-only requeue contract. - helm-unittest for the ingress `default-ssl-certificate` wiring (including a child-tenant controller pointing at its own-namespace replica) and for the cozystack-controller ClusterRole's new Secret-write rule. ### Screenshots N/A — no UI changes. ### Release note ```release-note feat(platform): the operator-provided wildcard TLS certificate (`publishing.certificates.wildcardSecretName`) is now served by per-tenant ingress controllers and Gateways automatically — the platform controller replicates it into each tenant namespace that terminates TLS, with no extra operator input and no cross-namespace Secret read ``` Closes #2820 Part of #2811 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Wildcard TLS Secret now replicates from the platform to tenant namespaces that terminate TLS via their own ingress controller or Gateway, including child-tenant scenarios. * **Documentation** * Clarified configuration, timing, and SAN matching requirements for wildcard replication. * Updated gateway and ingress-nginx default certificate documentation for replica mirroring behavior. * **Bug Fixes** * Improved handling for missing/invalid source secrets (no unnecessary pruning) and safe collision behavior. * **Tests** * Added comprehensive reconciliation/replication test coverage. * Added Helm unittest target for the controller chart. * **Chores** * Updated controller RBAC to support wildcard TLS Secret replication. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
scooby87
|
f91841fce6 |
fix(monitoring): surface missing VictoriaLogs and gate dead-end vlinsert ExternalName (#3215)
## What this PR does Addresses #3181, where platform log ingestion could fail **silently**: fluent-bit shipped to `vlinsert-generic` while the monitoring `HelmRelease` reported `Ready`, and a `vlinsert-generic` `ExternalName` in `cozy-monitoring` dead-ended at a `tenant-root` VictoriaLogs that did not exist when the root tenant had monitoring disabled. Logs were dropped with no failing signal. Three complementary layers make the failure loud instead of silent: 1. **Readiness gate** — `packages/extra/monitoring/templates/helmrelease.yaml` now sets a `poller` `waitStrategy` and a `healthCheckExprs` entry on the `VLCluster` `status.updateStatus`. A missing/unhealthy log store makes the `monitoring-system` release **not-ready** instead of flipping `Ready` the moment the CR is applied. Mirrors the existing seaweedfs CNPG pattern. 2. **No empty-log-store hole** — `packages/system/monitoring/templates/vlogs/vlogs.yaml` fails the render on an empty `logsStorages`. With no entry no `VLCluster` exists, the readiness gate has nothing to match, and fluent-bit would silently drop logs — so this is now a loud misconfiguration. 3. **No silent fallback** — the `cozy-monitoring → tenant-root` `vlinsert`/`vminsert` `ExternalName` redirects in `cozystack-basics` are created only when the root tenant hosts monitoring. The template reads the live `Tenant/root` `spec.monitoring` via `lookup` (source of truth, so it cannot drift from the tenant's real state) and falls back to a new platform value `monitoring.rootEnabled` (via `_cluster.monitoring-enabled`) when `lookup` is empty (dry-run / first render). The default preserves the historical always-create behaviour. Covered by helm-unittest across all four affected charts (141 tests): readiness-gate fields, empty-vs-populated `logsStorages` contract, `ExternalName` present/absent fallback contract, and the platform wiring into the `cozystack-values` Secret. Also adds the missing `test:` target to `packages/extra/monitoring/Makefile` so its suite runs in CI. **Known limitation / follow-up:** the legacy platform monitoring release installed directly into `cozy-monitoring` via the `cozystack.monitoring` PackageSource cannot carry the readiness gate, because the engine's `ComponentInstall`/HelmRelease builder and the vendored `helm-controller` API v1.4.3 lack `waitStrategy`/`healthCheckExprs`. That source is not referenced by any active bundle; full coverage there needs a helm-controller bump plus engine support. ### Release note ```release-note fix(monitoring): surface missing platform VictoriaLogs as not-ready and stop the dangling vlinsert-generic ExternalName from silently dropping logs (#3181) ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a platform setting (`monitoring.rootEnabled`) to control whether root-level monitoring is enabled. * Monitoring redirect ExternalName services are now rendered only when root monitoring is enabled (defaulting to enabled). * **Bug Fixes** * Ensures an explicitly set `monitoring.rootEnabled=false` is preserved when generating platform configuration. * Prevents incomplete monitoring deployments by failing rendering when `logsStorages` is empty. * Improves VictoriaMetrics readiness by delaying “ready” until the operator reports an operational status. * **Tests** * Added/expanded Helm-unittest suites covering monitoring wiring, redirects, readiness gating, and `logsStorages` requirements. * Added a `make test` target to run chart unittests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Алексей Артамонов
|
f700c9043c |
fix(monitoring): backfill release label onto pre-existing VM/VL storage PVCs
Re-review on #3094 (IvanHunters, MAJOR): the volumeClaimTemplate label lands on PVCs only at creation — the StatefulSet controller never re-labels existing PVCs and vm-operator's PVC reconcile is resize-only. So on an already-deployed cluster (exactly the #3091 population) the pre-upgrade VM/VL storage PVCs stay unlabeled and the cleanup hook's selector misses them. Add migration 49 (mirrors the ClickHouse-keeper backfill in migration 48 for the sibling #3072). It relabels the pre-existing vmstorage/vmselect/vlstorage PVCs of every cozystack-managed VMCluster/VLCluster with apps.cozystack.io/application.name read from the CR's own spec.managedMetadata, so only monitoring's clusters are touched (a user's own VMCluster carries no such label) and the value always matches what the cleanup hook selects. Best-effort like #48: a skipped relabel is no worse than the pre-existing leak, so transient apiserver errors never abort the platform upgrade. Bump platform targetVersion 49 -> 50. Also correct the misleading comments (IvanHunters MINOR): spec.managedMetadata is never applied to PVCs by vm-operator at any current version, so the claim-template label is the only mechanism — do not imply a v0.71.0 crossover. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Алексей Артамонов
|
2a78e7b247 |
fix(monitoring): match cleanup PVCs by release label alone, drop managed-by
Re-review on #3094 (IvanHunters): the AND selector kept managed-by=vm-operator, which on the shipped vm-operator v0.68.4 is not reliably present on the storage PVCs (it lives on the StatefulSet selector/pod labels, not the claim template), making the delete fragile. Match by apps.cozystack.io/application.name=<release>-system alone: that label is stamped on the VM/VL claim templates (67b087ab) and copied onto the PVCs by IntoSTSVolume even on v0.68.4, is already release-scoped, and is absent from the CNPG db PVCs (grafana-db/alerta-db carry only policy.cozystack.io + cnpg.io/*), so those stay untouched. Regression guard now forbids re-adding managed-by to the selector. Also apply the review's discipline items: the cleanup Job keeps only before-hook-creation (a Failed run survives for post-mortem; hook-succeeded stays on SA/Role/RoleBinding, cf. #3072) and sets backoffLimit: 0 (best-effort, no retry). Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Алексей Артамонов
|
e01b725a48 |
fix(monitoring): stamp release-scoping label on VM/VL storage PVCs so cleanup matches
Re-review on #3094 (IvanHunters, lexfrei): the release-scoped selector added in ba5797f4 was a silent no-op on the shipped vm-operator v0.68.4. The apps.cozystack.io/application.name label lived only under the VMCluster/VLCluster spec.managedMetadata.labels, and v0.68.4 does not propagate managedMetadata onto PVCs (that lands only in v0.71.0). So the delete matched nothing and the #3091 storage leak stayed open. Stamp the label where it actually reaches the PVCs: on spec.{vmselect,vmstorage,vlstorage}.storage.volumeClaimTemplate.metadata.labels of the VMCluster/VLCluster CRs (packages/system/monitoring). vm-operator copies claim-template labels onto the PVCs (IntoSTSVolume) even on v0.68.4, so the existing cleanup selector (managed-by=vm-operator,apps.cozystack.io/application.name= <release>-system) now matches and stays scoped to this release. Verified by rendering the CRs (label resolves to monitoring-system, matching the hook). Also add the explicit subjects[].namespace on the cleanup RoleBinding (lexfrei non-blocking) and a helm-unittest suite asserting the label lands on all three storage claim templates. Corrected the misleading test comment about how the label reaches the PVCs. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Алексей Артамонов
|
69c9e69ff4 |
fix(monitoring): scope VM PVC cleanup selector to this release
Address IvanHunters review on #3094: the PVC delete selector was release-agnostic (-l managed-by=vm-operator), matching the VM storage PVCs of every VMCluster/VLCluster in the namespace. If a namespace ever hosted more than one monitoring release, tearing one down would delete the other's VictoriaMetrics/VictoriaLogs storage. Scope the selector to this release via apps.cozystack.io/application.name= <release>-system, the ownership label the monitoring chart stamps on the VM CRs' managedMetadata and vm-operator propagates onto the PVCs — mirroring the merged seaweedfs hook's release scoping. deletecollection is still not needed: kubectl delete -l lists then deletes individually. Tests: assert the release-scoped selector and add a regression guard that the delete never falls back to the bare managed-by=vm-operator selector. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Алексей Артамонов
|
b5d1279ccd |
fix(monitoring): make cleanup deletes non-blocking and guard delete scope
Address review on #3094: - Add --wait=false to both kubectl delete calls so PVC finalizers (pvc-protection on pods still terminating) can never block the post-delete hook and stall the HelmRelease teardown. - Add activeDeadlineSeconds: 120 on the cleanup Job as a backstop. - Tests: assert the non-blocking flag, plus negative assertions that the PVC delete stays label-scoped (no --all) and the secret delete stays name-scoped (no -l/--all). deletecollection RBAC verb intentionally not added: kubectl delete -l lists then deletes individually, so the existing delete verb suffices. BREAKING CHANGE: disabling monitoring now permanently removes its VictoriaMetrics/VictoriaLogs storage PVCs and the metrics/logs they hold. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> Assisted-By: Claude <noreply@anthropic.com> |
||
|
Алексей Артамонов
|
73937ddff9 |
fix(monitoring): clean up orphaned storage PVCs and TLS secrets on tenant module delete (#3091)
When the monitoring tenant module is disabled, the vm-operator storage PVCs (VictoriaMetrics/VictoriaLogs) and the alerta/grafana TLS secrets were left orphaned. Add a post-delete cleanup hook that deletes the PVCs by the managed-by=vm-operator label and the two TLS secrets by name, plus a test target and helm-unittest suite (the package had neither). CNPG PVCs are not targeted (CNPG cleans them). The cleanup Job is hardened (non-root, read-only root fs, dropped caps, resource limits); failures are logged, never blocking teardown. NOTE: disabling monitoring now permanently removes its storage PVCs and the data they hold. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> |
||
|
Alexey Artamonov
|
f9afc73096 |
fix(monitoring): surface missing VictoriaLogs and gate dead-end vlinsert ExternalName (#3181)
Platform log ingestion could fail silently: fluent-bit shipped to vlinsert-generic while the monitoring HelmRelease reported Ready, and a vlinsert-generic ExternalName in cozy-monitoring dead-ended at a tenant-root VictoriaLogs that did not exist when the root tenant had monitoring disabled. Logs were dropped with no failing signal. Three complementary changes: - Readiness (do not stay silently Ready): the monitoring-system HelmRelease now uses a `poller` waitStrategy and a healthCheckExpr on its VLCluster (status.updateStatus), so a missing/unhealthy log store makes the release not-ready instead of flipping Ready as soon as the CR is applied. Mirrors the seaweedfs CNPG pattern. - No empty-log-store hole: system/monitoring rejects an empty logsStorages at render time (helm fail). With no entry no VLCluster is created, the readiness gate has nothing to match, and fluent-bit would silently drop logs — so this is made a loud misconfiguration rather than a documented footgun. - No silent fallback: the cozy-monitoring -> tenant-root vlinsert/vminsert ExternalName redirects in cozystack-basics are now created only when the root tenant hosts monitoring, gated on _cluster.monitoring-enabled — fed from a new platform value monitoring.rootEnabled (default true, preserving the historical always-create behaviour). monitoring.rootEnabled is the single render-time source of truth: changing it re-renders cozystack-basics (helm-controller watches the cozystack-values Secret it flows into). A live `lookup` of Tenant/root spec.monitoring is deliberately NOT used: cozystack-basics is an operator-generated HelmRelease with drift detection off, so it re-renders only on install/upgrade/values-change — never on a reconcile tick or when a lookup result changes — and a lookup would go stale in both directions. Operators must keep monitoring.rootEnabled in sync with Tenant/root spec.monitoring. Covered by helm-unittest across all four charts: readiness gate fields, empty-vs-populated logsStorages contract, ExternalName present/absent and null-guard contract, and the platform wiring into the cozystack-values Secret. The CEL health predicates are exercised live, not in helm-unittest (which cannot evaluate CEL). Adds the missing `test:` target to packages/extra/monitoring/Makefile so its suite runs in CI. Known limitation (follow-up): the legacy cozystack.monitoring PackageSource that installs system/monitoring directly into cozy-monitoring cannot carry the readiness gate, because the engine's ComponentInstall/HelmRelease builder and the vendored helm-controller API v1.4.3 lack waitStrategy/healthCheckExprs. That source is not referenced by any active bundle; full coverage needs a helm-controller bump plus engine support. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
43104c9075 |
fix(ingress): guard the immutable loadBalancerClass migration on existing Services
spec.loadBalancerClass is immutable once a Service is type LoadBalancer, so setting, changing, or clearing publishing.loadBalancerClass on a cluster whose host ingress Service already exists would make Flux wedge on an API-rejected patch instead of migrating. Fail the render early with the exact recreate command whenever an existing type: LoadBalancer Service's class differs from the requested value; a fresh install (lookup nil) or a ClusterIP Service transitioning to LoadBalancer is left untouched. Note the caveat in the values comment and cover the guard with helm-unittest. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
a1748009ac |
fix(ingress): scope host loadBalancerClass to the external-facing ingress
publishing.loadBalancerClass is a host-publishing setting, but its _cluster.load-balancer-class block was emitted on any ingress taking the LoadBalancer branch — including tenant ingresses — unlike expose-external-ips and proxy-protocol, which gate on the host namespace. A tenant ingress would then be claimed by an LB controller the tenant never selected. Gate it the same way, and add helm-unittest coverage for the host/tenant split and the platform to _cluster wiring. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrei Kvapil
|
3dbc7d3662 |
refactor(network): drop ExposureClass/ServiceExposure for native loadBalancerClass
Remove the network.cozystack.io API group (ExposureClass + ServiceExposure) and its reconciler. Managed-application charts own their Service, so native type: LoadBalancer + loadBalancerClass + an admin-provisioned pool covers external exposure without a dedicated API group and controller. The host ingress -- the only consumer -- keeps rendering type: LoadBalancer and gains an optional publishing.loadBalancerClass to pick the LB controller; the externalIPs (node-IP) default path is unchanged. Rationale in cozystack#3164. Assisted-By: Claude Signed-off-by: Andrei Kvapil <andrei.kvapil@aenix.io> |
||
|
Andrey Kolkov
|
cc5d688e15 |
feat(platform): migrate to etcd-operator v1alpha2
Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Aleksei Sviridkin
|
d89ad70698 |
fix(cozystack-basics): validate legacy Ingress hostnames against the tenant apex
The Gateway (cozystack-gateway-hostname-policy) and Route (cozystack-route-hostname-policy) admission policies constrain tenant hostnames on the Gateway API path. Gateway API is opt-in and off by default, so in the default configuration tenant applications publish through a legacy networking.k8s.io/Ingress on the shared ingress-nginx, which had no equivalent policy. Add a fail-closed ValidatingAdmissionPolicy + Binding for Ingress in tenant-* namespaces. A hostname on spec.rules[].host or spec.tls[].hosts[] is allowed when it is within the namespace's own apex (namespace.cozystack.io/host), or when it is a concrete (non-wildcard) name entirely outside the platform root apex (_cluster.root-host); a hostname under the platform root apex but outside the namespace's own apex, and a wildcard taken through the outside-root path (which could match the platform apex itself), are rejected. The outside-root branch keeps the kubernetes app's Proxied exposeMethod working, since it routes a user-supplied external domain to a nested cluster. A rule with no host (absent or empty, an unbounded catch-all) and spec.defaultBackend are also rejected. The policy templates the platform root apex into its CEL and renders only when _cluster.root-host is set, so it is never shipped in a state that would admit every hostname. A tenant-* namespace missing its own apex label is denied. Cover the policy with helm-unittest (render, including the root-host, empty-host, and wildcard guards) and behavioral admission tests against the live apiserver (in-apex allow, external-domain allow, own-apex-wildcard allow, external-tls allow, under-root-outside-apex deny, prefix-adjacent-under-root deny, wildcard-matching-apex deny, hostless deny, empty-string-host deny, defaultBackend deny), and document it as an additional layer in the gateway security model. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Matthieu
|
0ea94e7d2f |
[bootbox] Add resources requests/limits to matchbox container
The matchbox Deployment in the bootbox package had no resources block, triggering a 'no CPU limit' conformance warning. Set limits 200m/256Mi and requests 50m/128Mi — modest sizing since matchbox is a lightweight HTTP server serving iPXE profiles and boot assets from ConfigMaps. Bootbox is a standalone chart (no vendored upstream), so the values are set directly in the template, matching the existing pattern in csi-resizer and s3manager. Signed-off-by: Matthieu <matthieu@hidora.com> |
||
|
Алексей Артамонов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> |
||
|
Алексей Артамонов
|
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> |
||
|
Aleksei Sviridkin
|
8f60b27f9a |
feat(ingress): serve replicated wildcard on per-tenant ingress controllers
In operator wildcard mode each tenant's ingress controller now sets default-ssl-certificate to the wildcard Secret in its own namespace, so a child tenant serves the replicated wildcard instead of falling back to per-host ACME. ingress-nginx reads the default certificate only from its own namespace, and each ingress HelmRelease renders in the namespace that owns the controller, so the reference stays same-namespace for the root and every child alike. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
c841c70866 |
feat(operator): replicate operator wildcard certificate to tenant namespaces
The operator-provided wildcard MVP serves the root tenant only: the root ingress controller and Gateway read a TLS Secret from their own namespace. Child tenants run their own ingress controller / Gateway in a separate, default-deny namespace and cannot read that Secret cross-namespace. Add a controller that mirrors the operator wildcard Secret into every tenant namespace that owns a TLS termination point (its namespace.cozystack.io/ingress or .../gateway label equals its own name). The source is identified by name from the platform values channel (_cluster.wildcard-secret-name) with no extra operator input; replicas carry a managed-by label and refresh on rotation. An individual replica is removed when its namespace stops terminating TLS; every replica is torn down only on an explicit disable (clearing wildcard-secret-name). A missing or mistyped source, or a transiently-absent values channel, keeps existing replicas, so tenant TLS never drops on a transient gap. The controller's Secret cache is scoped to managed replicas and the values channel, not every cluster Secret. Only the platform controller's ServiceAccount gains Secret writes — no tenant RBAC is widened, and every consumer reads only its own-namespace copy. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
31c15b282f |
feat(network): mechanism-agnostic LoadBalancer exposure (ExposureClass + ServiceExposure) (#3081)
## What this PR does Lays the foundation for migrating tenant LoadBalancer exposure off the deprecated `Service.spec.externalIPs` (KEP-5707) and, eventually, off MetalLB onto Cilium-native announcement — the mechanism-agnostic shape prescribed when the earlier chart-side approach (#2468) was reverted in #2603. It adds a new `network.cozystack.io/v1alpha1` API group and a controller that translates intent into whichever LoadBalancer backend the cluster admin configured: - `ExposureClass` (cluster-scoped, admin): binds a logical class name to a backend (`externalIPs` | `metallb` | `cilium` | `robotlb`) and its VIP address scope — the StorageClass/IngressClass pattern. - `ServiceExposure` (namespaced, tenant intent): "expose this Service via class X"; status reports the resolved backend and assigned IPs. - A `serviceexposure` controller renders the backend's pool + announcer once per class (MetalLB `IPAddressPool` + `L2Advertisement`; Cilium `CiliumLoadBalancerIPPool` + `CiliumL2AnnouncementPolicy`), scoped to the namespaces that opted in. Backends are pluggable behind a small Go interface, so adding BGP or another cloud LB is one implementation, not an API change. The controller never mutates the target Service — the chart still owns its shape. Platform wiring: `publishing.exposureClass.*` renders the admin `ExposureClass` and the host ingress publishes through it. The default (no class set, `externalIPs`) leaves every existing cluster's behaviour byte-for-byte unchanged; the MetalLB→Cilium announcer migration becomes a single `ExposureClass.spec.backend` change. This is the foundation only — Part of #2586. Converting per-app exposure, flipping Cilium `l2announcements` on, changing the default, and removing MetalLB are deferred to later phases. ### Release note ```release-note feat(network): add ExposureClass and ServiceExposure for mechanism-agnostic LoadBalancer exposure. Opt-in foundation; the default externalIPs publishing path is unchanged. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added new `ExposureClass` and `ServiceExposure` custom resources with support for multiple backends (externalIPs, MetalLB, Cilium, RobotLB), including optional L2 and address configuration. * Added Helm wiring and templates to enable service/ingress publishing via `ServiceExposure`, selecting a configured (or default) `ExposureClass`. * **Bug Fixes** * Improved reconciliation, finalizer-based cleanup, and status updates for readiness and assigned IP reporting, including safer handling when backends or managed resources change. * **Tests** * Added end-to-end and unit test coverage for backend rendering, readiness, and cleanup flows. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
4404c5c5eb |
fix(etcd): pin tenant etcd to v3.5.31 to restore consistent watches (#3122)
## What this PR does Tenant Kubernetes control planes get their etcd from the `extra/etcd` chart, which creates an `EtcdCluster` (`etcd.aenix.io/v1alpha1`) reconciled by etcd-operator v0.4.5. That operator hardcodes the etcd image to `quay.io/coreos/etcd:v3.5.12`, and the v1alpha1 CRD exposes no version field, so every tenant etcd runs 3.5.12. kube-apiserver only enables the `RequestWatchProgress` storage feature on etcd >= 3.5.13 (or >= 3.4.31). Kubernetes >= 1.31 enables `ConsistentListFromCache` — locked-to-true in 1.35 — which depends on it. On etcd 3.5.12 every consistent watch therefore fails inside the stream with HTTP 500 (`the required storage feature RequestWatchProgress is disabled`), which breaks watch-based clients of aggregated APIs in modern (k8s 1.35) tenants. This pins the etcd container image to a >= 3.5.13 patch release (v3.5.31) by setting `image` on the `etcd` container in the EtcdCluster's `spec.podTemplate.spec`. etcd-operator v0.4.5 strategically merges `podTemplate.spec` over its generated pod spec (matched by container name), so the override takes effect with no operator change — the same mechanism the chart already uses for the metrics port, probes, and resources. It restores consistent watches without waiting on the etcd-operator v1alpha2 migration (#2859), which is the longer-term fix. A helm-unittest suite asserts the etcd container carries the pinned image and is no longer left on the operator default v3.5.12. Addresses #3080. ### Upgrade impact On merge, etcd-operator reconciles every existing tenant `EtcdCluster` and rolls its StatefulSet from 3.5.12 to 3.5.31. This is a safe in-minor rolling patch upgrade (no etcd storage-format change, quorum preserved by the operator's rolling strategy), so expect a brief, sequential etcd pod restart per tenant and no data migration. ### Release note ```release-note fix(etcd): pin tenant etcd to v3.5.31 so kube-apiserver can enable RequestWatchProgress (consistent watches) on Kubernetes 1.35 tenants ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Pinned the bundled etcd image to `quay.io/coreos/etcd:v3.5.31` to improve compatibility with Kubernetes watch-related storage behavior. * Prevented deployments from reverting to the older default etcd image. * **Tests** * Added a new test suite to verify the template uses the expected pinned etcd image and blocks regressions. * **Chores** * Updated dependency automation to recognize and manage the pinned etcd image version. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
26b3110c59 |
fix(etcd): pin etcd image to v3.5.31 for consistent watches
etcd-operator v0.4.5 hardcodes quay.io/coreos/etcd:v3.5.12 and the
v1alpha1 EtcdCluster CRD exposes no version field, so the tenant
control-plane etcd runs 3.5.12. kube-apiserver only enables the
RequestWatchProgress storage feature on etcd >= 3.5.13 / >= 3.4.31, and
Kubernetes >= 1.31 enables ConsistentListFromCache (locked-to-true in
1.35), which depends on it. On etcd 3.5.12 every consistent watch fails
inside the stream with HTTP 500 ("the required storage feature
RequestWatchProgress is disabled"), breaking watch-based clients of
aggregated APIs in modern tenants.
Override the etcd container image through spec.podTemplate.spec, which
the operator strategically merges over its generated pod spec (matched
by container name), pinning a >= 3.5.13 patch release. This restores
consistent watches without waiting on the etcd-operator v1alpha2
migration.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
|
||
|
Алексей Артамонов
|
6e093f11bc |
fix(gateway): clean up orphaned ACME account secret on tenant module delete (#3089)
When the gateway tenant module is disabled, cert-manager's ACME account private-key secret cozystack-acme-account was left orphaned (no ownerRef). Add a post-delete cleanup hook that deletes it by name. Verified the secret is referenced only by the gateway module's own cozystack-gateway Issuer, so removing it on gateway teardown is safe. The cleanup Job runs non-root with a read-only root fs and dropped capabilities; RBAC is scoped via resourceNames. Signed-off-by: Алексей Артамонов <aleksei.artamonov@aenix.io> |
||
|
Aleksei Sviridkin
|
67692b4544 |
feat(platform,ingress): publish host ingress via ServiceExposure
Add the operator-facing knob that drives the new exposure model. When publishing.exposureClass.name is set, the platform renders a cluster-scoped ExposureClass binding the chosen backend (externalIPs, metallb, cilium, robotlb) and VIP address scope, and carries the class name through the cozystack-values Secret. The host ingress then renders a ServiceExposure for its ingress-nginx Service, which the cozystack-controller translates into the backend's pool and announcer. The default (empty name) renders neither the ExposureClass nor the ServiceExposure, so clusters on the externalIPs path are unchanged on upgrade. The ingress Service shape is untouched: it is already published as type LoadBalancer whenever externalIPs is empty. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
4218e4cd19 |
feat(ingress): issue shared DNS-01 wildcard on the publishing controller
When the platform signals shared wildcard issuance via _cluster.wildcard-issue, the publishing controller (Release.Namespace == expose-ingress) renders one cert-manager Certificate for <root-host> + *.<root-host>. The resulting Secret is served as the controller's default SSL certificate and the existing wildcard-secret-name consumption path makes the system services drop per-host ACME. The Certificate uses the DNS-01 ClusterIssuer that cert-manager-issuers already renders in dns01 mode, so no per-tenant Issuer is minted. Only the publishing controller renders it — the Secret must be same-namespace for ingress-nginx to read it. Part of #2811. Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
cozystack-ci[bot]
|
e1aaa90ed3 |
Prepare release v1.5.0
Signed-off-by: cozystack-ci[bot] <274107086+cozystack-ci[bot]@users.noreply.github.com> |