feat(platform): add default backupclass - #2716

Code Review

This pull request implements a platform-managed backup credentials projection system, centralizing S3 configuration and deprecating per-tenant S3 settings in application charts. It introduces a CredentialsProjector that automatically formats and copies S3 secrets into tenant and system namespaces, enabling a unified cozy-default BackupClass for Postgres, MariaDB, Etcd, ClickHouse, FoundationDB, and Velero-based VM backups. Review feedback identifies several issues in the new Altinity strategy template, including non-deterministic backup naming, the use of apk add at runtime, the lack of non-root security contexts, and a logic error where the restore process defaults to the latest backup rather than the specific artifact requested by the RestoreJob.

Actionable comments posted: 5

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 `@hack/e2e-apps/backup-default-class.bats`:
- Line 77: The kubectl wait command for the HelmRelease (kubectl -n
"$TEST_NAMESPACE" wait hr "${TEST_POSTGRES_NAME}" --for=condition=ready
--timeout=600s) can fail with NotFound because the HelmRelease may not be
materialized yet; add an existence backstop by polling for the resource (e.g.,
loop calling kubectl -n "$TEST_NAMESPACE" get hr "${TEST_POSTGRES_NAME}" until
it returns success or a short timeout) before running kubectl wait, so the
script ensures the HelmRelease exists prior to waiting for the ready condition.

In `@hack/e2e-apps/backup-etcd.bats`:
- Around line 106-107: The two kubectl wait calls for the HelmRelease resources
(hr "bucket-${TEST_BUCKET_NAME}" and bucketclaims.objectstorage.k8s.io
"bucket-${TEST_BUCKET_NAME}") can race controller materialization; add an
existence backstop by polling with kubectl get in a loop (using TEST_NAMESPACE
and the HelmRelease name pattern "bucket-${TEST_BUCKET_NAME}") with a short
sleep and overall timeout, and only call kubectl wait after the resource is
observed; apply the same polling guard for the second
bucketclaims.objectstorage.k8s.io wait (also referenced around line 161) so both
waits are preceded by existence checks to avoid flakes.

In `@packages/system/backupstrategy-controller/templates/bucket.yaml`:
- Around line 5-6: When backupStorage.provisionBucket is true the template
should fail fast if bucketName or namespace are empty; update the manifest so
the name and namespace fields use the Helm required function when
.Values.backupStorage.provisionBucket is set. Concretely, guard the fields that
render .Values.backupStorage.bucketName and .Values.backupStorage.namespace with
an if on .Values.backupStorage.provisionBucket and call required("clear error
message", .Values.backupStorage.bucketName) and required("clear error message",
.Values.backupStorage.namespace) respectively so template rendering fails with a
useful message instead of producing an invalid manifest.

In
`@packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml`:
- Around line 45-72: The polling loop and curl calls can hang indefinitely: add
a bounded timeout/attempts and per-request timeouts. Update the curl invocations
that set/inspect backups (the POSTs that create/restore and the GETs that
populate target and row via curl in the blocks that reference
${api}/backup/create_remote, ${api}/backup/list/remote,
${api}/backup/restore_remote/${target} and ${api}/backup/actions) to include a
--max-time (e.g. 10s) option; then replace the unbounded while true that
inspects row/status with a bounded loop (use a max attempts counter or compute a
deadline using $(date +%s) and compare) that treats empty or missing row as “in
progress” until timeout and exits with a helpful error if the max
attempts/deadline is reached, ensuring the same error handling path used for
status=error is triggered on timeout.

In
`@packages/system/backupstrategy-controller/templates/strategy-foundationdb-default.yaml`:
- Around line 18-20: Replace the hardcoded "secure_connection=0" with a
conditional that derives secure_connection from .Values.backupStorage.endpoint;
e.g. in the strategy-foundationdb-default.yaml template use a Helm/Go template
test (hasPrefix or regexMatch) on .Values.backupStorage.endpoint to emit
"secure_connection=1" when the endpoint scheme is "https://" and
"secure_connection=0" otherwise, keeping the rest of the lines (region,
bucket_path) unchanged; reference the symbols secure_connection and
.Values.backupStorage.endpoint when making the change.

---

Nitpick comments:
In `@internal/backupcontroller/credentials_projector.go`:
- Around line 108-123: The target Secret's Data map in the credentials projector
(inside the function that performs CreateOrUpdate and writes target.Data) can
retain stale "endpoint", "bucketName", and "region" entries because they are
only set when non-empty; update the logic to clear or reset these optional keys
before applying new values—e.g., delete or set them to empty []byte{}
unconditionally prior to the conditional assignments for endpoint, bucket, and
region—so that previous values don't persist when the source omits them; adjust
the code around the existing target.Data assignments (the block that sets
"AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY", "cloud", "blob_credentials.json",
and the optional keys) to ensure stale keys are removed.

Actionable comments posted: 1

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 `@internal/backupcontroller/credentials_projector.go`:
- Around line 122-124: The fallback assignment leaves cfg.Endpoint unnormalized
causing scheme mismatch: when setting creds.endpoint from cfg.Endpoint do the
same normalization used for parsed source paths (i.e., strip/normalize the URL
scheme and trailing parts) before storing it; update the creds.endpoint =
cfg.Endpoint line to pass cfg.Endpoint through the existing endpoint
normalization/parsing routine used elsewhere in this file (or implement
equivalent scheme-stripping) so creds.endpoint matches parsed-source format.

---

Outside diff comments:
In `@packages/apps/clickhouse/templates/backup-strategy-secret.yaml`:
- Around line 16-23: The gating comment above the template is out of date: it
mentions only `backup.enabled` and an empty `backup.s3CredentialsSecret.name`
but the template condition also requires `backup.useSystemBucket` to be false;
update the inline description so it accurately reflects the rendered condition —
i.e. state that the template is rendered only when `backup.enabled=true`,
`backup.useSystemBucket=false`, and `backup.s3CredentialsSecret.name` is empty
(and keep the existing explanation about using a user-supplied Secret when
`s3CredentialsSecret.name` is set).

---

Duplicate comments:
In `@hack/e2e-apps/backup-default-class.bats`:
- Line 77: Add an existence backstop before calling `kubectl ... wait hr
"${TEST_POSTGRES_NAME}"`: poll for the HelmRelease resource (using `kubectl -n
"$TEST_NAMESPACE" get hr "${TEST_POSTGRES_NAME}"` or equivalent) in a short loop
with a total timeout and small sleep intervals until it returns found, then
proceed to the existing `kubectl -n "$TEST_NAMESPACE" wait hr
"${TEST_POSTGRES_NAME}" --for=condition=ready --timeout=600s`; this prevents
spurious NotFound flakiness when the controller hasn’t yet created the `hr`.

In `@hack/e2e-apps/backup-etcd.bats`:
- Around line 106-107: The two kubectl wait calls (kubectl -n "$TEST_NAMESPACE"
wait hr "bucket-${TEST_BUCKET_NAME}" --for=condition=ready --timeout=300s and
kubectl -n "$TEST_NAMESPACE" wait bucketclaims.objectstorage.k8s.io
"bucket-${TEST_BUCKET_NAME}" ...) can race if the HelmRelease/CRD isn't created
yet; add an existence backstop by polling with kubectl get for each resource
(e.g., loop until kubectl -n "$TEST_NAMESPACE" get hr
"bucket-${TEST_BUCKET_NAME}" and similarly for bucketclaims.objectstorage.k8s.io
"bucket-${TEST_BUCKET_NAME}" return success, with a short sleep and timeout),
then run the existing kubectl wait command to wait for ready.

In `@packages/system/backupstrategy-controller/templates/bucket.yaml`:
- Around line 5-6: When backupStorage.provisionBucket is true we must fail
template rendering if bucket identifiers are missing; update the bucket.yaml
template to check .Values.backupStorage.provisionBucket and call the Helm
required function on .Values.backupStorage.bucketName and
.Values.backupStorage.namespace (e.g. required "error: backupStorage.bucketName
is required when provisionBucket=true" .Values.backupStorage.bucketName) so
rendering errors out with a clear message instead of emitting an invalid
manifest.

In
`@packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml`:
- Around line 58-85: The script's curl calls (the POST to
"${api}/backup/create_remote", the GETs to "${api}/backup/list/remote",
"${api}/backup/restore_remote/${target}", and "${api}/backup/actions") and the
unbounded "while true" polling loop can block forever; add per-request timeouts
to all curl invocations (e.g. --max-time/--connect-timeout or equivalent) and
make the polling loop bounded by introducing a deadline/timeout variable (e.g.
POLL_TIMEOUT/POLL_MAX_RETRIES and POLL_INTERVAL) and check elapsed time or
retries inside the loop before exiting with an error; update references to
target/row/status handling to exit when the deadline is reached. Ensure all curl
calls in this template include the timeout flags and the "while true" is
replaced with a bounded loop that fails cleanly after the timeout.

---

Nitpick comments:
In `@internal/backupcontroller/credentials_projector_runnable.go`:
- Around line 46-48: The guard in NewSystemCredentialsProjector currently
replaces period only when period == 0; change it to treat non-positive durations
defensively by checking period <= 0 and setting period = time.Minute so Start()
(which calls time.NewTicker) cannot panic on zero or negative durations; update
the conditional around the period variable in NewSystemCredentialsProjector to
use <= 0 to harden against bad callers/config.

Actionable comments posted: 1

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 `@internal/backupcontroller/credentials_projector.go`:
- Around line 164-227: The preflight c.Get can race with
controllerutil.CreateOrUpdate; inside the CreateOrUpdate mutate callback (the
anonymous func passed to controllerutil.CreateOrUpdate) re-check
target.Labels[managedByLabel] and if it exists but != managedByValue return a
ProjectionError with ReasonTargetNotOwned (same semantics as the preflight
check) so the reconcile refuses to overwrite a concurrently-created unmanaged
Secret; reference the mutate callback, the target variable,
managedByLabel/managedByValue, and ProjectionError/ReasonTargetNotOwned when
implementing this guard.

---

Duplicate comments:
In
`@packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml`:
- Around line 58-85: The polling loop in the backup/restore flow (the while true
loop that computes row/status from the backup/actions endpoint) is unbounded and
all curl calls (the calls that set target via backup/list/remote,
create_remote/restore_remote, and polling backup/actions) lack per-request
timeouts; fix by adding a curl timeout flag (e.g., --max-time with a sensible
seconds value) to every curl invocation (the curl that sets target, the POST to
backup/create_remote and backup/restore_remote, and the curl that sets row from
backup/actions) and bound the polling loop in the function by either adding a
max-attempts counter or tracking start time and breaking with an error after a
configurable timeout (check the variables target, row, status and the polling
loop that inspects backup/actions) so the script exits instead of hanging
indefinitely.

In
`@packages/system/backupstrategy-controller/templates/strategy-foundationdb-default.yaml`:
- Around line 18-20: The template currently hardcodes "secure_connection=0";
change it to derive secure_connection from .Values.backupStorage.endpoint by
checking its scheme (set secure_connection=1 when the endpoint starts with
"https://" or has "https" scheme, otherwise 0). Update the YAML template around
the existing secure_connection line (the block that also uses
.Values.backupStorage.region and bucket_path) to compute the value via a Helm
template conditional or string test on .Values.backupStorage.endpoint so HTTPS
endpoints produce secure_connection=1 and others produce 0.

---

Nitpick comments:
In `@packages/system/backupstrategy-controller/templates/velero-bsl.yaml`:
- Line 2: The template defines a helper variable $endpointHost (trimmed from
.Values.backupStorage.endpoint) but never uses it, causing an unused variable;
either replace occurrences of .Values.backupStorage.endpoint used for the s3Url
configuration with $endpointHost (so s3Url uses the trimmed host) or remove the
$endpointHost declaration entirely if trimming was not intended; locate the
declaration of $endpointHost and the s3Url config reference to update one or the
other accordingly.

Actionable comments posted: 4

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 `@cmd/backupstrategy-controller/main.go`:
- Around line 185-190: When BACKUP_STORAGE_SYSTEM_NAMESPACES is set but the
projection configuration (source/target envs) is missing, the
SystemCredentialsProjector runnable is currently added and then silently
disables itself; change main.go to validate the projection config before calling
mgr.Add so the process fails fast. In practice, check the same environment
variables or config used by backupcontroller.NewSystemCredentialsProjector (the
source/target projection settings) when BACKUP_STORAGE_SYSTEM_NAMESPACES != ""
and if required source/target values are absent log a descriptive error via
setupLog.Error and exit(1) instead of adding the runnable; reference
NewSystemCredentialsProjector and SystemCredentialsProjector in the check so the
failure clearly ties to the projector configuration.

In `@internal/backupcontroller/backupjob_controller_test.go`:
- Around line 152-154: The test currently treats any non-nil error from
c.Get(context.Background(), types.NamespacedName{Namespace: "tenant-acme", Name:
"cozy-backups-creds"}, got) as success; change the assertion to explicitly
require a NotFound error by using apierrors.IsNotFound(err). Import
"k8s.io/apimachinery/pkg/api/errors" (aliased as apierrors) if not present, and
replace the err == nil check with a conditional that calls
apierrors.IsNotFound(err) and fails the test for any other error (or for nil),
so only a true NotFound result passes the "secret should be absent" expectation.

