mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 10:37:21 +09:00
Andrey KolkovandClaude Opus 4.8
9379729b31
Review notes from IvanHunters on #3385 (LGTM with non-blocking findings). 1. The backup-s3-ca volume carries no `optional: true` while the adjacent useSystemBucket credential refs deliberately do, and the asymmetry was undocumented — an operator naming a Secret that does not exist gets the whole ClickHouse Pod stuck in ContainerCreating, not just the sidecar. Kept the fail-closed behaviour and wrote down why the two cases differ: credentials are projected by the platform after install, so a strict ref would wedge every fresh release until the first BackupJob, whereas this CA is named by the tenant, must already exist, and is trust material — optional would mount an empty dir, the Pod would look healthy, and the first backup would fail deep inside clickhouse-backup with an x509 error naming neither the Secret nor the chart. The comment also states the blast radius plainly. 2. The CA probe swallowed stderr, so an RBAC denial on the cross-namespace read was reported as "CA not found" followed by advice (set S3_CA_SECRET) that cannot fix a permissions problem. It now separates present / absent / not-allowed-to-look and says so, and the README no longer labels step 03 simply "tenant": copying the CA reads a Secret in tenant-root, which is a platform capability. Added a section spelling out the three ways to satisfy that (admin credentials, a one-off admin copy, or skipping the CA entirely on a publicly-trusted endpoint). 3. Caveat, and a usability regression this branch introduced: on a cluster with a publicly-trusted endpoint and no seaweedfs, step 03 hard-exited on a default the user never chose. It now distinguishes an explicitly named Secret (still an error when missing — a typo should not be guessed past) from the unset default, where it warns and continues with backup.endpointCA unset. Verified all four paths against a fake kubectl: Forbidden, absent+default, absent+explicit, and present. 4. Caveat: endpointCA reaches the clickhouse-backup sidecar only. The legacy `schedule` CronJob writes to S3 through restic and does not consume it, so a private-CA endpoint combined with `schedule` still fails TLS. Documented on the field itself, so it lands in values.schema.json, the README and the cozyrds openAPISchema. The PR body's claim about select-e2e.sh/select-install.sh mappings was stale after the restructure that removed them; corrected there rather than in code. Assisted-By: Claude <noreply@anthropic.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Andrey Kolkov <androndo@gmail.com>