From 55b80e5691553df717d1b0a4d61466f481713556 Mon Sep 17 00:00:00 2001 From: "Diego Nieto (lesandie)" Date: Thu, 24 Sep 2026 10:56:36 +0200 Subject: [PATCH 1/2] docs: document installation-wide collection scope and AWS key mapping Operators collected a single replica prefix, missing the other replicas' orphans, and expected the standard AWS credential variables to work in static mode. Document collecting the parent prefix with the clustered comparison, and map AWS_* names to the S3GC_* Secret keys. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 11 +++++++++++ README.md | 7 +++++++ deploy/kubernetes/README.md | 21 ++++++++++++++++++++- 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8a0de2..d9f42b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,17 @@ expose. ClickHouse user default database. It also calls out the `REMOTE` grant for clustered preflight and the S3 IAM permissions needed by collect and delete. +- Documented the collection scope for an installation-wide cleanup. An object + layout with a `//` level tempted an operator to + collect one replica's prefix, which misses the other replicas' orphans. The + guides now say to collect the parent prefix into one inventory table and + rely on the clustered comparison against every replica's + `system.remote_data_paths`. + +- Documented that static mode does not read the standard `AWS_ACCESS_KEY_ID` + and `AWS_SECRET_ACCESS_KEY` variables, with a mapping to the `S3GC_*` + Secret keys. Operators expected the AWS names to work. + ## [0.7.0] - 2026-08-24 A safety release, and its headline is uncomfortable: the dangerous diff --git a/README.md b/README.md index ba96c78..8f4bcd5 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,10 @@ Deleting an object is irreversible. `s3gc` preserves the following controls: - The Kubernetes renderer requires a digest-pinned image and keeps credentials outside the manifest. +For an installation-wide cleanup, collect the parent prefix that holds every +replica's objects and set the cluster name, so the comparison covers all +replicas. A single-replica prefix misses the other replicas' orphans. + Use a unique collection-table prefix for each bucket and prefix. Never commit credentials, rendered manifests, target-cluster details, or run output. @@ -82,6 +86,9 @@ Choose one S3 authentication mode with `S3GC_S3AUTH`: | `aws` | boto3 chain; optional `S3GC_S3PROFILE` | AWS SSO or a named workstation profile | | `iam` | MinIO workload-identity provider | EKS IRSA, EC2 instance profiles, or ECS task roles | +Static mode does not read `AWS_ACCESS_KEY_ID` or `AWS_SECRET_ACCESS_KEY`. Pass +those values as `S3GC_S3ACCESSKEY` and `S3GC_S3SECRETKEY`. + `S3GC_S3PROFILE` selects `aws`; the tool rejects contradictory settings. An `aws` preview still needs `s3:ListBucket` for the configured bucket and prefix. diff --git a/deploy/kubernetes/README.md b/deploy/kubernetes/README.md index 4c51f34..7b77833 100644 --- a/deploy/kubernetes/README.md +++ b/deploy/kubernetes/README.md @@ -53,6 +53,15 @@ Set the values in `s3gc.env`, then check these five items before rendering: 4. Choose a unique, database-qualified `COLLECTTABLEPREFIX`, such as `s3gc.s3gc__`. Keep it after a partial delete so the replacement Job can use the deletion checkpoints. A bare prefix uses the ClickHouse user's current database, which can differ from `default` and cause a grant failure. 5. Set `USEAGE_HOURS` to 24 or more. Raise it for slow merges or long mutations; production phases cannot lower it. +For an installation-wide cleanup, set `S3PATH` to the parent prefix that +contains every replica's objects, such as `clickhouse//`, not a +single `//` prefix. Collect it once into one inventory +table on the pinned host. With `CLUSTERNAME` set, dry-run and delete compare +the inventory with `system.remote_data_paths` on all replicas, so an object +that any replica still references is never a candidate. A single-replica +prefix misses the other replicas' orphans. Collect per-replica prefixes only +when you intend to, and give each one its own `COLLECTTABLEPREFIX`. + Use an immutable multi-architecture image digest in `IMAGE`. CI prints the exact value after publishing. Do not use an image tag. @@ -106,7 +115,17 @@ its own S3 client and the Secret or workload identity instead. For `S3AUTH=static`, create the Secret from a private, shell-style credential file. Keep this file outside the repository, restrict it to its owner, and -never print or commit it. Its values must be exported as follows: +never print or commit it. s3gc does not read `AWS_ACCESS_KEY_ID` or +`AWS_SECRET_ACCESS_KEY` in static mode. Store those values under the s3gc +names instead: + +| AWS variable | Secret key | +| --- | --- | +| `AWS_ACCESS_KEY_ID` | `S3GC_S3ACCESSKEY` | +| `AWS_SECRET_ACCESS_KEY` | `S3GC_S3SECRETKEY` | +| `AWS_SESSION_TOKEN` | `S3GC_S3SESSIONTOKEN` (optional) | + +The file must export these values: ```bash export S3GC_CHUSER=s3gc From 2ec231ed0d2656dfef78d24b099619d7ecfc227d Mon Sep 17 00:00:00 2001 From: "Diego Nieto (lesandie)" Date: Fri, 25 Sep 2026 11:45:44 +0200 Subject: [PATCH 2/2] docs: address review feedback on scope and session-token guidance Restrict the parent-prefix collection advice to objects on a single S3DISKNAME: the orphan check matches ClickHouse's system.remote_data_paths by disk_name, so another disk's objects under the same prefix would be misclassified as orphaned. Also carry the optional session token through the Secret export/create/unset commands, which previously dropped it. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 7 +++++-- README.md | 5 ++++- deploy/kubernetes/README.md | 28 +++++++++++++++++++++------- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9f42b5..e6eb4e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,11 +35,14 @@ expose. collect one replica's prefix, which misses the other replicas' orphans. The guides now say to collect the parent prefix into one inventory table and rely on the clustered comparison against every replica's - `system.remote_data_paths`. + `system.remote_data_paths` — but only when that parent prefix holds one + `S3DISKNAME`'s objects, since the orphan check matches by disk name and + would misclassify another disk's objects under the same prefix as orphaned. - Documented that static mode does not read the standard `AWS_ACCESS_KEY_ID` and `AWS_SECRET_ACCESS_KEY` variables, with a mapping to the `S3GC_*` - Secret keys. Operators expected the AWS names to work. + Secret keys, including the optional session token in the copy-paste Secret + creation commands. ## [0.7.0] - 2026-08-24 diff --git a/README.md b/README.md index 8f4bcd5..0da9743 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,10 @@ Deleting an object is irreversible. `s3gc` preserves the following controls: For an installation-wide cleanup, collect the parent prefix that holds every replica's objects and set the cluster name, so the comparison covers all -replicas. A single-replica prefix misses the other replicas' orphans. +replicas. A single-replica prefix misses the other replicas' orphans. Only +collect a parent prefix that holds one `S3DISKNAME`'s objects — the orphan +check matches by disk name, so another disk's objects under the same prefix +would look orphaned. Use a unique collection-table prefix for each bucket and prefix. Never commit credentials, rendered manifests, target-cluster details, or run output. diff --git a/deploy/kubernetes/README.md b/deploy/kubernetes/README.md index 7b77833..de636e8 100644 --- a/deploy/kubernetes/README.md +++ b/deploy/kubernetes/README.md @@ -55,8 +55,14 @@ Set the values in `s3gc.env`, then check these five items before rendering: For an installation-wide cleanup, set `S3PATH` to the parent prefix that contains every replica's objects, such as `clickhouse//`, not a -single `//` prefix. Collect it once into one inventory -table on the pinned host. With `CLUSTERNAME` set, dry-run and delete compare +single `//` prefix — but only when that parent prefix +holds objects for the one `S3DISKNAME` being cleaned up. The orphan check +matches ClickHouse's `system.remote_data_paths` by `disk_name`, so an object +that belongs to a different disk stored under the same parent prefix would +never match and would look orphaned. When disks share a parent prefix, collect +a disk-scoped prefix instead, or give each disk its own S3 path layout. +Collect the chosen prefix once into one inventory table on the pinned host. +With `CLUSTERNAME` set, dry-run and delete compare the inventory with `system.remote_data_paths` on all replicas, so an object that any replica still references is never a candidate. A single-replica prefix misses the other replicas' orphans. Collect per-replica prefixes only @@ -132,6 +138,7 @@ export S3GC_CHUSER=s3gc export S3GC_CHPASS='' export S3GC_S3ACCESSKEY='' export S3GC_S3SECRETKEY='' +# export S3GC_S3SESSIONTOKEN='' # only for temporary credentials ``` Create the Secret. This command creates it once and refuses to overwrite an @@ -142,13 +149,20 @@ set -a . /private/path/s3gc-secrets.env set +a -kubectl -n create secret generic \ - --from-literal=S3GC_CHUSER="$S3GC_CHUSER" \ - --from-literal=S3GC_CHPASS="$S3GC_CHPASS" \ - --from-literal=S3GC_S3ACCESSKEY="$S3GC_S3ACCESSKEY" \ +literals=( + --from-literal=S3GC_CHUSER="$S3GC_CHUSER" + --from-literal=S3GC_CHPASS="$S3GC_CHPASS" + --from-literal=S3GC_S3ACCESSKEY="$S3GC_S3ACCESSKEY" --from-literal=S3GC_S3SECRETKEY="$S3GC_S3SECRETKEY" +) +if [ -n "${S3GC_S3SESSIONTOKEN:-}" ]; then + literals+=(--from-literal=S3GC_S3SESSIONTOKEN="$S3GC_S3SESSIONTOKEN") +fi + +kubectl -n create secret generic \ + "${literals[@]}" -unset S3GC_CHUSER S3GC_CHPASS S3GC_S3ACCESSKEY S3GC_S3SECRETKEY +unset S3GC_CHUSER S3GC_CHPASS S3GC_S3ACCESSKEY S3GC_S3SECRETKEY S3GC_S3SESSIONTOKEN ``` The S3 principal needs `s3:ListBucket` on the scoped bucket/prefix for