In `@packages/apps/clickhouse/templates/backup-strategy-secret.yaml`:
- Line 23: Update the docblock above the template gating expression to reflect
all three predicates now used in the condition: .Values.backup.enabled,
.Values.backup.useSystemBucket must be false (i.e.
backup.useSystemBucket=false), and .Values.backup.s3CredentialsSecret.name == ""
so the comment accurately documents the clause `if and .Values.backup.enabled
(not .Values.backup.useSystemBucket) (eq .Values.backup.s3CredentialsSecret.name
"")`.

---

Duplicate comments:
In `@hack/e2e-apps/backup-etcd.bats`:
- Around line 106-107: The kubectl wait calls on HelmReleases can race with
cozystack-controller not having materialized the HR; add an existence backstop
loop that polls for the HR resource before each kubectl wait: before the kubectl
wait hr "bucket-${TEST_BUCKET_NAME}" and before the kubectl wait hr
"${TEST_ETCD_NAME}" insert a short polling loop that repeatedly runs kubectl get
hr "<name>" in the target namespace (suppressing output) until it returns
success, with a small sleep/retry interval and a reasonable timeout, then
proceed to the existing kubectl wait commands so the waits no longer fail due to
controller-materialization flakiness.

In
`@packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml`:
- Around line 66-67: The script contains unbounded polling loops ("while true;
do") and curl calls without timeouts (e.g., curl -fsS ${auth} -X POST
"${api}/backup/create_remote?name=${target}") which can hang jobs; replace each
unbounded "while true" polling loop with a bounded retry loop (use a max
attempts counter and break on success) and add curl timeout flags (e.g.,
--max-time and --connect-timeout) to all curl invocations that call
"${api}/backup/create_remote", "${api}/backup/restore_remote" and any
backup/actions endpoints so requests fail fast; update the polling logic to
respect the same overall timeout and propagate non-retryable failures out of the
functions/blocks that reference ${api}, ${auth}, and ${target}.

Actionable comments posted: 3

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 `@internal/backupcontroller/credentials_projector_runnable.go`:
- Around line 89-90: The code only guards when period == 0 but time.NewTicker
will panic for negative durations; update the constructor or initializer that
sets p.Period (the Period field used by the Runnable and the call to
time.NewTicker(p.Period)) to normalize non-positive values by treating <= 0 the
same as 0 (e.g., set period = time.Minute when period <= 0) or return an error
if invalid; ensure the normalization happens before storing to p.Period so any
subsequent call to time.NewTicker(p.Period) is safe.

In `@internal/backupcontroller/restorejob_controller.go`:
- Around line 111-119: Projecting platform-managed S3 credentials happens before
strategy kind validation, so ProjectBackupCredentials (called in
restorejob_controller.go) can create the tenant Secret even for unsupported
strategy kinds; update the flow to validate the RestoreJob's strategy kind first
(the logic that rejects unknown kinds for strategy.backups.cozystack.io) before
calling ProjectBackupCredentials, or add an early check (e.g.,
validateStrategyKind/ensureSupportedStrategy) at the start of
reconcileRestoreJob to short-circuit and return a Failure for unsupported kinds;
ensure you still call handleProjectionError when projection fails, but never
call ProjectBackupCredentials for an unvalidated/unsupported strategy.

In `@packages/apps/postgres/values.schema.json`:
- Around line 243-247: Update the deprecated destinationPath value so new
installs don’t get a legacy S3 path by default: remove or set the default for
the destinationPath key to an empty string in the source values (the
values.yaml/tool entry that defines destinationPath), keep the deprecation
description, then run make generate to regenerate
packages/apps/postgres/values.schema.json so the schema no longer defaults to
"s3://bucket/path/to/folder/".

---

Duplicate comments:
In `@hack/e2e-apps/backup-default-class.bats`:
- Line 90: The test currently calls kubectl wait directly on the HelmRelease
(hr) referenced by TEST_POSTGRES_NAME (and the second hr at the later
occurrence), which can race with resource creation; add an existence backstop
that polls until kubectl -n "$TEST_NAMESPACE" get hr "${TEST_POSTGRES_NAME}"
(and the other hr) returns the resource (e.g., loop with a small sleep and a
timeout, using --ignore-not-found or checking exit code) before calling kubectl
-n "$TEST_NAMESPACE" wait hr "${TEST_POSTGRES_NAME}" --for=condition=ready
--timeout=600s so the wait never fails with NotFound.

In `@internal/backupcontroller/backupjob_controller_test.go`:
- Around line 152-154: Replace the loose error check after
c.Get(context.Background(), types.NamespacedName{Namespace: "tenant-acme", Name:
"cozy-backups-creds"}, got) so the test explicitly asserts the Secret is missing
by using apierrors.IsNotFound(err) instead of err == nil; import
"k8s.io/apimachinery/pkg/api/errors" as apierrors if not already present and
update the assertion to fail when err is nil or when apierrors.IsNotFound(err)
is false so only a NotFound result counts as the expected outcome.

In `@internal/backupcontroller/credentials_projector.go`:
- Around line 148-150: The fallback assignment for creds.endpoint currently
copies cfg.Endpoint raw, causing inconsistency with source-parsed endpoints
which are normalized via stripScheme; update the conditional in
credentials_projector (the block setting creds.endpoint when empty) to assign
stripScheme(cfg.Endpoint) instead of cfg.Endpoint so the projected endpoint
format matches the scheme-stripped values produced elsewhere.

In
`@packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml`:
- Around line 66-67: The template uses unbounded "while true" polling loops and
curl calls without per-request timeouts (see the curl POST to
"${api}/backup/create_remote?name=${target}" and the surrounding polling loops),
which can hang indefinitely; modify each polling loop to include a maximum
iteration count (e.g., max_attempts with a counter and exit/error after
exceeded) and add curl timeouts such as --max-time and a short --connect-timeout
to every curl invocation (including the POST to create_remote and subsequent
status/poll requests using ${api} and ${auth}) so each request fails fast and
the loop exits deterministically when retries are exhausted.

LGTM

Scope

Ships a platform-managed cozy-default BackupClass, a system cozy-backups Bucket, a credentials projector that copies the platform S3 Secret into tenant/system namespaces per BackupJob (and on a leader-elected tick for system namespaces), an opt-in useSystemBucket flag for the postgres and clickhouse charts, and a Velero BSL owned by the backupstrategy-controller chart.

Verification performed

• go build ./internal/backupcontroller/... ./cmd/backupstrategy-controller/... — clean (go 1.26.2).
• go vet on the same packages — clean.
• go test ./internal/backupcontroller/... — all pass, including the 20+ new projector/controller tests (ownership guard, transient-vs-terminal split, idempotency / no-ResourceVersion-bump, scheme stripping, BucketInfo fallback, empty-endpoint guard, leader-election requirement, terminal/transient reconcile paths for both BackupJob and RestoreJob).
• Secret scan of the diff — no hardcoded credentials (the only api_key hit is the FDB blob_credentials.json field name, populated from a variable).
• Doc cross-check: docs/operations/backup-classes.md endpoint-form table, key-format table, observability counters, and useSystemBucket instructions all match the code. --leader-elect is present in the deployment, so the leader-elected SystemCredentialsProjector will actually run.

