Skip to content

fix(backup): simplify UX with default backupclass and correct the admin override path (next + v1.5)#622

Merged
Andrey Kolkov (androndo) merged 9 commits into
mainfrom
feat/backup-default-class-v2
Jul 20, 2026
Merged

fix(backup): simplify UX with default backupclass and correct the admin override path (next + v1.5)#622
Andrey Kolkov (androndo) merged 9 commits into
mainfrom
feat/backup-default-class-v2

Conversation

@androndo

@androndo Andrey Kolkov (androndo) commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Supersedes #547 and #621, combining both into one PR based on main and covering the documentation versions that actually ship the feature: next/ and v1.5/.

Commit 1 (retarget of #547): simplify the backup guides around the platform-provided default strategies and the cozy-default BackupClass — a new Backup Classes admin page replaces managed-app-backup-configuration.md and velero-backup-configuration.md, and the applications / virtualization / velero-addon guides are rewritten around backupClassName: cozy-default. Applied to next/ and ported to v1.5/ (the v1.5 base pages were identical to the v1.4 ones modulo version refs). v1.4/ is untouched: release-1.4 does not ship cozy-default (packages/system/backupstrategy-controller there has no default BackupClass/strategy templates and no backupStorage values block), so the v1.4 guides for the manual flow remain correct as they are.

Commit 2 (override path + review findings, both versions): the admin-override example documented a path the cozystack operator silently ignores — spec.components.backupstrategy-controller.values on the Package cozystack.cozystack-platform (cozystack/cozystack#3245). cozystack/cozystack#3333 wires the working path: spec.components.platform.values.backupStorage. In next/ and v1.5/:

  • operations/services/backup-classes.md — the override example uses components.platform.values.backupStorage, a paragraph explains the forwarding and calls out the two plausible-looking paths that do not work; the external-S3 section uses the same platform Package path; aliases: frontmatter keeps the deleted pages' permalinks resolving (the release announcements link them as absolute URLs, which Hugo's ref checking cannot see).
  • operations/configuration/platform-package.md — the spec.components.platform.values.* reference table gains the new backupStorage key.
  • Review fixes from fix(backup): simplify UX with default backupclass and correct the admin override path (next + v1.5) #622: valid kubectl wait --for=jsonpath='{.status.phase}'=Available readiness checks instead of the malformed = Available tail, flux reconcile helmrelease backupstrategy-controller -n cozy-backup-controller (the namespace the HelmRelease actually lives in — not cozy-system), anchor link for the FoundationDB caveat, "Starting with v1.4" phrasing, Etcd mentioned in the tenant guide intro and escalation diagnostics (etcd.aenix.io/EtcdBackup), merged duplicate "See also" bullet.

Merge ordering

Summary by CodeRabbit

  • Documentation
    • Added a new Backup Classes guide for the platform-managed default cozy-default BackupClass, covering Postgres, MariaDB, ClickHouse, and Etcd, credential projection, tuning, and troubleshooting/observability.
    • Updated the platform values reference with a Backup storage (backupStorage) block for cozy-default.
    • Refreshed application and virtualization backup/restore docs to use cozy-default, updated cross-references to Backup Classes, and clarified the fresh-cluster availability window.
    • Removed legacy documentation pages for Velero and managed-application backup configuration.

@netlify

netlify Bot commented Jul 17, 2026

Copy link
Copy Markdown

Deploy Preview for cozystack ready!

Name Link
🔨 Latest commit 56ff318
🔍 Latest deploy log https://app.netlify.com/projects/cozystack/deploys/6a5dbe4dd1e15e00084464f9
😎 Deploy Preview https://deploy-preview-622--cozystack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The documentation introduces the platform-managed cozy-default BackupClass, documents storage overrides and credential projection, updates application and virtualization examples, adds Etcd coverage, and removes legacy backup configuration pages and references.

Changes

Platform-managed BackupClass model

Layer / File(s) Summary
BackupClass model and administration
content/en/docs/{next,v1.5}/operations/services/backup-classes.md, content/en/docs/{next,v1.5}/operations/configuration/platform-package.md
Documents cozy-default, supported drivers, credential projection, ClickHouse and FoundationDB behavior, observability, overrides, external S3, migrations, and tenant usage.

Tenant database backup workflow

Layer / File(s) Summary
Tenant database backup workflow
content/en/docs/{next,v1.5}/applications/backup-and-recovery.md
Updates prerequisites, examples, supported kinds, tenant workflows, and troubleshooting to use cozy-default and include Etcd.

Virtualization backup workflow

Layer / File(s) Summary
Virtualization backup workflow
content/en/docs/{next,v1.5}/virtualization/backup-and-recovery.md, content/en/docs/{next,v1.5}/kubernetes/backups-with-velero-addon.md
Replaces velero examples and admin links with cozy-default and the Backup Classes guide, including bootstrap availability notes.

Application backup cross-references

Layer / File(s) Summary
Application backup cross-references
content/en/docs/{next,v1.5}/applications/{clickhouse,mariadb,postgres}.md
Updates backup warnings and deprecation notices to reference the current tenant and administrator documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • cozystack/website#536 — Overlaps directly in the tenant application backup guide and BackupClass-based backup usage.

Suggested labels: do-not-merge

Suggested reviewers: kvaps, lllamnyp

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main changes: default BackupClass UX and corrected admin override path, scoped to next and v1.5.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/backup-default-class-v2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request consolidates backup configuration documentation by introducing a unified BackupClass (cozy-default) for both managed databases and VM backups, replacing previous per-application configurations. The changes update various documentation files to reflect this centralization and provide instructions for platform-managed storage. The review feedback provides actionable suggestions to improve documentation navigation, fix command syntax in examples, and clarify phrasing regarding versioning.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread content/en/docs/next/applications/backup-and-recovery.md Outdated
Comment thread content/en/docs/v1.4/applications/backup-and-recovery.md Outdated
Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/v1.4/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/v1.4/virtualization/backup-and-recovery.md Outdated
Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/v1.4/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/v1.4/operations/services/backup-classes.md Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@content/en/docs/next/operations/services/backup-classes.md`:
- Around line 167-169: Update the external S3 instructions in both
content/en/docs/next/operations/services/backup-classes.md lines 167-169 and
content/en/docs/v1.4/operations/services/backup-classes.md lines 167-169 to
configure provisionBucket through
spec.components.platform.values.backupStorage.provisionBucket: false instead of
editing backupstrategy-controller values directly; leave the remaining
credential and endpoint guidance unchanged.
- Line 96: Replace the malformed BSL readiness checks with valid kubectl wait
--for=jsonpath='{.status.phase}'=Available syntax in
content/en/docs/next/operations/services/backup-classes.md:96,
content/en/docs/v1.4/operations/services/backup-classes.md:96,
content/en/docs/next/virtualization/backup-and-recovery.md:45, and
content/en/docs/v1.4/virtualization/backup-and-recovery.md:45. Apply the same
correction to each cozy-default readiness check.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e89c8e55-8532-4ee6-b03d-59ea2ec99de1

📥 Commits

Reviewing files that changed from the base of the PR and between 4e85b4e and cb94b51.

📒 Files selected for processing (17)
  • content/en/docs/next/applications/backup-and-recovery.md
  • content/en/docs/next/applications/clickhouse.md
  • content/en/docs/next/applications/mariadb.md
  • content/en/docs/next/applications/postgres.md
  • content/en/docs/next/kubernetes/backups-with-velero-addon.md
  • content/en/docs/next/operations/configuration/platform-package.md
  • content/en/docs/next/operations/services/backup-classes.md
  • content/en/docs/next/operations/services/managed-app-backup-configuration.md
  • content/en/docs/next/operations/services/velero-backup-configuration.md
  • content/en/docs/next/virtualization/backup-and-recovery.md
  • content/en/docs/v1.4/applications/backup-and-recovery.md
  • content/en/docs/v1.4/kubernetes/backups-with-velero-addon.md
  • content/en/docs/v1.4/operations/configuration/platform-package.md
  • content/en/docs/v1.4/operations/services/backup-classes.md
  • content/en/docs/v1.4/operations/services/managed-app-backup-configuration.md
  • content/en/docs/v1.4/operations/services/velero-backup-configuration.md
  • content/en/docs/v1.4/virtualization/backup-and-recovery.md
💤 Files with no reviewable changes (4)
  • content/en/docs/v1.4/operations/services/managed-app-backup-configuration.md
  • content/en/docs/v1.4/operations/services/velero-backup-configuration.md
  • content/en/docs/next/operations/services/velero-backup-configuration.md
  • content/en/docs/next/operations/services/managed-app-backup-configuration.md

Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

NOT LGTM

The next/ half is accurate and matches cozystack/cozystack#3333, but the v1.4/ half documents a feature that release-1.4 does not ship, and the version that actually ships it (v1.5, docs already cut in this repo) is not updated at all.

Findings

[CRITICAL] content/en/docs/v1.4/** — the v1.4 rewrite documents the cozy-default BackupClass, which does not exist in release-1.4

Verified against the cozystack repo: on branch release-1.4, packages/system/backupstrategy-controller/templates/ contains only crds.yaml, deployment.yaml, rbac.yaml, rbac-bind.yaml, sa.yaml — no backupclass-default.yaml, no bucket.yaml, no velero-bsl.yaml, no strategy-*-default.yaml — and its values.yaml has no backupStorage block. The 1.4 release announcement (this repo, content/en/blog/2026-05-21-...) likewise describes the manual strategy/BackupClass flow. The entire feature ships in release-1.5 (backupclass-default.yaml etc. confirmed on branch release-1.5; the 1.5 announcement says "A platform-managed BackupClass named cozy-default is now shipped by default"). The PR body's stated assumption — that a cozystack/cozystack#3333 backport to release-1.4 makes the v1.4 copies valid — is insufficient: #3333 only forwards backupStorage values into the child Package; on release-1.4 the receiving chart has nothing to consume them. A 1.4 operator following the new v1.4/operations/services/backup-classes.md (and the new backupStorage row in v1.4/operations/configuration/platform-package.md:110-115) will set an override that silently does nothing and will believe tenants have working default backups; meanwhile the PR deletes the only correct 1.4 guides (managed-app-backup-configuration.md, velero-backup-configuration.md). Fix: drop the v1.4/ changes from this PR entirely, or gate them on an actual full feature backport (which is not just #3333).

[MAJOR] content/en/docs/v1.5/** (untouched) — the version that ships cozy-default keeps the obsolete guides this PR deletes elsewhere

content/en/docs/v1.5/ exists in the tree (cut with the 2026-06-28 release; the PR-body premise "release-1.5 when its docs version is cut" is stale). It still contains operations/services/managed-app-backup-configuration.md and velero-backup-configuration.md, and v1.5/applications/backup-and-recovery.md still instructs tenants to use per-app class names and admin-provisioned credential Secrets — while v1.5/applications/postgres.md:39-41 already documents backup.useSystemBucket + cozy-default, so v1.5 docs are now internally contradictory. This also breaks the PR-body claim "covering all documentation versions 1.4+", and CONTRIBUTING.md:41 requires a change applying to multiple versions to be copied to each. The correct target set for this rewrite is next/ + v1.5/, not next/ + v1.4/.

[MAJOR] content/en/docs/{next,v1.4}/operations/services/backup-classes.md:1-5 — deleted pages leave published permalinks 404 with no aliases

The 1.4 release announcement links to https://cozystack.io/docs/v1.4/operations/services/managed-app-backup-configuration/ twice (content/en/blog/2026-05-21-cozystack-1-4-.../index.md:86,196); these are absolute URLs, so the Hugo build stays green while the published links start 404ing the moment this merges. The same trap will apply on a v1.5 retarget: the 1.5 announcement links to the v1.5 copy (content/en/blog/2026-06-28-cozystack-1-5-.../index.md:214). Whichever released version ends up losing pages, add aliases: frontmatter on the replacing backup-classes.md covering both old URLs (.../managed-app-backup-configuration/, .../velero-backup-configuration/).

[MINOR] content/en/docs/next/applications/backup-and-recovery.md:8 (and the v1.4 copy) — intro and troubleshooting not updated for Etcd

The prerequisites and examples now include Etcd as a supported Kind, but the opening sentence still reads "Postgres, MariaDB, and ClickHouse", and the escalation paragraph (next/applications/backup-and-recovery.md:203) still enumerates only cnpg.io/Backup, k8s.mariadb.com/Backup, and ClickHouse Pod logs — no Etcd driver diagnostic.

[MINOR] content/en/docs/next/applications/backup-and-recovery.md:207-209 (and the v1.4 copy) — duplicate "See also" bullet

Two bullets both titled "[Backup Classes]" link to the same page with different descriptions (one inherited from the deleted Velero page). Merge into a single bullet.

Caveats

  • Phase 5b (existing-customer upgrade / fresh-install impact) is classified N/A: this PR is docs-only in the website repo and touches nothing under packages/; there is no chart, migration, RBAC, or image surface to analyse.
  • cozystack/cozystack#3333 is still OPEN as of this review (checked 2026-07-17). The PR body correctly orders this PR after it; the maintainer must enforce that ordering at merge time — until #3333 lands, the documented override path does not work on main either.
  • The documented override path was verified against the actual fix, not taken on trust: spec.components.platform.values.backupStorage and the "forwarded into the child Package cozystack.backupstrategy-controller" description match #3333's diff exactly (packages/core/platform/templates/bundles/system.yaml forwarding block plus the same correction in the cozystack repo's own docs/operations/backup-classes.md, which also names the two non-working paths). No stale references to the old spec.components.backupstrategy-controller path remain in next/ or v1.4/.
  • Dead-link sweep: no remaining {{% ref %}} references to the two deleted pages exist inside next/ or v1.4/; all remaining in-tree references live in other versions (v1.0v1.3, v1.5) pointing at their own still-present copies, so Hugo ref resolution is unaffected. The two new backup-classes.md files are byte-identical (same blob) and contain no version-scoped {{% ref %}} shortcodes, so the duplication does not cross-link versions.
  • Technical content of the new page (strategy CR names, backupStorage knobs, projected-Secret key formats) was spot-checked against the release-1.5 backupstrategy-controller chart (values.yaml comments, templates listing) and is consistent; I did not independently verify every knob row (e.g. bucketNameOverride, the per-driver endpoint-shape table) against chart template internals.

Recommended follow-ups

  • Port the v1.4/ half of this PR to content/en/docs/v1.5/ (same two-commit structure applies cleanly; v1.5 is where the feature actually ships).
  • Consider a repo convention (or CI check) requiring aliases: frontmatter whenever a page is deleted from a released docs version, since absolute blog links bypass Hugo's ref checking.

Andrey Kolkov (androndo) added a commit that referenced this pull request Jul 17, 2026
…review findings (next + v1.5)

The admin-override example documented a path the cozystack operator
silently ignores (cozystack/cozystack#3245): that PackageSource exposes
only the platform component. cozystack/cozystack#3333 wires the working
path — spec.components.platform.values.backupStorage — which the
platform chart forwards into the backupstrategy-controller component.
Update the example in both next/ and v1.5/, explain the forwarding,
call out the two plausible-looking paths that do not work, use the same
platform Package path in the external-S3 section, and add the new
backupStorage key to the platform-package values reference tables.

Review fixes from PR #622 (IvanHunters, gemini, coderabbit):

- add aliases frontmatter on backup-classes.md for the two deleted
  pages, so published permalinks (release announcements link them as
  absolute URLs) keep resolving
- replace the malformed BSL readiness check (jsonpath '=' 'Available'
  passed as extra kubectl args) with kubectl wait
  --for=jsonpath='{.status.phase}'=Available, and rephrase the
  tenant-facing variant to 'returns Available'
- namespace the flux reconcile example (-n cozy-backup-controller, the
  namespace the backupstrategy-controller HelmRelease actually lives
  in per packages/core/platform/sources/backupstrategy-controller.yaml)
- link the FoundationDB caveat section instead of quoting its title
- 'After v1.4' -> 'Starting with v1.4' in the upgrade note
- mention Etcd in the tenant guide intro/description and add
  etcd.aenix.io/EtcdBackup to the escalation diagnostics list
- merge the duplicated Backup Classes bullet in See also

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
@androndo
Andrey Kolkov (androndo) force-pushed the feat/backup-default-class-v2 branch from cb94b51 to 8d39146 Compare July 17, 2026 08:05
@androndo

Copy link
Copy Markdown
Contributor Author

IvanHunters thanks — the CRITICAL finding is confirmed and addressed. Verified against the cozystack repo: release-1.4's backupstrategy-controller chart has only the controller templates (no backupclass-default.yaml, no strategy templates, no backupStorage values block), while release-1.5 ships the full feature. The branch has been rebuilt (force-pushed):

  • v1.4 dropped entirelycontent/en/docs/v1.4/ is untouched now; its existing manual-flow guides stay as the correct documentation for that release.
  • v1.5 covered — commit 1 is the fix(backup): simplify UX with default backupclass #547 rewrite retargeted to next/ + v1.5/ (the v1.5 base pages were identical to the v1.4 ones modulo version refs, so the port is mechanical), removing the now-contradictory v1.5/operations/services/managed-app-backup-configuration.md / velero-backup-configuration.md and aligning v1.5/applications/backup-and-recovery.md with what v1.5/applications/postgres.md already documents.
  • aliases added — both backup-classes.md copies carry aliases: for the two deleted permalinks, covering the absolute links in the release announcements.
  • MINOR items fixed — Etcd added to the tenant guide intro/description and to the escalation diagnostics (etcd.aenix.io/EtcdBackup), duplicate "See also" bullet merged.

Bot findings addressed in the same commit: kubectl wait --for=jsonpath='{.status.phase}'=Available instead of the malformed = Available tail (all four spots), external-S3 section now uses the same spec.components.platform.values.backupStorage path, FoundationDB caveat is an anchor link, "Starting with v1.4" phrasing. One correction to a bot suggestion: the flux reconcile example is namespaced to cozy-backup-controller, not cozy-system — that is where the PackageSource installs the backupstrategy-controller HelmRelease (packages/core/platform/sources/backupstrategy-controller.yaml).

On merge ordering: the PR body now states the v1.5 half depends on a cozystack/cozystack#3333 backport to release-1.5 (the feature ships there, only the values forwarding is missing); #3333 carries the backport label, and the backport-previous label was removed since a 1.4 backport would have nothing to consume the forwarded values.

@IvanHunters

Copy link
Copy Markdown
Contributor

Verdict

NOT LGTM

The force-push resolves every prior-round finding (v1.4 no longer touched, v1.5 rewritten coherently, aliases: added, Etcd coverage, duplicate bullet merged), but the retarget left one factual version misattribution in the upgrade note that misinforms exactly the v1.4-to-v1.5 upgraders the note exists for — a one-word fix in two files.

Findings

[MAJOR] content/en/docs/next/operations/services/backup-classes.md:178 (and the identical content/en/docs/v1.5/operations/services/backup-classes.md:178) — upgrade note attributes the placeholder-credentials behavior change to v1.4; it ships in v1.5

The note says "The pre-v1.4 defaults ... were the literal "<your-access-key>" ... placeholders" and "Starting with v1.4 those defaults are empty strings and the chart NO LONGER renders the backup block". Verified against the source repo: on branch release-1.4, packages/apps/postgres/values.yaml:213-214 still contains s3AccessKey: "<your-access-key>" / s3SecretKey: "<your-secret-key>" and has no useSystemBucket key at all (0 occurrences); the empty-string defaults plus useSystemBucket first appear on release-1.5 (values.yaml:244,249-250), introduced by commit f17ead3d "feat(platform): add default backupclass" dated 2026-05-26 — after the v1.4 release (blog dated 2026-05-21). The monorepo source of this note (docs/operations/backup-classes.md:167 on main) says "pre-PR / After this PR"; the translation to "pre-v1.4 / Starting with v1.4" mapped it to the wrong release. Impact: this is migration guidance about barmanObjectStore (WAL archiving) silently disappearing on helm upgrade — an operator reading it concludes the change already happened on their v1.4 cluster and does not act during the actual 1.4-to-1.5 upgrade, which is precisely when archiving stops for placeholder-credential tenants. Fix: replace both version references with v1.5 ("pre-v1.5 defaults ... Starting with v1.5") in both copies.

[MINOR] content/en/docs/next/applications/backup-and-recovery.md:203etcd.aenix.io/EtcdBackup is stale for the next tree

The escalation diagnostics list names etcd.aenix.io/EtcdBackup as the operator-native CR the Etcd driver creates. That is correct for v1.5 (verified: internal/backupcontroller/etcdstrategy_controller.go on release-1.5 drives etcd.aenix.io EtcdBackup), so the v1.5 copy at the same line is fine. But next/ documents trunk, and on main the driver has moved to etcd-operator.cozystack.io/v1alpha2 EtcdSnapshot (internal/backupcontroller/etcdtypes/types.go:27GroupName = "etcd-operator.cozystack.io", registered kinds EtcdCluster/EtcdSnapshot; the controller's messages reference etcd-operator.cozystack.io/EtcdSnapshot). Update the next/ copy only.

[MINOR] PR title — still says "(next + v1.4)" while the diff, commits, and body are next + v1.5

The body and both commit subjects were updated to "(next + v1.5)", but the PR title (which becomes the squash-merge subject) still claims v1.4 coverage. Retitle to "(next + v1.5)".

Caveats

  • Phase 5b (existing-customer upgrade + fresh install) is N/A: this is the documentation website repo (Hugo, .md only); the PR touches no packages/, charts, migrations, or RBAC. Classification stated explicitly per review protocol.
  • Merge-ordering dependency: fix(platform): forward backupStorage overrides to the backupstrategy-controller Package cozystack#3333 is still OPEN (not merged). Until it lands on main (and is backported to release-1.5 — the PR carries the backport label), the documented override path does not work anywhere. The PR body states this correctly; the maintainer must enforce the ordering at merge time.
  • Verified, no breakage found: the documented override path spec.components.platform.values.backupStorage matches #3333's actual wiring (packages/core/platform/templates/bundles/system.yaml forwards .Values.backupStorage into components.backupstrategy-controller.values.backupStorage; values.yaml gains backupStorage: {}); the claim that platform is the only component under the cozystack-platform PackageSource matches cmd/cozystack-operator/main.go (single component platform on all four variants); flux reconcile helmrelease backupstrategy-controller -n cozy-backup-controller matches the install namespace in packages/core/platform/sources/backupstrategy-controller.yaml on both release-1.5 and main; kubectl wait --for=jsonpath='{.status.phase}'=Available is valid syntax; all eight backupStorage knob names in the override table exist in packages/system/backupstrategy-controller/values.yaml on both branches; the Prometheus metric names and the bucket-cozy-backups BucketClaim/Secret names exist in the controller source; aliases: frontmatter in both backup-classes.md copies covers both deleted permalinks, including the absolute URL in the v1.5 release announcement (content/en/blog/2026-06-28-.../index.md:214); zero remaining ref links to the deleted pages anywhere in next/ or v1.5/; the next/ and v1.5/ copies of all five shared pages are byte-identical modulo the version path prefix; commits are Conventional-Commit formatted and signed off.
  • Prior-round findings are all resolved at head 8d39146: v1.4 untouched, v1.5 updated consistently (obsolete guides deleted, tenant/virt/velero-addon guides coherent with cozy-default), aliases present, Etcd in intro and escalation, duplicate "See also" bullet merged.

Recommended follow-ups

  • content/en/docs/next/applications/postgres.md still documents only the legacy chart-managed backup flow (its source: header pins the v1.3.0 README), while content/en/docs/v1.5/applications/postgres.md already documents the two-layer model with backup.useSystemBucket. The next/ app page lagging behind v1.5/ is pre-existing drift on main, not introduced here, but it undercuts the "prefer the BackupClass flow" alert this PR fixes the link in — worth a separate sync PR.
  • When fixing the [MAJOR] version wording, consider aligning the monorepo copy (docs/operations/backup-classes.md, carried by fix(platform): forward backupStorage overrides to the backupstrategy-controller Package cozystack#3333, currently phrased "pre-PR / After this PR") to the same explicit version boundary so the two documents cannot diverge again.

Andrey Kolkov (androndo) added a commit that referenced this pull request Jul 17, 2026
…5, use EtcdSnapshot for next

Review round 2 on PR #622: release-1.4 packages/apps/postgres/values.yaml
still ships the placeholder s3AccessKey/s3SecretKey defaults and has no
useSystemBucket at all — the empty-string defaults land in release-1.5,
so the upgrade note's version boundary moves from v1.4 to v1.5 in both
copies. On main the etcd backup driver has moved from etcd.aenix.io
EtcdBackup to etcd-operator.cozystack.io EtcdSnapshot, so the next/
escalation diagnostics name the new CR while the v1.5 copy keeps
EtcdBackup.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
@androndo Andrey Kolkov (androndo) changed the title fix(backup): simplify UX with default backupclass and correct the admin override path (next + v1.4) fix(backup): simplify UX with default backupclass and correct the admin override path (next + v1.5) Jul 17, 2026
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 17, 2026
Same note as the website copy (cozystack/website#622 review round 2):
name the explicit version boundary instead of 'pre-PR / After this PR',
so the two documents cannot diverge — the empty-string defaults and
useSystemBucket shipped in release-1.5.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
@androndo

Copy link
Copy Markdown
Contributor Author

All three round-2 findings addressed in 4a1cb07:

  • [MAJOR] upgrade-note version boundary — confirmed against the source repo (release-1.4 packages/apps/postgres/values.yaml still ships the placeholder defaults and has no useSystemBucket); both copies now read "pre-v1.5 / Starting with v1.5". The follow-up you recommended is also done: the monorepo source of the note (docs/operations/backup-classes.md in fix(platform): forward backupStorage overrides to the backupstrategy-controller Package cozystack#3333) now names the same explicit v1.5 boundary instead of "pre-PR / After this PR" (pushed as 0962fbae7 there).
  • [MINOR] Etcd CR in next/ — the next/ escalation diagnostics now name etcd-operator.cozystack.io/EtcdSnapshot (confirmed as the driver's target on main), while the v1.5/ copy keeps etcd.aenix.io/EtcdBackup.
  • [MINOR] PR title — retitled to "(next + v1.5)".

The next/applications/postgres.md drift (v1.3.0-pinned README while v1.5/ already documents the two-layer model) is noted as a separate sync PR — it is pre-existing on main and the app pages are generator-owned (make update-apps), so the right fix is a regeneration from the upstream README rather than a hand edit here.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@content/en/docs/next/operations/services/backup-classes.md`:
- Line 105: Update the bootstrap resource description in the strategy-template
paragraph to state that the strategy configuration is incomplete until the
BucketClaim status is populated, while explicitly preserving that the
cozy-default Velero BackupStorageLocation is already present. Do not imply that
only the Bucket CR and BackupClass exist; keep the later Flux re-render behavior
unchanged.
- Line 12: Update the description of the system-managed `cozy-backups` reference
in the `BackupClass` documentation to identify it as the
`apps.cozystack.io/Bucket` resource name, not the underlying S3 bucket name.
Keep the existing `tenant-root` namespace detail and align the wording with the
COSI-assigned bucket name behavior described elsewhere.
- Line 116: Update the alert expression in the backup monitoring documentation
to use the fully qualified exported metrics and valid range vectors: replace the
failures condition with
rate(cozystack_backup_credentials_projection_failures_total[5m]) > 0 and the
stale-success condition with
absent_over_time(cozystack_backup_credentials_projection_successes_total[10m]).

In `@content/en/docs/v1.5/operations/services/backup-classes.md`:
- Line 4: Update the page description metadata to state that administrators can
tune BackupClass parameters while tenants only select a BackupClass name,
aligning it with the documented tenant workflow.
- Line 12: Update the BackupClass documentation to identify cozy-backups as the
system-managed Kubernetes Bucket/release resource in the tenant-root namespace,
not as the actual S3 bucket name. Reference the later COSI documentation for the
assigned S3 bucket name and preserve the existing provisioning details.
- Line 88: Update the credentials Secret description to clarify that the
original system-managed Secret remains in tenant-root, while only copies are
projected into tenant and configured system namespaces. Preserve the existing
details about projection timing, refresh behavior, and Secret naming.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6ef226b0-f5c1-4e1e-b066-02e80369ca86

📥 Commits

Reviewing files that changed from the base of the PR and between cb94b51 and 4a1cb07.

📒 Files selected for processing (17)
  • content/en/docs/next/applications/backup-and-recovery.md
  • content/en/docs/next/applications/clickhouse.md
  • content/en/docs/next/applications/mariadb.md
  • content/en/docs/next/applications/postgres.md
  • content/en/docs/next/kubernetes/backups-with-velero-addon.md
  • content/en/docs/next/operations/configuration/platform-package.md
  • content/en/docs/next/operations/services/backup-classes.md
  • content/en/docs/next/operations/services/managed-app-backup-configuration.md
  • content/en/docs/next/operations/services/velero-backup-configuration.md
  • content/en/docs/next/virtualization/backup-and-recovery.md
  • content/en/docs/v1.5/applications/backup-and-recovery.md
  • content/en/docs/v1.5/kubernetes/backups-with-velero-addon.md
  • content/en/docs/v1.5/operations/configuration/platform-package.md
  • content/en/docs/v1.5/operations/services/backup-classes.md
  • content/en/docs/v1.5/operations/services/managed-app-backup-configuration.md
  • content/en/docs/v1.5/operations/services/velero-backup-configuration.md
  • content/en/docs/v1.5/virtualization/backup-and-recovery.md
💤 Files with no reviewable changes (4)
  • content/en/docs/v1.5/operations/services/velero-backup-configuration.md
  • content/en/docs/v1.5/operations/services/managed-app-backup-configuration.md
  • content/en/docs/next/operations/services/velero-backup-configuration.md
  • content/en/docs/next/operations/services/managed-app-backup-configuration.md
🚧 Files skipped from review as they are similar to previous changes (7)
  • content/en/docs/next/kubernetes/backups-with-velero-addon.md
  • content/en/docs/next/applications/mariadb.md
  • content/en/docs/next/applications/postgres.md
  • content/en/docs/next/applications/clickhouse.md
  • content/en/docs/next/operations/configuration/platform-package.md
  • content/en/docs/next/virtualization/backup-and-recovery.md
  • content/en/docs/next/applications/backup-and-recovery.md

Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/next/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/v1.5/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/v1.5/operations/services/backup-classes.md Outdated
Comment thread content/en/docs/v1.5/operations/services/backup-classes.md Outdated
Andrey Kolkov (androndo) added a commit that referenced this pull request Jul 17, 2026
…main, fix external-S3 BSL claim

Branch-review round 3 on PR #622:

- next/ copy carried pre-derivation endpoint semantics: on cozystack
  main (e160a0dc8) the backupstrategy-controller.endpoint helper
  derives the endpoint from the COSI system-credentials Secret (forced
  https://) whenever provisionBucket: true, and backupStorage.endpoint
  is only the fallback for external S3 and offline renders. Port the
  corrected wording from the monorepo docs/operations/backup-classes.md
  into the next/ copy (intro, post-table paragraph, endpoint knob row).
  The v1.5/ copy keeps the direct-endpoint wording — release-1.5 has no
  endpoint helper — making this the second deliberate divergence
  between the copies after the Etcd CR name.
- both copies claimed the external-S3 path needs the chart's Velero BSL
  settings updated inside backupStorage; no such settings exist there
  and the platform Package path cannot carry velero.* values. The BSL
  is rendered from the same backupStorage block, so no separate step is
  needed — say that instead.
- add a See also section to both copies (tenant guides,
  platform-package reference); the page previously had no outbound
  links.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 17, 2026
… namespace knob and bslEnabled gap

Keep the monorepo doc aligned with the website copy
(cozystack/website#622 branch-review round 3): full
cozystack_backup_credentials_projection_* metric names with range
vectors in the alerting examples, the intro no longer conflates the
Bucket CR name with the COSI-assigned S3 bucket name, the knob table
gains the backupStorage.namespace row, and the external-S3 section
states that the Velero BSL derives from the same backupStorage block
while velero.bslEnabled is not carried by the override path.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 17, 2026
…ax, leader-election phrasing

Same corrections as the website copy (cozystack/website#622 round 4):
the re-render interval is 5m via the operator's helmrelease-interval
flag (not a 10-minute Flux default), the FoundationDB backup CR group
is apps.foundationdb.org, the BSL readiness check uses valid kubectl
wait --for=jsonpath syntax, and the projector's first synchronous round
is gated on leader election rather than pod readiness.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 17, 2026
The projector runs from both the BackupJob and RestoreJob controllers
(restorejob_controller.go), not only before BackupJobs — keep the
monorepo doc aligned with the website copy (cozystack/website#622).

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

LGTM

Independent re-verification of every cross-repo technical claim against cozystack/cozystack source on both main and release-1.5 found no inaccuracies, and a full-repo grep (not just the diffed files) found no dangling references to the deleted pages.

Caveats

  • Phase 5b/5c/5d: N/A. This PR touches only content/en/docs/{next,v1.5}/**/*.md — no packages/, chart, values, migration, or RBAC surface exists to evaluate upgrade/fresh-install impact, config-toggle matrices, or regression-test adequacy against.
  • Merge-order dependency (disclosed by the author, not newly discovered). The next/ copy's override-path text, and the v1.5/ copy additionally, assume cozystack/cozystack#3333 lands on main (confirmed via gh api: OPEN, mergeable, reviewDecision: REVIEW_REQUIRED) and is backported to release-1.5. Confirmed release-1.5's packages/core/platform/values.yaml currently has no backupStorage key and packages/core/platform/templates/bundles/system.yaml still emits cozystack.backupstrategy-controller via the unwired .package.default form — so the documented override genuinely is silently ignored on every release-1.5 patch shipped today. The PR body already states "do not merge before #3333 lands"; noting this so the merge sequencing is gated on that, separate from this review's technical sign-off.
  • Ref-integrity verified beyond the diff. grep -rn "managed-app-backup-configuration\|velero-backup-configuration" content/ across the whole working tree shows zero remaining {{% ref %}} shortcodes pointing at the two deleted pages anywhere in next/ or v1.5/; the v1.0-v1.4 trees still have their own independent copies of those pages (untouched, unaffected). The aliases: added to the new backup-classes.md pages cover exactly the two absolute-URL permalinks the v1.5/next release-announcement blog posts link to. A full hugo build was attempted as the authoritative check but did not finish within the sandbox's time budget (large module/asset graph); the targeted grep is the verification of record for this pass.
  • Cross-repo technical claims independently re-verified against source (not taken from the PR body or prior review threads): endpoint-derivation split between next (COSI-secret-derived, forced https://) and v1.5 (plain backupStorage.endpoint) matches a real diff between docs/operations/backup-classes.md on main vs release-1.5 in cozystack/cozystack; Postgres backup.s3AccessKey/s3SecretKey are literal "<your-access-key>"/"<your-secret-key>" placeholders with no useSystemBucket key on release-1.4, and empty-string placeholders plus useSystemBucket on release-1.5; the etcd backup driver is etcd-operator.cozystack.io/EtcdSnapshot on main and etcd.aenix.io/EtcdBackup on release-1.5 (confirmed in internal/backupcontroller/etcdstrategy_controller.go on both branches); FoundationDB's strategy template derives secure_connection from the endpoint scheme identically on both branches; the backupstrategy-controller HelmRelease's actual namespace is cozy-backup-controller (not cozy-system) on both branches; the HelmRelease reconcile interval defaults to 5m via the --helmrelease-interval flag (pkg/cmd/server/start.go:89); the credentials projector runs on a leader-elected 1-minute ticker and also fires on RestoreJob reconcile, not just BackupJob (internal/backupcontroller/restorejob_controller.go, credentials_projector_runnable.go).

Recommended follow-ups

  • The upstream cozystack/cozystack release-1.5 in-repo doc (docs/operations/backup-classes.md) still omits FoundationDB from the endpoint knob's TLS-enabling sentence, while this PR's v1.5/backup-classes.md correctly includes it (verified: the FoundationDB strategy template derives secure_connection from the endpoint scheme identically on main and release-1.5). Worth a one-line fix in the monorepo's own copy so the two don't drift apart.

Andrey Kolkov (androndo) and others added 2 commits July 20, 2026 10:20
…and backupclass (next + v1.5)

Retarget of the released-version half of the original commit from
cherry-pick c1a3ff8 (PR #547): the cozy-default BackupClass feature
ships in release-1.5, not release-1.4, so the rewritten guides land in
content/en/docs/v1.5/ instead of v1.4/. The v1.5 base pages were
identical to the v1.4 ones modulo version refs, so the port is the
same content with /docs/v1.5/ refs.

Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…review findings (next + v1.5)

The admin-override example documented a path the cozystack operator
silently ignores (cozystack/cozystack#3245): that PackageSource exposes
only the platform component. cozystack/cozystack#3333 wires the working
path — spec.components.platform.values.backupStorage — which the
platform chart forwards into the backupstrategy-controller component.
Update the example in both next/ and v1.5/, explain the forwarding,
call out the two plausible-looking paths that do not work, use the same
platform Package path in the external-S3 section, and add the new
backupStorage key to the platform-package values reference tables.

Review fixes from PR #622 (IvanHunters, gemini, coderabbit):

- add aliases frontmatter on backup-classes.md for the two deleted
  pages, so published permalinks (release announcements link them as
  absolute URLs) keep resolving
- replace the malformed BSL readiness check (jsonpath '=' 'Available'
  passed as extra kubectl args) with kubectl wait
  --for=jsonpath='{.status.phase}'=Available, and rephrase the
  tenant-facing variant to 'returns Available'
- namespace the flux reconcile example (-n cozy-backup-controller, the
  namespace the backupstrategy-controller HelmRelease actually lives
  in per packages/core/platform/sources/backupstrategy-controller.yaml)
- link the FoundationDB caveat section instead of quoting its title
- 'After v1.4' -> 'Starting with v1.4' in the upgrade note
- mention Etcd in the tenant guide intro/description and add
  etcd.aenix.io/EtcdBackup to the escalation diagnostics list
- merge the duplicated Backup Classes bullet in See also

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) and others added 7 commits July 20, 2026 10:20
…5, use EtcdSnapshot for next

Review round 2 on PR #622: release-1.4 packages/apps/postgres/values.yaml
still ships the placeholder s3AccessKey/s3SecretKey defaults and has no
useSystemBucket at all — the empty-string defaults land in release-1.5,
so the upgrade note's version boundary moves from v1.4 to v1.5 in both
copies. On main the etcd backup driver has moved from etcd.aenix.io
EtcdBackup to etcd-operator.cozystack.io EtcdSnapshot, so the next/
escalation diagnostics name the new CR while the v1.5 copy keeps
EtcdBackup.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…main, fix external-S3 BSL claim

Branch-review round 3 on PR #622:

- next/ copy carried pre-derivation endpoint semantics: on cozystack
  main (e160a0dc8) the backupstrategy-controller.endpoint helper
  derives the endpoint from the COSI system-credentials Secret (forced
  https://) whenever provisionBucket: true, and backupStorage.endpoint
  is only the fallback for external S3 and offline renders. Port the
  corrected wording from the monorepo docs/operations/backup-classes.md
  into the next/ copy (intro, post-table paragraph, endpoint knob row).
  The v1.5/ copy keeps the direct-endpoint wording — release-1.5 has no
  endpoint helper — making this the second deliberate divergence
  between the copies after the Etcd CR name.
- both copies claimed the external-S3 path needs the chart's Velero BSL
  settings updated inside backupStorage; no such settings exist there
  and the platform Package path cannot carry velero.* values. The BSL
  is rendered from the same backupStorage block, so no separate step is
  needed — say that instead.
- add a See also section to both copies (tenant guides,
  platform-package reference); the page previously had no outbound
  links.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…hrasing; document namespace knob and bslEnabled gap

Branch-review round 3 blockers (both copies):

- the alerting guidance used truncated metric names and a rate() call
  without a range vector; spell out
  cozystack_backup_credentials_projection_{failures,successes}_total
  with proper windows
- the frontmatter description claimed tenants tune BackupClass
  parameters; tenants only reference a class by name — parameter tuning
  is admin-only via sibling classes
- the intro conflated the Bucket CR name cozy-backups with the real
  S3 bucket name (COSI-assigned via BucketClaim.status.bucketName)

Non-blocking recommendations from the same review:

- add the backupStorage.namespace knob row (where the Bucket CR lands;
  must be a tenant-* namespace per the cozy-lib RBAC helper)
- state explicitly that velero.bslEnabled is not carried by the
  backupStorage override path

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…-election phrasing

Deep verification against the cozystack repo (branch-review round 4)
found two factual errors and one simplification, in both copies:

- the re-render interval is 5 minutes, set by the cozystack operator's
  helmrelease-interval flag (cmd/cozystack-operator/main.go), not a
  '10 minutes Flux standard interval'
- the FoundationDB backup CR group is apps.foundationdb.org, not
  foundationdb.org (internal/backupcontroller/foundationdbtypes)
- the projector's first synchronous round is gated on leader election,
  not literally on pod readiness; say so while keeping the practical
  timing guidance

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
… pin the override to the #3333 backport

Branch-review round 5: release-1.5's strategy-foundationdb-default.yaml
derives secure_connection from the endpoint scheme exactly like the
MariaDB strategy does, so the endpoint knob row must name both (the
page's own driver table already says 'derived secure flag' for FDB).
And the v1.5 page documented the platform-Package override without
saying that no shipped v1.5 patch carries the forwarding yet — state
that it requires the cozystack/cozystack#3333 backport, so a
v1.5.0-v1.5.2 admin is not handed the same silently-ignored block that
#3245 was filed about.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
…the #3333 backport, mention RestoreJob projection

Branch-review round 6: the v1.5 platform-package.md reference row
documented backupStorage with no version caveat — a reader landing on
the standalone table on v1.5.0-v1.5.2 would set a silently-ignored key,
the exact trap #3245 describes; the row now points at the required
cozystack/cozystack#3333 backport. Also credentials are projected for
RestoreJob reconciles too (restorejob_controller.go), not only
BackupJob — say so everywhere the projection timing is described.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Mirrors the monorepo fix from the cozystack/cozystack#3333 review: with
provisionBucket: false the bucketName value is taken verbatim as the
real S3 bucket name by every strategy CR and the Velero BSL (no COSI
lookup, the Secret's bucketName key alone is not read by the
strategies), so the external-S3 section now requires updating it and
the knob table states the two-mode semantics. Same helper behavior
verified on release-1.5 for the v1.5 copy.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
@androndo
Andrey Kolkov (androndo) force-pushed the feat/backup-default-class-v2 branch from 53cebd7 to 56ff318 Compare July 20, 2026 06:20

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@content/en/docs/next/operations/services/backup-classes.md`:
- Line 116: The stale-success alert using absent_over_time() is not scoped per
namespace, so failures can be hidden by successes elsewhere. Update the alert
guidance in content/en/docs/next/operations/services/backup-classes.md:116-116
and content/en/docs/v1.5/operations/services/backup-classes.md:116-116 to
evaluate each expected system namespace independently, preserving the existing
failure-rate condition and detection guarantee.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 044bc669-2993-4acf-8dfa-b365ff888331

📥 Commits

Reviewing files that changed from the base of the PR and between 53cebd7 and 56ff318.

📒 Files selected for processing (17)
  • content/en/docs/next/applications/backup-and-recovery.md
  • content/en/docs/next/applications/clickhouse.md
  • content/en/docs/next/applications/mariadb.md
  • content/en/docs/next/applications/postgres.md
  • content/en/docs/next/kubernetes/backups-with-velero-addon.md
  • content/en/docs/next/operations/configuration/platform-package.md
  • content/en/docs/next/operations/services/backup-classes.md
  • content/en/docs/next/operations/services/managed-app-backup-configuration.md
  • content/en/docs/next/operations/services/velero-backup-configuration.md
  • content/en/docs/next/virtualization/backup-and-recovery.md
  • content/en/docs/v1.5/applications/backup-and-recovery.md
  • content/en/docs/v1.5/kubernetes/backups-with-velero-addon.md
  • content/en/docs/v1.5/operations/configuration/platform-package.md
  • content/en/docs/v1.5/operations/services/backup-classes.md
  • content/en/docs/v1.5/operations/services/managed-app-backup-configuration.md
  • content/en/docs/v1.5/operations/services/velero-backup-configuration.md
  • content/en/docs/v1.5/virtualization/backup-and-recovery.md
💤 Files with no reviewable changes (4)
  • content/en/docs/v1.5/operations/services/managed-app-backup-configuration.md
  • content/en/docs/next/operations/services/velero-backup-configuration.md
  • content/en/docs/next/operations/services/managed-app-backup-configuration.md
  • content/en/docs/v1.5/operations/services/velero-backup-configuration.md
🚧 Files skipped from review as they are similar to previous changes (11)
  • content/en/docs/v1.5/kubernetes/backups-with-velero-addon.md
  • content/en/docs/next/operations/configuration/platform-package.md
  • content/en/docs/next/applications/postgres.md
  • content/en/docs/v1.5/operations/configuration/platform-package.md
  • content/en/docs/next/applications/clickhouse.md
  • content/en/docs/next/kubernetes/backups-with-velero-addon.md
  • content/en/docs/v1.5/virtualization/backup-and-recovery.md
  • content/en/docs/next/applications/mariadb.md
  • content/en/docs/next/virtualization/backup-and-recovery.md
  • content/en/docs/next/applications/backup-and-recovery.md
  • content/en/docs/v1.5/applications/backup-and-recovery.md

- `cozystack_backup_credentials_projection_successes_total`
- `cozystack_backup_credentials_projection_failures_total`

Alert on `rate(cozystack_backup_credentials_projection_failures_total[5m]) > 0` or `absent_over_time(cozystack_backup_credentials_projection_successes_total[10m])` to catch a stale BSL credential or a malformed source Secret without log scraping.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Scope the stale-success alert per namespace in both versioned pages.

The success counter is labeled by namespace, but unfiltered absent_over_time() only detects a complete absence of the metric. Failures in one namespace can be missed while another namespace continues reporting successes.

  • content/en/docs/next/operations/services/backup-classes.md#L116-L116: scope the alert to each expected system namespace, or revise the stated detection guarantee.
  • content/en/docs/v1.5/operations/services/backup-classes.md#L116-L116: apply the same namespace-scoped alert correction.
📍 Affects 2 files
  • content/en/docs/next/operations/services/backup-classes.md#L116-L116 (this comment)
  • content/en/docs/v1.5/operations/services/backup-classes.md#L116-L116
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@content/en/docs/next/operations/services/backup-classes.md` at line 116, The
stale-success alert using absent_over_time() is not scoped per namespace, so
failures can be hidden by successes elsewhere. Update the alert guidance in
content/en/docs/next/operations/services/backup-classes.md:116-116 and
content/en/docs/v1.5/operations/services/backup-classes.md:116-116 to evaluate
each expected system namespace independently, preserving the existing
failure-rate condition and detection guarantee.

@androndo
Andrey Kolkov (androndo) merged commit eace3f8 into main Jul 20, 2026
6 checks passed
@androndo
Andrey Kolkov (androndo) deleted the feat/backup-default-class-v2 branch July 20, 2026 06:27
@androndo

Copy link
Copy Markdown
Contributor Author

Follow-up from review #622 (review) (release-1.5 in-repo docs/operations/backup-classes.md omitting FoundationDB from the endpoint TLS sentence) is now tracked in cozystack/cozystack#3365.

Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 20, 2026
Same note as the website copy (cozystack/website#622 review round 2):
name the explicit version boundary instead of 'pre-PR / After this PR',
so the two documents cannot diverge — the empty-string defaults and
useSystemBucket shipped in release-1.5.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 20, 2026
… namespace knob and bslEnabled gap

Keep the monorepo doc aligned with the website copy
(cozystack/website#622 branch-review round 3): full
cozystack_backup_credentials_projection_* metric names with range
vectors in the alerting examples, the intro no longer conflates the
Bucket CR name with the COSI-assigned S3 bucket name, the knob table
gains the backupStorage.namespace row, and the external-S3 section
states that the Velero BSL derives from the same backupStorage block
while velero.bslEnabled is not carried by the override path.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 20, 2026
…ax, leader-election phrasing

Same corrections as the website copy (cozystack/website#622 round 4):
the re-render interval is 5m via the operator's helmrelease-interval
flag (not a 10-minute Flux default), the FoundationDB backup CR group
is apps.foundationdb.org, the BSL readiness check uses valid kubectl
wait --for=jsonpath syntax, and the projector's first synchronous round
is gated on leader election rather than pod readiness.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 20, 2026
The projector runs from both the BackupJob and RestoreJob controllers
(restorejob_controller.go), not only before BackupJobs — keep the
monorepo doc aligned with the website copy (cozystack/website#622).

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
Same note as the website copy (cozystack/website#622 review round 2):
name the explicit version boundary instead of 'pre-PR / After this PR',
so the two documents cannot diverge — the empty-string defaults and
useSystemBucket shipped in release-1.5.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
… namespace knob and bslEnabled gap

Keep the monorepo doc aligned with the website copy
(cozystack/website#622 branch-review round 3): full
cozystack_backup_credentials_projection_* metric names with range
vectors in the alerting examples, the intro no longer conflates the
Bucket CR name with the COSI-assigned S3 bucket name, the knob table
gains the backupStorage.namespace row, and the external-S3 section
states that the Velero BSL derives from the same backupStorage block
while velero.bslEnabled is not carried by the override path.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
…ax, leader-election phrasing

Same corrections as the website copy (cozystack/website#622 round 4):
the re-render interval is 5m via the operator's helmrelease-interval
flag (not a 10-minute Flux default), the FoundationDB backup CR group
is apps.foundationdb.org, the BSL readiness check uses valid kubectl
wait --for=jsonpath syntax, and the projector's first synchronous round
is gated on leader election rather than pod readiness.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
The projector runs from both the BackupJob and RestoreJob controllers
(restorejob_controller.go), not only before BackupJobs — keep the
monorepo doc aligned with the website copy (cozystack/website#622).

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
Same note as the website copy (cozystack/website#622 review round 2):
name the explicit version boundary instead of 'pre-PR / After this PR',
so the two documents cannot diverge — the empty-string defaults and
useSystemBucket shipped in release-1.5.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
… namespace knob and bslEnabled gap

Keep the monorepo doc aligned with the website copy
(cozystack/website#622 branch-review round 3): full
cozystack_backup_credentials_projection_* metric names with range
vectors in the alerting examples, the intro no longer conflates the
Bucket CR name with the COSI-assigned S3 bucket name, the knob table
gains the backupStorage.namespace row, and the external-S3 section
states that the Velero BSL derives from the same backupStorage block
while velero.bslEnabled is not carried by the override path.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
…ax, leader-election phrasing

Same corrections as the website copy (cozystack/website#622 round 4):
the re-render interval is 5m via the operator's helmrelease-interval
flag (not a 10-minute Flux default), the FoundationDB backup CR group
is apps.foundationdb.org, the BSL readiness check uses valid kubectl
wait --for=jsonpath syntax, and the projector's first synchronous round
is gated on leader election rather than pod readiness.

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
The projector runs from both the BackupJob and RestoreJob controllers
(restorejob_controller.go), not only before BackupJobs — keep the
monorepo doc aligned with the website copy (cozystack/website#622).

Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Andrey Kolkov (androndo) added a commit to cozystack/cozystack that referenced this pull request Jul 21, 2026
…controller Package (#3333)

## What this PR does

Fixes #3245.

The documented admin override for the `cozy-default` backup S3
coordinates — `spec.components.backupstrategy-controller.values` on the
`Package cozystack.cozystack-platform`
(`docs/operations/backup-classes.md`, "Admin overrides for
`cozy-default`") — had no effect: the `cozystack-platform` PackageSource
exposes exactly one component (`platform`), so the operator silently
ignored the block, and the platform chart rendered the child `Package
cozystack.backupstrategy-controller` via the `.default` helper with no
components argument, so nothing forwarded `backupStorage` either. Admins
running against an external or TLS S3 had no working supported path to
set `backupStorage.endpoint` / `region` / `systemSecretName`.

This PR wires the override through the platform chart, the same way
`bundles/iaas.yaml` forwards kubevirt and gpu-operator values:

- `packages/core/platform/values.yaml` gains a top-level `backupStorage:
{}` value.
- `templates/bundles/system.yaml` renders it into the emitted `Package
cozystack.backupstrategy-controller` as
`components.backupstrategy-controller.values.backupStorage`, from where
the cozystack operator merges it into the `backupstrategy-controller`
HelmRelease over the chart defaults. When `backupStorage` is unset the
components block is omitted and the emitted Package is byte-identical to
the historical render.
- `docs/operations/backup-classes.md` now documents the working path
(`spec.components.platform.values.backupStorage`) and explicitly calls
out the two plausible-looking paths that do not work (the old documented
one, and patching the child Package directly, which the platform
helm-reconcile reverts).
- A new helm-unittest suite
(`packages/core/platform/tests/bundles_backupstorage_wiring_test.yaml`)
is the behavioural regression test: it proves the unset case emits no
components block and the set case lands the values on the child Package,
on both `isp-full` and `isp-full-generic`.

Verified with `make test` in `packages/core/platform` (22 suites, 91
tests, all green). `make show` requires a live cluster (`cozyhr` +
`lookup` in `templates/repository.yaml`), so the unittest render of
`templates/bundles/system.yaml` is the offline verification path.

### Screenshots

Not a UI change.

### Downstream repositories

- [ ] No downstream repository is affected by this change
- [x] [cozystack/website](https://github.com/cozystack/website) -
follow-up: cozystack/website#622
- [ ]
[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:

Walking the trigger map against the diff: the change to
`packages/core/platform/values.yaml` reaches **cozystack/website** twice
— `content/en/docs/next/operations/configuration/platform-package.md`
(hand-written table of `spec.components.platform.values.*` keys gains
`backupStorage`) and
`content/en/docs/next/operations/services/backup-classes.md` (mirrors
the broken override example this PR corrects). The follow-up is
cozystack/website#622, which also carries the superseded website#547
content and covers both next/ and v1.4/ docs, marked not to merge before
this PR lands; its v1.4 copy assumes this fix is backported to
release-1.4. No other downstream repository restates the touched files:
the templates change is chart-internal, no app `values.schema.json`,
CRD, variant list, `hack/` file, or namespace changed.

### Release note

```release-note
fix(platform): the documented admin override for the cozy-default backup S3 coordinates now works — set spec.components.platform.values.backupStorage on the cozystack.cozystack-platform Package and the platform chart forwards it to the backupstrategy-controller component (previously the documented spec.components.backupstrategy-controller path was silently ignored)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants