mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-27 23:27:58 +09:00
feat/postgres-credentials
103
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Timofei Larkin
|
23b3d40bb4 |
docs(postgres): cut comments to what the code cannot say
Comments carry the mechanism, not the rationale; every argument removed here is in the design proposal, the migration's own template or the PR. Where one claim appeared at several sites, the copy at the site that constrains the code stays and the others point at it. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Timofei Larkin
|
d5d2a90ce1 |
feat(postgres)!: mint tenant passwords through the credentials API
The chart generated every user's password at render time into <release>-credentials, which tenants could read again at any time and which Helm history kept for MaxHistory revisions, so a deployment whose policy requires a credential shown once could not run PostgreSQL here. The accounts move under CloudNativePG's managed roles, each pointing at the account Secret the Cozystack API seeds, so the operator creates the role with the SCRAM verifier the Secret holds and applies a changed Secret live, and a password set inside PostgreSQL is reverted. Every role is rendered as the init-script created it, because the operator reverts an attribute that differs, revokes a parent role missing from inRoles and clears a comment the spec leaves unset, while the init-script still finds the roles to drop by that comment and by membership in the marker. The init-script stops creating, passwording and altering accounts, where it would be a second writer, and keeps the two things the operator has no field for: the marker membership narrowed to a record, and createrole_self_grant. The refusal to adopt a role the chart did not mark moves into a pre-upgrade hook, because the operator adopts a managed role within seconds of the spec reaching it and the post-upgrade init-job would find it adopted already. The definition declares the credentials block, so the API seeds, serves and mints for the kind, and no longer selects the credentials Secret, which the chart no longer renders or grants. BREAKING CHANGE: PostgreSQL passwords are no longer readable from <release>-credentials. A password is issued once, by a mint on the account's Credential in core.cozystack.io, and replaced by the next mint; deleting a Postgres application whose release is suspended is refused. The conversion needs a platform variant with migrations enabled. Assisted-by: LLM Signed-off-by: Timofei Larkin <lllamnyp@gmail.com> |
||
|
Andrey Kolkov
|
9cc3a9e459 |
test(vmi): add VMInstance and standalone VMDisk backup/restore e2e round-trips
The disabled hack/e2e-chainsaw/backup/ suite was a Chainsaw port of the retired
backup.bats, written against the old backup API, so the VMInstance and standalone
VMDisk Velero paths had no e2e coverage. Replace it with two round-trips in the
vminstance suite that drive the validated examples/backups/{vmi,vmdisk} scripts,
so the documented demo and the automated test stay in step.
Both run un-gated in CI:
- vminstance-2-backup-roundtrip boots a tiny cirros VM (u1.nano, 2Gi), backs it
up via BackupClass + BackupJob, restores in place, and asserts the VM returns
Running off a disk PVC the restore created: a new uid, Velero's restore-name
label and the -orig- PVC keepOriginalPVC leaves behind. The label alone is not
enough, because existingResourcePolicy: update also stamps it on a PVC Velero
only patched, which moves no data. A guest-side boot proof is unavailable: the
KubeVirt CR disables serial-console logging and cirros ships no guest-agent.
- vmdisk-2-backup-roundtrip imports a 5Gi disk, backs it up, and restores it in
place with the same three assertions. The controller prepares the target
(suspend HelmRelease, delete DataVolume, rename PVC). On a WaitForFirstConsumer
class the Velero data mover waits for a Pod to select a node for the restored
PVC, which a detached disk never gets, so the script starts a pause Pod on the
new PVC, standing in for the VM that would attach it.
The disk storageClass is an override (default replicated; the CI steps pass the
lane's COZY_E2E_STORAGE_CLASS, 'local' on the DRBD-off gate) and the script waits
are bounded so their worst case stays under the 40m op timeout, sized to keep the
whole parallel:1 suite inside the 215m job cap. On failure the catch also dumps
the Velero Restore, DataDownload and DataUpload objects. Each harness stands up a
throwaway in-cluster BSL (the platform cozy-default BSL derives the external S3
endpoint, unroutable in kind); its finally resumes the suspended HelmReleases,
reclaims the keepOriginalPVC orig PVC and its LINSTOR volume, and drains the
Velero backups before deleting the BSL. The example scripts are non-interactive
so the harness can drive them.
Assisted-by: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
|
||
|
Andrey Kolkov
|
43b02bda30 |
test(e2e): run the Redis backup round-trip against Valkey
The Redis backup demo now takes APP_KIND=Valkey, which switches the app kind and the chart prefix the helpers derive sentinel and Secret names from. The demo BackupClass routes Valkey to the same demo strategy. The valkey suite gains valkey-backup-roundtrip, which drives that demo end to end: seed a marker, back it up, restore it in place and into a copy. It writes to tenant-root's seaweedfs through seaweedfs-s3, so a seaweedfs change now selects the valkey suite too, and since the round-trip runs examples/backups/redis as its harness, an edit there selects both the redis and the valkey suites. The reduced E2E install keeps the backup stack for the valkey suite, as it does for the other backup round-trip suites. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Andrey Kolkov
|
0c2f131192 |
feat(backups): back up Valkey with the Redis strategy
Valkey runs on the same spotahome RedisFailover operator as Redis and speaks the same replication protocol and RDB format, so it needs no strategy kind of its own. The only difference the backup Job sees is the chart's resource-name prefix: valkey-<app> instead of redis-<app> for the sentinel Service and the auth Secret. The Redis driver now accepts applicationRef kind Valkey as well as Redis and hands the template .Release.Prefix, derived from the app's kind, which cozy-default-redis uses for every name it builds. A kind with no known prefix fails the render instead of producing an empty one. cozy-default routes apps.cozystack.io/Valkey to cozy-default-redis. A restore must target an app of the same kind as the backup's source: replaying a Redis dump into Valkey, or the reverse, is not something the strategy can promise across engine releases. The shipped template reads the prefix as `.Release.Prefix | default "redis-"`: during an upgrade the previous controller, which has no Prefix in its render context and serves only Redis, renders the new CR, and a bare reference would print "<no value>" into the names. The redis:8 image already loads RDB v11, which both shipped Valkey versions (7.2, 8.1) write. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Andrey Kolkov
|
647407545d |
fix(backupstrategy-controller): run backup clients on a package-owned image
The Altinity and RabbitMQ strategy Pods ran platform-migrations, a 349 MB image carrying kubectl, helm and etcd-migrate whose ENTRYPOINT is the migration runner, and whose pins were stamped from packages/core/platform so every migration change moved the backup clients too. Build images/backup-http-client (alpine + curl + jq + ca-certificates, non-root) from this package's Makefile and stamp both client keys from it. The cross-package stamp in packages/core/platform/Makefile and the platform-migrations-specific check in hack/image-pin-consistency.bats go away; the general one-digest-per-repository check still covers the two keys sharing one image. Fixes #3420 Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
Aleksei Sviridkin
|
ff3c409881 |
fix(backups): refuse a foreign -orig PVC and record the kept disk's reclaim policy (#4671)
## What this PR does Two defects in `renamePVC` (`internal/backupcontroller/velerostrategy_controller.go`), the step of an in-place VMInstance restore that keeps each disk as `<dv>-orig-<shortHash(RestoreJob name)>` when `keepOriginalPVC` is true, the default. ### A taken `-orig` name skipped the rename, and the restore went on `renamePVC` returned `nil` as soon as a PVC with the `-orig` name existed (`main:1263-1266`), whatever that PVC was. `prepareForRestore` then reached Step 4 (`main:1172-1183`), which deletes the DataVolume of every disk regardless of what Step 3 did, because a Step 3 error was only ever turned into a Warning event (`main:1165-1168`). The restore therefore ran exactly as `keepOriginalPVC: false` does and reported `Succeeded`: the live disk was not kept, and the `-orig` name held a different, older disk that the user takes for the pre-restore copy. When that name can already exist: - A RestoreJob created under the name of a deleted one. The hash covers only the RestoreJob name, and nothing in the controller ever deletes an `-orig` PVC, so the predecessor's PVC is still there. This is the realistic case, and it is what a scripted restore with a fixed RestoreJob name runs into on its second run unless the `-orig` PVC was removed in between. - Two RestoreJobs with different names whose 4-hex-digit hashes collide (16 bits) while restoring the same disk in the same namespace. Rare, but nothing guards it. - A reconcile of the same RestoreJob after a controller restart in the middle of a rename. The name is ours, but the early return treated a half-done rename as complete: the original PVC stayed in place and the PV stayed bound to it. - Truncation does not apply: the name is not truncated, and one longer than 253 characters makes the `Create` fail, an error that was swallowed in the same way. Each `-orig` PVC now carries the UID of the RestoreJob that created it, in `backups.cozystack.io/restore-job-uid`. `prepareForRestore` checks every `-orig` name before it suspends or halts anything, and fails the RestoreJob, naming the PVC, when any of them is held by a PVC another RestoreJob created. When the PVC is ours, `renamePVC` resumes: it deletes the original PVC only while it still claims the kept PV, and repoints the PV unless it already points at the `-orig` PVC, so a rerun is a no-op. Any other rename error now holds the restore back before Step 4 deletes a DataVolume, and the restore is retried, instead of the error becoming an event (see the next section). An `-orig` PVC created by an earlier controller has no UID annotation and is refused like any foreign PVC. The only RestoreJob that loses by this is one whose controller was upgraded after its rename and before its Velero Restore was created; it fails with the message above rather than going on. ### The rename read its own write back through the cache `renamePVC` created the `-orig` PVC, then read it back through the cached client to learn its UID for the PV's claimRef (`main:1314-1316`). The informer often has not seen a PVC created a moment earlier. On our lab, running Cozystack v1.6.3, an in-place restore of a two-disk VM with `keepOriginalPVC: true` renamed and restored the system disk, while the data disk lost the race, with this event on the RestoreJob: `Failed to keep old PVC vm-disk-e2e-dc-fix-data: failed to get -orig PVC vm-disk-e2e-dc-fix-data-orig-a574: PersistentVolumeClaim "vm-disk-e2e-dc-fix-data-orig-a574" not found`. As the error was only an event, the data disk's DataVolume was deleted and its original PVC stayed in place. Velero skipped that PVC and created no DataDownload for it, so its content was not restored, yet the restore finished `Succeeded`. The `-orig` PVC was left `Pending` on the same PV, which was by then `Retain`. `Create` already fills in the UID, so the read-back is gone. The other reads the rename decides on (the `-orig` PVC, the original PVC and the PV, before and after its own writes) go through the manager's uncached reader, wired as `APIReader`. The PV rebind retries on conflict, since the PV controller also writes the PV once the original PVC is deleted. A rename that fails for any reason other than a foreign `-orig` name requeues the restore rather than failing it. The DataVolume is still there, and the retry finds its own `-orig` PVC by the RestoreJob UID and resumes where it stopped. A cause that never clears keeps the RestoreJob `Running`, with a Warning event on each attempt. #4673 adds the same `APIReader` field and `apiReader()` helper to `RestoreJobReconciler`, for its finished-job guard. Whichever of the two lands second drops its own copy when it rebases. ### The PV's reclaim policy was overwritten without being recorded `renamePVC` switches the PV to `Retain` so it survives the deletion of the original PVC (`main:1289-1293`), and the policy it had was lost. I looked for a path in the controller that releases or cleans up `-orig` PVCs, and there is none: `cleanupOnDelete` on a Velero RestoreJob removes only the Velero `Restore` and the resource-modifier ConfigMaps, and no code deletes an `-orig` PVC or touches its PV again. A PV that was `Delete` therefore stays `Retain` for good, and its volume leaks when an operator deletes the `-orig` PVC. The original policy is now recorded in `backups.cozystack.io/original-reclaim-policy`, on the PV and on the `-orig` PVC, following the `backups.cozystack.io/` prefix the controller already uses for `underlying-resources`. A value already present on the PV is kept, as it predates any rename. Since there is no release path to restore it from, `examples/backups/vmi/scenario-user-restore.md` gains a section for the operator who deletes an `-orig` PVC: set the recorded policy back on the PV first, with a snippet that leaves a PV without the annotation on `Retain`. Restoring the policy from the controller itself, for instance once the Velero Restore has completed and the PV is `Bound` to the `-orig` PVC, would close the leak without the operator. I did not do it, because it changes what deleting an `-orig` PVC destroys, from nothing to the volume; that is a maintainer call, and the annotation is what it would read. ### How this fits with #3427 #3427 is complementary, and the two do not touch the same code: it changes `collectUnderlyingResources`, this PR changes `prepareForRestore` and `renamePVC`. Both append tests to `velerostrategy_controller_test.go`, so whichever lands second may need a trivial rebase. - #3427 makes a standalone VMDisk report its own disk, which sends VMDisk in-place restores through this same `renamePVC`. Once both land, the VMDisk path gets the same guard and the same annotation. - Its `examples/backups/{vmi,vmdisk}/cleanup.sh` and the Chainsaw `finally` reclaim each `-orig` PVC by patching its PV to `Delete` before deleting the PVC. That works around, in the e2e teardown, the leak described above. It is correct on the LINSTOR classes, which are `Delete`, but it would destroy a volume whose class was `Retain`. With this PR the scripts can read `backups.cozystack.io/original-reclaim-policy` instead of hard-coding `Delete`. I have not changed them here, since they only exist on that branch. - Its round-trips use fixed RestoreJob names and assert that a `<pvc>-orig-*` PVC exists after the restore. If a failed teardown leaves one behind, today the next run skips the rename and the assertion still passes on the stale PVC; with this PR the RestoreJob fails and names it. - Its cleanup comments describe the `-orig` PVC as a bare ObjectMeta with no labels or owner. That stays true: this PR adds annotations only, so its name-based lookup is unaffected. ### Tests New tests in `internal/backupcontroller/velerostrategy_controller_test.go`, run through `prepareForRestore` with the DataVolume in the dynamic fake client: - `TestPrepareForRestore_KeepOriginalPVC_RefusesForeignOrigPVC`: an `-orig` PVC without the annotation, and one carrying another RestoreJob's UID, both fail the restore with the PVC named, and leave the DataVolume, the live PVC, the PV and the foreign PVC untouched. - `TestPrepareForRestore_KeepOriginalPVC_ResumesOwnRename`: a rename interrupted after the `-orig` PVC was created completes on the next pass, and a second pass changes nothing. - `TestPrepareForRestore_KeepOriginalPVC_FailedRenameKeepsDataVolume`: a rename that fails for another reason (missing PV) requeues the restore without failing it and keeps the DataVolume, and once the PV exists the retry completes the rename. - `TestPrepareForRestore_KeepOriginalPVC_CacheLagsBehindWrites`: the reconciler's cached client serves every read from the objects as they were seeded, through an `interceptor` over the fake client, as an informer that has not caught up with this reconcile's writes would, while `APIReader` sees them. The rename must complete. - `TestRenamePVC_RecordsOriginalReclaimPolicy`: a `Delete` PV and a `Retain` PV both end up `Retain`, with their original policy recorded on the PV and on the `-orig` PVC. With the controller reverted to `main` (only the two annotation constants kept so the tests compile), all four fail: `prepareForRestore() = ready true, nil error` with `DataVolume was deleted although the disk could not be kept` for both foreign cases and for the failed rename, `original PVC still exists` and the PV still bound to it for the interrupted rename, and an empty `original-reclaim-policy` on the PV and on the PVC. For the cache race, the first commit alone fails `CacheLagsBehindWrites` with the lab's error, `failed to keep PVC vm-disk-ubuntu-source: failed to get -orig PVC vm-disk-ubuntu-source-orig-de3e: persistentvolumeclaims "vm-disk-ubuntu-source-orig-de3e" not found`, and `FailedRenameKeepsDataVolume` with a terminal error instead of a requeue. On `main` the race test reproduces the lab outcome: `original PVC still exists`, and the PV still claimed by it. With both commits, `go test -count=1 ./internal/backupcontroller/...` passes. No API type, CRD, chart or values file changes, so there is nothing to regenerate. ### Screenshots N/A, no UI change. ### Downstream repositories Walked the trigger map against the diff: the restore preparation in `internal/backupcontroller/velerostrategy_controller.go`, its tests, and `examples/backups/vmi/scenario-user-restore.md`. No CRD, ApplicationDefinition, values, schema, enum, namespace, label, metric or tooling change, so terraform-provider-cozystack, ansible-cozystack, ccp, talm, cozyhr, cozy-proxy, cozystack-telemetry-server, external-apps-example, examples and community are not reached. `RestoreJob.spec.options` stays opaque to the provider. cozystack/website is reached: `content/en/docs/next/virtualization/backup-and-recovery.md` (and each released copy) tells users to delete the `*-orig-*` PVCs "to free up storage". A PV left on `Retain` frees nothing, and the page should explain the recorded policy and the new failure for a reused RestoreJob name. No follow-up exists there yet, so the box stays empty until one is filed and linked. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(backups): an in-place VM restore with keepOriginalPVC now fails, before the VM is halted, when the <disk>-orig-<hash> PVC it would keep the disk under already exists and was not created by this RestoreJob (for instance one left by a deleted RestoreJob of the same name), instead of skipping the rename and restoring without keeping the current disk. Any other failure to keep a disk, including a race in which the rename read its just-created -orig PVC back through the cache and failed, now holds the restore back and retries it before its DataVolume is deleted. Previously such a restore reported success without restoring the disk. The PV's reclaim policy before the switch to Retain is recorded in the backups.cozystack.io/original-reclaim-policy annotation on the PV and the -orig PVC; set it back on the PV before deleting the -orig PVC, or its volume is not reclaimed. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * In-place restores that preserve original PVCs can now resume safely after interruptions, including when reads lag behind recent changes. * Restores stop before suspending the VM if a conflicting `-orig` PVC already exists. PVC rename failures can be retried, while conflicting names remain fatal. * Original PV reclaim policies are recorded and restored before preserved PVCs are deleted. * **Documentation** * Updated the restore guide with disk-reclamation steps and guidance for resolving leftover PVC name conflicts. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Aleksei Sviridkin
|
534f8f1db9 |
docs(backups): stop presenting admin-only demo steps as tenant work
The kafka and nats walkthroughs put steps 03, 04, 06, 07 and cleanup under the tenant persona, and the scenario docs and helper comments had the tenant create the backup Secret. The scripts read, create and delete Secrets, wait on HelmReleases, COSI objects and operator resources, exec into a CLI Pod, read Pod logs, and delete the Backup and the cluster-scoped BackupClass and strategy. The tenant roles grant none of that, so a tenant following the docs hit Forbidden at the first read. The tables now say admin and name the part a tenant admin can do on its own, and the scenario docs say the same. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
874e1e44b7 |
fix(backups): run the kafka-metadata demo CLI Pod as restricted
The kafka-metadata walkthrough started its long-lived CLI Pod with a bare kubectl run, so the Pod carried no securityContext and a namespace enforcing the restricted Pod Security level would reject it. The kafka topic-data walkthrough already layers the restricted context onto its CLI Pod; the metadata demo now applies the same JSON patch. A running CLI Pod left by an earlier run without that context is replaced rather than reused, since a Pod's securityContext cannot change in place. The new bats cases read the overrides kubectl run receives and fail unless both the Pod-level and the container-level fields are present and the patch is sent as --override-type=json, and check that an owned Pod without the restricted context is recreated. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Mattia Eleuteri
|
d8d9608e22 |
fix(backups): refuse a foreign -orig PVC and record the kept disk's reclaim policy
renamePVC returned early whenever a PVC named <dv>-orig-<hash> already existed, without checking it came from this rename. The hash covers only the RestoreJob name, so a RestoreJob created under the name of a deleted one finds the -orig PVC its predecessor left behind. The rename was skipped, the DataVolume deleted anyway, and the restore ran as if keepOriginalPVC were false while reporting success: the live disk was not kept, and the -orig name held an older one. Any other rename failure ended the same way, since its error only became an event. A -orig PVC now records the UID of the RestoreJob that created it. A name held by any other PVC fails the restore before the VM is halted, an interrupted rename of our own resumes instead of stopping halfway, and a rename failure fails the restore before the DataVolume goes. The PV is switched to Retain so it outlives the original PVC, and the policy it had was discarded. Nothing in the controller deletes the -orig PVC or restores that policy, so a PV that was Delete stayed Retain and its volume leaked once an operator removed the claim. The policy is now recorded on the PV and on the -orig PVC, and the restore scenario documents how to set it back. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <mattia@hidora.io> |
||
|
Aleksei Sviridkin
|
04d469fc85 |
fix(tests): let the backup walkthrough waits outlive their installs
The backup round-trips drive examples/backups, whose bucket waits gave up at 300s and whose postgres waits at 360s, under the 600s install timeout cozystack-api gives each of those releases. The kafka example's bucket wait passed no budget and took the helper's 300s default. A failed install there reported only that the wait ran out. Raise each to 660s, and grow the Chainsaw op around every walkthrough by what its waits gained, so a failure at any later wait keeps the headroom it had and is still reported by the script rather than cut off by the op. The kafka and kafka-metadata Kafka waits stay: their 7h install timeout cannot be cleared inside the E2E job cap. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
873d9f4c8e |
fix(backups): stop the nats and kafka-metadata demos using a Pod they refused
Both demos read their CLI output through command substitutions, where bash does not apply errexit. When the ownership check refused a same-named Pod another party owns, the refusal was ignored and the exec went into that very Pod. A failed read of the Pod's phase or owner was likewise read as "no Pod", so the helper went on to delete and replace a Pod whose owner it had not seen. Every step of the Pod setup now returns on failure, and the phase and owner reads tell a missing Pod (--ignore-not-found) from a failed read. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
ead7048b35 |
refactor(backups): define the walkthrough wait helpers once
Every backup walkthrough carried its own wait_for_field, and most their own wait_hr_ready and wait_deleted: twelve, eight and two copies, which had already drifted into five, three and two bodies. A fix made in one copy did not reach the others, and a diagnostics change carried by hand between two of them once went into the wrong function and was caught only by reading the diff. The helpers now live in examples/backups/_lib/wait-helpers.sh, sourced by each walkthrough's 00-helpers.sh. Where copies differed, the shared body keeps the diagnostics any of them had: wait_for_field leaves kubectl's stderr attached, and wait_hr_ready prints the conditions and history dump on Stalled or timeout. Call sites and budgets are unchanged. A new unit test fails when any walkthrough defines one of these helpers again, which replaces the byte-identity pin between copies. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
5318b5e507 |
fix(backups): exec into a CLI Pod in the kafka topic-data demo
kafka_run seeded and read topics through a throwaway `kubectl run -i` Pod per call. That Pod's stdout returns over an attach that carries only what the container writes after the attach registers, so a CLI that finishes first reads as an empty reply with exit 0 -- the empty-read defect already fixed in the redis, nats and kafka-metadata demos. The topic count, dump and replication-factor checks all read that reply. Run one long-lived CLI Pod, keeping the restricted security context, and exec into it. It carries its own name and owner label so it cannot collide with the kafka-metadata demo's Pod in a shared namespace, and cleanup.sh removes it by that label. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
9b56e70ca6 |
feat(mongodb): support the system-bucket backup flow (useSystemBucket) (#3967)
## What this PR does MongoDB was the only `cozy-default` app with **no `useSystemBucket` flow**: its backup storage was always static on the `PerconaServerMongoDB` CR (bucket/endpoint/credentials from `backup.*` values plus a chart-emitted `<release>-s3-creds` Secret), so a tenant could not back up to the platform system bucket out of the box the way Postgres/ClickHouse/MariaDB/etcd do. This closes that gap so the default MongoDB backup-restore works OOTB. PBM takes `bucket`/`endpointUrl`/`region`/`prefix` as **static CR fields** (not secret refs), and the platform bucket name/endpoint are only known at BackupJob time — so, unlike ClickHouse's runtime `secretKeyRef` env, the chart cannot render them. The implementation mirrors the CNPG driver's pattern: - **Chart** (`packages/apps/mongodb`): a new `backup.useSystemBucket` flag keeps `spec.backup.enabled` (so the pbm agents run) but omits the static `storages`/`tasks`/`pitr` and the `<release>-s3-creds` Secret. Helm never owns `spec.backup.storages`, so a Flux re-render can't fight the driver. A `bootstrap.enabled + useSystemBucket` render guard mirrors Postgres, and `useSystemBucket: true` with `backup.enabled: false` is refused both at render time and by an `x-kubernetes-validations` rule on `backup` in the schema (the same mechanism as the `storageClass` immutability rule). Note on the schema rule: the aggregated apiserver does not evaluate CEL on application-backed resources yet (#2657), so today it is a UI/schema-level marker and the render-time refusal is what stops a Flux release; it also trips the API review gate (“validation rule added”), so this PR needs an API-owner approval on top of the code review. - **Strategy** (`cozy-default-mongodb`): now carries the S3 coordinates (`endpointUrl` already scheme'd by the `backupstrategy-controller.endpoint` helper, `credentialsSecret: cozy-backups-creds`, `prefix: <namespace>/<application>`). - **Driver** (`psmdbstrategy_controller.go`): SSA-injects `spec.backup.storages[s3-storage]` onto the live `PerconaServerMongoDB` at BackupJob time under a dedicated field manager (`cozystack-psmdb-backup-driver`) with `ForceOwnership`. Injection is **gated on the app's `backup.useSystemBucket` flag** (a legacy app leaves it false and is never touched; a useSystemBucket app is re-injected once per mint (once per BackupJob, plus once per storage-race re-mint), before its operator CR is created, so a later coordinate change catches up on the next job while a streaming dump is never patched under). The backup CR is minted only once the operator reports the storage pushed into pbm (`status.observedGeneration` caught up and `PBMReady` True): pbm writes to what its own config says, not to the spec, so a CR minted before that push would land at the previous coordinates while its Backup recorded the new ones; the job waits at `PerconaServerMongoDBPBMConfigPending` meanwhile, bounded by the backup deadline. Once a job has its CR, none of the mint-time reads or holds can fail it: the strategy and app reads the artifact still needs are bounded by the 24h running ceiling instead, and the cluster is not read at all **and on the release having rendered that flag**: the app CR is projected from the HelmRelease's desired values, so the flag reads true before helm-controller renders the revision, while the tenant's own storage, scheduled tasks and PITR stream from the previous render still sit on the cluster. As long as the cluster carries `spec.backup.tasks` or `spec.backup.pitr`, the driver does not inject and holds the BackupJob at `Ready=False PerconaServerMongoDBLegacyRender` (failing it at the deadline), so a wedged legacy revision never has its nightly dump and oplog redirected into the shared bucket. The backup snapshot falls back to the strategy coordinates when the operator status echo is empty; that fallback is gated on the CR's own `percona.com/delete-backup` finalizer (the flow the CR was minted on), not on the app flag re-read at completion, so a legacy backup never records the platform bucket and a flag flipped mid-dump does not change the artifact. The restore path rebuilds `backupSource` from it (the `cozy-backups-creds` reference is projected into the restore namespace before dispatch). - **Retention/cleanup**: on the `useSystemBucket` flow the driver owns the shared-bucket archive (the chart's psmdb `tasks[].keep` retention is gated out, and an on-demand backup is not covered by task retention). It stamps the operator's `percona.com/delete-backup` finalizer on each backup it mints **against a storage that carries the platform coordinates** — the strategy's bucket and `credentialsSecret`, read back from the live cluster after the injection — never on the flag alone: a strategy without an `s3` block, or a storage that points anywhere else, holds the job (`MongoDBStrategyHasNoS3` / `PerconaServerMongoDBStorageForeign`) instead of minting a finalized dump against a bucket the platform does not own. On Cozystack `Backup` deletion (by hand today; by platform retention once it lands, since a `Plan` only creates BackupJobs and never deletes a `Backup`) the driver deletes the operator `PerconaServerMongoDBBackup` CR so that finalizer frees the object — the finalizer's presence is the ownership marker cleanup reads back. That reading is unambiguous because cleanup resolves the CR by the name recorded in `driverMetadata` (the one the driver minted), not by scanning: the psmdb operator does stamp `percona.com/delete-backup` on its own `cron-*` task-minted backups, but those are never in scope. A legacy backup keeps the pre-existing hands-off contract, decided from the snapshot before any read of the operator CR. A terminating namespace or the `backups.cozystack.io/skip-artifact-cleanup: "true"` annotation strips the driver's finalizer and releases; a `Get` of the operator CR that fails with anything other than NotFound/NoMatchError still requeues, and the operator's own `release-lock` finalizer can hold the CR while the operator is down, so those two are what can keep a namespace in `Terminating`. `schedule` and `retentionPolicy` are inert on this flow (there is no engine-side retention and a `Plan` has no retention field yet); platform-level retention is tracked in [#4192](https://github.com/cozystack/cozystack/pull/4192) and teardown GC of orphaned backups in [#3966](https://github.com/cozystack/cozystack/issues/3966), so until those land bound `cozy-backups` with an S3 lifecycle policy. - **Bounds on the waits**: the injection race with the operator's cluster cache (an `unable to get storage` error on a freshly injected storage) is retried by deleting the errored CR and minting a fresh one, with the retry count kept on the BackupJob (`backups.cozystack.io/mongodb-storage-race-retries`) because the deleted CR is reaped before the next poll; after three retries the operator's error is terminal. A system-bucket dump that has started streaming is not cancelled on the 30-minute deadline, but fails after 24 hours in `running` (`BackupAbandoned` warning naming the operator CR, which is left in place as the handle to whatever pbm still writes). The deadline cancel of a not-yet-started CR pins its delete to the resourceVersion the live read answered for. - **RBAC**: `patch` on `perconaservermongodbs` for the injection (server-side apply), and `create`/`delete`/`patch` on `perconaservermongodbbackups` — `delete` to prune the shared-bucket archive on `Backup` deletion, `patch` to strip the driver's finalizer when releasing a wedged cleanup. Coverage: Go unit tests over `internal/backupcontroller` — the injection gate keyed off the app flag (legacy cluster never patched) and off the applied render (a cluster still carrying tasks or pitr is held, untouched), the SSA-injection precondition with the whole entry (per-application prefix included) asserted on the cluster, strategy templating per application, the finalizer refused against a storage without platform coordinates (strategy without `s3` over a legacy entry, foreign coordinates on the merged entry), the retry budget driven with the operator simulated reaping the deleted CR between polls, the restore snapshot fallback and credential swap across all four legacy/system flow combinations, the finalizer-owned cleanup with its escape hatches (skip annotation before any read, `NoMatchError` treated as gone, terminating-namespace release using the pre-read CR, a failed strip surfaced as an Event, a legacy CR never deleted, a legacy snapshot released without a read, a system-bucket snapshot reaching the prune), the deadline cancelling a not-yet-running operator backup only on a positive live read (including no live client at all, and a CR that moved since the read), and the running ceiling. Plus helm-unittest on both charts (chart skips the creds Secret and omits `spec.backup.storages`/`tasks`/`pitr` on `useSystemBucket` and keeps them on the legacy flow; the strategy renders the S3 coordinates and the `insecureSkipTLSVerify` knob). A live `useSystemBucket` backup+restore e2e against the platform bucket is a follow-up (needs the system bucket wired in CI, same status as the Postgres/ClickHouse useSystemBucket e2e). One item to confirm in e2e: the psmdb operator accepting `spec.backup.enabled=true` with an empty `storages` between install and the first BackupJob. ### Screenshots Not applicable (no UI changes). ### Downstream repositories The tenant-facing schema surface is one new field, `spec.backup.useSystemBucket`, on the MongoDB `values.schema.json` / CRD (regenerated), plus a validation rule on `spec.backup`. `terraform-provider-cozystack` hand-models MongoDB and may want to expose it, but it is a new optional boolean that defaults false (no drift for existing users), and the chart `README.md` regenerates on release. Leaving every box unticked per the trigger-map guidance so a maintainer decides on the provider follow-up. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note feat(mongodb): MongoDB backups now support the platform system bucket via backup.useSystemBucket, so the default cozy-default BackupClass works without per-app S3 credentials — the backup driver injects the system-bucket storage onto the cluster at backup time, once the release has rendered the flag. On this flow nothing prunes the shared bucket automatically yet (a Plan carries no retention field; tracked in #4192 and #3966), so an S3 lifecycle policy on cozy-backups is a prerequisite for enabling it, not a suggestion. useSystemBucket requires backup.enabled=true; the chart refuses the combination with backups disabled at render time, and the schema carries the same rule as a marker until the apiserver enforces CEL (#2657). A system-bucket dump still running after 24 hours fails its BackupJob with the operator backup left in place. A MongoDB restore now adopts the target cluster's S3 credentials only for a storage on the same bucket as the archive (it used to adopt them unconditionally); a legacy restore into a target on a different bucket that relied on one key pair covering both must keep or re-create the source release's -s3-creds Secret by name, and is held at Ready=False RestoreCredentialsMissing until it exists. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added opt-in system bucket mode for MongoDB backups. * Automatically injects platform bucket storage without application-managed S3 credentials. * Supports endpoint, region, path prefix, addressing, and TLS settings. * Manages shared-bucket backup artifacts when system bucket backups are deleted. * **Bug Fixes** * Improved restore credential selection to avoid cross-bucket mismatches. * Preserved backup metadata when storage details are unavailable. * **Documentation** * Documented configuration, scheduling, retention, PITR limitations, and restore guidance. * **Validation** * Prevented incompatible bootstrap and system bucket restore configurations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Andrey Kolkov
|
46a8bd7363 |
fix(mongodb): apply the storage once per job, and pin the terms that decide ownership
The system-bucket storage was applied on every reconcile of a job, not once per job: with the operator CR already minted nothing returned before the apply, so a streaming dump saw an SSA patch every poll for up to a day, and the only thing keeping that patch off a cluster that had re-acquired the chart's tasks and pitr since the mint was a predicate term no test held. The storage a CR names is fixed for its dump, so the apply now runs only before the CR exists; the legacy-render hold covers that window, and a test pins that an existing CR is never injected over. The other predicate that decides whether a Backup owns an archive compared the live entry's bucket and credentialsSecret against the strategy, and only the bucket half had a fixture. The credential is what tells a tenant's entry from the platform's when a bucket name collides, so a case varying only that term joins the foreign-bucket one. The cleanup shortcut's snapshot-kind term and the 24h running ceiling are pinned the same way, the latter as an absolute boundary pair. The generated field description said the schema rejects useSystemBucket without backup.enabled. The aggregated apiserver does not evaluate CEL on application schemas yet, so the render-time refusal is the one that holds; the description and the runbook now say so. The runbook's restore paragraph still described the unconditional credential swap this branch narrowed, exempted MariaDB from a retention warning its shipped strategy does not earn, and pointed at a Plan retention field that does not exist; the example README and a dispatcher test's comment still carried the pre-branch claim that the driver never injects and never deletes. Assisted-by: LLM Signed-off-by: Andrey Kolkov <androndo@gmail.com> |
||
|
scooby87
|
03e2da5805 |
feat(postgres,mariadb)!: drop plaintext user passwords (#4078)
## What this PR does Removes the plaintext `users[].password` field from the `apps/postgres` and `apps/mariadb` charts (and the mariadb `root` account), so managed-database passwords can no longer be pinned in tenant values or committed to git. Passwords are always chart-generated into the `<release>-credentials` Secret and preserved across reconciles via `lookup`, matching the model `redis`/`valkey` already use. This closes the "User passwords" item of the portal upstream epic (aenix-org/cozyportal#1060). The generated password never lands in the init-script Secret or the SQL it runs: the postgres init-job reads each user's password from the mounted `<release>-credentials` Secret at run time and applies it through a psql variable (`ALTER ROLE … WITH PASSWORD :'pw'`), so it does not linger in the init-script manifest a tenant can read. It does live in cleartext `stringData` in the `<release>-credentials` Secret itself, which is a chart manifest and therefore kept in Helm release history for `MaxHistory` revisions like any chart-generated secret — that residue is exactly why rotation cannot be built on the chart (see below). A `password` or a `passwordRotation` key left over in a tenant's values is accepted-but-inert and draws an admission warning, so an operator editing it is told it has no effect rather than getting a silent 200. Backup/restore into a copy keeps the target's chart-managed credentials working: the CNPG driver clears `bootstrap.enabled` once recovery is healthy so the init-job re-applies the generated passwords onto the recovered roles, and the MariaDB driver excludes `mysql.global_priv` from the dump (`ignoreGlobalPriv`) for backups captured from this release onward, so restoring one of those into a copy does not clobber the target's chart-managed users or `root`. Excluding the grant table has a flip side, and it applies to every MariaDB restore, not only a to-copy one: only accounts declared through `User` CRs (the chart's `users` map) are captured, so an account created out of band directly in MySQL is in no backup and a restore brings back its data and `mysql.db` grants but not the account itself. This is also a backup-side fix only: a restore from a backup taken **before** this change still replays the source's `mysql.global_priv` and overwrites the target's users and `root`, and `root` then needs an out-of-band reset (the operator applies `rootPasswordSecretKeyRef` only at datadir bootstrap, so there is no in-band path). See `docs/operations/backup-classes.md` for the convergence window, the out-of-band-account limitation, and this old-backup caveat. Password **rotation** is deliberately **not** in this PR. A chart-rendered credentials Secret keeps its cleartext in Helm release history for `MaxHistory` revisions, so a rotation built on it cannot revoke a leaked password; rotation is being reworked as a controller that owns the Secret — design proposal in cozystack/community#72. Migration is seamless: existing releases keep their current passwords on upgrade (the value already lives in `<release>-credentials`, which `lookup` re-reads and preserves); a `password` still present in a tenant's values is ignored, so no existing HelmRelease fails validation. Verification: `helm unittest` passes for both charts (postgres 87, mariadb 126, including render-time username guards that reject any name k8s would later reject as an invalid Secret key or User resource name); `go build ./...`, `go vet`, the backup-controller tests and the apps-apiserver tests are green; `make generate` was run with the CI-pinned `cozyvalues-gen` v1.6.0 so `values.schema.json`, the READMEs, the API types, and the `cozyrds` ApplicationDefinitions are all in sync (the branch was rebased onto `main`, which reconciled cleanly with the read-replica autoscaling feature that landed there). ### Screenshots N/A — no UI change. ### Downstream repositories - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: affected — `content/en/docs/next/cozystack-api/go-types.md` is hand-written and still shows `Password:` in the Go examples, and the managed-app reference pages regenerate from these chart READMEs, so both document `users[name].password` until refreshed (not yet filed). - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: affected — the provider hand-maintains the postgres/mariadb schemas and per-field defaults; this PR removes `users[].password`. A follow-up issue/PR is needed there (not yet filed). `terraform-provider-cozystack` and `website` are reached by this diff; both boxes are left unticked because the follow-ups are not yet filed. Same-repo future work this change suggests: the controller-owned password rotation (design in cozystack/community#72), and extending the "passwords only in the Secret" model to the other engines that still accept plaintext (`clickhouse`, `mongodb`, `nats`, `opensearch`, `rabbitmq`, `vpn`). ### Release note ```release-note feat(postgres,mariadb)!: passwords for managed PostgreSQL and MariaDB users can no longer be set via `users[].password` — they are always auto-generated and stored only in the `<release>-credentials` Secret, never rendered into the init-script or the SQL it runs. Existing releases keep their current passwords on upgrade; a `password` (or a `passwordRotation`) left in values is ignored with an admission warning. Password rotation is deferred to a follow-up controller design (cozystack/community#72). MariaDB backups now exclude the `mysql.global_priv` grant table (`ignoreGlobalPriv`), so a restore recreates only accounts declared through the chart's `users` map — an account created out of band directly in MySQL is not captured and must be recreated after a restore. ``` |
||
|
IvanHunters
|
c355de63ed |
fix(backups): exec into a CLI Pod in the nats demo
nats_cli seeded and verified JetStream streams through a throwaway `kubectl run -i` Pod per call, with the same attach-race empty-read the redis demo fixed. NATS ships no long-lived Pod carrying the `nats` CLI, so run one nats-box Pod and `kubectl exec` into it; stream_message_count keeps the exec's stderr so a failed read says why. The password travels in NATS_PASSWORD on stdin rather than the --server URL the exec request records in the apiserver audit log. The Pod carries cozystack.io/backup-demo=nats: nats_cli_pod reuses only a Ready Pod the demo owns, replaces a leftover in a terminal phase, and refuses a same-named Pod it does not own; cleanup.sh reaps by that label with --grace-period=1. Unit tests under hack/ pin the exec path, the preserved stderr, and the password staying out of argv. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
9cea7dcd90 |
fix(backups): exec into a CLI Pod in the kafka-metadata demo
kafka_run seeded and verified topics through a throwaway `kubectl run -i` Pod per call. That Pod's stdout returns over an attach that only carries what the container writes after the attach registers, so a CLI that finishes first comes back empty with exit 0 -- the empty-read defect fixed for the redis demo. Run one long-lived CLI Pod and `kubectl exec` into it: an exec'd process owns its pipes, so its output cannot be lost that way, and stderr stays attached (the topic_meta / topic_partitions reads keep it) so a failed read says why. The Pod carries cozystack.io/backup-demo=kafka-metadata: kafka_cli_pod reuses only a Ready Pod the demo owns, replaces a leftover in a terminal phase, and refuses a same-named Pod it does not own; cleanup.sh reaps by that label with --grace-period=1. Unit tests under hack/ pin the exec path -- a throwaway run Pod or a re-added stderr redirect reddens them. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
7b94287970 |
fix(backups): wait for the platform strategy in rabbitmq and kafka demos
The rabbitmq and kafka-metadata backup demos read the platform's cozy-default-<engine> strategy once, right after install, where the Helm upgrade that renders it is gated on dependsOn and can leave it absent for minutes; the demo then failed immediately instead of waiting. Replace the single read with kubectl wait --for=create, so it polls through NotFound and fails only on a real error. Assisted-by: LLM Signed-off-by: IvanHunters <xorokhotnikov@gmail.com> |
||
|
IvanHunters
|
9e3ed1f2cb |
fix(backups): stop the redis backup demo from reading empty values (#4399)
<!-- Thank you for making a contribution! Here are some tips for you: - Use Conventional Commits for the PR title: `type(scope): description` - Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore - Scopes are not an exhaustive list — pick the most specific scope for the change and extend the list when a genuinely new area appears. Examples: - System components: dashboard, platform, operator, cilium, kube-ovn, linstor, fluxcd, cluster-api - Managed apps: postgres, mariadb, redis, kafka, clickhouse, virtual-machine, kubernetes - Development and maintenance: api, hack, tests, ci, docs, maintenance - Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or add a `BREAKING CHANGE:` footer - If it's a work in progress, consider creating this PR as a draft. - Don't hesistate to ask for opinion and review in the community chats, even if it's still a draft. - Add the label `kind/backport` if it's a bugfix that needs to be backported to a previous version. --> ## What this PR does The redis backup round-trip suite went red on PRs that do not touch redis, each time on an empty read (#4395). The report snapshots of those runs point to two separate causes, and I fix both here. The empty marker reads came from the demo helper that ran redis-cli in a throwaway `kubectl run -i --rm` Pod. kubectl gets that Pod's stdout over an attach, and containerd's attach only carries output written after the attach is registered. kubectl falls back to the Pod log only when the attach fails. A redis-cli that finishes in that gap comes back as an empty string with exit 0. In the failed runs, the master's log shows the key present, and sentinel logged no failover, yet the GET read ''. `redis_cmd` now runs redis-cli by `kubectl exec` in a sentinel Pod, which already has it. An exec'd process gets its own pipes, so its output can't be lost that way, and its exit code comes back. The password now goes over stdin rather than a Pod env var. The missing-strategy failure was real: `cozy-default-redis` did not exist yet. It gets rendered behind a lookup of the platform bucket, so it appears only after a forced Helm upgrade. helm-controller holds that upgrade while `cozystack-basics` is not Ready, and the e2e install's late platform patches keep it not Ready for several minutes. The demo read the object once. It now waits for the object with `kubectl wait --for=create` and a 15 minute bound. That wait fails right away on anything other than NotFound. The demo's reads also stop sending stderr to `/dev/null`, so the next failure names its cause. `hack/redis-backup-demo-helpers.bats` covers `redis_cmd` with stubbed kubectl and redis-cli: it reads through exec, keeps the password out of argv, sends no AUTH when there is no Secret, and fails loudly when the exec fails or sentinel has no master. Closes #4395 ### Screenshots <!-- REQUIRED for UI changes: attach screenshots or screen recordings demonstrating the visual impact of your changes. PRs with UI changes without screenshots will not be merged. --> No UI changes. ### Downstream repositories <!-- Cozystack is upstream for repositories that are NOT kept in sync with it automatically, and nothing in CI compares the two sides, so a change here can break them silently. Which change forces what, and which file to touch, is the trigger map in docs/agents/contributing.md under "Downstream Repositories". Walk it before you tick anything. IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM TO FILL IN: 1. Walk the trigger map against the actual diff of this PR, file by file. Do not decide from the PR title or from your memory of what you changed. 2. For every repository you tick, open the follow-up PR there and link it on the same line. A ticked box with no link claims work that does not exist, which is worse than an empty box. Search that repository for an open PR or issue covering it first, and link that instead of filing a duplicate. 3. When the follow-up is out of scope, or needs a decision that is not yours to make, open an issue in that repository instead, link that, and say so in the PR body. Do not push a speculative PR into another repository just to fill a line here. 4. Do not tick "No downstream repository is affected" to make the checklist go away. If you are unsure, leave every box empty and say so in the PR body, so a human decides. --> - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: The diff touches only `examples/backups/redis/` and a new unit test under `hack/`. No entry in the trigger map covers either. ### Release note <!-- Write a release note: - Explain what has changed internally and for users. - Start with the same `type(scope):` prefix as in the PR title - Follow the guidelines at https://github.com/kubernetes/community/blob/master/contributors/guide/release-notes.md. --> ```release-note fix(backups): the Redis backup demo reads its keys through `kubectl exec`, so a short-lived Pod can no longer drop its output, and it waits for the platform's `cozy-default-redis` strategy to appear instead of failing on its first read. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Redis backup and restore commands now report Kubernetes and Secret read errors instead of hiding them, while still treating a missing password Secret as empty. * Redis commands pass authentication securely and fail clearly when Sentinel cannot identify a master. * Backup demos wait for the Redis backup strategy to appear and provide clearer error messages. * **Tests** * Added coverage for Redis authentication handling, command failures, and missing Sentinel masters. <!-- end of auto-generated comment: release notes by coderabbit.ai --> |
||
|
Alexey Artamonov
|
f02ca8cbc2 |
test(backups): log in as an application user after each restore
The backup examples and e2e suites no longer set passwords. They read the generated ones from <release>-credentials with jq, since a username may contain a dot that jsonpath treats as a path step, and log in as an application user after each restore, so a restore that leaves the credentials unusable fails the run. The MariaDB run also checks that an account created out of band on the source is absent from the copy. The unreachable-target restore step asserts a terminal Failed attributed to the recovery window, since the reason itself is best effort. Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io> |
||
|
Myasnikov Daniil
|
cadd72a4be |
fix(kafka): use the shared wait_hr_ready body in the topic-data example
The topic-data example was branched before the backup examples moved to one wait_hr_ready body and merged after they did, so it carried the old copy. hack/mariadb-first-boot-waits.bats finds every copy in the tree and runs its failure branches under errexit, and the old one does not report a kubectl that cannot answer. That turned the unit job red on main and on every pull request tested against it. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com> |
||
|
Timofei Larkin
|
95ece888ba |
feat(kafka): add topic-data backup/restore example and e2e roundtrip (#3580)
## What this PR does
Adds a worked backup/restore example for a Cozystack-managed `Kafka`
application's **topic data**, built on the generic `Job` backup strategy
— the same pattern as `examples/backups/nats`, with no purpose-built
backup image. A stock Strimzi Kafka image (the `kafka-*.sh` CLI plus
`bash`, `curl` and `tar`) runs one shell script that branches on `{{
.Mode }}`: on backup it freezes each partition's end offset and drains
`[begin, end)` to a tarball it `PUT`s to S3; on restore it `GET`s the
tarball, recreates the topics with their original partition count and
replication factor, and replays every partition file. S3 access is `curl
--aws-sigv4`, verified against the endpoint's CA projected from the
`<app>-backup-s3` Secret, so no extra client image is needed.
The consistency model is a frozen-end-offset cut: `kafka-get-offsets
--time -1` is read once per partition at backup start and the consumer
drains only up to that offset, so anything produced during the run is
excluded and every partition of a topic is captured as of the same
instant — a cut by log position, independent of record timestamps.
This is deliberately a DATA-only backup. Topic configs, ACLs, SCRAM
users and **consumer-group offsets** are not captured, and re-produced
records receive fresh offsets — the README spells out the scope and
points at a volume-snapshot strategy for a faithful, offset-preserving
backup. Keyed records reproduce into their original partition because
the topic is recreated with the same partition count and the default
(murmur2) partitioner is deterministic.
The script fails closed rather than reporting a partial backup or a
lossy restore as success: it refuses an offset listing short of the
topic's partition count, a drain short of `end - begin`, a `topics`
value with a duplicate or a name outside Kafka's charset, a manifest
that disagrees with the data files, a target topic that already holds
records (checked before the replay, so a retried Pod cannot append a
second copy), a pre-existing topic with a different partition count or
replication factor, a replication factor below the target's effective
`min.insync.replicas`, and a post-replay count that does not match the
manifest. Each topic's replication factor is recorded in the manifest
and restored as captured; the `replicationFactor` BackupClass parameter
overrides it for restoring into a smaller cluster.
Also adds a `kafka-2-backup-roundtrip` Chainsaw test to
`hack/e2e-chainsaw/kafka`, which drives the example `run-all.sh` as its
harness (Bucket → two-broker source Kafka with a seeded 3-partition,
2-replica topic plus a decoy topic that the demo topic matches as a
regex but not as a literal → BackupJob → in-place restore with count,
content and replication-factor assertions → a second RestoreJob into the
populated topic that must be refused with the topic untouched →
two-broker target Kafka + to-copy restore + verify), mirroring
`mariadb-2-backup-roundtrip`. Two brokers, not one, because that is the
shape on which the chart sets `min.insync.replicas: 2` and a restore
with the wrong replication factor cannot land a record. It resolves the
Kafka image from the running operator so the backup Pod reuses the image
already cached on the nodes.
The strategy script itself is pinned by
`hack/kafka-topic-backup-strategy.bats`, on the shape of
`hack/kafka-backup-script.bats`: it renders the strategy through a stub
`kubectl` exactly as step 01 applies it, lifts the container body out,
and executes it against stubbed `kafka-*.sh`, a stubbed `curl` and a
directory standing in for S3, with fault injection for the broker
behaviours the guards exist for. Fourteen cases, one per guard; removing
the partition-listing guard reddens one of them.
Verified: `chainsaw/kafka-2-backup-roundtrip` passed in this PR's CI on
the two-broker shape (record count, byte-identical content dump and
replication factor 2 asserted after both restores, the refused
RestoreJob settled Failed with the topic untouched);
`hack/kafka-topic-backup-strategy.bats` passes under both
`hack/cozytest.sh` and bats.
Files:
- `examples/backups/kafka/` — 14 files (`00-helpers.sh`, `01`..`07`,
`run-all.sh`, `cleanup.sh`, `README.md`, `90`/`91`/`92` scenario
walk-throughs).
- `hack/kafka-topic-backup-strategy.bats` — hermetic test of the
strategy script.
- `hack/e2e-chainsaw/kafka/chainsaw-test.yaml` — new
`kafka-2-backup-roundtrip` Test appended.
- `hack/backup-access-preflight.bats` — the kafka example and Test join
the S3-preflight roster.
- `hack/select-e2e.sh`, `hack/e2e-chainsaw/.chainsaw.yaml` —
comment-only: their enumerations of preflight-gated suites are replaced
by a pointer.
- `.gitignore` — the example's `.source-dump.txt` snapshot.
No package sources (`values.yaml`, `values.schema.json`, `Chart.yaml`,
package `README.md`) are touched, so `make generate` produces no diff.
### Screenshots
N/A — no UI changes.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` file-by-file
against the diff. The change adds example scripts under
`examples/backups/kafka`, one unit test under `hack/`, and appends one
Chainsaw Test to an existing file under `hack/e2e-chainsaw/kafka` — it
adds no package under `packages/apps` or `packages/extra`, changes no
CRD, no `ApplicationDefinition`, no `values.schema.json`, no
platform/installer values, no `hack/*.mk` anchor or make-target
behaviour, and no node prerequisites in `hack/e2e-prepare-cluster.bats`.
None of the downstream triggers match.
- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
### Release note
```release-note
feat(kafka): add a topic-data backup/restore example and e2e round-trip on the generic Job backup strategy (data-only: topic records with a frozen-end-offset consistency cut and partition-preserving replay; consumer offsets, topic configs, ACLs and users are out of scope)
```
|
||
|
Andrey Kolkov
|
7ce85a2f7f |
test(kafka): pin the strategy script's guards with a hermetic bats suite
The in-Pod script had grown eight fail-closed guards across as many rounds, and the only thing exercising any of them was a 40-minute cluster test that drives the happy path plus one refusal. The partition listing guard in particular - the whole of the previous round's second fix - could be deleted with nothing in the tree going red. Add hack/kafka-topic-backup-strategy.bats on the shape of hack/kafka-backup-script.bats: render the strategy through a stub kubectl exactly as step 01 applies it, lift the container body out of it, and execute it against stubbed kafka-*.sh, a stubbed curl and a directory standing in for S3, with fault injection for the broker behaviours the guards exist for. Deleting that listing guard now fails one case; the script gains BIN/SCRATCH/CA_FILE overrides so it can run outside the Pod, each defaulting to the Pod's value. The same rounds' smaller findings: the manifest's topic name is held to Kafka's charset like the topics parameter, since it reaches unquoted expansions with globbing on; the shape check after --create compares the replication factor as well as the partition count, and refuses a replication factor below the topic's effective min.insync.replicas before replaying, because kafka-console-producer only logs a rejected acks=all write and the run would otherwise end at the count check with the cause unnamed; the manifest's replication factor must agree across a topic's lines; topic_records carries its own || return 1 on each command, as a function called as the operand of || runs with errexit off; the CA lookup in step 03 distinguishes a denied read from an absent Secret, and the README's persona column says the copy needs the admin; the e2e comment no longer claims the 2-label endpoint fails the serving cert's SANs, which include '*.tenant-root'. Rebased onto main, which moved the chart to KRaft: the demo's Kafka CRs drop the zookeeper block the chart no longer has. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-By: LLM |
||
|
Andrey Kolkov
|
5bd029c46a |
fix(kafka): restore with the captured replication factor and a complete listing
Record each topic's ReplicationFactor in the backup manifest, off the same --describe line the partition count already comes from, and recreate the topic with it on restore. The value used to be a BackupClass parameter defaulting to 1, and the Cozystack Kafka chart sets the broker-level min.insync.replicas to 2 as soon as kafka.replicas reaches 2: kafka-console-producer writes with acks=all, the broker rejects every write into a topic with fewer replicas than that, and the producer logs the rejection and exits 0. A restore onto any multi-broker cluster therefore landed zero records and failed the post-replay count with a message naming the wrong cause; in place, the documented delete had already taken the live topic with it. replicationFactor stays as an override for restoring into a smaller cluster, and a value the target cannot host now fails at --create in Kafka's own words. Hold the pre-replay emptiness check to the topic's partition count, as the backup side already does. kafka-get-offsets skips a partition it could not read and still exits 0, so a leader election in flight dropped the populated partitions from the listing, the surviving empty ones summed to zero, and the replay appended a full copy into a live topic - the exact outcome the check was moved ahead of the replay to prevent. Both restore-side counts now go through one helper that refuses a short listing. The data-file-versus-manifest check runs both ways before anything is created, and the replay walks the manifest topic by topic, announcing each one, so a retried Pod's log says which topics landed. The demo now runs two brokers with two-replica topics, which is the shape the chart puts min.insync.replicas=2 on; the restore steps assert the restored topic's replication factor, and step 06 submits a second RestoreJob against the populated topic and requires it to be refused with the topic untouched. Smaller findings from the same round: the topics parameter is held to Kafka's name charset (a literal "*" no longer path-expands) and refused on a repeated name, which produced a Succeeded backup no restore could balance; the strategy verifies S3 TLS against the seaweedfs CA projected into the <app>-backup-s3 Secret instead of curl -k, following the rabbitmq example, so the e2e endpoint moves to the .svc name the serving cert covers; the image lookup keeps digest-pinned references; the default namespace is tenant-root, the only one the demo ever ran in; step 05 caches the resolved Backup name for the restore steps; the decoy count subtracts begin offsets like the topic count; the bats test name carries its count again; and the scenario walk-throughs the sibling examples ship are added. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-By: LLM |
||
|
Andrey Kolkov
|
b141166cde |
fix(kafka): key begin-offset lookups on topic:partition
The begin-offset lookups matched on the partition index alone, so a line belonging to another topic could supply this partition's begin. The cut then shifts silently and the drain check cannot see it, because n is derived from that same wrong begin. Match on the whole topic:partition key in all four lookups, which holds whatever order the listing arrives in and no longer depends on the topic filter being pinned. Hold the tarball's file names to the manifest as well. The manifest is validated as untrusted input, but the replay derived its topic from the data file names, so a file naming a topic the manifest never listed was replayed into a topic that skipped the create, shape, emptiness and post-replay checks while the restore still reported success. Correct what the decoy is documented to prove. It reliably covers the offset listing, the in-place delete and the two record counts; it does not cover the two PartitionCount reads, because kafka-topics --describe yields topics in the admin client's result-map order rather than sorted, so which one their head -1 sees is not something a fixture can pin down. The previous text claimed every pin reddens the run, and explained the decoy's name by a sort order that does not govern that output. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
17475f1ee5 |
fix(kafka): refuse a non-empty restore target before replaying
The replay appends and the driver retries a Pod that died mid-replay, so checking the record count only afterwards meant every attempt had already written another full copy into a live topic and the check merely reported the damage. Read the target's count before replaying and refuse a topic that already holds records, leaving it as the failed attempt left it; the post-replay check still verifies what landed. Name the decoy so it sorts before the demo topic. Both PartitionCount reads pipe --describe through head -1, so an unpinned describe reads whichever matching topic comes first: with the decoy sorting after the topic those two pins were never exercised and could be removed with the suite staying green. "-" precedes ".", so orders-v1 now sorts first and an unpinned describe reads its partition count instead. Also state in the README that an empty backup is refused on restore rather than merely uploaded, so the empty-tarball note does not read as if such an artifact were usable. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
2c9b3dfeea |
fix(kafka): validate the restore manifest and pin topic matching in the suite
The manifest arrives inside the downloaded tarball but was consumed as trusted input. A non-numeric partition field slipped through the numeric test that computes a topic's partition count: an if condition is exempt from errexit, so test exits 2, the term reads false, the count comes out short, the topic is created smaller, and the shape check then compares that short value against itself and passes. The per-topic total still added up, so the restore reported Succeeded with every keyed record re-placed. Validate each manifest field on read, refuse an empty manifest whose verification loops would otherwise run zero times and report a successful restore of nothing, and guard the restore-side PartitionCount read the way the backup side already guards its own. The delete-wait still matched the topic name as a regex; use grep -qxF. The suite now pins that class of bug instead of relying on review: the demo topic is orders.v1 and a decoy ordersXv1 sits beside it, matching the first as a regex but not as a literal. The decoy is never named in the BackupClass, so dropping any \Q...\E pin fails the run on its own - the partition-set guard counts the decoy's partitions, or the in-place delete takes the decoy with it. Also: the host-side record count skipped a partition missing from the begin-offset listing instead of defaulting it to 0, undercounting the topic; the two preflight invariants now cover the kafka flow they already describe; and the README names the disabled TLS verification. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
9cb9c70083 |
fix(kafka): match topic names literally, not as regexes
kafka-topics and kafka-get-offsets compile --topic into a Java regex and full-match it, so any name carrying a metacharacter also selects its siblings - "audit.events" matches "audit-events". Two consequences here: - The in-place step's `--delete --topic "$TOPIC"` deleted every topic the name happened to match, which for a demo meant destroying a bystander topic to "simulate data loss". - The partition-set guard read another topic's partitions into `ends` and another topic's PartitionCount into `pc`, so it either refused a legal backup or, when the counts coincided, wrote duplicate manifest lines and overwrote a data file. Pin every filtering call with \Q...\E, the same fix the kafka-metadata example carries. --create and the console producer/consumer take a literal name and stay unquoted. Also: verify the restore target's partition count matches the backup, since --if-not-exists leaves an existing topic's shape alone and the post-replay total cannot see re-placed records; stop wrapping the whole kafka_run in 2>/dev/null, which hid pod and API failures rather than in-pod noise; and point the warned image fallback at the newest entry of the operator's map, the one the resolver itself picks, instead of the oldest. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
d405a36e3c |
fix(kafka): validate the backup's partition set against the topic
The drain guard only covers a short read within a partition it was told about. kafka-get-offsets lists just the partitions whose lookup succeeded - GetOffsetShell logs a per-partition KafkaException to stderr, skips it and still exits 0 - so a partition lost to a leader election got no manifest line, no data file and no drain check, and the tarball was uploaded with the Backup reporting Succeeded. The restore-side count check could not catch it either, since it sums the same short manifest, and a missing highest-numbered partition made restore recreate the topic with fewer partitions, re-placing every keyed record. Read the topic's own PartitionCount before the drain and fail closed when the offset listing is short. A short "begins" needs no separate check: its per-partition default of 0 can only overstate n, which the existing drain check then catches. Also run the shared BucketAccess preflight in the e2e - accessGranted does not prove the S3 server reloaded the new identity, and a fresh grant answering the SigV4 PUT with 403 reds the run as a backup defect - and state the .chainsaw.yaml namespace override as the rule it is rather than as a claim about which Tests match a name pattern. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
7d8dee66a5 |
fix(kafka): verify restore landed and namespace-scope the S3 key
- Restore verification: kafka-console-producer, like the consumer on backup, exits 0 without every record being accepted, so a replay that dropped records still reported Succeeded. After the replay, compare each restored topic's record count (sum of end-begin over partitions) against the manifest and fail on any shortfall or overshoot - the latter being a restore into a non-empty topic, which this appending DATA restore does not support. - Namespace-scope the S3 object key (<namespace>/<app>/kafka-topics.tar) so two same-named apps in different namespaces sharing one bucket do not collide. The namespace is .Release.Namespace in both modes and always names the source, since a to-copy target shares the source's namespace (a TypedLocalObjectReference). - Warn instead of failing open when the Kafka image cannot be resolved from the operator: the pinned public literal is unreachable on an air-gapped cluster, so tell the caller to set KAFKA_IMAGE. - Word the .chainsaw.yaml namespace note as "most" (etcd-2-backup- roundtrip sets no namespace) and document the empty all-topics backup. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
9673af6aca |
fix(kafka): guard short backup drains and settle example teardown
- Short-drain guard: kafka-console-consumer exits 0 after a timed-out short read, so a truncated drain was tarred and reported Succeeded. Compare the drained line count against end-begin per partition and fail the backup on any shortfall, making the README's "fails closed" promise real for compacted/transactional topics. - Settling teardown: cleanup.sh returned before the Kafka apps and Bucket had uninstalled and could not report failure, letting the neighbouring chainsaw/kafka Test contend on PVC reclaim. Add a wait_deleted helper and a teardown_rc gate that waits for the heavy objects and exits non-zero when the teardown does not settle. - Fully qualify the ambiguous delete kinds (backup/bucket/backupclass/ restorejob/backupjob) so a bare shortcut cannot target a foreign group and silently leave the Cozystack artifact behind. - Default KAFKA_IMAGE from the running operator (newest STRIMZI_KAFKA_ IMAGES entry), falling back to the literal only when the operator cannot be read; drop the duplicate resolution note. - Make the strategy and throwaway CLI Pods restricted-clean (runAsNonRoot + JSON-patch securityContext on the CLI Pods). - gitignore .source-dump.txt; replace the deprecated "small" preset with c1.small; capture the to-copy cluster in the e2e catch; document the all-topics risk; de-enumerate the decaying suite-list comments. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
21f5b2db2d |
fix(kafka): correct internal-topic filter and harden the delete-wait
Two accuracy fixes in the all-topics path and the in-place delete-wait. The all-topics default filtered `^_`, which also drops single-underscore user topics such as `_schemas`, silently excluding user data from a backup that still reports success; filter `^__` (Kafka's internal topics are double-underscore). The in-place delete-wait ran `--list | grep` in a pod snippet under `set -eu` without pipefail, so a transient `--list` failure read as empty output and false-positived "deleted"; capture `--list` on its own and check its exit before trusting it. Sharpen the docs: the frozen-end-offset cut is per-topic, not cross-topic; restore uses the configured replication factor (default 1), not the source's; compacted and transactional topics are out of scope; and the endpoint parse is demo-scope (no IPv6 / path-carrying forms). Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
34a04e6b37 |
chore(kafka): document copy-safety caveats and harden all-topics loop
Document the caveats that make the example safe to copy - a null key round-trips as the literal string "null" (changing key and compaction identity), the drain stages the topic twice under /tmp with no ephemeral-storage bound, every backup of an app overwrites the same S3 key, and the Pods target warn-only Pod Security. Harden the all-topics partition-count loop: the final `[ ... ] && parts=...` returned non-zero under `set -e` when the last test was false, so use `if ... then ... fi`. Align the default KAFKA_IMAGE with the vendored operator (0.45.1-rc1) and list kafka-2- in the .chainsaw.yaml tenant-root header. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
cc8c56849d |
test(kafka): assert restore content and exact record count
The backup roundtrip only summed offset counts, so a restore that replayed garbage, scrambled keys, or ran twice (the strategy Job's backoffLimit retry re-appends from the start) still passed. Both verifications also compared with `< MESSAGE_COUNT`, so an inflated count slipped through. Snapshot the source topic in step 04 as a per-partition, offset-ordered partition/key/value dump, and diff the restored topic against it in the in-place and to-copy steps: byte-equality proves keys, values, partition placement and per-partition ordering survived, not merely the count. Make both count checks exact. Document that restore appends into a non-empty topic and is safe only against an absent or empty one. Raise the helper Pods' --pod-running-timeout to 5m so a cold image pull on a first run outside CI does not abort before seeding. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
d38b538d98 |
test(kafka): fail fast on terminal backup phases and capture strategy Job logs
Two CI-quality fixes, both diagnostics/wall-clock only (neither can turn a run falsely green): - The chainsaw comment claimed run-all.sh fails fast on terminal BackupJob/RestoreJob phases and Stalled HelmReleases, but the kafka helpers never implemented it (carried over from mariadb). A BackupJob that settled phase=Failed was polled the full 600s and then mislabelled 'Timeout', burning ~10 of the 40 chainsaw minutes per already-red run. Port the mariadb variants: wait_for_field gains a terminal fail_value arg (call sites pass Failed), and a new wait_hr_ready fails fast on Stalled=True with an existence backstop. HR waits in steps 03/04/07 now use it; the Strimzi Kafka CR waits keep their genuine Ready condition. run-all.sh runs under set -e, so a non-zero step aborts the flow immediately, matching the comment. - The catch block only captured the Strimzi broker Pods (app.kubernetes.io/instance=kafka-kafka-test); the generic Job strategy runs the drain/replay in controller-created Pods labelled job.strategy.backups.cozystack.io/mode=backup|restore (never that instance label) and not TTL-reaped, so the actual S3/SigV4 error left no log in the report. Add a second podLogs selector keyed on that label. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
b099f8dde4 |
fix(kafka): fail closed when listing topics for an all-topics backup
In the empty-TOPICS (back up all non-internal topics) branch, the topic list was resolved with a single pipeline under set -eu without pipefail: TLIST=$(kafka-topics.sh --list | grep -v '^_' || true) The pipeline's exit status is grep's alone, so a real broker error (unreachable / auth) that emits no stdout makes grep exit 1, || true swallows it, and TLIST is empty - indistinguishable from 'no non-internal topics'. The backup then drains nothing, tars an empty dir, PUTs it and reports success: a silent empty backup a later restore would treat as authoritative. Split the --list into its own command substitution so a broker error aborts the Pod under set -e; filter internal topics only afterwards, so an empty TLIST means genuinely no user topics. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
c84f77ac16 |
fix(kafka): sign S3 host without non-default port in backup strategy
The Strimzi image ships curl 7.76.1, whose --aws-sigv4 omits a non-default port from the SigV4 canonical Host while still sending it in the Host header (a bug fixed in curl 7.87.0). seaweedfs recomputes the signature from the received host:port, so every PUT/GET to the :8333 in-cluster S3 Service the e2e harness targets was rejected with HTTP 403, failing kafka-2-backup-roundtrip at the backup upload. It passed on dev7 only because that run used a :443 ingress, where the port is absent from the Host anyway. Strip the port from the signed and sent URL and redirect the connection to the real port with --connect-to, so the signed, sent and server-side Host all agree on the bare host. Verified against the image's curl 7.76.1: the emitted SigV4 signature matches a portless canonical Host, and the request still reaches the :8333 listener. Correct on curl >= 7.87 too, which drops the default port from the canonical Host as well. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Andrey Kolkov
|
7842b88676 |
feat(kafka): add topic-data backup/restore example and e2e roundtrip
Add examples/backups/kafka, a worked backup/restore of a Cozystack-managed Kafka application's topic data via the generic Job backup strategy - the same pattern as examples/backups/nats, with no purpose-built backup image: a stock Strimzi Kafka image (kafka-*.sh CLI + bash/curl/tar) runs one shell script that branches on .Mode. Consistency is a frozen-end-offset cut: kafka-get-offsets --time -1 is read once per partition at backup start and the consumer drains only up to that offset, so anything produced during the run is excluded and every partition is captured as of the same instant. Restore recreates each topic with its original partition count and replays every partition file; keyed records reproduce into their original partition via the default partitioner. This is deliberately a DATA-only backup: topic configs, ACLs, SCRAM users and consumer-group offsets are not captured, and re-produced records get fresh offsets. The README spells out the scope and points at a volume-snapshot strategy for faithful, offset-preserving backups. Add kafka-2-backup-roundtrip to hack/e2e-chainsaw/kafka, driving the example run-all.sh as the harness (Bucket -> source Kafka + seeded topic -> BackupJob -> in-place restore -> to-copy restore + verify) so the test and the documented flow cannot drift, mirroring mariadb-2-backup-roundtrip. The image is resolved from the running operator so the backup Pod reuses the cached Kafka image. Verified end to end on a live cluster: freeze cut orders:0 [0,8), orders:1 [0,11), orders:2 [0,11) = 30 records; in-place and to-copy restores both return 30 with the original 8/11/11 partition distribution. Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io> Assisted-by: LLM |
||
|
Aleksei Sviridkin
|
f629280944 |
fix(tests): raise the mongodb, rabbitmq and clickhouse HR waits
Those walkthroughs gave their database HelmReleases 300s, under the 600s install timeout the generated releases carry, so a failed install surfaced as a bare timeout rather than its own reason. None of these charts sets a startup probe, so 660s clears both floors, as it does for mariadb. Each Chainsaw op that runs one of the scripts grows by exactly what the waits it reaches gained, so it keeps the margin it had. The clickhouse op still sits above the sum of every inner ceiling. The rabbitmq one covers a failed install at the source wait, while the ceilings up to its target wait add up to more than the op, as they did before. The mongodb round-trip runs with SKIP_RESTORE, so only its source wait counts there. postgres keeps its 360s: whether a suite pays the extra minutes on every red run is a per-suite call, and this one is not made here. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
289acb2ed6 |
fix(tests): let the mariadb backup example cover a first boot
The walkthrough gave each MariaDB HelmRelease 300s, less than the release can legitimately take, so a slow first boot failed the harness and a failed install surfaced as a bare timeout with no reason. A wait here has two floors and must beat the larger. The chart grants a 310s startup budget, and the install can span it: helm-controller defaults to the poller strategy, and kstatus reads the operator CR's Ready=False as InProgress. The generated release also carries a 600s install timeout, and a wait equal to it expires first, because its clock starts at the apply and the install's only once the chart is fetched. The install timeout is the binding floor, hence 660s. On timeout and on Stalled, wait_hr_ready now dumps every condition and the release history: a scheduled retry can overwrite the Ready message before anyone reads it. It also stops folding different answers into one "(none recorded)" line. An absent release, a release deleted during the wait, a lookup kubectl could not answer, an empty history and a failed history read each say so, with kubectl's own stderr kept, because that message is usually the cause. A release seen earlier still gets its dump when only the last lookup failed. Every backup example whose wait_hr_ready carries this body takes the same change, so the copies stay byte-identical. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <3811295@gmail.com> |
||
|
Aleksei Sviridkin
|
01feb368a1 |
fix(backups): wait for the platform redis strategy in the backup demo
The redis backup demo read cozy-default-redis once and gave up if it was absent. That strategy is rendered behind a lookup of the platform bucket, so it only exists after a Helm upgrade that runs once the bucket is provisioned. The backupstrategy-controller forces that upgrade, but helm-controller will not run it while a release in the chart's dependsOn is not Ready. A platform change shortly before the suite starts is enough to hold it back. The e2e install patches the platform package late (exposed services, then OIDC), the operator rewrites the cozystack-basics release, which then waits on its own dependencies, and backupstrategy-controller waits on cozystack-basics. On one run the strategy was still missing eight minutes after those patches, and the demo's single read failed the suite as if the backups stack were not installed. Wait for the object with a bound instead. 15 minutes covers the nine minutes seen between those patches and the strategy's creation on another run, and still leaves the demo inside its 30 minute step. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Aleksei Sviridkin
|
294b9699c3 |
fix(backups): read the redis demo's keys through exec, not kubectl run
The redis backup demo read and wrote its marker key from a throwaway `kubectl run -i --rm` Pod. kubectl returns that Pod's stdout over an attach, the container runtime delivers only what the container writes after the attach is registered, and kubectl falls back to the Pod log only when the attach fails. A redis-cli that finishes in between comes back as an empty reply with exit 0, which is how a GET of a key the master held read as '' and failed the round-trip on PRs that do not touch redis. Run redis-cli by `kubectl exec` in a sentinel Pod instead, whose image already carries it. An exec'd process writes into pipes created with it, so nothing it prints is lost, and its exit status comes back too. The demo's reads also stop discarding stderr. The same empty string used to stand for a missing object, an API error and a lost stream alike, so a red run could not say which one it was. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la> |
||
|
Andrey Kolkov
|
4ea4888685 |
fix(mongodb): key system-bucket injection off the app flag, not live storage
The MongoDB backup driver decided what to inject and what to record from the live cluster's storage set rather than from the app's opt-in flag. That had three consequences. The snapshot fallback fired for legacy apps too. Its gate was `snap.S3 == nil && rendered.S3 != nil`, but the coordinates moved onto the shared cozy-default strategy, so rendered.S3 is now non-nil for every MongoDB app. A legacy backup written to the tenant's own bucket got its snapshot rewritten to the platform bucket, sending restore to the wrong place. Gate the fallback on the app's backup.useSystemBucket flag instead, threaded through createMongoDBBackupArtifact, and pin the legacy case. The injected storage never caught up with the strategy. Injection was gated on the named storage being absent, so it ran once per cluster and froze the coordinates at the first backup; a later endpoint/region change or bucket re-provision was a silent no-op, and Helm could not repair it because the useSystemBucket chart renders no storages. Key injection off the app flag and re-apply on every BackupJob (the prefix is deterministic, so this never splits the archive the way CNPG's serverName would). Add UseSystemBucket to mongodbapp.MongoDBSpec so a legacy app is protected by the flag, not by whatever happens to be on the cluster. The docs said the opposite of what the driver does. backup-classes.md claimed "the MongoDB driver cannot inject its S3 target per-backup"; correct it and add an opt-in-to-the-system-bucket section that names which values stop being read and that scheduled backups + PITR must move to a Plan. Fix the endpoint table row, the tuning reference, the RBAC comment that said the driver never patches, the strategy header/inline comments, and the example comment. Also assert notExists on spec.backup.pitr in the helm test, and lift the duplicated cozy-backups-creds default into a const. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-By: LLM |
||
|
Andrey Kolkov
|
caae13a60a |
docs(backups): drop the inaccurate NAMESPACE-override note from the demo runner
run-all.sh said NAMESPACE could be overridden through the environment, but the numbered manifests pin namespace: tenant-root while only the waits read $NAMESPACE, so an override would create resources in one namespace and wait in another. The demo is tenant-root-specific anyway: the system bucket credentials and the derived strategy live there. Say that instead. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
bd7d4abe84 |
fix(backups): size the Kafka metadata demo brokers at the chart default
The demo's source and to-copy Kafka fixtures requested resourcesPreset "small", the deprecated legacy alias worth 1 CPU / 512Mi — half the Kafka chart's own c1.small default (1 CPU / 1Gi). 512Mi is too little for the broker: it crashlooped before the HelmRelease went Ready, so the demo never reached the backup. Use c1.small on the kafka and zookeeper blocks of both fixtures, matching the chart default and the in-tree kafka e2e fixture that comes up cleanly. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
c5a8a39bcd |
docs(backups): name the demo BackupClass in the Kafka flow headers
The run-all.sh flow header and the chainsaw harness header both said the ad-hoc BackupJob runs against the platform cozy-default BackupClass. It routes to this demo's kafka-metadata BackupClass instead, which maps Kafka to the kafka-strategy-default strategy that run-all.sh derives from the shipped cozy-default-kafka at run time. The numbered manifests already described this; only the two summary headers overstated the one piece of wiring the demo exists to exercise. Reword them to match. Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |
||
|
Andrey Kolkov
|
2cb2b459fa |
fix(backups): reject a non-numeric recorded partition count on Kafka restore
The restore path validated only the shape parsed from the live cluster,
not the partition count and replication factor read from the stored
object. A non-numeric recorded partition count made `[ "${lp}" -lt
"${parts}" ]` and its `elif` exit 2; an `if` condition is exempt from
errexit, so both read as false, the topic was skipped, and the loop still
printed its per-topic success line - so the RestoreJob reached Succeeded
without ever touching the topic. A well-formed but wrong-content object,
which curl -f cannot catch, must fail loudly like the unrecognised-record
guard.
Validate the recorded parts and rf are numeric before the comparisons and
--create, with a bats case that reddens when the guard is dropped.
Also correct a stale to-copy example comment: the strategy reads the exact
object the source backup recorded in status.artifact.uri (injected as
ARTIFACT_URI), not a key derived from .Backup.ApplicationRef.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
|
||
|
Andrey Kolkov
|
4925a84c26 |
fix(backups): pin the restore-side guards and bound a running Kafka Job
- delete_topic waited with `grep -qx` (BRE), so deleting `audit.events` matched the live `audit-events` and spun to its timeout — the e2e failed at step 25 on every run that reached it. Use `grep -qxF`. - The two -k/--connect-to guards were written as `! grep`, which cannot fail under hack/cozytest.sh (it wraps each @test in `set -e` + a trailing `return 0`, and bash exempts a `!`-negated command from errexit). Rewrite them as positive `if grep ...; then return 1; fi`; verified the -k reinstatement now reddens the suite under cozytest. - Cover the restore-side literal-topic guard that was unpinned at every level: a bats case runs the existing-topic branch with a colliding sibling and asserts the `--alter` argv carries `\Q…\E`, and run-all.sh now leaves `audit.events` live so the e2e exercises that branch instead of only `--create`. Add bats cases for the partition-shrink refusal, the `-f` upload guard, and `--exclude-internal`. - Bound a running Job: reconcileKafka / reconcileKafkaRestore now fail the BackupJob/RestoreJob once kafkaDefaultBackupDeadline elapses in the Job-running branch (buildJobStrategyBatchJob sets no activeDeadlineSeconds), so an unschedulable pod no longer pins it in Running forever. Pinned by TestReconcileKafka_FailsWhenJobNeverCompletes. - Fail closed on an unknown record kind and an unknown MODE, and validate the live partition/RF parsed on restore (an empty value silently skipped reconciliation). Correct the stale "image pinned in the strategy" comment in the example helpers and the chainsaw test (the controller now resolves the target broker's image as .ClientImage). Signed-off-by: Andrey Kolkov <androndo@gmail.com> Assisted-by: LLM |