Findings (no blockers)

The implementation is solid and the test coverage is unusually thorough for each non-trivial path. No bugs, security gaps, error-handling holes, or doc drift found. A few non-blocking observations:

1. Recommended (doc precision, not a blocker) — docs/operations/backup-classes.md:90 says the projector's first synchronous round runs "immediately when the backupstrategy-controller Pod becomes Ready." The runnable is leader-elected (credentials_projector_runnable.go:75 NeedLeaderElection() = true) and the readiness probe is healthz.Ping, so a non-leader replica is Ready without ever projecting; the first projection happens when a replica wins the lease, not merely when it is Ready. The practical timing claim (tens of seconds) is fine; only the "becomes Ready" wording is loose. Optional reword to "becomes the elected leader."

2. Note (intentional, documented) — in backupjob_controller.go:112-118, a BackupJob whose resolved StrategyRef.APIGroup is not the platform group returns ctrl.Result{}, nil with no status write and no requeue, leaving it phaseless. This is deliberate (another controller owns that group) and the comment says so; flagging only so it is a conscious decision rather than an oversight.

Note on local generated-doc validation

The schema/README/values for the postgres and clickhouse charts are generated from api/.../types.go via cozyvalues-gen, which the repo pins at v1.4.0 (commit 7cc0194f2). I could not validate chart-doc drift locally because of a cozyvalues-gen version mismatch on my end — the pinned v1.4.0 in CI's run-make-generate hook is the authoritative check here, so trust that over any local generation.

Requesting changes: two documentation-drift blockers. The Go code is well-structured and thoroughly unit-tested (go build, go vet, go test ./internal/backupcontroller/... all pass; gofmt clean), and the chart renders correctly offline. The blockers are both in the operator-facing docs this PR adds/changes, and would actively mislead an operator following the runbook.

Summary of what the change does

Adds a platform-managed default backup stack on top of the existing backups.cozystack.io API:

• A cozy-default BackupClass (cluster-wide), plus pre-rendered Strategy CRs for CNPG/MariaDB/Etcd/Altinity/FoundationDB/Velero (vminstance+vmdisk).
• An apps.cozystack.io/Bucket cozy-backups CR and a Velero BackupStorageLocation cozy-default.
• A credentials projector (internal/backupcontroller/credentials_projector.go + _runnable.go) that copies the bucket Secret into tenant namespaces on demand (per BackupJob/RestoreJob) and into system namespaces (cozy-velero) on a leader-elected periodic tick.
• Reconciler changes: terminal-phase skip, BackupClass/strategy gating before projection, transient-vs-terminal classification, MariaDB backup deadline, FoundationDB cleanup branch.
• useSystemBucket opt-in on Postgres + ClickHouse apps; the legacy chart-managed S3 fields are marked DEPRECATED.

I verified the bootstrap gating directly: with no resolved bucket name, only the Bucket CR + BackupClass render; strategy CRs and the BSL stay absent until the BucketClaim status populates.

BLOCKER 1 — Docs name the wrong namespace for the platform bucket and source Secret

docs/operations/backup-classes.md and api/backups/v1alpha1/DESIGN.md repeatedly state that the cozy-backups Bucket and the source credentials Secret live in the cozy-backup-controller namespace. The implementation puts them in tenant-root.

Evidence (implementation — tenant-root):

