Repository navigation
docs: installation-wide collection scope and AWS key mapping - #14
Conversation
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) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55b80e5691
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| For an installation-wide cleanup, set `S3PATH` to the parent prefix that | ||
| contains every replica's objects, such as `clickhouse/<installation>/`, not a | ||
| single `<installation>/<replica>/` prefix. Collect it once into one inventory |
There was a problem hiding this comment.
Restrict parent-prefix advice to the selected disk
When an installation root contains objects for multiple S3 disks, this recommendation inventories all of them, but make_antijoin() only recognizes references whose rdp.disk_name equals the single configured S3DISKNAME. Objects belonging to another disk will therefore appear orphaned and can be irreversibly deleted after approval. Qualify this advice to require a parent prefix containing only the selected disk's objects, or document disk-isolated collection prefixes.
AGENTS.md reference: AGENTS.md:L16-L19
Useful? React with 👍 / 👎.
| | --- | --- | | ||
| | `AWS_ACCESS_KEY_ID` | `S3GC_S3ACCESSKEY` | | ||
| | `AWS_SECRET_ACCESS_KEY` | `S3GC_S3SECRETKEY` | | ||
| | `AWS_SESSION_TOKEN` | `S3GC_S3SESSIONTOKEN` (optional) | |
There was a problem hiding this comment.
Carry the mapped session token into the Secret
For temporary AWS credentials, following this new mapping still produces an unusable Secret: the subsequent export example, kubectl create secret command, and unset command omit S3GC_S3SESSIONTOKEN. Even if the private file defines the mapped token, the literal-only command silently drops it, so static authentication fails. Add the optional token to the copy-paste workflow.
Useful? React with 👍 / 👎.
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 <noreply@anthropic.com>
Summary
clickhouse/<installation>/) for an installation-wide cleanup, instead of one<installation>/<replica>/prefix. WithCLUSTERNAMEset, dry-run and delete compare against every replica'ssystem.remote_data_paths(s3gc.py:1258-1260).AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY/AWS_SESSION_TOKEN, and add a mapping table to theS3GC_*Secret keys.These are doc-only changes, so they are exempt from spec+TDD under CLAUDE.md rule 2.
Test plan
pytest -v -m "not dev_cluster"passes (docs only, no code change)deploy/kubernetes/README.mdstep 2 and the static Secret section🤖 Generated with Claude Code