mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-30 08:37:31 +09:00
648eb42cae3f12bd367102048fa89089fc40407a
238
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Myasnikov Daniil
|
9bd8c17971 |
fix(ci): shape-filter the promote postcondition and close test gaps
Review of the previous commit found the postcondition itself could abort
a release. It grepped the bare "X.Y.Z-rc.N" substring across the tree, and
three live files carry such strings without being image refs:
kubevirt-csi-driver's go.sum pins github.com/golang/protobuf v1.4.0-rc.2
-- a version cozystack actually cut, so promoting v1.4.0 would have
failed on a Go checksum line -- metallb_test.yaml names v1.5.0-rc.2 in a
comment, and the console pnpm-lock.yaml carries rolldown@1.0.0-rc.15. The
remediation the error message gave would have made things worse: declaring
go.sum as a ref source would sed a checksum file.
Match the version only in image-reference position now: after an
image/repository/tag key with no intervening '#', or immediately before an
@sha256 digest. Narrowing it to "${RC}@sha256:" alone would have been
wrong -- cilium keeps `tag: v1.5.0` and `digest:` under separate keys, so
the version is not always adjacent to the digest. Verified against four
real rc versions that previously aborted.
The suite also only tested the fail-closed direction, which is why that
defect survived, and the collector had no completeness guard at all: a
first-party ref added at an unenumerated path on a `latest@` tag was
mirrored by nothing and passed all eighteen tests. Since the mirror's
failure mode is the worst of the three -- the published nightly points at
the private build registry -- that gap mattered most where coverage was
thinnest.
Added: a fail-open test using the three real rc versions above; a
collector completeness diff that discovers refs by content and fails on
any the enumeration misses; coverage for the OCI-artifact shape, whose
deletion previously left every suite green; and an unreadable-file test,
since a read error was being treated as "no match" and silently skipped.
The component-versioned test was vacuous -- its rc version shared no
substring with the kamaji tag it claimed to protect, so it passed by
construction. It now uses an rc version sharing kamaji's 0.19.0 prefix,
which a loosely-anchored implementation would corrupt.
Round-trip fixture discovery matched a combined 'ghcr.io/cozystack/
cozystack/' prefix, which skips keycloak-operator (host in a sibling
registry: key) and kubeovn (host in global.registry.address). It now
matches 'cozystack/cozystack' and stamps split `tag:` keys too.
Also documented two pre-existing gaps rather than half-fixing them: the
nightly host rewrite is a contiguous-substring replace and cannot reach a
split host, and a ref inside a gzip is invisible to every tool including
the postcondition, which skips binaries. Both are host-dimension defects;
the version dimension this change owns is unaffected, because the version
always lives in a tag key regardless of where the host sits.
Assisted-By: Claude <noreply@anthropic.com>
Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
|
||
|
Myasnikov Daniil
|
09323fff13 |
fix(ci): cover every image-ref storage shape in promote, retag and mirror
Promotion rewrote the rc version substring across the depth-2 package values.yaml plus packages/apps/kubernetes/images/*.tag alone, on the premise that the kubernetes app was the only one whose .tag files carry the cozystack version. Nine other .tag files do, and system/multus seds its ref straight into a vendored upstream manifest. v1.6.0 was therefore staged with 33 of 55 first-party refs still reading v1.6.0-rc.4. promote-retag.sh and nightly-mirror.sh had the same blind spot from the other direction: both scanned only the depth-2 values.yaml, so those images were never retagged to the stable version (30 refs selected before, 42 after) and never mirrored to the public registry for a nightly. A retag miss merely leaves an image short of a tag, since the digest still resolves inside one registry; a mirror miss is worse, because the host rewrite walks the same file list and a ref that is neither mirrored nor rewritten leaves the published tree pointing at the private build registry. The three call sites each carried their own idea of where a ref can live, which is how they drifted apart. Move that knowledge into hack/lib/image-refs.sh as the single enumeration -- image_ref_files() for the files, collect_image_refs() for the refs inside them -- and have all three source it, so a new storage location is declared once and reaches every consumer. The rewrite itself moves out of promote-rc.yaml into hack/promote-rewrite-tags.sh. Being workflow-inline is why this was only observable by cutting a release: there was nothing to unit test. It now also asserts its own postcondition, scanning wider than it rewrites and failing the promotion when any ref survives in a location the enumeration does not know about, rather than shipping it quietly. Component-versioned images (kamaji's v0.19.0-cozystack.N) and third-party pass-through images do not ride the cozystack version line and are deliberately left alone; a test pins that so a future fix here cannot over-reach. capi-providers-cpprovider is a documented known gap: its ref is also embedded in a gzipped copy that the chart actually ships, and rewriting only the readable copy would diverge the two. docs/agents/image-refs.md records the contract -- three tag classes, three storage shapes, the invariants, and which consumer does what. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Andrey Kolkov
|
e73ad68a85 |
fix(backups): support cnpg-operator with barman plugin (#3313)
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `backport` if it's a bugfix that needs to be backported
to a previous version.
-->
## What this PR does
Fixes #3300. Migrates CNPG Postgres backups off the deprecated native
`spec.backup.barmanObjectStore` (removed in CNPG 1.29; the
`barman-cloud-*` binaries it needs are absent from the `standard` image
variant keycloak-db pins, and the bare system-flavor tags apps/postgres
pins are deprecated upstream) onto the [CloudNativePG Barman Cloud
plugin](https://github.com/cloudnative-pg/plugin-barman-cloud), across
every Postgres backup surface: `apps/postgres`, `system/keycloak`, and
the `cozy-default-cnpg` BackupClass strategy driver. The chart renders
`spec.plugins` + a `barmancloud.cnpg.io/ObjectStore` CR; the
backup-controller driver SSA-applies an `ObjectStore` and patches
`spec.plugins`; `Backup`/`ScheduledBackup` use `method: plugin`. The
`postgres-operator` chart vendors and co-locates the
`plugin-barman-cloud` operator (with its cert-manager-issued mTLS) so
CNPG can discover it.
One platform-level change is required for the plugin to run and ships
with this PR (see the upgrade notes in
`docs/operations/backup-classes.md` and the release note below): the
CloudNativePG operator is bumped to 1.28.1 (also fixing an operator/CRD
version skew that broke all backups). The other prerequisite — the
LINSTOR scheduler admission webhook stripping
`initContainers[].restartPolicy` cluster-wide, which broke the plugin's
restartable sidecar — is already fixed on `main` (
|
||
|
Myasnikov Daniil
|
b4f98af940 |
docs(seaweedfs): fail closed in the rename runbook's reclaim-policy handling
The 1.5.x->1.6 rename recovery runbook deletes each volume PVC after switching its PV to Retain, but the switch was never gated on success. With no `set -e`, a failed `kubectl patch ... Retain` left the PV at Delete and the following `delete pvc` took the data with the claim. Gate the delete on both the annotation record and the Retain patch succeeding, aborting the re-bind loop otherwise, so no PVC is deleted while its PV is unprotected. In the Step 5 restore loop, chain the annotation removal after the restore patch with `&&` so a failed patch keeps the saved policy for a retry. Address review feedback from coderabbitai on docs/operations/seaweedfs-431-rename-recovery.md. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov Daniil
|
95fcc76902 |
docs(seaweedfs): scope the rename runbook to the supported instance name
The tenant module is the supported way to run SeaweedFS and it hardcodes the instance name: packages/apps/tenant/templates/seaweedfs.yaml creates the HelmRelease as `seaweedfs`, and a tenant can only enable or disable it. Every shell selector in this runbook assumes that name, so say so up front, and route instances created directly against the API under other names to escalation instead of through loops whose name filters provably cannot see truncated claims. The ~40-char zone/pool key limit of the guard's reconstruction is called out as an accepted limit for default-named instances too. Two content fixes ride along, both for findings the final review pass raised: - The emptiness check told the operator to kubectl exec the candidate's own volume pod — which for a D-wedged duplicate can never start; that is the definition of the class, so the check was unexecutable exactly where it matters most and every D-wedged tenant funneled to escalation. Mount each bound candidate claim read-only in a scratch pod instead (a Pending claim has no PV and is empty by construction). The weed shell cross-check stays on the authoritative master, which is running. - Step 1's description of the audit now matches the audit itself: direction is decided by the relative strictly-newer PV rule (the same precondition Step 2a enforces), and an interrupted re-bind reads as overlapping vintages with no candidate — not by a clock window anchored on first_deployed. 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
|
ae93356ed1 |
fix(seaweedfs): name cluster-scoped RBAC per namespace, not after the pinned fullname
ClusterRoles and ClusterRoleBindings are cluster-scoped: one object per name, cluster-wide. This chart installs once per tenant, so every cluster-scoped name it renders must be unique per install. Four of them were not. Upstream 4.31 switched them from global.serviceAccountName to seaweedfs.fullname. That is fatal in combination with the #3282 pin: fullnameOverride: seaweedfs is what adopts the running NAMESPACED workloads across the rename, so fullname is by construction the SAME string in every tenant. Every SeaweedFS instance in the fleet therefore renders seaweedfs-objectstorage-provisioner, seaweedfs-rw-cr and their bindings — one object, five claimants. NOT a #3282 regression. Rendering v1.4.5, v1.5.3 and main for the same tenants: v1.4.5 tenant-root-seaweedfs-* tenant-named-seaweedfs-* unique v1.5.3 seaweedfs-system-* foo-system-seaweedfs-* COLLIDES (identical for every default-named instance; a non-default instance name was the only thing that escaped) main seaweedfs-* seaweedfs-* COLLIDES (nothing escapes: the pin removes the last distinguishing input) So the collision shipped in 1.5.0 with the 4.31 bump; #3282 widens it from "every default-named instance" to "every instance". Observed live on the upgrade stand: one ClusterRole/seaweedfs-objectstorage-provisioner annotated to a single owning tenant, four other tenants' COSI provisioners left with no RBAC, and tenant-dsplit's helm history recording Upgrade failed: original object ClusterRole "seaweedfs-objectstorage-provisioner" not found The binding is the sharp end: its subject names a ServiceAccount in the release namespace, so the single live ClusterRoleBinding grants cluster-wide secrets RBAC to whichever tenant reconciled last, and silently revokes it from the rest. Fix: put the four names back on global.seaweedfs.serviceAccountName, which extra/seaweedfs already sets to <namespace>-seaweedfs and which cosi-provisioner-sa-name.patch already uses for the ServiceAccount and the binding's subject — the cluster-scoped names were simply left behind. Three of the four come out byte-identical to pre-4.31, so a 1.4.x tenant adopts them in place; the fourth (rw-crb) replaces upstream's username-shaped system:serviceaccount:<sa>:default and is pruned from the release's own manifest. The namespaced pin is untouched: workloads still render seaweedfs-master/-filer/ -volume, so #3282's adoption is intact. Uniqueness now rests on a value, so assert it rather than trust it: templates/cluster-scoped-rbac-guard.yaml refuses to render when the name would not carry the release namespace. It sits behind the same namespace canary as the naming guard, because system/seaweedfs's own values.yaml ships a placeholder that only extra/seaweedfs overrides — a client-side render must not trip on it. Tests fail on unfixed code: cluster_scoped_names_test asserts two tenants get two names, and unpatched both render seaweedfs-objectstorage-provisioner (5 failed, 0 passed). They test the vendored chart templates directly, which is where the rename happens. The guard includes seaweedfs.compat before reading. extra/seaweedfs sets the OLD flat key global.serviceAccountName, and the shim is what folds it into the canonical global.seaweedfs.serviceAccountName; without it the guard compares against this chart's placeholder default and refuses every real upgrade. Caught by helm upgrade --dry-run=server against the stand, not by unit tests — the mocks never carry the flat key. Verified live with --dry-run=server on the stand: tenant-l (installed pre-4.31, never upgraded) already owns tenant-l-seaweedfs-objectstorage-provisioner and tenant-l-seaweedfs-rw-cr, and the fixed chart renders exactly those — adopted in place, no ownership error. tenant-root renders tenant-root-seaweedfs-* and also dry-runs clean. Leftovers on the release-based names that a 1.5.x tenant passed through are inert once every tenant is upgraded (no release renders them any more) but are not always pruned, since more than one release claimed them; the runbook adds the sweep. Review round 2: - The uniqueness guard used `contains .Release.Namespace $sa`, a substring test that accepts namespace `tenant-a` for service account `tenant-ab-seaweedfs` — a different namespace's. Anchored to hasPrefix "<namespace>-". It still asserts the VALUE the names are built from, not the rendered names, so it cannot catch the chart being reverted to name them after the pinned fullname; cluster_scoped_names_test.yaml covers that axis and is what actually bites. - Added the case that would have caught the compat bug: every test set the CANONICAL global.seaweedfs.serviceAccountName, while production sets the OLD FLAT global.serviceAccountName that only seaweedfs.compat folds in. Deleting the include left the suite passing 5/5 while every real upgrade refused. The new case sets the flat key; with the include removed it now fails with the production error. - Runbook sweep missed the fourth pre-4.31 name: 4.31 renamed the master-rw binding from upstream's username-shaped system:serviceaccount:<sa>:default to <sa>-rw-crb, and the old one carries neither suffix nor a tenant prefix, so the grep could not see it (live on the stand as system:serviceaccount:tenant-l-seaweedfs:default). Also documents the rolling- upgrade window: Helm prunes by name without checking ownership, so the first tenant onto the fixed chart deletes shared objects a not-yet-upgraded tenant is still bound through — COSI provisioning 403s until they reconcile, S3 traffic unaffected. Review round 3: - hasPrefix "<namespace>-" was still too loose. It accepts `tenant-a-b-seaweedfs` for namespace `tenant-a` — which is ALSO the natural value for namespace `tenant-a-b`, so two namespaces would accept one string and collide on the very object this guard exists to keep unique. extra/seaweedfs sets exactly `<namespace>-seaweedfs`, so assert equality with it and nothing looser. 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> |
||
|
Andrey KolkovandClaude Fable 5
|
f1627354b4 |
docs(contributing): point the ansible migration note at loadBalancerClass
The downstream-coordination checklist still directed contributors at an
"externalIPs to exposureClass" migration; exposureClass was removed in
|
||
|
Andrey KolkovandClaude Fable 5
|
572492c276 |
docs(backups): scope the barman-binaries claim, drop the stale linstor webhook note
The db.yaml plugins-block comment still carried the over-generalized claim that the pinned postgresql image variants no longer ship the barman-cloud-* binaries; mirror the scoped wording from docs/operations/backup-classes.md (only the standard variant keycloak-db pins lacks them; this chart's bare system-flavor tags still ship them but the flavor is deprecated upstream). After the rebase onto main this PR no longer touches linstor-scheduler: main re-vendored it to upstream chart 0.3.1 / extender v0.3.6, which keeps the admission webhook and stops stripping unknown pod fields (including initContainers[].restartPolicy the barman sidecar needs). The upgrade note claiming the webhook is removed and locality scheduling becomes advisory is rewritten as a hold-back warning. Also: afterwards -> afterward (American English, review nit). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
1ea7ef54c0 |
docs(backups): correct the CRD-upgrade and barman-binaries claims
Review follow-ups: the vendored cloudnative-pg chart ships the CNPG CRDs as ordinary release manifests (crds.create), so helm upgrade updates them with the operator — the out-of-band kubectl apply claim was wrong (the historical skew came from the image.tag pin, which this PR removes). And the missing barman-cloud-* binaries only affect the standard image variant keycloak-db pins; apps/postgres pins bare system-flavor tags that still ship them — its breakage was the operator/CRD skew. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Fable 5
|
8967205353 |
docs(backups): document the mixed-version BackupJob window caught by the e2e
Running the gated postgres backup e2e against a cluster whose apps still
render the pre-upgrade chart surfaced the CNPG webhook rejection
("Cannot enable a WAL archiver plugin when barmanObjectStore is
configured"): the driver cannot attach spec.plugins while the native
config is still present. Document the window and its resolution in the
upgrade notes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey KolkovandClaude Opus 4.8
|
604e79437b |
docs(backups): disclose CNPG-bump + LINSTOR scheduler change; fix stale e2e fixture
Review follow-up (cozystack#3300): - docs/operations/backup-classes.md: add an operator-facing upgrade note for the two platform-wide changes this migration carries — the CloudNativePG operator bump to 1.28.1 (with the CRD-skew fix and the Helm-CRD-upgrade caveat) and the LINSTOR scheduler admission-webhook removal, which makes data-locality scheduling advisory cluster-wide (spec.schedulerName no longer auto-assigned; opt in via explicit schedulerName). These affect more than Postgres backups and were previously only noted in a chart values.yaml comment. - hack/e2e-chainsaw/postgres/postgres.yaml: drop stale restic-era backup fields (s3Region, s3Bucket, cleanupStrategy, resticPassword) that no longer exist in the Postgres backup schema; use current fields (destinationPath, endpointURL, retentionPolicy, 6-field schedule). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Andrey KolkovandClaude Opus 4.8
|
302a0193f1 |
docs+test(backups): update upgrade notes for the plugin flow + cover recovery rendering
Review follow-ups for the CNPG barman-cloud plugin migration (cozystack#3300): - docs/operations/backup-classes.md: the 'Upgrade notes from chart-managed backups' section still described the removed native spec.backup.barmanObjectStore behavior. Rewrite it for the plugin flow (chart renders spec.plugins + a barmancloud.cnpg.io/ObjectStore; the driver SSA-applies an ObjectStore and patches spec.plugins), and document the real upgrade hazard: the plugin-barman-cloud operator + cert-manager must be present in cozy-postgres-operator or the Cluster references an unregistered plugin and backups/WAL archiving stall. - apps/postgres backup_storage_test.yaml: add a positive bootstrap-recovery rendering test. Asserts externalClusters[].plugin references barman-cloud.cloudnative-pg.io with barmanObjectName=<release>-recovery and serverName on the plugin parameter, and that the -recovery ObjectStore carries spec.configuration but NOT serverName (the plugin forbids it there). Previously the recovery path had only fail-fast guard tests, no positive coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
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 --> |
||
|
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
|
7e34e87cd9 |
docs(backups): document derive-from-Secret S3 endpoint behavior
The `backupstrategy-controller.endpoint` helper changed how the S3 endpoint is resolved (for a provisioned bucket it derives from the COSI system Secret and forces https://, with `backupStorage.endpoint` demoted to a fallback), but docs/operations/backup-classes.md still described the old single-verbatim-value behavior and directly contradicted the code: - the "Endpoint format per driver" intro said the templates adapt "the single backupStorage.endpoint value"; - it claimed drivers "pull from backupStorage.endpoint in chart values, not from the Secret" — now precisely backwards for provisioned buckets; - the values-table `endpoint` row omitted the fallback/derive semantics. Rewrite those to match the new helper + the values.yaml comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Ivan Okhotnikov
|
0164a6afa4 |
fix(monitoring): set groups_attribute_path=groups so allowed_groups actually works on Grafana v11.x
Verified end-to-end on dev3 with Grafana v11.6.15: OAuth flow reaches Grafana, id_token and userinfo response both carry the correct `groups` claim (`[cozystack-cluster-admin, tenant-root-super-admin]` for kvaps), `allowed_groups` in grafana.ini lists `tenant-root-super-admin`, and login still fails with `user not a member of one of the required groups`. Grafana debug log shows `Attributes: map[]` — the extracted user's Groups slice is empty. On v11.x+ Grafana does not auto-map the `groups` claim onto user.Groups unless `groups_attribute_path` is set explicitly (the field is documented as optional but the gate is a silent no-op without it — a claim-name-string is not enough). Fix: hard-wire `groups_attribute_path=groups` in the chart-forced System-mode block alongside `allowed_groups`. "groups" points at the top-level `groups` array from userinfo, matching the shape Keycloak's oidc-group-membership-mapper emits (verified on the same dev3 run). Two helm-unittest cases updated to assert `groups_attribute_path` lives next to `allowed_groups` in both the users-map-active and empty-users branches (rejecting the silent no-op regression). Docs also updated to name the requirement. Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
874ca9427f |
fix(apps/kubernetes): close cross-tenant kube-apiserver OIDC hole via CEL claim gate
Symmetric with the Grafana-side allowed_groups gate — round-3 review from lllamnyp on PR#3176 covered both surfaces, and #3044's original System-mode wiring shipped without this guard. The rationale is identical: audience binding stops REPLAY of a minted token across tenant kube-apiservers, but nothing stopped Keycloak from ISSUING a usable token to a caller outside the tenant. Absent a claim-side gate, any authenticated cozy-realm user could `kubectl oidc-login` against another tenant's cluster and land at `system:authenticated`. RBAC default-denies named-resource access, but `system:authenticated` still leaks discovery (kubectl api-resources, OpenAPI schemata) — tenant-alice could enumerate tenant-bob's cluster's CRD shape and built-in resource surface. Fix: add a CEL `claimValidationRules` entry to the rendered AuthenticationConfiguration (System mode only) that requires at least one of the release's four tenant-scoped Keycloak groups (`<namespace>-{view,use,admin,super-admin}`) to be present in the token's `groups` claim. The `has(claims.groups) &&` guard is required — CEL evaluation on a missing claim surfaces as HTTP 500 from the authenticator instead of a clean 401. CustomConfig mode is not touched — the tenant's own AuthenticationConfiguration is authoritative and the tenant is responsible for claim-side guards on their own issuer. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
b002c967ac |
fix(monitoring): close cross-tenant Grafana OIDC hole via tenant-group gate
Round-3 review from lllamnyp: any user in the shared `cozy` Keycloak
realm could sign into any tenant's Grafana. The audience mapper stops
replay of a minted token across releases, but does not gate who may
mint one — tenant-alice could complete the OAuth flow against
tenant-bob's Grafana. With `spec.oidc.users` non-empty the earlier
`allow_sign_up=false` fix mostly closed the door (though tenant-alice
still hit `error 401` after landing in Grafana, revealing the release
exists); with empty users under mode: System — a config the schema
permits and the docs describe as hands-off — the login succeeded and
minted a Viewer account for every cozy-realm identity.
The fix uses the KeycloakRealmGroups the tenant chart already
provisions (`<namespace>-{view,use,admin,super-admin}` in
packages/apps/tenant/templates/keycloakgroups.yaml) — no new
directory objects owned by the Monitoring release. Two chart edits:
1. oidc-keycloak.yaml: add the platform-managed realm-default `groups`
scope to `defaultClientScopes` explicitly. EDP's KeycloakClient
does NOT merge realm-level defaults into `defaultClientScopes`, so
the scope must be listed here or the `groups` claim would be
missing from the token.
2. grafana.yaml: in System mode, unconditionally emit
`allowed_groups: <ns>-view <ns>-use <ns>-admin <ns>-super-admin`
in `auth.generic_oauth` and add `groups` to `scopes`. Grafana
rejects any login whose token does not carry at least one of the
groups.
The gate is chart-owned and does not depend on the users-map — with
`users: []` tenant members still get a self-service Viewer through
`auto_assign_org_role`, cross-tenant users get 401 in Keycloak's OAuth
callback. Docs updated to reflect the two-layered contract
(tenant-membership gate + optional users-map).
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
f64ba91872 |
fix(monitoring): do not render users-Job when spec.oidc.users is empty
Address PR#3176 review round-2 gap-2: the users-Job's prune pass deletes every non-admin Main-Org member whose email is not in the desired list. When the desired list is empty every helm-upgrade wiped the whole org. Three empty-users states used to fall into this trap: * customConfig.secretRef: users[] is forbidden by assertCustomSecretRefUsersEmpty, so the desired list was structurally always empty and the chart wiped an org whose auth.ini fragment the operator advertised as authoritative. * System with users[] unset: the operator opted into OIDC but not into the chart-managed users-map, yet the Job still pruned every Main-Org member on their behalf. * CustomConfig-inline with users[] unset: same shape as System. The mode=None guard (from an earlier round) already had the same shape; extend the render gate to cover the other empty-list doors — now the Job renders only when `mode != None AND users non-empty`. The trade-off: taking `users:` back to `[]` no longer prunes the last entry — matching the already-documented `System → None` posture — called out explicitly in the docs. Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
1422495b88 |
fix(monitoring): do not render users-Job when mode=None (upgrade safety)
Caught by dev3 end-to-end test on the PR working tree
(cozystack-pr-test skill): the users-Job was rendered in every mode
including `None` on the rationale that a `mode: System → None`
switch should garbage-collect OIDC-provisioned local accounts. That
rationale ignored the upgrade path from the pre-PR chart: an
existing tenant that never had `spec.oidc` on their Monitoring CR
would receive `mode: None` on first render under the PR, the Job
would fire on every Flux reconcile of the outer HR, and its prune
pass would DELETE every non-admin Main-Org member — including users
the tenant added by hand through the Grafana UI on the pre-PR chart.
Silent breaking change with data loss.
Wrap the whole Job resource in `{{- if ne $mode "None" }} … {{- end }}`
so `mode: None` = "chart owns nothing in Grafana orgs, we don't
touch what's there". Operators who need to prune OIDC-provisioned
accounts after switching System | CustomConfig → None do it
themselves through the Grafana UI or admin API — same posture as
"we don't own the Keycloak directory objects" from the earlier
architectural change.
The `assertUsersPrologue` include stays outside the `if` so an
operator listing users in `mode: None` still gets the schema and
email-shape validation at render time — the Job body just doesn't
emit.
Test coverage:
- Flip the previous "mode=None still renders the users-Job so a
mode switch can prune stale members" case to
`mode=None does NOT render the users-Job (upgrade safety guard)`
as a regression lock.
- Add `mode=None with non-empty users still fails render` covering
the prologue-only-fires-on-schema-issue path.
Docs: rewrite the "Users and RBAC" section step 3 to name the
upgrade-safety motivation and the manual-cleanup handoff.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
|
||
|
IvanHunters
|
02305eae59 |
docs(oidc-grafana): correct post-mortem window claim
The failed-hook post-mortem paragraph in the users-Job troubleshooting section still said the Job "stays around until the next helm-upgrade for post-mortem", but the shipped Job spec carries `ttlSecondsAfterFinished: 3600` — the Kubernetes TTL controller reaps the Job + Pod one hour after the terminal Failed condition regardless of when the next `helm upgrade` lands. Reword to name the 1h window explicitly and instruct the operator to re-trigger the hook if they missed the window. The edit was authored alongside commit 1f664566 but slipped out of that commit's file list; this is the cleanup. 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
|
affe9beca5 |
docs(oidc-grafana): reflect users-map replacement of the role-attribute-path design
Rewrite `docs/oidc-grafana.md` for the new authorization model: - Selector example carries a `users:` map. - System-mode "what the chart provisions" list drops the three KeycloakRealmGroups + role_attribute_path, adds the audience scope-only KeycloakClient defaults, and adds the users-Job step with `skip_org_role_sync` + `oauth_allow_insecure_email_lookup` wiring notes. - Fresh "Users and RBAC" section describes the users-Job reconcile contract: pre-provision -> add-to-org -> PATCH role -> prune. Explains the "user not in list -> gets nothing" semantics (replaces the default-Viewer fallback). - CustomConfig section explicitly notes that `role_attribute_path` in the operator's payload is now dead config: the chart forces `skip_org_role_sync = true` regardless of mode. - "Failure modes" gains "user logs in but sees no dashboards" (missing from `users:`) and a users-Job troubleshooting entry. - Drop the `GrafanaAdmin` deferral notes and the "Mode toggle destroys chart-owned KeycloakRealmGroup membership" failure mode — both moot now that the groups are gone. - Removed the stale `KeycloakRealmUser` example that used `tenant-acme-monitoring-admin` (the chart never rendered a group by that name — the inner release is `monitoring-system` and the example was a live footgun). Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
Ivan Okhotnikov
|
b46543e5f2 |
fix(monitoring): defer platform GrafanaAdmin lever to a future phase
The MAJOR finding from the cozy-review agent: the platform-side `oidc.grafanaAdmin: true` documented in the PR body and in docs/oidc-grafana.md never actually reached the platform Grafana's HelmRelease. `Package.spec.components["monitoring-system"].values.oidc.grafanaAdmin` was set in packages/core/platform/templates/bundles/system.yaml, but `monitoring-system` (and `monitoring`) have no `install:` block in packages/core/platform/sources/monitoring-application.yaml — only `monitoring-rd` does. PackageReconciler skips components without an `install:` block before ever reaching the assignment that copies `pkg.Spec.Components[].Values` onto `hr.Spec.Values` (internal/operator/package_reconciler.go around lines 197-201 and 292-293). The values-override dictionary was silently dropped, no HR carried `oidc.grafanaAdmin: true`, and the platform Grafana rendered with the chart default (`false`). The `bundles_monitoring_grafana_admin_test.yaml` unit test only asserted against the emitted Package CR spec — exactly the layer where the value still existed but was about to be discarded — so the broken propagation slipped past. Trim the broken promise from Phase 1. The chart-side helper (`monitoring.oidc.allowAssignGrafanaAdmin`) stays as-is so a future phase that routes the override through the tenant-root chart's monitoring HR values (or adds an `install:` block on the `monitoring-system` component) lands on a green chart-side contract. The helm-unittest that verifies chart-side emission stays too, but with a docstring calling out that end-to-end wiring is deferred. Everything else that promised the lever works — bundle line, docs, platform bundle test — is either removed or rewritten to match reality. Also picks up two MINOR findings from the same review: * Drop the misleading `secret-hash` annotation on the OIDC client-secret Secret. The comment claimed it "rotates the KeycloakClient CR and therefore the Grafana Deployment env when the client-secret changes", but nothing downstream watches an annotation on the Secret itself — no rotation is triggered. The `lookup`-based fallback already keeps the value stable across reconciles, so no rotation is even needed. Removing the dead annotation avoids planting a broken assumption for future readers. * Add `release.cozystack.io/helm-install-timeout: "15m"` on the monitoring cozyrds ApplicationDefinition. Under `mode: System` the chart now emits, on top of the existing Grafana + Alerta + VMCluster + CNPG stack, four additional Keycloak CRs whose reconciliation is asynchronous against an operator in `cozy-keycloak` (not the tenant namespace). Flux's default 5m install window was already tight for the pre-OIDC stack; 15m matches the precedent set by tenant-rd/kubernetes-rd cozyrds. 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
|
be52e80289 |
fix(monitoring): split monitoring-application PackageSource into default/oidc variants
Follow-up to the previous self-review-fix commit: the unconditional `- cozystack.keycloak-operator` in dependsOn was itself a CRITICAL cold-install regression on any cluster where `authentication.oidc.enabled: false` (the default). The keycloak-operator Package is only emitted by `packages/core/platform/templates/bundles/system.yaml:112` when that flag is on — a hard dep against it from the default variant would leave `cozystack.monitoring-application` stuck on DependenciesNotReady forever, cascading a `cozystack-platform` HR timeout that hits every OIDC-off customer. Fix by mirroring the cozystack-engine variant split: - `default` variant keeps the baseline reconciliation edges (no keycloak-operator dep). - `oidc` variant additionally waits for `cozystack.keycloak-operator` so the v1.edp.epam.com CRDs are registered before the monitoring chart reconciles and the `.Capabilities.APIVersions.Has` guard in templates/grafana/oidc-keycloak.yaml can safely emit KeycloakClient/Scope/Groups. The bundle template swings between variants on `.Values.authentication.oidc.enabled`, mirroring the cozystack-engine invocation two lines above. The `monitoring-system` component override (oidc.grafanaAdmin: true) is preserved across both branches — the values field is a no-op when mode: None (no auth.generic_oauth block is emitted at all), so keeping it identical avoids two divergent code paths. Also widened the platform helm-unittest coverage to lock: - default variant does NOT depend on keycloak-operator (permanent DependenciesNotReady guard). - oidc variant DOES. - Bundle emits the right variant based on authentication.oidc.enabled AND keeps the GrafanaAdmin uplift on the monitoring-system component in either branch. 83 platform tests + 23 monitoring tests still green. 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
|
ed4aa841c8 |
docs(oidc-grafana): operator guide for Phase-1 OIDC on the Grafana instance
New docs/oidc-grafana.md parallel to docs/oidc-tenant.md from cozystack/cozystack#3044. Covers: - The three modes on the Monitoring CR (None / System / CustomConfig) and how the selector maps to what the chart provisions. - What System mode creates on the cozy realm: a per-instance confidential KeycloakClient, a KeycloakClientScope with the audience mapper, three KeycloakRealmGroups, and the persistent client-secret Secret; plus the Grafana CR side (auth.generic_oauth section, GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET env). - Both CustomConfig payload paths (inline vs secretRef) and the XOR guard. - User-role mapping via role_attribute_path and how an operator adds users to the three groups through a KeycloakRealmUser. - Failure modes (assertSystemEnabled, XOR, wrong claims, emailVerified prescriptive requirement) and what is out of scope for Phase 1 (per-tenant realms, backend-logout-url, disabled login form, CEL claimValidationRules). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Aleksei SviridkinandMyasnikov Daniil
|
5dc363d398 |
docs(seaweedfs): add 4.31 rename audit & recovery runbook
Classify each tenant from its volume PVCs, re-bind the volumes of a tenant installed fresh on 1.5.x so the adopted names pick them up, stop a split cluster before upgrading it, and clear the leftovers the upgrade cannot remove itself. The recovery steps select the renamed set by a precise filter and derive secret names from the fullname helper, so they work for an instance running under a non-default name and for pooled or zoned volumes. Co-authored-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
44507d2cd0 |
docs(agents): add downstream repository checklist to the PR template
Cozystack is upstream for repositories that nothing keeps in sync with it, and no CI job compares the two sides, so a change here breaks them in silence. Only two narrow paths are automated, and both fire on a release rather than on a PR. It has already happened. The website docs generator works from an app list hardcoded in its own Makefile, and packages/apps/opensearch was never added to it, so an application we ship has no reference page at all. The Terraform provider still offers a Kubernetes version this repo removed. The plugin repo still sends operators to hack/ scripts that were deleted, one of them rewritten as a Go command. Add a Downstream Repositories trigger map to the contributing guide, naming per repository which change here forces a change over there and which file to touch, and a checklist in the PR template that links to it. The template addresses AI agents directly: they author a large share of PRs and will otherwise tick a box without opening the follow-up it claims. The coupling is wider than the obvious ones, and mostly invisible. talm restates the node contract, overlapping ours but already diverged, so it must be diffed rather than overwritten. cozyhr reads an annotation the operator writes and silently merges the wrong values if it is renamed. cozy-proxy is vendored in from its own repo, yet consumes labels produced here and quietly stops handling external VM services if they move. The telemetry server queries metric names emitted here, and a rename just empties its dashboards. None of these fail loudly. Guard the map with a unit test, because a map about silent drift must not drift silently itself. It pins every in-repo path the map cites to a file that exists, pins the map and the checklist to the same repository list, and pins the workflow carve-out below to the map's location, to the job that reads the flag, and to the output that carries it: cutting any one of those three leaves the other two looking wired up. The header states plainly what the suite cannot check, so a green tick is not mistaken for a correct map: whether a coupling is real, and whether it still holds over there, stays a human's job. Editing the map alone is a docs-only PR, which skips the unit tests and would exempt the map from its own guard. Rather than widen the docs-only rule, which also gates the Talos image build, the plan step exports a separate flag that only the unit-test job reads: a docs PR now runs the tests and still builds nothing. It detects the map by its old path too, since a rename is reported as the new path alone. Document that gh pr create --body and --body-file replace the body wholesale, which would otherwise drop the checklist from every CLI-authored PR. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
18715f8c63 |
docs(agents): document Chainsaw v0.2.15 gotchas in the e2e-testing guide
The v0.2.15 syntax/semantics traps were tribal knowledge scattered across PR review notes (the number-literal caveat lived only in #2826's body, which evaporates on merge). Consolidate them into a durable subsection so the next author finds them before hitting each one fresh: filter-as-list conditions, no number-literals/backticks in JMESPath, error: fails hard on an unknown GVK, no conditional Test-level skip, step-level-only finally, bounded/blocking cleanup surfacing latent teardown bugs, projection-style failure diffs, and the catch-script cwd. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
302a72f14a |
Merge origin/main into feat/e2e-chainsaw-example
Reconciles the talos-image-cache conflicts created when #3254 merged to main: keep the Chainsaw _lib/ port (hack/e2e-chainsaw/_lib/talos-image-cache.sh, a superset of #3254's logic + the node-join diagnose helper), drop the BATS-era hack/e2e-apps/talos-image-cache.sh, and keep the _lib/-path versions of the mirror manifest, install bats, and unit test. Also picks up main's zizmor action-pinning (#3223, unblocks pre-commit/zizmor) and Talos v1.13.6 (#3240). Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov DaniilandClaude Opus 4.8
|
47eec0157f |
feat(release): add Cut Pre-release Tag workflow (#2677)
Pre-release tags (vX.Y.Z-alpha.N / -beta.N / -rc.N) were cut by a manual `git push origin HEAD:refs/tags/<tag>`. That leaves the immutable-tag work in #2677 only partly enforceable: a repo ruleset cannot restrict `v*` tag *creation* to CI while still letting maintainers hand-push pre-releases (fnmatch has no way to match "v* except v*-rc.*"), so "do not hand-cut stable tags" stays a matter of discipline backed only by an in-workflow guard. Add cut-prerelease.yaml — a workflow_dispatch that becomes the sole entry point for creating a pre-release tag: - Generates the CozyStack CI app token (same app as promote-rc.yaml) and pushes the tag as the app, so tags.yaml fires (a tag pushed with the default GITHUB_TOKEN would not trigger it) and the push-event base_ref is populated (tags.yaml's Get-base-branch step needs it). - Validates ^v\d+\.\d+\.\d+-(alpha|beta|rc)\.\d+$ and rejects stable vX.Y.Z — stable tags are cut write-once by the promote flow at a PR merge commit. - Enforces dispatch from main or the matching release-X.Y branch, and refuses to move an existing tag (write-once). - Pushes HEAD:refs/tags/<tag> from the branch tip — the exact form docs/release.md prescribes — on ubuntu-latest (no build runner needed). With every pre-release cut by CI, repo admins can then enable a "Restrict creations" tag ruleset on `v*` with the CI app as the sole bypass actor, turning tag-creation discipline into hard enforcement. Also update tags.yaml's two human-facing error messages and docs/release.md (the RC and patch flows, the workflow list, and a new repo-side tag-ruleset enforcement note) to point at the workflow instead of a manual push. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Myasnikov DaniilandClaude Fable 5
|
9297e51170 |
Merge origin/main into feat/e2e-chainsaw-example
Reconcile the Chainsaw e2e migration with ~500 commits of main drift: - keep main's run-kubernetes.sh helpers (kubectl_wait_retry, tenant drain, LINSTOR pool-free wait, SC-fallback probe) under the branch's _lib/ rename with the Chainsaw finally/snapshot deltas re-applied - resolve modify/delete on 10 drifted hack/e2e-apps/*.bats as deleted; their post-branch assertion drift is re-ported into the Chainsaw suites in follow-up commits - drop the 4 new main-side bats (kubernetes-oidc-*, securitygroup, serviceexposure) from _lib/ relocation; ported as Chainsaw suites in follow-up commits - docs/agents/e2e-testing.md: merge main's plan-job TIA wording with the Chainsaw suite phrasing Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Aleksei Sviridkin
|
12c3cd03f8 |
docs(vm-instance): document attaching a VM to an external VLAN (#3196)
## What this PR does Adds `docs/vm-external-vlan.md`, a guide for attaching a `vm-instance` VM directly to an external, physically-routed VLAN via a Linux bridge and a `bridge`-type NetworkAttachmentDefinition. The documented VM networking story is overlay-only (pod network + KubeOVN VPC subnets). Attaching a VM to a real VLAN — so it shares a broadcast domain with external hardware and takes an address from that VLAN's subnet — is undocumented, and the obvious `macvlan` approach silently fails with KubeVirt's default bridge binding: macvlan demultiplexes ingress by the child interface's MAC while bridge binding puts the guest's own MAC on the wire, so gateway replies reach the parent interface and are dropped before the guest. The working recipe is a Linux bridge + `bridge` CNI NAD. The guide covers why bridge and not macvlan, the host bridge, the NAD (one per tenant namespace), assigning a static guest address through cloud-init (the chart has no `networkData`), the always-present dual-homed pod NIC, and the post-recreate ARP staleness from KubeVirt regenerating the guest MAC. Docs-only; no chart or manifest changes. The recipe depends on the `bridge` CNI plugin being present in `/opt/cni/bin`; #3195 makes Cozystack's multus package stage the reference plugins there. ### Screenshots N/A — no UI changes. ### Release note ```release-note docs(vm-instance): add a guide for attaching a VM to an external VLAN via a Linux bridge and a bridge-type NetworkAttachmentDefinition ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new guide for connecting a `vm-instance` directly to an externally routed VLAN. * Includes setup steps for creating a bridged VLAN interface, defining a network attachment, and configuring the VM with a static IP. * Notes important networking caveats such as dual-homing behavior, MAC changes after recreation, and host-to-VM connectivity requirements. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
myasnikovdaniil
|
cece5c23b5 |
feat(release): immutable tags & rc→stable promotion (#2677) (#3017)
## What this PR does Implements the immutable-tag + rc-promotion flow from #2677. A **stable release becomes a renamed release-candidate**: the bytes shipped as `vX.Y.Z` are bit-for-bit the bytes built and e2e-tested as `vX.Y.Z-rc.N`. No tag is ever force-moved, and stable is never rebuilt — it is *promoted* by retagging the rc's existing images. Layered onto the build matrix from #2937/#2983 (this PR is **stacked on `refactor/build-matrix-2937`** and must merge after it). ## The five force-retag sites, removed | Site | Before | After | |---|---|---| | `tags.yaml` api/apps/v1alpha1 tag | `git tag -f` / `push -f` | write-once (create-if-absent, fail if it would move) | | `tags.yaml` release-X.Y.Z branch | `git branch -f` / `push -f` | compare-before-force (no-op if unchanged; staging branch only) | | `pull-requests-release.yaml` stable tag | `git tag -f` / `push -f` | write-once at the PR merge commit (force impossible by construction) | | `pull-requests-release.yaml` maintenance branch | `updateRef force:true` | fast-forward-only | | `auto-release.yaml` patch tags | delete-recreate (cron) | **workflow deleted** — stable only via explicit promote | ## Version decoupling (the enabler) The operator baked its version into the image at build time, so an rc image self-reported the rc string — blocking retag-promotion. It now reads `COZYSTACK_VERSION` from the environment (threaded via `cozystackOperator.platformVersion` → Deployment env, stamped by `make manifests`), falling back to the build-time value when unset. The same image bits can report any release name. The only runtime reader is the telemetry metric `cozy_cluster_info{cozystack_version=...}`. ## Promotion flow `promote-rc.yaml` (`workflow_dispatch`, `rc_tag=vX.Y.Z-rc.N`): 1. Validate the rc release exists and the stable tag does not. 2. `hack/promote-retag.sh` reads the rc's digest-pinned image refs from `packages/*/*/values.yaml` and `skopeo copy`s each — **by digest** — to `:vX.Y.Z` and `:latest`, verifying with `skopeo inspect`. 3. Rewrite the cosmetic `-rc.N` substring in vendored tags to the stable version (the `@sha256` wins regardless), restamp the version-stamped assets (operator manifests, cozypkg, openapi; the heavy Talos assets are copied verbatim from the rc draft), open a `release-X.Y.Z` PR. 4. Merging the PR reuses the existing release-PR e2e and `pull-requests-release.yaml` finalize to cut the **write-once** stable tag at the merge commit and publish the release. Squash is disallowed (the tag needs a real merge commit). `nightly.yaml` cuts write-once `*-nightly.<date>` tags (gated by `NIGHTLY_ENABLED`); `retention.yaml` keeps the newest 14 per line (dry-run by default). ## Validation **Local end-to-end — the core claim is proven, not asserted.** The whole design rests on "copy-by-digest to a new tag preserves the digest, so stable == the e2e-tested rc, bit-for-bit." This was exercised against a real registry (`ttl.sh`) with real `skopeo`: - Pushed a multi-arch image under a `:v1.4.0-rc.2` tag, built a temp tree exercising all three `values.yaml` digest shapes (single `image:` string, split `repository`/`tag`/`digest` map, and the `platformSourceRef` OCI artifact), then ran the actual `hack/promote-retag.sh v1.4.0`. - **Result: `:v1.4.0-rc.2`, `:v1.4.0`, and `:latest` all resolve to the identical digest** (`sha256:fd8d9aa6…`), across all 17 platform manifests. The script's own `skopeo inspect` post-check passed, and an independent re-inspection confirmed it. Local validation caught (and this PR fixes) two bugs that only surface against a real registry — never in static lint: 1. `skopeo copy --multi-arch all --all` — mutually exclusive flags, fatal error; every retag would have failed on CI. Now `--multi-arch all`. 2. The same digest was retagged twice when an image appeared in two value shapes; the ref set is now deduped on the canonical `repo@digest`. **Other local checks:** `go test ./pkg/version` + `go build`/`vet`; `helm template` renders `COZYSTACK_VERSION` in all 3 operator variants (omitted when unset); `make manifests` stamps the version into the install assets; `shellcheck` clean on `promote-retag.sh`; `actionlint` clean and `act -l` resolves the job graph on all workflows; the rc-tag parser and nightly version-math unit-tested (accept/reject + minor-vs-patch bumps). **Still CI-only (cannot be exercised offline):** the full workflow runtime — github-script API calls, OCIR auth, the rc-release / staging-branch preconditions, the nightly `base_ref` push mechanics, and self-hosted runners. The *logic* inside the steps is unit-tested; the *orchestration* is not. Treat it as unproven until the workflows run. ## Out of scope / required follow-up (needs repo admin — not doable from a PR) - Tag-protection rules: `v*` and `api/apps/v1alpha1/*` create-only by the CI app, no delete/update; "limit branches/tags updated in a single push" = 1. - Enable `NIGHTLY_ENABLED` (and `RETENTION_APPLY` when ready) repo variables. - Registry-side pruning of `*-nightly.*` image tags (no OCIR delete parity yet — tracked TODO in `retention.yaml`). ## Release note ```release-note NONE ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added nightly publishing (mirror-by-digest, disk build, e2e validation) and nightly retention pruning. * Added RC-to-stable promotion via digest retagging (no rebuild). * Operator/installer and console now expose version/platform metadata, including runtime override via `COZYSTACK_VERSION`. * **Bug Fixes** * Enforced write-once tag behavior and fast-forward-only maintenance updates across release workflows. * Made nightly mirroring/selection and retention pruning more selective and safer. * **Tests** * Added Bats coverage for nightly mirroring and RC retagging, plus new Helm/unit checks. * **Documentation** * Updated the release model to center RC promotion and tag immutability. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
tym83andClaude
|
60704530ed |
docs(security): align security docs with current controls for incubation DD
Bring the security documentation in line with controls that are already in place, and add the two coordinated-disclosure items flagged during the incubation due-diligence review. - self-assessment: DCO is enforced by an automated check; private intake is GitHub Private Vulnerability Reporting; OpenSSF passing badge achieved 2026-04-01 (working toward silver); GHSA/PVR framing for advisory history; incident-response now summarized publicly. - SECURITY.md: route reports to Private Vulnerability Reporting with a MAINTAINERS.md fallback; add a 90-day coordinated-disclosure window and a CVSS-based remediation-target table. - CODEOWNERS: document that it encodes the area-scoped Reviewer role, which is intentionally distinct from the MAINTAINERS.md governance roster, and that security reporting is not routed through this file. - Add docs/security/incident-response.md as the public incident runbook. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
9a4bcfb13c |
docs(security): mark self-assessment stage as completed
Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
tym83andClaude
|
19ab910f66 |
docs(security): add security self-assessment
Add the CNCF security self-assessment required for the Incubation due diligence review, alongside the existing design-level threat model. It documents the trust boundaries, actors/actions, security functions, secure development practices, and known gaps. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: tym83 <6355522@gmail.com> |
||
|
Aleksei Sviridkin
|
286f260570 |
docs(security): add design-level threat model
Document the platform's actors, boundary-crossing actions, and security goals/non-goals, organized by trust boundary (T0-T4) per the CNCF TAG-Security Actors/Actions model. Records the primary trust-boundary invariant (tenant RBAC grants no HelmRelease/core writes; the aggregated API server fixes the chart reference server-side) and the two admission enforcement points (core kube-apiserver vs cozystack-api). Every claim cites a source file so the model can be re-verified as the code evolves. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Myasnikov DaniilandClaude Fable 5
|
d252fc93a0 |
ci(release): make rc promotion transactional; defer registry writes to finalize
Promotion mutated the registry at dispatch time — retagging ~37 images to vX.Y.Z (+:latest), publishing the stable cozy-installer chart — before the promote PR's e2e ran and before any merge. An abandoned promotion then left stable-named bytes for a version that never shipped, and the (correct) image write-once check wedged any later re-promotion to the same version. Move every irreversible registry side effect into pull-requests-release.yaml finalize, which runs only after the promote PR merged (= after its e2e passed), making promotion atomic. promote-rc.yaml now only stages the release-X.Y.Z branch, drafts the stable release, and opens the PR; an abandoned promotion leaves nothing but an overwritable draft + branch. Folded into the same restructure: - Cut the api/apps/v1alpha1/vX.Y.Z Go-module tag in finalize, at the stable tag's commit. It was gated in tags.yaml on `release_exists==false && is_stable`, unsatisfiable under promotion (the draft always pre-exists), so promoted stables never got their submodule tag — a regression for Go consumers. Finalize is where the stable tag is born, so it cuts the matching submodule tag write-once (stable only). - Gate :latest by max-semver. promote-retag.sh now moves :latest only when MOVE_LATEST=1 (default off); finalize passes it the same make_latest decision the release gets, so a patch on an older line moves neither the release's `latest` nor the images' :latest. Added MOVE_LATEST bats coverage. - Stamp platformVersion into the published stable chart's default values (uncommitted, at package time) so `helm --install --version X.Y.Z` reports the stable version in telemetry instead of the rc version baked into the operator image. Not committed to the branch — that would leak platformVersion=vX.Y.Z onto main and mis-stamp the next rc. - promote-rc: serialize all promotions (static concurrency group, not per-rc); label the PR full-e2e so the full suite runs; scope the rc->stable sed to the depth-2 values.yaml glob plus the kubernetes .tag files (the build's own write set); tolerate a leftover draft so re-promotion is not wedged. - finalize: assert the PR author is cozystack-ci[bot], and run on an ephemeral oracle-vm runner (needs skopeo/helm). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Ivan Okhotnikov
|
de6240270c |
docs(oidc): reframe emailVerified as prescriptive, not chart-enforced
Prior wording overstated Phase 1: the rendered
AuthenticationConfiguration carries no claimValidationRules, so the
chart itself does not gate on email_verified. Rewrite the failure-mode
bullet to describe the layered guarantees operators actually rely on:
1. provisioning KeycloakRealmUsers with emailVerified: true,
2. Keycloak's realm-level duplicateEmails: false as an
impersonation-through-second-account guard,
3. the k8s upstream side-effect that rejects tokens explicitly
carrying email_verified: false.
Also flag CEL claimValidationRules as a reasonable follow-up hardening
path, but explicitly out of scope for Phase 1 per the design doc.
Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
|
||
|
Ivan Okhotnikov
|
ef6dd50570 |
docs(oidc): note emailVerified prerequisite for cozy-realm users
The tenant kube-apiserver auto-applies `claims.?email_verified.orValue(true) == true` when username.claim is email, so a Keycloak user with `emailVerified: false` fails login with `oidc: email not verified` even though token issuance succeeds. Document the requirement in the operator guide alongside the other System-mode failure modes. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Ivan Okhotnikov
|
d3a5bf745f |
docs(oidc): clarify — chart requests email scope explicitly (not a realm-default)
Adversarial review flag: the previous rewording implied the cozy realm has `email` in its client-scope defaults, which is misleading — the per-cluster client my chart creates lists only `groups` and the per-cluster audience scope as defaults. The `email` claim reaches the token because the chart-generated `kubectl oidc-login` block passes `--oidc-extra-scope=email` explicitly, not because of realm-side default-scope hygiene. State the actual mechanic: `email` is a built-in OIDC scope available in every conformant issuer, requested by the client at auth time via the kubeconfig's exec block. No realm-side configuration required. For `CustomConfig`, note that the operator should verify their BYO issuer emits `email` when the client requests the `email` scope (every conformant provider does). Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |
||
|
Ivan Okhotnikov
|
0bf26182da |
fix(apps/kubernetes,docs): explicit --oidc-extra-scope=email + doc alignment
Two follow-ups from the final adversarial review:
- The chart-generated OIDC kubeconfig exec block requested only
`--oidc-extra-scope=groups`. On a Keycloak realm where `email` is
not a default client scope, kubectl-oidc-login would fetch a token
that lacks the `email` claim — apiserver would then fail to
identify the user, causing a silent auth failure. Add
`--oidc-extra-scope=email` explicitly so the flow does not depend
on realm-level default-scope hygiene.
- `docs/oidc-tenant.md` still said the `User:` subject "must match
the `preferred_username` claim emitted by the issuer" — obsolete
after the claim switch. Rewrite to say `email`, and add a note
that the chart requests the `email` scope explicitly.
Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
|
||
|
Ivan Okhotnikov
|
b6fba8cabf |
docs(oidc): fix Secret name + CRB hash separator in the operator guide
Self-review fixes for two factual errors:
- The "How a user logs in" example named the Secret
`prod-oidc-kubeconfig`, but the cozystack-api prefixes the
cluster name with `kubernetes-` when materialising the
`HelmRelease`, so the actual Secret is
`kubernetes-prod-oidc-kubeconfig`. The NOTES.txt template was
already correct; only the doc example was off.
- The CRB hash formula was described as `<release>:<email>` but
the code uses `<release>-<email>` as the SHA input.
Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io>
|
||
|
Ivan Okhotnikov
|
b7c553e653 |
docs(oidc): Phase-1 operator guide + chart NOTES
Add the operator-facing guide for the new selector — what the three modes mean, what the chart provisions, how a user logs in, what the failure modes are, and what is intentionally out of scope (per-tenant realms, federation, cross-cluster SSO). The architectural rationale points back to cozystack/community#24. Update the chart NOTES.txt to print the OIDC kubeconfig retrieval snippet, the per-cluster client/issuer values, and the listed `users[]` roster — only when OIDC is on, otherwise the previous admin-kubeconfig hint is unchanged. Signed-off-by: Ivan Okhotnikov <ivan.okhotnikov@aenix.io> |