• packages/system/backupstrategy-controller/values.yaml:41 → backupStorage.namespace: tenant-root, with an in-file comment explaining the Bucket must live in a tenant- prefixed namespace (the apps.cozystack.io/Bucket chart's RBAC helper cozy-lib/_rbac.tpl fails Helm render otherwise), so tenant-root was chosen.
• packages/system/backupstrategy-controller/templates/bucket.yaml:6 → namespace: {{ .Values.backupStorage.namespace }} → renders namespace: tenant-root (verified via helm template).
• packages/system/backupstrategy-controller/templates/_helpers.tpl:37 → BucketClaim lookup uses .Values.backupStorage.namespace (= tenant-root).
• packages/system/backupstrategy-controller/templates/deployment.yaml:40 → BACKUP_STORAGE_SECRET_NAMESPACE = {{ .Values.backupStorage.namespace }} (= tenant-root), which is the source namespace the projector reads from (cmd/backupstrategy-controller/main.go → BackupCredentialsConfig.SourceNamespace).

Evidence (docs — cozy-backup-controller, wrong):

• docs/operations/backup-classes.md:3 — "...references the system-managed S3 bucket cozy-backups in the cozy-backup-controller namespace."
• docs/operations/backup-classes.md:74-75 — kubectl -n cozy-backup-controller get bucket cozy-backups / kubectl -n cozy-backup-controller get secret bucket-cozy-backups-system-credentials (both return NotFound — the objects are in tenant-root).
• docs/operations/backup-classes.md:79 — "The bucket lives in cozy-backup-controller..."
• docs/operations/backup-classes.md:96 and :98 — "ships an apps.cozystack.io/Bucket cozy-backups CR in cozy-backup-controller" and kubectl get bucketclaim -n cozy-backup-controller bucket-cozy-backups ....
• docs/operations/backup-classes.md:161 — "create the source credentials Secret in cozy-backup-controller manually..."
• api/backups/v1alpha1/DESIGN.md:25 — "A platform-managed S3 bucket cozy-backups (provisioned through apps.cozystack.io/Bucket in cozy-backup-controller)."

Why it's blocking: an operator following the inspection/troubleshooting commands in the docs (the explicit purpose of docs/operations/backup-classes.md) gets NotFound on every namespaced lookup, and an external-S3 admin following :161 creates the manual source Secret in the wrong namespace, so the projector reads nothing and every BackupJob requeues with SourceSecretMissing. The PR introduces this doc and this code together, so the drift is owned by this PR.

Fix: replace every cozy-backup-controller occurrence above with tenant-root (or, if cozy-backup-controller is genuinely the intended home, change backupStorage.namespace — but note the chart comment that the Bucket RBAC helper rejects non-tenant- namespaces, so tenant-root is the deliberate choice and the docs are what's wrong). The cozy-system (HelmRelease targetNamespace) and cozy-velero references are correct and should stay.

Test that fails without the fix: an e2e that installs the chart and asserts the BackupClass functions end to end — e.g. create a BackupJob against cozy-default and assert the projected cozy-backups-creds Secret materialises and the job leaves the transient SourceSecretMissing state. Such a test exercises the real backupStorage.namespace=tenant-root path; a docs-following operator runbook would hit the NotFound. (Do not add a test that greps the markdown for the namespace string — fix the docs and let the e2e prove the runbook.)

BLOCKER 2 — Doc claims the Altinity strategy renders during the bootstrap window; it does not

docs/operations/backup-classes.md:96 states: "...until [the BucketClaim status populates], the strategy templates render empty and only the Bucket CR + Altinity strategy + BackupClass are present in the cluster."

This is wrong. packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml wraps the entire CR in {{- if $bucketName -}} (the same gate as every other strategy template). Verified via helm template with no override / no live lookup: top-level Strategy CR instances rendered = 0; only kind: Bucket and kind: BackupClass appear. The Altinity strategy is gated on $bucketName exactly like CNPG/Etcd/MariaDB/FDB/Velero and is therefore absent during the bootstrap window.

Why it's blocking: it is in the same docs paragraph being added by this PR and describes the area being changed, and it misleads an operator debugging a bootstrap install into thinking the Altinity strategy should already exist (it intentionally does not).

Fix: change :96 to "...only the Bucket CR + BackupClass are present in the cluster." (drop "Altinity strategy").

Test that fails without the fix: the same bootstrap-window e2e/render assertion — install with an unpopulated BucketClaim and assert the rendered set is exactly {Bucket, BackupClass} with zero Strategy CRs. (Again: assert against the rendered/applied cluster objects in e2e, not by grepping the template source.)

Non-blocking observations (worth a glance, not gating)

• internal/backupcontroller/credentials_projector.go:199-250: the ownership-guard race (concurrent Create of an unlabelled Secret → AlreadyExists → projection fails this round, next round hits the guard) is correctly reasoned and self-healing; IsTransient keeps ReasonTargetNotOwned terminal and ReasonAPIError/ReasonSourceMissing transient. The AlreadyExists from the racing Create is wrapped as ReasonAPIError (transient) so it requeues — consistent with the comment.
• packages/system/backupstrategy-controller/templates/strategy-altinity-default.yaml: ClickHouse restore matches remote backups by name prefix clickhouse-<ApplicationRef.Name>-, while the sidecar's S3 path is scoped by <namespace>/<release> (clickhouse.yaml). These are different axes (object-key prefix vs backup-name prefix) and are internally consistent for same-release restore; cross-release restore is an advanced path outside the default flow. No action needed — just noting the two-prefix design is easy to misread.
• internal/backupcontroller/mariadbstrategy_controller.go:123-138,168-189: StartedAt is persisted (with a fresh-read idempotency guard against stale informer cache) before the MariaDB-CR existence check, so the mariadbDefaultBackupDeadline (30m) clock is correctly started; mariadbBackupDeadlineExceeded(nil) returns false so a not-yet-started job won't be failed prematurely. Covered by TestMariaDBBackupDeadlineExceeded and the two StartedAt subtests.
• The new chBackupClientImage (packages/system/backupstrategy-controller/values.yaml) and platform-migrations digest must stay in lockstep with packages/core/platform/values.yaml; the in-file comment says so but there is no enforcement. A future bump can drift.

Requesting changes. The namespace and bootstrap-window documentation are now correct. This round found one credential-handling regression in the Postgres chart, plus test-coverage gaps and a behavioral change worth bounding.

Summary

Large, well-structured change: a platform-managed cozy-default BackupClass, a system cozy-backups Bucket, a credentials projector that copies the platform S3 Secret into tenant/system namespaces, an opt-in useSystemBucket flag on Postgres + ClickHouse, default Strategy CRs for every driver, and a Velero BSL. The Go code is careful (typed projection errors, transient/terminal split, ownership guard, leader-elected system projector, cache disabled for Secrets to keep RBAC minimal), and the Go-side test coverage is genuinely thorough. Build, go vet, and go test ./internal/backupcontroller/ all pass. The committed Helm schemas/READMEs are in sync with cozyvalues-gen (verified by regenerating; no drift).

BLOCKER 1 — Postgres bootstrap recovery renders a dangling Secret reference when no creds are supplied

packages/apps/postgres/templates/db.yaml (bootstrap block, lines 129-157) + packages/apps/postgres/templates/backup-secret.yaml (guard at line 13).

What's wrong: This PR changed the s3AccessKey/s3SecretKey defaults from the placeholders "<your-access-key>"/"<your-secret-key>" to "" (api/apps/v1alpha1/postgresql/types.go lines 78-86), and tightened the <release>-s3-creds Secret guard to require $hasInlineCreds (both keys non-empty):

{{- $hasInlineCreds := and (ne (default "" .Values.backup.s3AccessKey) "") (ne (default "" .Values.backup.s3SecretKey) "") }}
{{- if and (or .Values.backup.enabled .Values.bootstrap.enabled) (not $credCfg.name) (not .Values.backup.useSystemBucket) $hasInlineCreds }}

But the bootstrap.recovery / externalClusters block in db.yaml is gated only by .Values.bootstrap.enabled and unconditionally references $credSecretName (default <release>-s3-creds) in externalClusters[0].barmanObjectStore.s3Credentials. With bootstrap.enabled=true, backup.enabled=false, no s3CredentialsSecret.name, and the new empty cred defaults, the chart renders a cnpg.io Cluster whose recovery s3Credentials points at <release>-s3-creds — but that Secret is never materialised. CNPG then crash-loops on bootstrap recovery with an opaque "secret not found" error.

Reproduced with helm template r packages/apps/postgres -n tenant-test and values {size: 10Gi, backup.enabled: false, bootstrap: {enabled: true, oldName: old}, _cluster.scheduling: {}}: the output contains name: r-s3-creds under externalClusters[].barmanObjectStore.s3Credentials and zero rendered Secrets.

This is a behavioral regression. Pre-PR (git show 1810263f9:packages/apps/postgres/templates/backup-secret.yaml) the Secret was materialised from the placeholder defaults whenever bootstrap.enabled=true, so the recovery reference always resolved (and an explicit empty value tripped the required "s3AccessKey is not specified!" error). Both safety nets are now gone. This is precisely the failure mode the PR's own guard at db.yaml:126-128 prevents for the useSystemBucket=true path — the legacy bootstrap path has the identical hole left open.

Fix: Fail fast (mirroring the existing useSystemBucket+bootstrap guard) when bootstrap.enabled=true and no usable credentials are available — i.e. when useSystemBucket=false AND s3CredentialsSecret.name is empty AND inline creds are empty. Add, alongside the existing guard at db.yaml:126-128:

{{- if and .Values.bootstrap.enabled (not .Values.backup.useSystemBucket) (not $credCfg.name) (not $hasInlineCreds) }}
{{- fail "postgres: bootstrap.enabled=true requires S3 credentials: set backup.s3AccessKey+backup.s3SecretKey, backup.s3CredentialsSecret.name, or backup.useSystemBucket=true (with a RestoreJob)." }}
{{- end }}

($hasInlineCreds is already computed at db.yaml — hoist it above this guard, or recompute it.)

Test that fails without the fix (and passes with it): a helm unittest case in packages/apps/postgres/tests/ rendering bootstrap.enabled=true, backup.enabled=false, no creds, asserting the render fails (asserts: [{failedTemplate: {errorMessage: "..."}}]). Without the guard the render currently succeeds and emits the dangling reference; with the guard it fails closed.

BLOCKER 2 — New Postgres/ClickHouse chart branches have no helm-unittest coverage

packages/apps/postgres/tests/backup_test.yaml, packages/apps/clickhouse/tests/ (no backup test suite exists).

What's wrong: The PR adds substantial new template logic, none of which is exercised by helm unittest:

• Postgres: barmanObjectStore gating ($renderBarman = enabled AND not useSystemBucket AND destinationPath != "" AND $hasLegacyCreds), the <release>-s3-creds skip-when-empty behaviour, and the useSystemBucket+bootstrap fail guard at db.yaml:126-128.
• ClickHouse: the useSystemBucket switch in clickhouse.yaml (sidecar Secret name → cozy-backups-creds, S3_PATH → <namespace>/<release>, optional: true on the env secretKeyRefs), the s3PathOverride+useSystemBucket fail guard (clickhouse.yaml:172-174), and the (not useSystemBucket) exclusion on backup-secret.yaml / backup-script.yaml.

The existing packages/apps/postgres/tests/backup_test.yaml only covers the pre-existing ScheduledBackup gating (4 it: cases, all about schedule). These are template-logic contracts (not source-grep drift guards), so helm-unittest is the right tool.

Fix / tests required: Add helm unittest cases asserting, at minimum:

• Postgres useSystemBucket=true, backup.enabled=true: barmanObjectStore is absent from the Cluster, and no <release>-s3-creds Secret is rendered.
• Postgres legacy backup.enabled=true with inline creds: barmanObjectStore is present and <release>-s3-creds is rendered.
• Postgres backup.enabled=true, no creds, useSystemBucket=false: barmanObjectStore is NOT rendered (the "half-empty barman would feed CNPG a broken archive_command" contract).
• Postgres useSystemBucket+bootstrap: render fails (already guarded — lock it in).
• Postgres bootstrap without creds: render fails (BLOCKER 1's new guard).
• ClickHouse useSystemBucket=true: sidecar env S3_BUCKET/S3_ENDPOINT/... reference cozy-backups-creds with optional: true, S3_PATH = <namespace>/<release>, and <release>-backup-s3 / restic Secrets are skipped.
• ClickHouse s3PathOverride+useSystemBucket: render fails.

BLOCKER 3 — Strategy-not-found switched from terminal failure to unbounded requeue, with no deadline on the strategy-not-found path itself

internal/backupcontroller/backupjob_controller.go:302-314 (requeueStrategyNotReady) and the call sites in cnpgstrategy_controller.go:182, altinitystrategy_controller.go:198, velerostrategy_controller.go:246, etcdstrategy_controller.go:185, foundationdbstrategy_controller.go:162, mariadbstrategy_controller.go:143 (and the restore mirrors).

What's wrong: Previously, a strategyRef.name that resolves to a non-existent Strategy CR was markBackupJobFailed (terminal). This PR changes every driver to requeueStrategyNotReady, which sets Ready=False/StrategyNotReady and requeues every CredentialsProjectionRequeue (30s) forever. The justification (bootstrap window: cozy-default BackupClass ships before its Strategy CRs, which are gated on the BucketClaim status) is legitimate and well-documented. But the change also converts a genuine misconfiguration — a typo in a custom BackupClass's strategyRef.name that will never resolve — from a clear terminal Failed into an infinite silent requeue with no operator-visible "this will never succeed" signal and no deadline bound.

Note the asymmetry: the MariaDB driver added a bounded deadline for the case where the operator-side MariaDB CR never appears (mariadbstrategy_controller.go:160-176, mariadbBackupDeadlineExceeded), explicitly because "a BackupJob whose applicationRef.name typo never resolves would pin in Running forever." The same reasoning applies to the Strategy-CR-not-found path, but no deadline was added there.

Fix: Bound requeueStrategyNotReady with a deadline (reuse j.Status.StartedAt like the MariaDB CR-existence path, or a dedicated budget): after the bootstrap-window grace period elapses, flip to terminal markBackupJobFailed with a message naming the missing strategy. This keeps the self-heal behaviour during bootstrap while still failing closed on a permanent misbinding.

Test that fails without the fix: a reconciler test seeding a BackupJob whose resolved strategyRef.name points at a non-existent Strategy CR, advancing StartedAt past the deadline, and asserting Phase == Failed (today it stays Ready=False and requeues indefinitely). The existing TestReconcile_BackupClassNotFound_IsTransient covers the transient direction but nothing asserts eventual terminal failure for a permanently-missing strategy.

Recommended to fix (non-blocking)

1. Stale comment in packages/system/backupstrategy-controller/templates/rbac.yaml:98 — says the source Secret is projected "from cozy-backup-controller", but values.yaml sets backupStorage.namespace: tenant-root (and the deployment wires BACKUP_STORAGE_SECRET_NAMESPACE from it). The RBAC is cluster-scoped so it still works, but the comment names the wrong namespace and will mislead the next reader. (The same cozy-backup-controller string in the test fixture credentials_projector_test.go:27 is harmless — it's an arbitrary fixture.) Align the comment with the configured namespace.

2. internal/backupcontroller/credentials_projector.go:163-165 — creds.region falls back to cfg.Region only after the endpoint check; fine functionally, but region is never validated as required (unlike endpoint). That's intentional per the comment ("region carried verbatim"), so no change needed — just confirming it's deliberate.

Things checked and found correct

• Credentials projector: typed ProjectionError + IsTransient split, ownership guard inside the CreateOrUpdate mutator, setOrDelete keeping the projected Secret in sync with the source, COSI BucketInfo per-field fallback, stripScheme handling bare host:port (the url.Parse mis-parse note is accurate). Strong test coverage: ownership refusal, stale-key removal, idempotency, endpoint forms, garbled-BucketInfo soft-skip, endpoint-missing-everywhere terminal.
• main.go disables the controller-runtime cache for Secrets and the RBAC grants only get/create/update/patch (no list/watch) — consistent, minimal blast radius.
• SystemCredentialsProjector is leader-elected (avoids double-projection across the 2 replicas), disabled cleanly on empty namespace list, monotonic failure/success counters matching the metric names documented in docs/operations/backup-classes.md:104-107.
• Terminal-phase skip on BackupJob (backupjob_controller.go:58-61) prevents re-projection after Succeeded/Failed.
• Credentials projection is correctly ordered AFTER BackupClass resolution + supported-Kind check in both BackupJob and RestoreJob reconcilers, so creds don't leak into third-party-strategy namespaces.
• All default Strategy CR templates gate on a non-empty $bucketName (including Altinity), and the cozy-default BackupClass renders unconditionally — matching the documented bootstrap self-heal design.
• Velero BSL sources endpoint/bucket/region from the same backupStorage values block as the projector (no drift).
• ClickHouse useSystemBucket sidecar uses optional: true secretKeyRefs so the Pod starts before first-BackupJob projection — verified the rendered env.
• Generated artifacts (values.schema.json, README.md) are in sync with cozyvalues-gen output; deepcopy/CRD untouched in scope. Docs (backup-classes.md) metric names, endpoint-scheme-stripping claim, and FDB caveat all match the code.

Requesting changes. The package test suite is currently red on this branch, and there's a deadline gap on the restore path. Details below; the rest of the change verifies cleanly.

Blocker 1 — package test suite is RED on this branch

go test ./internal/backupcontroller/... fails:

--- FAIL: TestBackupJobReconciler_MissingBackupClassErrors (0.00s)
    reconcilers_test.go:275: expected error from missing BackupClass, got nil
FAIL	github.com/cozystack/cozystack/internal/backupcontroller

What's wrong: this commit changed BackupJobReconciler.Reconcile's handling of a missing BackupClass. Before, a not-found BackupClass propagated the apiserver error up to controller-runtime (returned err != nil). This commit (internal/backupcontroller/backupjob_controller.go:88-99) now treats apierrors.IsNotFound as a transient bootstrap condition: it sets Ready=False/BackupClassNotFound and returns ctrl.Result{RequeueAfter: CredentialsProjectionRequeue}, nil — i.e. nil error.

The commit added a NEW test asserting the new behavior (TestReconcile_BackupClassNotFound_IsTransient in backupjob_controller_test.go:1064, which asserts err == nil and a requeue) but left the OLD test TestBackupJobReconciler_MissingBackupClassErrors (internal/backupcontroller/reconcilers_test.go:256-277) in place, which asserts the exact opposite (if err == nil { t.Errorf("expected error...") }). The two tests now encode contradictory contracts and the old one fails.

git log -S confirms the transient BackupClassNotFound handling was introduced by this commit, while reconcilers_test.go predates it — so this is the PR's regression to own: it changed the behavior and shipped a contradicting, now-failing test.

Fix: delete or rewrite TestBackupJobReconciler_MissingBackupClassErrors in internal/backupcontroller/reconcilers_test.go:252-277 to match the new transient contract (assert err == nil, res.RequeueAfter > 0, and Ready=False/BackupClassNotFound). Do NOT just delete it silently — fold its intent into the new test or replace its body so the "missing BackupClass is handled, not panicking" coverage survives. Note reconcilers_test.go uses an ApplicationRef with no APIGroup and Kind Harbor; after normalization that resolves under apps.cozystack.io, and the BackupClass lookup is what returns NotFound — so the rewrite should keep that exact setup and just flip the assertion.

Test that fails without the fix: the existing TestBackupJobReconciler_MissingBackupClassErrors (currently red). After the fix, go test ./internal/backupcontroller/... must be green; the corrected test is the one that proves the fix.

Blocker 2 — requeueRestoreStrategyNotReady uses StartedAt that the restore drivers do not all set before calling it

internal/backupcontroller/restorejob_controller.go:2863 (requeueRestoreStrategyNotReady) gates terminal-vs-transient on restoreJob.Status.StartedAt via strategyNotReadyDeadlineExceeded. For the BackupJob side the comments repeatedly assert "Every driver sets StartedAt before this is called, so the clock has already started" (backupjob_controller.go:863-865). The restore-side helper inherits the same assumption (restorejob_controller.go:2857-2862).

What's wrong / what to verify: the restore path calls requeueRestoreStrategyNotReady from the Altinity and CNPG restore reconcilers when the Strategy CR is NotFound (altinitystrategy_controller.go:531, cnpgstrategy_controller.go:1327). If any restore driver reaches that NotFound branch BEFORE it has persisted restoreJob.Status.StartedAt, then strategyNotReadyDeadlineExceeded(nil) returns false forever and that RestoreJob requeues indefinitely with no terminal deadline — defeating the "fail closed on a permanently-missing strategy" guarantee the helper claims to provide. The BackupJob side is safe only because StartedAt is set earlier in each driver; the restore side needs the same guarantee at every call site.

Fix: confirm (and if necessary make) every restore driver persist Status.StartedAt before it can hit the requeueRestoreStrategyNotReady NotFound branch. If a driver checks the Strategy CR existence before stamping StartedAt, move the stamp earlier or stamp it inside requeueRestoreStrategyNotReady itself (set StartedAt when nil, persist, and treat that first call as within-grace).

Test that fails without the fix: a unit test that calls reconcileCNPGRestore (or reconcileAltinityRestore) with a RestoreJob whose Status.StartedAt is nil and NO Strategy CR present, then asserts that after StrategyNotReadyDeadline of simulated wall-clock the RestoreJob eventually flips to Phase=Failed. Today such a RestoreJob requeues forever. (The existing TestRequeueRestoreStrategyNotReady_BoundedByDeadline only tests the helper in isolation with StartedAt pre-populated — it does not exercise the driver call sites, so it does not catch a driver that never stamps StartedAt.)

Recommended (not blocking)

• internal/backupcontroller/credentials_projector.go:176-187 documents a known race: a concurrent actor creating an unlabelled cozy-backups-creds between CreateOrUpdate's internal Get and Create yields AlreadyExists, the projection fails that round, and the next reconcile hits the ownership guard. The handling is correct (no stomp, terminal on next cycle), but there is no test for it. Add a test that pre-seeds an unlabelled target Secret and asserts ProjectBackupCredentials returns a ProjectionError{Reason: ReasonTargetNotOwned} and does NOT overwrite the Secret's data. (credentials_projector_test.go covers the parse paths heavily; confirm the ownership-guard path is among them — if it is, disregard.)

• stripScheme (credentials_projector.go:410-425) only strips http:// / https://. A source endpoint with an uppercase scheme (HTTP://) or a bare s3:// would pass through unstripped and produce a malformed FDB accountName / MariaDB endpoint. Given the platform always writes lowercase http(s):// from the bucket controller this is low-risk, but a defensive case-insensitive check (or an allowlist assertion) would harden the FDB blob_credentials host key.

Things checked and found correct

• Build is clean: go build ./internal/backupcontroller/... ./cmd/backupstrategy-controller/... ./api/... and go vet ./internal/backupcontroller/... both pass.
• chBackupClientImage value is actually consumed (strategy-altinity-default.yaml:33).
• supportedBackupStrategyKinds() (backupjob_controller.go:184) includes all 7 kinds and is the single source the dispatch switch, the pre-projection guard, and the restore guard all read from.
• Secret naming chain is consistent: bucket-rd prefix: bucket- + Bucket release cozy-backups + user system → bucket-cozy-backups-system-credentials, matching backupStorage.systemSecretName default and the BucketClaim lookup name bucket-cozy-backups in _helpers.tpl.
• metrics.enable → metrics.enabled (deployment.yaml:30,52) is a genuine bug fix; previously the template read a non-existent .enable key, so metrics/port were never bound. Now consistent with values.yaml (enabled: true).
• Credentials projection is correctly ordered AFTER BackupClass resolution + APIGroup filtering + supported-Kind check in both backupjob_controller.go and restorejob_controller.go, so a foreign-APIGroup or unsupported-Kind job never leaks cozy-backups-creds into a tenant namespace (covered by TestReconcile_SkipsProjection_ForeignAPIGroup, TestReconcile_UnsupportedKindInPlatformAPIGroup_IsTerminal, and the restore counterpart).
• Postgres db.yaml conditional rendering matches its documentation exactly: barmanObjectStore renders only when enabled && !useSystemBucket && destinationPath != "" && (inline creds || external secret). The two fail guards for incompatible bootstrap.enabled + useSystemBucket and bootstrap.enabled without creds are render-time fail-fast (they block a broken CR, not a comment-only "fix").
• ClickHouse clickhouse.yaml useSystemBucket path correctly sets S3_PATH=<ns>/<release>, switches the sidecar to cozy-backups-creds, marks the secretKeyRefs optional: true (so the Pod starts before first projection), and fails on s3PathOverride + useSystemBucket.
• README value tables (postgres/README.md, clickhouse/README.md) and the values.schema.json / cozyrds openAPISchema blocks are consistent with the deprecated-field descriptions and the new useSystemBucket field — these are generated artifacts and match the source values.yaml doc comments.
• docs/operations/backup-classes.md and api/backups/v1alpha1/DESIGN.md: the endpoint-format-per-driver table, the FoundationDB-not-bound caveat, the bootstrap-window note, and the override-via-Package path all match the templates and code (FDB is genuinely absent from backupclass-default.yaml's strategies; CNPG/Etcd/Velero consume full-URL .Values.backupStorage.endpoint while MariaDB/FDB strip the scheme — matches the table).
• user-credentials.yaml endpoint now strips both http:// and https://, consistent with parseSourceSecret's stripScheme.

LGTM. Approving. Verification details below; one cosmetic doc nit at the end (non-blocking).

What I verified in depth

• "Fail-closed" deadline guarantee across all drivers. requeueStrategyNotReady gates terminal-vs-transient on Status.StartedAt; if any driver could reach the Strategy-NotFound branch with StartedAt==nil the job would requeue forever. Confirmed all five backup drivers (CNPG reconcileCNPG, Etcd reconcileEtcd, Altinity reconcileAltinity, Velero reconcileVelero, FoundationDB reconcileFoundationDB) stamp+persist StartedAt before the strategy lookup, and the CNPG restore path is end-to-end covered by TestReconcileCNPGRestore_MissingStrategyFailsClosedAfterDeadline (internal/backupcontroller/cnpgstrategy_controller_test.go). MariaDB stamps StartedAt at mariadbstrategy_controller.go:132 before both its strategy lookup and its bounded MariaDB-CR-existence deadline. Guarantee holds.

• Projection ordering / secret-leak guard. Both BackupJobReconciler.Reconcile and RestoreJobReconciler.Reconcile resolve the BackupClass, reject foreign APIGroups, and reject unsupported Kinds inside the platform APIGroup before calling ProjectBackupCredentials. Covered by TestReconcile_SkipsProjection_ForeignAPIGroup, TestReconcile_UnsupportedKindInPlatformAPIGroup_IsTerminal, TestReconcile_SkipsProjection_TerminalPhase.

• Ownership guard on the projected Secret. ProjectBackupCredentials refuses to overwrite a pre-existing target Secret lacking apps.cozystack.io/managed-by=cozystack-backups. setOrDelete keeps the projected Secret strictly in sync with source. Source parsing handles both flat-key and COSI BucketInfo formats with sensible per-field fallback.

• stripScheme correctness. Bare host:port returned as-is (avoids url.Parse mis-parsing the port as a scheme); only explicit http(s):// go through url.Parse with a path-preserving manual-trim fallback. Empty endpoint fails terminally rather than projecting a half-broken Secret.

• Per-driver endpoint contract. Verified against the strategy type definitions: CNPG endpointURL and Etcd destination.s3.endpoint correctly get the full URL; MariaDB and FDB accountName get the scheme-stripped host; Velero BSL s3Url gets the full URL; ClickHouse S3_ENDPOINT reads the scheme-stripped projected key. MariaDB strategy S3 type has no forcePathStyle field, so omitting it is correct.

• Chart gating. Postgres useSystemBucket=true leaves barmanObjectStore unset (driver SSA-patches) with two fail guards placed before the bootstrap block; the <release>-s3-creds Secret and ScheduledBackup are gated off. ClickHouse makes the cozy-backups-creds secretKeyRefs optional: true only on the useSystemBucket path and fails on s3PathOverride+useSystemBucket. Source/projected Secret naming is consistent with the bucket-rd prefix and user-credentials.yaml rendering.

• Bootstrap ordering. PackageSource dependsOn covers backup-controller, bucket-application, objectstorage-controller, velero. The bucketName helper emits empty string (skip-render) until the BucketClaim status populates while bucket.yaml/backupclass-default.yaml always render — Flux re-renders to self-heal.

• Secrets cache disabled in main.go for corev1.Secret so the projector needs no list/watch RBAC (rbac.yaml grants only get/create/update/patch) and holds no tenant Secrets in RAM.

• Build / vet / tests. go build ./..., go vet, go test ./internal/backupcontroller/... all pass. helm-unittest passes for backupstrategy-controller (10), postgres (19), clickhouse (9). No uncommitted/generated drift.

Recommended (non-blocking, cosmetic)

docs/operations/backup-classes.md:30 illustrates the endpoint as http://seaweedfs-s3.tenant-root.svc:8333, but the actual values.yaml default (and line 125 of the same doc) is http://seaweedfs-s3.tenant-root.svc.cozy.local:8333. Line 30 is illustrative prose, not a literal config, but aligning it avoids confusion. Cosmetic only.

No bugs, security issues, missing error handling, logic gaps, or untested paths found. Credentials never land in CR specs or Backup.status; the projected Secret deliberately omits the internal.cozystack.io/tenantresource label so tenant RBAC cannot read it (operators mount via kubelet). The transient-vs-terminal split, deadline bounds, and observability counters are all test-covered.