diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..201997f --- /dev/null +++ b/.dockerignore @@ -0,0 +1,7 @@ +.git +.github +.venv +__pycache__ +tests +deploy +*.pyc diff --git a/.github/workflows/container.yml b/.github/workflows/container.yml new file mode 100644 index 0000000..7c70bac --- /dev/null +++ b/.github/workflows/container.yml @@ -0,0 +1,105 @@ +name: Container + +# Publishes a PUBLIC multi-architecture image to GHCR. +# +# Public on purpose: a private image forces every operator to copy a registry +# credential into the customer's namespace as an imagePullSecret and remember to +# delete it afterwards. A public image removes that step entirely. +# +# Multi-arch on purpose: ClickHouse node pools are frequently arm64 (one customer +# cluster is 5x arm64 + 1x amd64), and an amd64-only image can only ever schedule +# on a fraction of such a pool. + +on: + pull_request: + push: + branches: [master] + tags: ['v*.*.*'] + +env: + REGISTRY: ghcr.io + IMAGE_NAME: altinity/s3gc + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + - run: python -m pip install --disable-pip-version-check -r requirements.txt -r requirements-dev.txt + # Development-cluster tests require explicit credentials and are never + # part of the offline pull-request gate. + - run: pytest -v -m "not dev_cluster" + # The renderer is part of the contract: it refuses images that are not + # digest-pinned, so a broken template breaks every deployment. + - run: python deploy/kubernetes/render.py deploy/kubernetes/example.env > "$GITHUB_WORKSPACE/.s3gc-job.yaml" + # Client-side kubectl validation requires a Kubernetes API server for + # OpenAPI discovery. Kubeconform validates the rendered standard-resource + # manifest against pinned Kubernetes schemas without cluster access. + - name: Rendered manifest must conform to Kubernetes schemas + uses: docker://ghcr.io/yannh/kubeconform@sha256:85dbef6b4b312b99133decc9c6fc9495e9fc5f92293d4ff3b7e1b30f5611823c + with: + entrypoint: /kubeconform + args: -strict -summary /github/workspace/.s3gc-job.yaml + + publish: + needs: test + # Fork pull requests cannot write packages, and there is nothing to publish + # from them anyway. + if: github.event_name == 'push' + runs-on: ubuntu-latest + permissions: + contents: read + packages: write + steps: + - uses: actions/checkout@v4 + + - uses: docker/setup-qemu-action@v3 + + - uses: docker/setup-buildx-action@v3 + + - name: Log in to GHCR + uses: docker/login-action@v3 + with: + registry: ${{ env.REGISTRY }} + username: ${{ github.actor }} + # Automatic token: this repository lives in the Altinity organisation, + # so it already grants packages:write for ghcr.io/altinity/*. + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Docker metadata + id: meta + uses: docker/metadata-action@v5 + with: + images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }} + tags: | + type=semver,pattern={{version}} + type=sha,format=long + type=raw,value=latest,enable={{is_default_branch}} + + - name: Build and push + id: image + uses: docker/build-push-action@v6 + with: + context: . + file: docker/Dockerfile + platforms: linux/amd64,linux/arm64 + push: true + tags: ${{ steps.meta.outputs.tags }} + labels: ${{ steps.meta.outputs.labels }} + annotations: ${{ steps.meta.outputs.annotations }} + cache-from: type=gha + cache-to: type=gha,mode=max + + - name: Record immutable image reference + # render.py requires IMAGE to be digest-pinned, so this is the exact + # string an operator pastes into their .env file. + run: | + { + echo "### Image published" + echo '```' + echo "IMAGE=${REGISTRY}/${IMAGE_NAME}@${{ steps.image.outputs.digest }}" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" diff --git a/.gitignore b/.gitignore index 1d17dae..a180d07 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,5 @@ +__pycache__/ +*.pyc +options.lst +.pytest_cache/ .venv diff --git a/.python-version b/.python-version new file mode 100644 index 0000000..2c07333 --- /dev/null +++ b/.python-version @@ -0,0 +1 @@ +3.11 diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..ea94093 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,38 @@ +# Codex guide — s3gc + +Read `CLAUDE.md`, `CHANGELOG.md`, and `TODO.md` before making substantive +changes in this repository. `CLAUDE.md` is the full contributor guide and the +primary source of truth; the changelog provides recent historical context and +the TODO file records possible, interesting, and deliberately deferred +improvements. + +This file is intentionally short so agent tooling can find the critical rules +quickly, then defer to `CLAUDE.md` for complete repository guidance. + +## Critical rules + +1. **Read `CLAUDE.md` first, then `CHANGELOG.md` and `TODO.md`.** Treat them + as required repository context, not optional background reading. +2. **Protect destructive-operation safeguards.** `s3gc` deletes orphaned S3 + objects only after collection, dry-run review, explicit confirmation, and + the applicable ClickHouse cluster/replica preflight. Do not weaken these + controls without explicit approval and matching tests and documentation. +3. **Test coverage and required checks are mandatory.** For every feature, + bug fix, or material behaviour change, add a focused test when no existing + test covers it; use test-driven development for behaviour changes. Run the + relevant offline pytest suite and Kubernetes renderer/manifest dry-run + checks described in `CLAUDE.md`. Automated tests must not contact live + ClickHouse or object storage, or delete objects. +4. **No secrets or customer data in Git.** Never commit credentials, customer + configuration, target-cluster details, or rendered customer manifests. Use + Kubernetes Secrets or workload identity for production credentials. +5. **Keep deployments immutable and least-privileged.** Preserve digest-pinned + images, non-root/read-only container settings, and renderer validation. +6. **Keep dependencies deliberate.** Production dependencies belong in + `requirements.txt`; testing-only dependencies belong in + `requirements-dev.txt`. Do not add either casually. + +## Working rule + +When `AGENTS.md` and `CLAUDE.md` differ, update them to match, but follow the +more complete guidance in `CLAUDE.md` for the current task. diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..5d23e83 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,186 @@ +# Changelog + +Notable changes to `s3gc`, newest first. + +This file records **what changed and why**, with enough context that someone +picking the repository up later — human or agent — can tell a deliberate design +decision from an accident. Defects found by running the tool against real +clusters carry their evidence, because the reasoning is usually the expensive +part to reconstruct. + +Format loosely follows [Keep a Changelog](https://keepachangelog.com/). +Customer names, cluster identifiers and credentials never appear here (see +`CLAUDE.md`, rule 3); findings are described in terms of the behaviour they +expose. + +## [Unreleased] + +Changes below are on `feature/kubernetes-job-runner` and not yet released. + +### Fixed + +- **`--age` silently collected nothing for anything older than a day.** The + filter used `timedelta.seconds`, the sub-day remainder (0..86399), so computed + age never exceeded 23 h: a 30 d 5 h old object reported **5**. Harmless at the + default `age=0`, which is why it went unnoticed — but `--age 24`, the natural + choice by analogy with `--useage 24`, would produce an empty auxiliary table + and a dry-run reporting a clean bucket. Now uses `total_seconds()`. + +- **`--usecollected` against a missing or empty auxiliary table exited 0**, + which is indistinguishable from success. That is exactly what a load-balanced + ClickHouse Service produces, because the auxiliary table is a *node-local* + `ReplacingMergeTree`: collect writes it on one replica and a later phase looks + for it on another. It now fails loudly and explains the replica-pinning + requirement. + +- **GCS endpoints now fall back to per-object deletion automatically.** Google + Cloud Storage's S3-compatible API has no batch `DeleteObjects`, so + `remove_objects()` fails there. Detected from the endpoint, with a warning + that the per-object path is markedly slower. + +### Added + +- **S3 authentication modes — `--s3auth=static|aws|iam`**, merging + [PR #2](https://github.com/Altinity/s3gc/pull/2) by **@realyota**, which added + `--s3auth=aws`, `--s3profile` and `--s3-session-token` so credentials can come from + the boto3 chain (AWS SSO profiles) or as explicit temporary credentials. + + That PR replaced the workload-identity path outright. Both are kept instead, + because they are not interchangeable: `iam` uses MinIO's own provider, needs no + `boto3`, and is what the validated Kubernetes deployments use — it hands MinIO the + *provider* rather than frozen keys so credentials refresh across a long collect or + delete. `aws` resolves through boto3 and suits a workstation with SSO. + + | Mode | Credentials | boto3 | + |---|---|---| + | `static` (default) | explicit keys, optional session token | no | + | `aws` | boto3 chain / `--s3profile` | **yes** | + | `iam` | MinIO workload identity (IRSA/IMDS/ECS) | no | + + `--s3profile` implies `aws`. Contradictory combinations are rejected rather + than silently resolved, so nobody ends up authenticating with an identity they + did not ask for. + +- **Operator-facing S3 listing errors**, also from PR #2: a failed listing now names + the required permission (`s3:ListBucket` on the bucket ARN, **even for + `--dry-run`**) and prints the `aws sts get-caller-identity` / + `aws s3api list-objects-v2` commands to verify the same credentials. `UserVisibleError` + reports such failures without a traceback. + +- **Wider log-secret filtering** (PR #2): `s3accesskey` and `s3sessiontoken` are now + redacted alongside `chpass` and `s3secretkey`. + +- **Kubernetes wiring for the new auth**, which PR #2 did not include: `S3AUTH` and + `S3PROFILE` are exposed by `job.yaml.tmpl`, required and validated by `render.py` + (`S3PROFILE` without `S3AUTH=aws` is an error), and default to `iam` in + `example.env`. Without this the flags were unreachable from a Job. The session token + stays out of the non-secret env file — it belongs in the credentials Secret. + +- **Warning when `--samples` disagrees with the auxiliary table's + `PARTITION BY`.** The table is created as `PARTITION BY CRC32(objpath) % + ` at collect time, so a different value during the use phase loses + partition pruning. Measured on production-scale data: ~2 min per sample when + matched against ~26 min when not. + +- **Cumulative deletion total alongside the per-attempt one.** The closing + `N objects … are removed` line counts only the process that printed it, which + understated one resumed run by 16.61 TiB. It now says "in this attempt" and + logs the auxiliary table's cumulative tombstone count. + +- **30 regression tests** covering the boolean matrix (per flag, per spelling, + plus bare-CLI compatibility), the age filter, the fail-loud path, the samples + warning, the GCS fallback, and renderer pull-secret handling. + +### Changed + +- **Images are published publicly to `ghcr.io/altinity/s3gc`** instead of a + private Docker Hub repository, matching `altinity-mcp` and + `altinity-sql-browser`. CI authenticates with the automatic `GITHUB_TOKEN`, + so there is no registry secret to manage or rotate. + + *Why it matters operationally:* a private image forces whoever runs a Job to + copy a registry credential into the target namespace as an `imagePullSecret` + and remember to delete it afterwards. A public image removes that step + entirely. + +- **`boto3` is a pinned runtime dependency** (`boto3==1.43.65`), required by + `--s3auth=aws`. It is imported lazily, so the other modes never load it. PR #2 added + it unpinned; the pinned set is kept per `CLAUDE.md` rule 5 — unpinned `jsonargparse` + resolves to 4.50.x and fails at import. + +- **`IMAGE_PULL_SECRET` is now optional.** `render.py` omits the + `imagePullSecrets` block when the value is empty, rather than emitting a + meaningless `- name: ""`. Set it only for a private mirror. + +- **Multi-architecture builds are mandatory, not advisory.** ClickHouse node + pools are frequently arm64 — one observed pool was 5× arm64 and 1× amd64, + where an amd64-only image can only ever schedule on a sixth of the capacity. + CI builds `linux/amd64,linux/arm64` in a single step. + +- **CI validates the rendered manifest without a Kubernetes cluster.** Strict, + digest-pinned Kubeconform schema validation replaces client-side `kubectl`, + which attempts OpenAPI discovery against a nonexistent API server on GitHub + runners. + +- **CI explicitly excludes `dev_cluster` tests.** The pull-request suite stays + offline even after opt-in environment-dependent coverage is added. + +- Removed the deprecated S3 IAM selector. `S3AUTH=static|aws|iam` is now the + only supported authentication interface. + +### Documentation + +- `CHHOST` must be a **per-replica** Service, never the load-balanced one, and + every phase of a cleanup must use the same host. +- S3 authentication guidance now keeps static, AWS SSO/profile, and workload + identity modes together; the Kubernetes guide calls out dedicated ClickHouse + user provisioning and only the required table and system-table grants. +- Contributor guidance now requires test-driven, focused coverage for every + feature, defect, and material operational behaviour change. +- Sharding recipe for `--collectonly`, which has no resume: a crash re-lists + from the beginning, which is expensive on multi-million-object buckets. + Re-running a shard is safe because the auxiliary table is a + `ReplacingMergeTree` keyed on `objpath`. +- Per-cluster values that cause silent failure when wrong: `S3PATH` may + legitimately be empty (bucket-root layouts); the disk is not always named + `s3` (GCS-backed clusters commonly use `gcs`, which changes both the + anti-join scope and the auxiliary table name); a `*_cache` disk is a + filesystem cache over the same blobs, not a second reference scope. +- Buildx builder containers cache `/etc/resolv.conf` at creation, so a builder + left running across a network change fails with + `lookup registry-1.docker.io: i/o timeout` while the host resolves fine. + +### Migration notes + +- **Pull secrets are registry-scoped.** Moving from Docker Hub to GHCR silently + invalidates an existing `imagePullSecret` even though its *name* still looks + right: a secret holding `index.docker.io` credentials does not apply to + `ghcr.io`, so the kubelet falls back to an anonymous token and the pod sits in + `ImagePullBackOff` with `failed to fetch anonymous token: 401 Unauthorized`. + With the public image the correct action is to **remove** the secret reference + (leave `IMAGE_PULL_SECRET` empty), not to repoint it. + +- **The GHCR package must be made public once**, in the organisation's package + settings. It cannot be done through the REST API — the visibility endpoint + returns 404 and the standard token lacks `write:packages`. Until it is flipped, + every pull still needs credentials and the benefit above is not realised. + +### Verified + +All three phases were exercised end to end against a development ClickHouse cluster using the +image built from this branch, pulled anonymously from the public registry with no +`imagePullSecret`: `collect` (188 objects), `dry-run` (exactly the 16 seeded orphan +fixtures), `delete` (cluster preflight, per-batch checkpoints, cumulative total) and a +verifying `dry-run` reporting zero. `auth=iam` in the log confirms workload identity still +resolves after the credential-resolution rewrite, and the referenced tables were untouched. +Unit tests cover the `static`/`aws` modes; the GCS per-object fallback is still only +unit-tested, pending a real GCS endpoint. + +## v0.2 — 2025-01-31 + +- Added an option to avoid batch deletion for services such as GCS. + +## v0.1 — 2024-06-12 + +- Added object last-modified timestamps to the auxiliary table. +- Added the object age option. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..6f7e953 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,109 @@ +# Contributor guide — s3gc + +`s3gc` is a Python command-line tool and Kubernetes Job workflow for finding +and deleting orphaned objects from ClickHouse S3 disks and compatible object +storage. It is an operationally destructive tool: quality is held by offline +tests, explicit delete controls, and conservative deployment defaults. + +## Hard rules + +1. **The deletion lifecycle is non-negotiable.** The normal production flow is + `collect → dry-run → explicit customer approval → delete → verify`. Preserve + the delete confirmation token, ClickHouse cluster and expected-replica + preflight, deletion checkpoints, and the rule that a failed delete job does + not retry automatically. A behavior change in this path needs regression + tests and matching README/deployment documentation. + +2. **Test coverage and TDD are mandatory.** For every new feature, bug fix, or + material operational behaviour change, identify the test that covers it; if + none exists, add a focused test in the same change. Start behaviour changes + with a failing test, and add a regression test for every defect. Purely + editorial changes are exempt. + +3. **Tests stay offline by default.** Install both requirements files, then + run `pytest -v` for relevant changes. Tests must use fakes, local + subprocesses, and manifest dry-runs; they must not contact live ClickHouse, + S3-compatible storage, or delete objects. The `dev_cluster` marker is + explicitly environment-dependent and never runs in CI. Do not add live + credentials or a live-delete test path to the default suite. + +4. **No secrets or customer artifacts in Git.** Do not commit S3 keys, + ClickHouse passwords, customer `.env` files, rendered customer manifests, + target-cluster details, or command output containing them. Keep credentials + in an approved secret manager, Kubernetes Secret, or workload identity. + `deploy/kubernetes/example.env` is a non-secret template only. + +5. **Kubernetes deployment stays immutable and least-privileged.** + `deploy/kubernetes/render.py` must continue to reject unpinned images and + invalid phase/confirmation input. Use image digests, never mutable tags. + Preserve the Job template's non-root user and read-only root filesystem; + do not embed credentials in the image or manifest. + +6. **Dependencies are deliberate and reproducible.** Runtime dependencies are + pinned in `requirements.txt`; test-only dependencies are pinned in + `requirements-dev.txt`. Use both files when preparing a development or CI + environment. Add or update a dependency only when it is necessary for the + requested capability, and test the resulting workflow. + +7. **Read project history and backlog before substantive changes.** Review + `CHANGELOG.md` for recent behaviour and operational evidence, then `TODO.md` + for possible, interesting, and deliberately deferred improvements. Do not + treat a TODO item as already implemented or use the changelog as a backlog. + +## Repository map + +| Path | Purpose | +| --- | --- | +| `s3gc.py` | CLI arguments, ClickHouse inventory/anti-join, S3 collection and deletion, safety preflight, and checkpoints. | +| `tests/` | Pytest regression tests using fakes and local subprocesses. | +| `docker/Dockerfile` | Minimal Python 3.11 production image. | +| `docker/kubernetes-entrypoint.sh` | Phase dispatcher and delete/dev-automation confirmation gate. | +| `deploy/kubernetes/render.py` | Validates a non-secret environment file and renders the Job manifest. | +| `deploy/kubernetes/job.yaml.tmpl` | Kubernetes Job template with security context and environment wiring. | +| `deploy/kubernetes/example.env` | Non-secret rendering example; copy it outside the repository for a real run. | +| `.github/workflows/container.yml` | CI test, render, manifest validation, and container publication workflow. | +| `CHANGELOG.md` | Shipped behaviour and **why**, including evidence for defects found in production use. Update it in the same change as any behaviour, safety, or deployment change. | +| `TODO.md` | Possible, interesting, and deliberately deferred engineering or operational improvements. Move completed work to the changelog when it lands. | + +## Required checks + +For a change to Python, shell, renderer, manifest, dependencies, or deployment +workflow, run the relevant checks after installing the pinned development +requirements: + +```bash +python3.11 -m venv .venv +.venv/bin/python -m pip install -r requirements.txt -r requirements-dev.txt +.venv/bin/python -m pytest -v -m "not dev_cluster" +.venv/bin/python deploy/kubernetes/render.py deploy/kubernetes/example.env > /tmp/s3gc-job.yaml +docker run --rm --entrypoint /kubeconform -v /tmp:/tmp:ro \ + ghcr.io/yannh/kubeconform@sha256:85dbef6b4b312b99133decc9c6fc9495e9fc5f92293d4ff3b7e1b30f5611823c \ + -strict -summary /tmp/s3gc-job.yaml +``` + +Add a regression test in the same change as each behavior or safety fix. Cover +failure paths as well as the intended path, especially delete confirmation, +cluster/replica preflight, S3 delete errors, checkpointing, boolean environment +parsing, and renderer input validation. Do not claim or impose a coverage +percentage until coverage tooling and an enforceable threshold are introduced. + +## Working discipline + +- Keep the command-line and `S3GC_*` environment interfaces compatible unless + the task explicitly authorizes a breaking operational change. +- Record behaviour, safety and deployment changes in `CHANGELOG.md` as part of + the same change. Write down *why*, and keep the evidence for defects found in + production — the reasoning is the expensive part to reconstruct later. Never + put customer names, cluster identifiers or credentials there. +- Record possible, interesting, or pending engineering and operational + improvements in `TODO.md`, not in the changelog. Remove or update the TODO + item when the work lands. +- Treat the renderer, entrypoint, README, and Kubernetes guide as part of the + same operator-facing contract. Update the affected documentation in the same + change as an operational behavior change. +- Surface out-of-scope safety defects rather than silently changing them. State + the file and risk, and defer the fix unless it is necessary to keep the + current task safe. +- When using subagents for discovery or review, make them read-only unless the + task explicitly authorizes writes. Inspect the working tree after any agent + batch before continuing. diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index a52e231..0000000 --- a/Dockerfile +++ /dev/null @@ -1,48 +0,0 @@ -FROM python:3 - -WORKDIR /usr/src/app - -COPY requirements.txt ./ -RUN pip install --no-cache-dir -r requirements.txt - -COPY . . - -ENV S3GC_CHHOST=localhost \ - S3GC_CHPORT=8123 \ - S3GC_CHUSER=default \ - S3GC_CHPASS='' \ - S3GC_S3IP=127.0.0.1 \ - S3GC_S3PORT=9001 \ - S3GC_S3BUCKET=root \ - S3GC_S3PATH=data/ \ - S3GC_S3ACCESSKEY='' \ - S3GC_S3SECRETKEY='' \ - S3GC_S3SECURE_FLAG=false \ - S3GC_S3SSLCERTFILE='' \ - S3GC_S3REGION=null \ - S3GC_S3DISKNAME=s3 \ - S3GC_KEEPDATA_FLAG=false \ - S3GC_COLLECTONLY_FLAG=false \ - S3GC_USECOLLECTED_FLAG=false \ - S3GC_COLLECTTABLEPREFIX=s3objects_for_ \ - S3GC_COLLECTBATCHSIZE=1024 \ - S3GC_TOTAL=null \ - S3GC_COLLECTAFTER=null \ - S3GC_USEAFTER=null \ - S3GC_USETOTAL=null \ - S3GC_DRYRUN_FLAG=false \ - S3GC_CLUSTERNAME='' \ - S3GC_AGE=0 \ - S3GC_USEAGE=0 \ - S3GC_SAMPLES=4 \ - S3GC_CHTIMEOUT=1800 \ - S3GC_CREATEDATABASE_FLAG=false \ - S3GC_DROP_COLLECTTABLE_FLAG=false \ - S3GC_INTERACTIVE_FLAG=true \ - S3GC_VERBOSE_FLAG=false \ - S3GC_DEBUG_FLAG=false \ - S3GC_SILENT_FLAG=false - - -ENTRYPOINT ["python", "./s3gc.py"] -# CMD ["--help" ] diff --git a/Dockerfile.in b/Dockerfile.in deleted file mode 100644 index a30fde4..0000000 --- a/Dockerfile.in +++ /dev/null @@ -1,13 +0,0 @@ -FROM python:3 - -WORKDIR /usr/src/app - -COPY requirements.txt ./ -RUN pip install --no-cache-dir -r requirements.txt - -COPY . . - -# @@ - -ENTRYPOINT ["python", "./s3gc.py"] -# CMD ["--help" ] diff --git a/Makefile b/Makefile deleted file mode 100644 index 6e320df..0000000 --- a/Makefile +++ /dev/null @@ -1,5 +0,0 @@ -Dockerfile: Dockerfile.in options.lst - python3 -c "import sys; sys.stdout.write(sys.stdin.read().replace('# @@', open('./options.lst', 'r').read()))" < Dockerfile.in > Dockerfile - -options.lst: ./s3gc.py - python3 ./s3gc.py --listoptions > options.lst diff --git a/README.md b/README.md index 4ed0c77..a2f9321 100644 --- a/README.md +++ b/README.md @@ -1,100 +1,308 @@ # s3gc -Garbage collector for ClickHouse S3 disks -## description -The script removes orphaned objects from s3 object storage - Ones that are not mentioned in system.remote_data_paths table +`s3gc` finds and removes orphaned objects from ClickHouse S3 disks and other +S3-compatible storage. An object is a candidate only when it exists under the +configured bucket/prefix but is absent from ClickHouse +`system.remote_data_paths` for the configured disk. -There are two stages: -1. Collecting. - Paths of all objects found in object storage are put in auxiliary ClickHouse table. - It's name is a concatenation of 's3objects_for_' and disk name by default. - Created in the same ClickHouse instance where data from system.remote_data_paths selected -2. Removing. - All objects that exist in s3 and not used according to system.remote_data_paths - are removed from object storage. +## How it works -It is possible to split these stages or do everything at one go. +1. Collect object names, sizes, and timestamps into an auxiliary ClickHouse + table. +2. Anti-join that inventory with `system.remote_data_paths` (or all replicas of + a configured cluster). +3. Report candidates in dry-run mode, or delete them in batches and record + confirmed deletion checkpoints in the auxiliary table. -Besides this, it is possible to calculate objects to remove without actual removing AKA dry run. -If dryrun is set together with usecollected, it uses collected data. -If dryrun is set together with collectonly, error is raised. +The command-line script supports these actions directly. For Kubernetes, the +repository supplies a one-shot Job runner that separates collection, review, +and deletion. -It is important to use `--s3diskname` if your disk name is not `s3` which is by default. +## Safety -WARNING!: Please use `--dry-run` to check and compare results of what is going to be deleted, just to be on the safe side. +Deleting an object is irreversible. Always run and review a dry-run before +deletion, and scope the configured bucket and prefix as narrowly as possible. -## script invocation -### help +- Use a unique collection-table prefix for each cleanup. +- For clustered ClickHouse, use the cluster name and expected replica count. +- A failed delete Job does not automatically retry. Successfully deleted + batches remain checkpointed, so a replacement delete Job can resume safely. +- Never put credentials, customer manifests, or target-cluster details in Git. + +## Requirements + +- Python 3.11 for local development; the container image also uses Python 3.11. +- Network access to ClickHouse and the target S3-compatible endpoint. +- A ClickHouse user that can read `system.remote_data_paths` and manage the + auxiliary table. +- S3 permissions appropriate to the action: list for collection, plus delete + for deletion. + +## Quick start + +Create a local environment and inspect the available options: + +```bash +python3.11 -m venv .venv +.venv/bin/python -m pip install -r requirements.txt -r requirements-dev.txt +.venv/bin/python s3gc.py --help ``` -python3 s3gc.py --help + +Configuration can be supplied as command-line arguments or `S3GC_*` +environment variables. Set the ClickHouse connection, S3 endpoint/bucket/prefix, +region, disk name, and either static S3 keys or workload identity. Keep secrets +in your approved secret manager or environment, not in command history. + +Run a dry-run first: + +```bash +.venv/bin/python s3gc.py --verbose --dry-run ``` -### typical usage -#### all together with dry-run -for https://altinity-clickhouse-data-demo20565656565620663600000001.s3.amazonaws.com/github + +For a production or customer cleanup, use the Kubernetes procedure below rather +than a one-line delete command. + +## Direct script examples + +The following is a non-secret target configuration. Replace every +`` value and do not commit this environment to Git: + +```bash +export S3GC_CHHOST='' +export S3GC_CHPORT=8123 +export S3GC_CHUSER='' +export S3GC_S3IP='s3.eu-central-1.amazonaws.com' +export S3GC_S3PORT=443 +export S3GC_S3BUCKET='' +export S3GC_S3PATH='/' +export S3GC_S3REGION='eu-central-1' +export S3GC_S3SECURE_FLAG=true +export S3GC_S3DISKNAME=s3 +export S3GC_CLUSTERNAME='' +export S3GC_EXPECTED_REPLICAS=2 +export S3GC_COLLECTTABLEPREFIX='s3gc_example_' +export S3GC_AGE=24 +export S3GC_USEAGE=24 ``` -S3GC_S3ACCESSKEY=sdfasfaerasasf \ -S3GC_S3SECRETKEY=werqwsdfqwersdfasf \ -S3GC_S3IP=s3.amazonaws.com \ -S3GC_S3PORT=443 \ -S3GC_S3REGION=us-east-1 \ -S3GC_S3BUCKET=altinity-clickhouse-data-demo20565656565620663600000001 \ -S3GC_S3PATH=github/ \ -S3GC_S3SECURE_FLAG=true \ -python3 ./s3gc.py --verbose --dry-run + +### S3 authentication modes + +Select one with `S3GC_S3AUTH` (or `--s3auth`): + +| Mode | Credentials | Needs boto3 | Typical use | +|---|---|---|---| +| `static` (default) | `S3GC_S3ACCESSKEY` + `S3GC_S3SECRETKEY`, optionally `S3GC_S3SESSIONTOKEN` | no | long-lived keys, or explicit temporary credentials | +| `aws` | boto3 credential chain, optionally `S3GC_S3PROFILE` | **yes** | AWS SSO / named profiles on a workstation | +| `iam` | MinIO workload identity provider | no | EKS IRSA, EC2 instance profile, ECS task role | + +`S3GC_S3PROFILE` implies `aws`. Contradictory combinations are rejected rather +than silently resolved. + +#### Static credentials + +Inject static credentials from a secret manager or interactive shell rather +than saving them in a file: + +```bash +export S3GC_CHPASS='' +export S3GC_S3ACCESSKEY='' +export S3GC_S3SECRETKEY='' ``` -#### GCS and object storage that do not support batch delete operations + +Every `S3GC_*` boolean accepts `true/false`, `yes/no`, `on/off`, `1/0`, or an +empty value for false. Unset also means false. + +#### AWS SSO or a named profile + +Authenticate with the AWS CLI first, then let `s3gc` resolve temporary +credentials through the boto3 chain: + +```bash +aws sso login --profile my-sso-profile + +export S3GC_S3AUTH=aws +export S3GC_S3PROFILE=my-sso-profile +export S3GC_S3IP=s3.amazonaws.com +export S3GC_S3PORT=443 +export S3GC_S3REGION=us-east-1 +export S3GC_S3SECURE_FLAG=true +.venv/bin/python ./s3gc.py --verbose --dry-run ``` -S3GC_S3ACCESSKEY=GOOG1xxxxxxxxx \ -S3GC_S3SECRETKEY=xxxxxxxxxxx \ -S3GC_S3IP=storage.googleapis.com \ -S3GC_S3PORT=443 \ -S3GC_S3BUCKET=clickhouse-altinity-main-disk \ -S3GC_S3PATH=chi-main-main-0-0/ \ -S3GC_S3SECURE_FLAG=true \ -S3GC_S3DISKNAME=gcs \ -python3 ./s3gc.py --verbose --use-remove-objects=false + +`S3GC_S3ACCESSKEY` and `S3GC_S3SECRETKEY` are unused in `aws` mode, and setting +them is an error rather than a silent override. The resolved credentials must +allow `s3:ListBucket` on the bucket for that prefix **even for `--dry-run`** — +collection lists objects. On failure `s3gc` prints the required permission and +the commands to verify it: + +```bash +aws sts get-caller-identity --profile my-sso-profile +aws s3api list-objects-v2 --bucket --prefix --max-keys 1 --profile my-sso-profile ``` -GCS_HMAC_KEY = S3GC_S3ACCESSKEY -GCS_HMAC_SECRET = S3GC_S3SECRETKEY +#### Workload identity (EKS/IRSA, EC2, ECS) + +```bash +export S3GC_CHPASS='' +export S3GC_S3AUTH=iam +``` + +`iam` uses MinIO's AWS IAM credential provider and refreshes temporary +credentials from EKS IRSA/workload identity, an EC2 instance profile, or an ECS +task role. Prefer it over `aws` inside Kubernetes: it keeps `boto3` out of the +request path and avoids credentials expiring during a long collect or delete. + +It does not read AWS CLI profiles, `aws sso login` state, `~/.aws/config`, or +`AWS_PROFILE`; use `aws` mode for that workstation workflow. + +#### GCS and other stores without batch delete +GCS has no batch `DeleteObjects`. `s3gc` detects a `storage.googleapis.com` +endpoint and falls back to per-object deletion automatically, warning that it is +slower; `--use-remove-objects=false` sets it explicitly. Note the disk name is +usually `gcs`, not `s3`, and GCS needs **HMAC/interop** keys: -#### collect only +```bash +export S3GC_S3ACCESSKEY='GOOG1...' +export S3GC_S3SECRETKEY='...' +export S3GC_S3IP=storage.googleapis.com +export S3GC_S3PORT=443 +export S3GC_S3SECURE_FLAG=true +export S3GC_S3DISKNAME=gcs +.venv/bin/python ./s3gc.py --verbose --use-remove-objects=false ``` -S3GC_S3PORT=19000 S3GC_S3ACCESSKEY=minio99 S3GC_S3SECRETKEY=minio123 python3 ./s3gc.py --verbose --collectonly + +### Safe split workflow + +Collection makes an auxiliary table; the second command reads it and reports +candidates without deleting objects: + +```bash +.venv/bin/python s3gc.py --collectonly --keepdata +.venv/bin/python s3gc.py --usecollected --dry-run +``` + +### Collect has no resume — shard large buckets + +A crashed or interrupted `--collectonly` restarts its listing from the +beginning; there is no checkpoint. On a multi-million-object bucket that can +cost hours, and long runs are exactly where a rotating password or a dropped +connection tends to strike. + +Shard the listing by prefix and re-run only the shards that failed. This is safe +to repeat: the auxiliary table is a `ReplacingMergeTree` keyed on `objpath`, so +re-listing a shard is idempotent. + +```bash +# buckets laid out as /<3-char hash>/ +for shard in 0 1 2 3 4 5 6 7 8 9 a b c d e f g h i j k l m n o p q r s t u v w x y z; do + S3GC_S3PATH="/${shard}" ./s3gc.py --collectonly --keepdata || \ + echo "shard ${shard} FAILED — re-run just this one" +done ``` -#### use collected + +The same variables can be passed as flags (for example, +`--ch-host` or `--s3-bucket`). Run `.venv/bin/python s3gc.py --help` for the +complete flag and environment-variable reference. Avoid direct deletion for +customer or production work; use the reviewed Kubernetes workflow instead. + +## Container image + +Released images are **public** at `ghcr.io/altinity/s3gc`, so Kubernetes needs no +`imagePullSecret`. Always reference them **by digest**, never by tag — tags get +re-pushed and stop reproducing what you tested: + +```bash +docker pull ghcr.io/altinity/s3gc@sha256: +``` + +CI prints the exact `IMAGE=` line in its job summary; paste that into your +`.env`. `render.py` refuses anything not digest-pinned. + +Build locally for a quick check: + +```bash +docker build -f docker/Dockerfile -t s3gc:local . ``` -S3GC_S3PORT=19000 S3GC_S3ACCESSKEY=minio99 S3GC_S3SECRETKEY=minio123 S3GC_USECOLLECTED=true python3 ./s3gc.py --debug + +To publish by hand, **both architectures are mandatory** — ClickHouse node pools +are frequently arm64, and an amd64-only image will not schedule there: + +```bash +docker buildx build --platform linux/amd64,linux/arm64 \ + -f docker/Dockerfile -t ghcr.io/altinity/s3gc: --push . +docker buildx imagetools inspect ghcr.io/altinity/s3gc: # expect amd64 AND arm64 ``` -## docker -There is a docker image for the script. +> Buildx builder containers cache `/etc/resolv.conf` at creation time. A builder +> left running across a network or VPN change fails with +> `lookup registry-1.docker.io: i/o timeout` while the host resolves fine. +> Recreate the builder, or create one with `--driver-opt network=host`. + +The CI workflow runs tests on every pull request and publishes from pushes to +`master` and version tags, authenticating to GHCR with the automatic +`GITHUB_TOKEN`. -### rebuild +## Kubernetes + +The Kubernetes runner lives in [`deploy/kubernetes/`](deploy/kubernetes/). It +uses a digest-pinned image and external Kubernetes Secrets; it does not create +or store credentials in the repository. + +For customer and production work, follow: + +```text +collect → dry-run → approved delete → verify ``` -make -sudo docker buildx build --platform linux/arm/v7,linux/arm64/v8,linux/amd64 -t ilejn/s3gc . + +The concise operator procedure, Secret requirements, and renderer configuration +are in [deploy/kubernetes/README.md](deploy/kubernetes/README.md). A guarded +`dev-automation` phase is available only for non-production testing; it runs +collect, dry-run, and delete in one Job and still requires an explicit delete +confirmation. + +## Testing + +Run all isolated unit tests: + +```bash +.venv/bin/python -m pytest -v ``` -### usage +Run only the development-automation tests: + +```bash +.venv/bin/python -m pytest -v -k dev_automation ``` -sudo docker run ilejn/s3gc --help -sudo docker run --network="host" -e S3GC_S3PORT=19000 -e S3GC_S3ACCESSKEY=minio99 -e S3GC_S3SECRETKEY=minio123 ilejn/s3gc + +Validate that the example Kubernetes configuration renders without creating a +cluster resource: + +```bash +python3 deploy/kubernetes/render.py deploy/kubernetes/example.env > /tmp/s3gc-job.yaml +docker run --rm --entrypoint /kubeconform -v /tmp:/tmp:ro \ + ghcr.io/yannh/kubeconform@sha256:85dbef6b4b312b99133decc9c6fc9495e9fc5f92293d4ff3b7e1b30f5611823c \ + -strict -summary /tmp/s3gc-job.yaml ``` -## changelog +The unit suite does not contact ClickHouse, S3, or Kubernetes. The reserved +`dev_cluster` pytest marker is excluded from CI; any future tests using it must +be selected explicitly with `.venv/bin/python -m pytest -m dev_cluster` after +reviewing their fixture scope. The collect/dry-run/delete exercise is manual +because it can intentionally delete development objects. + +## Repository layout -### v_0.1 Wed Jun 12 2024 +- `s3gc.py` — collection, anti-join, and deletion logic. +- `docker/` — Python 3.11 container image and Kubernetes entrypoint. +- `deploy/kubernetes/` — plain Job template, renderer, example configuration, + and operator guide. +- `tests/` — pytest safety, renderer, and entrypoint tests. -- object last modified in auxiliary table -- useage command line parameter - -### v_0.2 Fri Jan 31 2025 -- added option to avoid batch deletion for services like GCS +## History and roadmap -## to do list -~~1. option to avoid `remove_objects` which is reportedly not supported by GCE~~ +See [`CHANGELOG.md`](CHANGELOG.md) for the full history, including why each +change was made and the evidence behind defects found in production use. -- concurrency / async +Planned: concurrency and asynchronous collection/deletion; a `--collectafter` +checkpoint so an interrupted collect can resume instead of re-listing. diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..dc68c6d --- /dev/null +++ b/TODO.md @@ -0,0 +1,18 @@ +# TODO + +Possible, interesting, and deliberately deferred engineering or operational +improvements. Completed behaviour belongs in `CHANGELOG.md`; this file is the +forward-looking backlog. + +- [ ] Add resumable `--collectonly` collection checkpoints. A crash currently + restarts listing from the beginning; `--collectafter` would allow a large + bucket collection to resume safely. +- [ ] Add an opt-in `dev_cluster` GCS end-to-end test. It must require explicit + environment configuration, create an isolated tiered-policy fixture, run + `collect → dry-run → delete → verify`, clean up ClickHouse and object-storage + fixtures, and remain excluded from CI. +- [ ] Make the `ghcr.io/altinity/s3gc` package public in the organisation's + package settings after its first publish, so Kubernetes pulls need no + registry credentials. +- [ ] Require the `Container / test` GitHub Actions check before pull-request + merges in the repository branch-protection settings. diff --git a/deploy/kubernetes/README.md b/deploy/kubernetes/README.md new file mode 100644 index 0000000..56f0fec --- /dev/null +++ b/deploy/kubernetes/README.md @@ -0,0 +1,140 @@ +# Kubernetes Job runner + +Run `s3gc` as a one-shot Job in the ClickHouse namespace. For customer and +production work, always run: + +```text +collect → dry-run → approved delete → verify +``` + +The Job reaches ClickHouse through an in-cluster Service; no laptop tunnel is +needed. + +> **`CHHOST` must be a PER-REPLICA Service, never the load-balanced one.** +> The auxiliary table is a node-local `ReplacingMergeTree`, not a Replicated +> table. A load-balanced Service round-robins, so `collect` can write the table +> on one replica while `dry-run`/`delete` land on another and find nothing. +> Use `chi---0-0` (replica 0), not `clickhouse-`, and use +> the **same** host for every phase of a cleanup. + +## Before the first Job + +- Use an immutable, multi-architecture image digest: + `ghcr.io/altinity/s3gc@sha256:`. CI prints the exact `IMAGE=` line in + its job summary. Pin by digest, never by tag — tags get re-pushed. +- The image must be multi-arch. ClickHouse node pools are often arm64 (one + customer cluster is 5x arm64 + 1x amd64), and an amd64-only image simply will + not schedule there. +- Create or reuse a namespace-local ServiceAccount. +- Create a runtime Secret named by `CREDENTIALS_SECRET`: + - `S3AUTH=static`: `S3GC_CHUSER`, `S3GC_CHPASS`, `S3GC_S3ACCESSKEY`, + `S3GC_S3SECRETKEY`, plus `S3GC_S3SESSIONTOKEN` for temporary credentials. + - `S3AUTH=iam`: only `S3GC_CHUSER` and `S3GC_CHPASS`, with an + identity-enabled ServiceAccount. Note the template sets + `automountServiceAccountToken: false`, so `iam` also needs a ServiceAccount + that actually projects a token. +- Confirm the ClickHouse Service name, cluster macro, expected replica count, + S3 bucket/prefix, and disk name. Use a unique `COLLECTTABLEPREFIX` per + bucket/prefix cleanup. + +### Choosing `S3AUTH` + +| `S3AUTH` | Credentials | Needs boto3 | Use when | +|---|---|---|---| +| `iam` (default here) | MinIO workload identity — IRSA, IMDS, ECS task role | no | the normal Kubernetes case | +| `static` | `S3GC_S3ACCESSKEY`/`S3GC_S3SECRETKEY` (+ optional `S3GC_S3SESSIONTOKEN`) from the Secret | no | no workload identity available | +| `aws` | boto3 chain, optionally `S3PROFILE` | **yes** | rarely in-cluster; this is a workstation SSO path | + +`S3PROFILE` requires `S3AUTH=aws` and the renderer rejects other combinations. + +Prefer `iam`: it hands MinIO the credential provider, so temporary credentials +refresh during a long collect or delete instead of expiring mid-run. + +### Minimum ClickHouse grants + +Create or provision a dedicated ClickHouse user for `s3gc`, then grant it: + +```sql +GRANT SELECT ON system.* TO s3gc; -- remote_data_paths, one, disks, tables +GRANT SELECT, INSERT, CREATE TABLE ON .* TO s3gc; -- the auxiliary table +``` + +### Values that vary per cluster, and bite when wrong + +- **`S3PATH` may legitimately be empty** — some buckets keep blobs at the root. + A wrong prefix silently lists nothing and reports a clean bucket. +- **The disk is not always called `s3`** — GCS-backed clusters commonly use + `gcs`. `S3DISKNAME` sets both the anti-join scope and the aux table name, so + the wrong value makes *every* blob look orphaned. +- **A `*_cache` disk is a filesystem cache over the same blobs**, not a second + reference scope; scope the anti-join to the underlying object disk. +- **`SAMPLES` must match the value used at collect time** — the aux table is + `PARTITION BY CRC32(objpath) % SAMPLES`, and a mismatch loses partition + pruning (measured: ~2 min vs ~26 min per sample). s3gc now warns on mismatch. +- **GCS has no batch delete** — s3gc detects a `storage.googleapis.com` endpoint + and falls back to one request per object, which is markedly slower. + +Never commit credentials, rendered customer manifests, or customer `.env` +files to this repository. + +## Local run directory + +Keep generated files outside this repository: + +```bash +export S3GC_RUN_DIR=/path/to/private-s3gc-runs/customer-cluster +mkdir -p "$S3GC_RUN_DIR" +cp deploy/kubernetes/example.env "$S3GC_RUN_DIR/s3gc.env" +``` + +```text +$S3GC_RUN_DIR/ +├── s3gc.env +├── collect.yaml +├── dry-run.yaml +├── delete.yaml +└── verify.yaml +``` + +Fill `s3gc.env` from `example.env`. For production, start with +`SAMPLES=4`, `USEAGE_HOURS=24`, `ORDER_BY_OBJPATH=false`, and a 12-hour +deadline. + +## Run each phase + +For each phase, update only `PHASE`, `JOB_NAME`, and (for delete) +`DELETE_CONFIRMATION` in `s3gc.env`, then render and apply: + +```bash +python3 deploy/kubernetes/render.py "$S3GC_RUN_DIR/s3gc.env" \ + > "$S3GC_RUN_DIR/.yaml" +kubectl apply --dry-run=server -f "$S3GC_RUN_DIR/.yaml" +kubectl apply -f "$S3GC_RUN_DIR/.yaml" +kubectl -n logs -f job/ +``` + +| Phase | Required values | Result | +|---|---|---| +| `collect` | `PHASE=collect` | Lists S3 objects into the auxiliary ClickHouse table. No deletion. | +| `dry-run` | `PHASE=dry-run` | Reports candidates and total size. Review this result. | +| `delete` | `PHASE=delete`, `DELETE_CONFIRMATION=DELETE_ORPHANS` | Checks cluster/replicas, deletes candidates, and checkpoints confirmed progress. | +| `verify` | `PHASE=dry-run` | Must report zero candidates. | + +### Development automation only + +`PHASE=dev-automation` runs `collect → dry-run → delete` in one Job. It always +starts with a fresh auxiliary table and requires +`DELETE_CONFIRMATION=DELETE_ORPHANS`, `CLUSTERNAME`, and `EXPECTED_REPLICAS`. +Any failed stage stops the Job and later stages do not run; successful delete +batches remain checkpointed. Do not use this phase for customer or production +work because it removes the manual dry-run approval gate. + +## Safety + +- Delete checks the local cluster macro and expected replica count before S3 + calls. +- Confirmed deletions are tombstoned in the auxiliary table. If a delete Job + fails, create a new delete Job name with the same table prefix; do **not** + re-collect. +- No Job retries automatically (`backoffLimit: 0`). +- Do not run delete until the customer explicitly approves the dry-run result. diff --git a/deploy/kubernetes/example.env b/deploy/kubernetes/example.env new file mode 100644 index 0000000..d8aaba8 --- /dev/null +++ b/deploy/kubernetes/example.env @@ -0,0 +1,45 @@ +# Copy this file outside the repository, fill non-secret target values, and render it. +# Credentials must be supplied separately by the named Kubernetes Secret. +JOB_NAME=s3gc-example-dry-run +NAMESPACE=clickhouse +# Always digest-pinned; CI prints the exact line in its job summary. +IMAGE=ghcr.io/altinity/s3gc@sha256:0000000000000000000000000000000000000000000000000000000000000000 +# The published image is PUBLIC, so leave this empty: the renderer then omits +# the imagePullSecrets block entirely. Set it only for a private mirror. +IMAGE_PULL_SECRET= +PHASE=dry-run +# Development automation only: set PHASE=dev-automation and +# DELETE_CONFIRMATION=DELETE_ORPHANS to run collect, dry-run, and delete in one Job. +DELETE_CONFIRMATION= +CREDENTIALS_SECRET=s3gc-credentials +SERVICE_ACCOUNT=s3gc + +CHHOST=clickhouse.example.svc.cluster.local +CHPORT=8123 +CLUSTERNAME=example-cluster +EXPECTED_REPLICAS=2 +COLLECTTABLEPREFIX=s3gc_example_ + +S3IP=s3.example.com +S3PORT=443 +S3BUCKET=example-bucket +S3PATH=clickhouse-data/ +S3REGION=us-east-1 +S3SECURE_FLAG=true +S3DISKNAME=s3 +# static = explicit keys in the Secret; aws = boto3 chain / SSO profile; iam = IRSA/IMDS. +# Prefer iam in Kubernetes: it refreshes credentials and needs no boto3. +S3AUTH=iam +# Only for S3AUTH=aws. The session token, if any, belongs in the Secret. +S3PROFILE= + +SAMPLES=4 +DELETE_BATCH_SIZE=1000 +USEAGE_HOURS=24 +# Leave false for Kubernetes Jobs; global ordering is unnecessary for deletion. +ORDER_BY_OBJPATH=false +ACTIVE_DEADLINE_SECONDS=14400 +TTL_SECONDS_AFTER_FINISHED=604800 +MEMORY_REQUEST=1Gi +MEMORY_LIMIT=4Gi +VERBOSE=true diff --git a/deploy/kubernetes/job.yaml.tmpl b/deploy/kubernetes/job.yaml.tmpl new file mode 100644 index 0000000..0a11910 --- /dev/null +++ b/deploy/kubernetes/job.yaml.tmpl @@ -0,0 +1,96 @@ +apiVersion: batch/v1 +kind: Job +metadata: + name: "${JOB_NAME}" + namespace: "${NAMESPACE}" + labels: + app.kubernetes.io/name: s3gc + app.kubernetes.io/component: object-storage-garbage-collection + s3gc.altinity.com/phase: "${PHASE}" +spec: + backoffLimit: 0 + activeDeadlineSeconds: ${ACTIVE_DEADLINE_SECONDS} + ttlSecondsAfterFinished: ${TTL_SECONDS_AFTER_FINISHED} + template: + metadata: + labels: + app.kubernetes.io/name: s3gc + app.kubernetes.io/component: object-storage-garbage-collection + s3gc.altinity.com/phase: "${PHASE}" + spec: + restartPolicy: Never + serviceAccountName: "${SERVICE_ACCOUNT}" + imagePullSecrets: + - name: "${IMAGE_PULL_SECRET}" + automountServiceAccountToken: false + securityContext: + runAsNonRoot: true + runAsUser: 65532 + runAsGroup: 65532 + seccompProfile: + type: RuntimeDefault + containers: + - name: s3gc + image: "${IMAGE}" + imagePullPolicy: IfNotPresent + command: ["/app/kubernetes-entrypoint.sh"] + securityContext: + allowPrivilegeEscalation: false + readOnlyRootFilesystem: true + capabilities: + drop: ["ALL"] + resources: + requests: + cpu: "500m" + memory: ${MEMORY_REQUEST} + limits: + cpu: "2" + memory: ${MEMORY_LIMIT} + envFrom: + - secretRef: + name: "${CREDENTIALS_SECRET}" + env: + - name: S3GC_PHASE + value: "${PHASE}" + - name: S3GC_DELETE_CONFIRMATION + value: "${DELETE_CONFIRMATION}" + - name: S3GC_CHHOST + value: "${CHHOST}" + - name: S3GC_CHPORT + value: "${CHPORT}" + - name: S3GC_CLUSTERNAME + value: "${CLUSTERNAME}" + - name: S3GC_EXPECTED_REPLICAS + value: "${EXPECTED_REPLICAS}" + - name: S3GC_COLLECTTABLEPREFIX + value: "${COLLECTTABLEPREFIX}" + - name: S3GC_S3IP + value: "${S3IP}" + - name: S3GC_S3PORT + value: "${S3PORT}" + - name: S3GC_S3BUCKET + value: "${S3BUCKET}" + - name: S3GC_S3PATH + value: "${S3PATH}" + - name: S3GC_S3REGION + value: "${S3REGION}" + - name: S3GC_S3SECURE_FLAG + value: "${S3SECURE_FLAG}" + - name: S3GC_S3DISKNAME + value: "${S3DISKNAME}" + - name: S3GC_S3AUTH + value: "${S3AUTH}" + - name: S3GC_S3PROFILE + value: "${S3PROFILE}" + - name: S3GC_SAMPLES + value: "${SAMPLES}" + - name: S3GC_DELETEBATCHSIZE + value: "${DELETE_BATCH_SIZE}" + - name: S3GC_USEAGE + value: "${USEAGE_HOURS}" + - name: S3GC_ORDER_BY_OBJPATH + value: "${ORDER_BY_OBJPATH}" + - name: S3GC_CHTIMEOUT + value: "${ACTIVE_DEADLINE_SECONDS}" + - name: S3GC_VERBOSE_FLAG + value: "${VERBOSE}" diff --git a/deploy/kubernetes/render.py b/deploy/kubernetes/render.py new file mode 100644 index 0000000..bdabd30 --- /dev/null +++ b/deploy/kubernetes/render.py @@ -0,0 +1,132 @@ +#!/usr/bin/env python3 +"""Render the plain s3gc Job template from a non-secret KEY=VALUE file.""" + +import re +import sys +from pathlib import Path +from string import Template + + +ROOT = Path(__file__).parent +TEMPLATE = ROOT / "job.yaml.tmpl" +REQUIRED = { + "ACTIVE_DEADLINE_SECONDS", + "CHHOST", + "CHPORT", + "CLUSTERNAME", + "COLLECTTABLEPREFIX", + "CREDENTIALS_SECRET", + "DELETE_BATCH_SIZE", + "EXPECTED_REPLICAS", + "IMAGE", + "IMAGE_PULL_SECRET", + "JOB_NAME", + "MEMORY_LIMIT", + "MEMORY_REQUEST", + "NAMESPACE", + "ORDER_BY_OBJPATH", + "PHASE", + "S3BUCKET", + "S3DISKNAME", + "S3IP", + "S3PATH", + "S3PORT", + "S3REGION", + "S3SECURE_FLAG", + "S3AUTH", + "S3PROFILE", + "SAMPLES", + "SERVICE_ACCOUNT", + "TTL_SECONDS_AFTER_FINISHED", + "USEAGE_HOURS", + "VERBOSE", +} +DELETE_CONFIRMATION = "DELETE_ORPHANS" +JOB_NAME_RE = re.compile(r"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$") + + +def read_values(path: Path) -> dict[str, str]: + values: dict[str, str] = {} + for line_number, line in enumerate(path.read_text().splitlines(), start=1): + line = line.strip() + if not line or line.startswith("#"): + continue + if "=" not in line: + raise ValueError(f"{path}:{line_number}: expected KEY=VALUE") + key, value = line.split("=", 1) + if not key.isidentifier(): + raise ValueError(f"{path}:{line_number}: invalid key {key!r}") + values[key] = value + return values + + +def validate(values: dict[str, str]) -> None: + missing = sorted(REQUIRED - values.keys()) + if missing: + raise ValueError("missing required values: " + ", ".join(missing)) + if values["PHASE"] not in {"collect", "dry-run", "delete", "dev-automation"}: + raise ValueError("PHASE must be collect, dry-run, delete, or dev-automation") + if values["PHASE"] in {"delete", "dev-automation"} and values.get("DELETE_CONFIRMATION") != DELETE_CONFIRMATION: + raise ValueError( + f"{values['PHASE']} requires DELETE_CONFIRMATION={DELETE_CONFIRMATION}" + ) + if not JOB_NAME_RE.fullmatch(values["JOB_NAME"]) or len(values["JOB_NAME"]) > 63: + raise ValueError("JOB_NAME must be a DNS label of at most 63 characters") + if "@sha256:" not in values["IMAGE"]: + raise ValueError("IMAGE must be pinned by digest (for example, altinity/s3gc@sha256:...)") + for key, value in values.items(): + if any(character in value for character in ('"', "\\n", "\\r")): + raise ValueError(f"{key} may not contain quotes or newlines") + for numeric_key in ("DELETE_BATCH_SIZE", "EXPECTED_REPLICAS", "SAMPLES", "ACTIVE_DEADLINE_SECONDS", "TTL_SECONDS_AFTER_FINISHED"): + if not values[numeric_key].isdigit() or int(values[numeric_key]) < 1: + raise ValueError(f"{numeric_key} must be a positive integer") + if not values["USEAGE_HOURS"].isdigit() or int(values["USEAGE_HOURS"]) < 0: + raise ValueError("USEAGE_HOURS must be a non-negative integer") + if values["S3AUTH"] not in {"static", "aws", "iam"}: + raise ValueError("S3AUTH must be static, aws or iam") + if values["S3PROFILE"] and values["S3AUTH"] != "aws": + raise ValueError("S3PROFILE requires S3AUTH=aws") + if values["VERBOSE"] not in {"true", "false"}: + raise ValueError("VERBOSE must be true or false") + if values["ORDER_BY_OBJPATH"] not in {"true", "false"}: + raise ValueError("ORDER_BY_OBJPATH must be true or false") + + +def drop_empty_image_pull_secret(manifest: str) -> str: + """Remove the imagePullSecrets block when no secret was configured. + + The published image is public, so most deployments need no pull secret at + all — and rendering `- name: ""` would be both meaningless and rejected. + string.Template has no conditionals, so this is done after substitution. + """ + lines = manifest.splitlines(keepends=True) + out = [] + index = 0 + while index < len(lines): + if lines[index].strip() == "imagePullSecrets:" and index + 1 < len(lines): + following = lines[index + 1].strip() + if following in ('- name: ""', "- name: ''", "- name:"): + index += 2 + continue + out.append(lines[index]) + index += 1 + return "".join(out) + + +def main() -> int: + if len(sys.argv) != 2: + print(f"usage: {Path(sys.argv[0]).name} CONFIG.env", file=sys.stderr) + return 64 + try: + values = read_values(Path(sys.argv[1])) + validate(values) + rendered = Template(TEMPLATE.read_text()).substitute(values) + sys.stdout.write(drop_empty_image_pull_secret(rendered)) + except (OSError, ValueError, KeyError) as exc: + print(f"render error: {exc}", file=sys.stderr) + return 64 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docker/Dockerfile b/docker/Dockerfile new file mode 100644 index 0000000..3a1dca8 --- /dev/null +++ b/docker/Dockerfile @@ -0,0 +1,14 @@ +FROM python:3.11-slim + +WORKDIR /app + +COPY requirements.txt ./ +RUN pip install --no-cache-dir --disable-pip-version-check -r requirements.txt + +COPY s3gc.py ./ +COPY docker/kubernetes-entrypoint.sh ./kubernetes-entrypoint.sh +RUN chmod 0555 /app/kubernetes-entrypoint.sh + +USER 65532:65532 + +ENTRYPOINT ["python", "/app/s3gc.py"] diff --git a/docker/kubernetes-entrypoint.sh b/docker/kubernetes-entrypoint.sh new file mode 100644 index 0000000..27e51d7 --- /dev/null +++ b/docker/kubernetes-entrypoint.sh @@ -0,0 +1,52 @@ +#!/usr/bin/env sh +set -eu + +phase="${S3GC_PHASE:-dry-run}" + +case "${phase}" in + collect) + set -- --collectonly --keepdata + if [ "${S3GC_FRESH_RUN:-false}" = "true" ]; then + set -- "$@" --drop-collecttable + fi + ;; + dry-run) + set -- --usecollected --dry-run + ;; + delete) + if [ "${S3GC_DELETE_CONFIRMATION:-}" != "DELETE_ORPHANS" ]; then + echo "Refusing delete: set S3GC_DELETE_CONFIRMATION=DELETE_ORPHANS" >&2 + exit 64 + fi + if [ -z "${S3GC_CLUSTERNAME:-}" ] || [ -z "${S3GC_EXPECTED_REPLICAS:-}" ]; then + echo "Refusing delete: S3GC_CLUSTERNAME and S3GC_EXPECTED_REPLICAS are required" >&2 + exit 64 + fi + set -- --usecollected --keepdata --non-interactive + ;; + dev-automation) + if [ "${S3GC_DELETE_CONFIRMATION:-}" != "DELETE_ORPHANS" ]; then + echo "Refusing dev automation: set S3GC_DELETE_CONFIRMATION=DELETE_ORPHANS" >&2 + exit 64 + fi + if [ -z "${S3GC_CLUSTERNAME:-}" ] || [ -z "${S3GC_EXPECTED_REPLICAS:-}" ]; then + echo "Refusing dev automation: S3GC_CLUSTERNAME and S3GC_EXPECTED_REPLICAS are required" >&2 + exit 64 + fi + + # A fresh collection avoids mixing prior runs and their tombstones into an + # automated development run. `set -e` stops subsequent stages on error. + echo "s3gc dev automation: collect" + python /app/s3gc.py --collectonly --keepdata --drop-collecttable + echo "s3gc dev automation: dry-run" + python /app/s3gc.py --usecollected --dry-run + echo "s3gc dev automation: delete" + exec python /app/s3gc.py --usecollected --keepdata --non-interactive + ;; + *) + echo "Invalid S3GC_PHASE=${phase}; use collect, dry-run, delete, or dev-automation" >&2 + exit 64 + ;; +esac + +exec python /app/s3gc.py "$@" diff --git a/pytest.ini b/pytest.ini new file mode 100644 index 0000000..c993f2f --- /dev/null +++ b/pytest.ini @@ -0,0 +1,5 @@ +[pytest] +addopts = -ra +testpaths = tests +markers = + dev_cluster: requires the dedicated development Kubernetes cluster and is never run in CI diff --git a/requirements-dev.txt b/requirements-dev.txt new file mode 100644 index 0000000..2c78728 --- /dev/null +++ b/requirements-dev.txt @@ -0,0 +1 @@ +pytest==8.3.5 diff --git a/requirements.txt b/requirements.txt index 839b8f5..b7a7fa7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,4 @@ -Minio -clickhouse_connect -jsonargparse[all] +minio==7.2.20 +clickhouse-connect==1.6.0 +jsonargparse==4.36.0 +boto3==1.43.65 diff --git a/s3gc.py b/s3gc.py index f668589..d2aabe0 100644 --- a/s3gc.py +++ b/s3gc.py @@ -19,6 +19,8 @@ from io import StringIO from minio import Minio from minio.deleteobjects import DeleteObject +from minio.credentials import IamAwsProvider +from minio.error import S3Error from contextlib import redirect_stdout import clickhouse_connect @@ -31,13 +33,39 @@ import urllib3 import logging import datetime -from distutils.util import strtobool usage = """ s3 garbage collector for ClickHouse example: $ ./s3gc.py """ + +def strtobool(value): + """Minimal stdlib-compatible replacement for distutils.util.strtobool.""" + normalized = value.lower() + if normalized in {"y", "yes", "t", "true", "on", "1"}: + return 1 + if normalized in {"n", "no", "f", "false", "off", "0"}: + return 0 + raise ValueError(f"invalid truth value {value!r}") + + +def coerce_bool(value): + """Normalise anything an option may arrive as into a real bool. + + Flags declared with action="store_true" are set to a real bool on the command + line, but jsonargparse populates them from the environment as the RAW STRING. + Every non-empty string is truthy in Python, so S3GC_DRYRUN_FLAG=false used to + mean *true*. Treat unset/empty as false and parse the usual spellings. + """ + if isinstance(value, bool): + return value + if value is None or value == "": + return False + if isinstance(value, (int, float)): + return bool(value) + return bool(strtobool(str(value))) + parser = ArgumentParser( usage=usage, env_prefix="S3GC", default_env=True, exit_on_error=False ) @@ -102,6 +130,31 @@ default="", help="S3 secret key", ) +parser.add_argument( + "--s3-session-token", + "--s3sessiontoken", + dest="s3sessiontoken", + default="", + help="S3 session token for explicit temporary credentials", +) +parser.add_argument( + "--s3auth", + "--s3-auth", + dest="s3auth", + default="static", + help=( + "S3 auth mode: static, aws or iam. static uses explicit keys (optionally with a " + "session token); aws uses the boto3 credential chain, including AWS SSO profiles; " + "iam uses MinIO's workload identity provider (IRSA/IMDS/ECS) and needs no boto3" + ), +) +parser.add_argument( + "--s3profile", + "--s3-profile", + dest="s3profile", + default="", + help="AWS profile name for S3 auth. Setting this enables aws auth mode", +) parser.add_argument( "--s3secure", "--s3-secure", @@ -113,7 +166,7 @@ parser.add_argument( "--s3secureflag", "--s3-secure-flag", - type=bool, + type=coerce_bool, dest="s3secure_flag", default=False, help="S3 secure mode", @@ -150,7 +203,7 @@ parser.add_argument( "--keepdataflag", "--keep-data-flag", - type=bool, + type=coerce_bool, dest="keepdata_flag", default=False, help="keep auxiliary data in ClickHouse table", @@ -166,7 +219,7 @@ parser.add_argument( "--collectonlyflag", "--collect-only-flag", - type=bool, + type=coerce_bool, dest="collectonly_flag", default=False, help="put object names to auxiliary table", @@ -182,7 +235,7 @@ parser.add_argument( "--usecollectedflag", "--use-collected-flag", - type=bool, + type=coerce_bool, dest="usecollected_flag", default=False, help="auxiliary data is already collected in ClickHouse table", @@ -243,7 +296,7 @@ "--dryrunflag", "--dryrun-flag", "--dry-run-flag", - type=bool, + type=coerce_bool, dest="dryrun_flag", default=False, help="Calculate objects to remove without actual removing", @@ -256,6 +309,12 @@ default="", help="Consider an objects unused if there is no host in the cluster refers the object", ) +parser.add_argument( + "--expected-replicas", + dest="expected_replicas", + type=Optional[int], + help="Fail before deleting when clusterAllReplicas() does not return this many replicas", +) parser.add_argument( "--age", "--hours", @@ -284,6 +343,49 @@ default=4, help="Number of partitions in auxiliary table", ) +parser.add_argument( + "--deletebatchsize", + "--delete-batch-size", + dest="deletebatchsize", + type=int, + default=1000, + help="S3 objects to delete and checkpoint per progress batch", +) +parser.add_argument( + "--order-by-objpath", + action="store_true", + dest="order_by_objpath", + default=False, + help="Order anti-join output by object path (costly for large Kubernetes Jobs)", +) +parser.add_argument( + "--order-by-objpath-flag", + dest="order_by_objpath", + type=coerce_bool, + default=False, + help="Order anti-join output by object path (costly for large Kubernetes Jobs)", +) +parser.add_argument( + "--s3-connect-timeout", + dest="s3_connect_timeout", + type=int, + default=15, + help="S3 connection timeout in seconds", +) +parser.add_argument( + "--s3-read-timeout", + dest="s3_read_timeout", + type=int, + default=120, + help="S3 read timeout in seconds", +) +parser.add_argument( + "--s3-retries", + dest="s3_retries", + type=int, + default=3, + help="S3 HTTP retries for transient failures", +) parser.add_argument( "--chtimeout", "--ch-timeout", @@ -306,7 +408,7 @@ "--create-database-flag", "--createdatabase-flag", dest="createdatabase_flag", - type=bool, + type=coerce_bool, default=False, help="create database for collecttable", ) @@ -322,7 +424,7 @@ "--drop-collecttable-flag", "--dropcollecttable-flag", dest="drop_collecttable_flag", - type=bool, + type=coerce_bool, default=False, help="drop collecttable and recreate; beware of ClickHouse DROP TABLE constraints", ) @@ -330,7 +432,7 @@ "--useremoveobjects", "--use-remove-objects", dest="use_remove_objects", - type=bool, + type=coerce_bool, default=True, help="use remove_objects (not supported by GCE). Set it to false to use remove_object", ) @@ -345,7 +447,7 @@ parser.add_argument( "--interactive-flag", dest="interactive_flag", - type=bool, + type=coerce_bool, default=True, help="confirm deleting", ) @@ -359,7 +461,7 @@ parser.add_argument( "--verboseflag", "--verbose-flag", - type=bool, + type=coerce_bool, dest="verbose_flag", default=False, help="debug output", @@ -374,7 +476,7 @@ parser.add_argument( "--debugflag", "--debug-flag", - type=bool, + type=coerce_bool, dest="debug_flag", default=False, help="trace output (more verbose)", @@ -386,7 +488,7 @@ "--silentflag", "--silent-flag", dest="silent_flag", - type=bool, + type=coerce_bool, default=False, help="no log", ) @@ -407,6 +509,39 @@ args = parser.parse_args() +# Every flag declared with action="store_true" arrives from the environment as a +# raw string, and every non-empty string is truthy. Normalise all boolean +# options in one place, immediately after parsing, so the rest of the program +# can rely on real bools. +BOOLEAN_DESTS = ( + "s3secure_flag", + "use_remove_objects", + "keepdata_flag", + "collectonly_flag", + "usecollected_flag", + "dryrun_flag", + "order_by_objpath", + "createdatabase_flag", + "drop_collecttable_flag", + "verbose_flag", + "debug_flag", + "silent_flag", + "listoptions", +) + +for _dest in BOOLEAN_DESTS: + if not hasattr(args, _dest): + continue + _raw = getattr(args, _dest) + try: + setattr(args, _dest, coerce_bool(_raw)) + except ValueError: + parser.error( + f"invalid boolean value {_raw!r} for {_dest} " + f"(environment variable S3GC_{_dest.upper()}); " + "use one of true/false, yes/no, on/off, 1/0" + ) + if args.listoptions: with redirect_stdout(StringIO()) as f: try: @@ -422,7 +557,7 @@ if key in ["listoptions"]: continue if backslash: - print(" \\ ") + print(" \\") print(f" S3GC_{key.upper()}={value}", end="") backslash = True @@ -446,10 +581,14 @@ class LogFormatter(logging.Formatter): def get_filter_strings(): filter_strings = [] - if len(args.chpass) > 3: - filter_strings.append(args.chpass) - if len(args.s3secretkey) > 3: - filter_strings.append(args.s3secretkey) + for secret in [ + args.chpass, + args.s3accesskey, + args.s3secretkey, + args.s3sessiontoken, + ]: + if len(secret) > 3: + filter_strings.append(secret) return filter_strings filter_strings = get_filter_strings() @@ -500,6 +639,49 @@ def graceful_exit(): ch_client = None +class S3DeletionError(RuntimeError): + """A delete failed after successful deletions were checkpointed.""" + + +def _query_single_value(query): + result = ch_client.query(query) + if not result.result_rows or not result.result_rows[0]: + raise RuntimeError(f"ClickHouse returned no result for preflight query: {query}") + return result.result_rows[0][0] + + +def preflight_cluster(): + """Make destructive cluster-wide cleanup fail closed when topology is unexpected.""" + if not args.expected_replicas: + return + if not args.clustername: + raise ValueError("--expected-replicas requires --cluster") + + actual_cluster = _query_single_value("SELECT getMacro('cluster')") + if actual_cluster != args.clustername: + raise RuntimeError( + f"cluster preflight failed: expected local cluster macro {args.clustername!r}, " + f"got {actual_cluster!r}" + ) + + cluster_name = args.clustername.replace("'", "\\\\'") + actual_replicas = _query_single_value( + f"SELECT count() FROM clusterAllReplicas('{cluster_name}', system.one)" + ) + if actual_replicas != args.expected_replicas: + raise RuntimeError( + f"replica preflight failed: expected {args.expected_replicas}, got {actual_replicas}" + ) + + logger.info( + f"cluster preflight passed: cluster={args.clustername}, replicas={actual_replicas}" + ) + + +class UserVisibleError(RuntimeError): + """An operator-facing failure: reported without a traceback.""" + + def connect_to_ch(): logger.info( f"Connecting to ClickHouse, host={args.chhost}, port={args.chport}, username={args.chuser}, password={args.chpass}, s3path={args.s3path}, bucket={args.s3bucket}, s3path={args.s3path}" @@ -514,25 +696,184 @@ def connect_to_ch(): ) +def resolve_static_s3_credentials(): + if bool(args.s3accesskey) != bool(args.s3secretkey): + raise ValueError("s3accesskey and s3secretkey must be specified together") + if args.s3sessiontoken and not args.s3accesskey: + raise ValueError("s3sessiontoken requires s3accesskey and s3secretkey") + + if args.s3accesskey: + return args.s3accesskey, args.s3secretkey, args.s3sessiontoken or None, args.s3region, "static" + + return None, None, None, args.s3region, "anonymous" + + +def resolve_aws_s3_credentials(): + if args.s3accesskey or args.s3secretkey or args.s3sessiontoken: + raise ValueError("s3auth=aws cannot be combined with explicit S3 access keys") + + try: + import boto3 + except ImportError as exc: + raise UserVisibleError("boto3 is required for s3auth=aws") from exc + + session = boto3.Session( + profile_name=args.s3profile or None, + region_name=args.s3region, + ) + credentials = session.get_credentials() + if credentials is None: + profile_hint = f" profile {args.s3profile}" if args.s3profile else "" + raise UserVisibleError(f"unable to resolve AWS credentials{profile_hint}") + + frozen_credentials = credentials.get_frozen_credentials() + if not frozen_credentials.access_key or not frozen_credentials.secret_key: + profile_hint = f" profile {args.s3profile}" if args.s3profile else "" + raise UserVisibleError(f"resolved AWS credentials{profile_hint} are incomplete") + + return ( + frozen_credentials.access_key, + frozen_credentials.secret_key, + frozen_credentials.token, + args.s3region or session.region_name, + "aws", + ) + + +AUTH_MODES = ("static", "aws", "iam") + + +def resolve_iam_s3_credentials(): + """Workload identity via MinIO's own provider — IRSA, IMDS or ECS task role. + + Kept as a first-class mode rather than folded into `aws`: it needs no boto3, + and it is what the validated Kubernetes deployments use. `credentials=` is + returned instead of keys so MinIO can refresh the temporary credentials. + """ + return None, None, None, args.s3region, "iam" + + +def resolve_s3_credentials(): + auth_mode = args.s3auth.lower() + if auth_mode not in AUTH_MODES: + raise ValueError(f"s3auth must be one of {', '.join(AUTH_MODES)}") + + # Implied modes. Both are conveniences, so a contradiction is an error rather + # than a silent winner: picking one would send credentials nobody asked for. + if args.s3profile: + if auth_mode not in ("static", "aws"): + raise ValueError(f"s3profile implies s3auth=aws, which conflicts with s3auth={auth_mode}") + auth_mode = "aws" + if auth_mode == "aws": + return resolve_aws_s3_credentials() + if auth_mode == "iam": + return resolve_iam_s3_credentials() + + return resolve_static_s3_credentials() + + def connect_to_s3(): if args.s3secure_flag: logger.debug(f"using SSL certificate {args.s3sslcertfile}") os.environ["SSL_CERT_FILE"] = args.s3sslcertfile + access_key, secret_key, session_token, s3_region, s3_auth = resolve_s3_credentials() logger.info( - f"Connecting to S3, host:port={args.s3ip}:{args.s3port}, access_key={args.s3accesskey}, secret_key={args.s3secretkey}, secure={args.s3secure_flag}, region={args.s3region}" + f"Connecting to S3, host:port={args.s3ip}:{args.s3port}, auth={s3_auth}, " + f"secure={args.s3secure_flag}, region={s3_region}" ) + + # Google Cloud Storage's S3-compatible API has no batch DeleteObjects, so + # remove_objects() fails there. Switch to the per-object path automatically + # rather than letting every delete fail at run time. + if "storage.googleapis.com" in args.s3ip and args.use_remove_objects: + logger.warning( + "GCS endpoint detected: batch remove_objects is not supported there, " + "falling back to per-object remove_object. This is markedly slower " + "(one request per object); pass --use-remove-objects false to silence this." + ) + args.use_remove_objects = False global minio_client + connection_options = { + "secure": args.s3secure_flag, + "region": s3_region, + "http_client": urllib3.PoolManager( + cert_reqs="CERT_NONE", + timeout=urllib3.Timeout( + connect=args.s3_connect_timeout, read=args.s3_read_timeout + ), + retries=urllib3.Retry( + total=args.s3_retries, + connect=args.s3_retries, + read=args.s3_retries, + status=args.s3_retries, + backoff_factor=1, + status_forcelist=[429, 500, 502, 503, 504], + allowed_methods=frozenset({"DELETE", "GET", "HEAD", "POST"}), + ), + ), + } + if s3_auth == "iam": + # Hand MinIO the provider, not frozen keys, so it can refresh the + # temporary credentials for the lifetime of a long collect or delete. + connection_options["credentials"] = IamAwsProvider() + else: + # static, aws and anonymous all arrive here as resolved values; + # session_token is None unless temporary credentials were supplied. + connection_options["access_key"] = access_key + connection_options["secret_key"] = secret_key + connection_options["session_token"] = session_token minio_client = Minio( f"{args.s3ip}:{args.s3port}", - access_key=args.s3accesskey, - secret_key=args.s3secretkey, - secure=args.s3secure_flag, - region=args.s3region, - http_client=urllib3.PoolManager(cert_reqs="CERT_NONE"), + **connection_options, ) +def remove_objects_reconnecting(batch_rows): + """Delete one batch, reconnecting once if the S3 transport is stale. + + DeleteObject requests are idempotent: retrying after an interrupted response + can only leave the object absent, never delete a different object. + """ + for attempt in range(2): + try: + return list( + minio_client.remove_objects( + args.s3bucket, [DeleteObject(row[0]) for row in batch_rows] + ) + ) + except (S3Error, urllib3.exceptions.HTTPError) as exc: + if attempt: + raise + logger.warning( + "S3 delete transport failed (%s); reconnecting and retrying once", exc + ) + connect_to_s3() + + raise AssertionError("unreachable") + + +def format_s3_list_error(exc): + code = getattr(exc, "code", "unknown") + message = getattr(exc, "message", str(exc)) + profile_arg = f" --profile {args.s3profile}" if args.s3profile else "" + return ( + f"unable to list S3 objects for bucket={args.s3bucket!r}, prefix={args.s3path!r}: " + f"{code}: {message}. " + f"s3gc collection requires s3:ListBucket on arn:aws:s3:::{args.s3bucket} " + f"for this prefix, even with --dry-run. Verify the same credentials with: " + f"aws sts get-caller-identity{profile_arg}; " + f"aws s3api list-objects-v2 --bucket {args.s3bucket} --prefix {args.s3path} --max-keys 1{profile_arg}" + ) + + +def next_s3_object(objects): + try: + return next(objects) + except S3Error as exc: + raise UserVisibleError(format_s3_list_error(exc)) from exc + + def do_collect(): logger.debug(f"start_after {args.collectafter}") objects = minio_client.list_objects( @@ -568,9 +909,12 @@ def do_collect(): objs = [] for batch_element in range(0, args.collectbatchsize): try: - obj = next(objects) + obj = next_s3_object(objects) delta = datetime.datetime.now(datetime.timezone.utc) - obj.last_modified - hours = int(delta.seconds / 3600) + # total_seconds(), not .seconds: the latter is the sub-day + # remainder (0..86399), so any object older than a day reported + # at most 23 hours and --age 24 collected nothing at all. + hours = int(delta.total_seconds() // 3600) if hours >= args.age: objs.append([obj.object_name, obj.size, obj.last_modified, True]) total_size += obj.size @@ -594,7 +938,38 @@ def do_collect(): ) +def check_samples_match_partitioning(): + """Warn when --samples disagrees with the aux table's PARTITION BY. + + The table is created as PARTITION BY CRC32(objpath) % at COLLECT + time. Running the use phase with a different --samples silently loses + partition pruning: on one production cluster the matching case scanned a + sample in ~2 min where the mismatching case took ~26 min. + """ + try: + rows = ch_client.query( + "SELECT partition_key FROM system.tables " + f"WHERE database = currentDatabase() AND name = '{tname.strip('`').split('.')[-1]}'" + ).result_rows + except Exception as exc: + logger.debug(f"could not read partition_key for {tname}: {exc}") + return + if not rows or not rows[0][0]: + return + partition_key = rows[0][0] + expected = f"% {args.samples}" + if "CRC32" in partition_key and expected not in partition_key.replace(" ", " "): + logger.warning( + f"--samples {args.samples} does not match the auxiliary table's " + f"partitioning ({partition_key}). Partition pruning will be lost; " + "use the same --samples value that the collect phase used." + ) + + def do_use(): + if not args.dryrun_flag: + preflight_cluster() + srdp = "system.remote_data_paths" if args.clustername: srdp = f"clusterAllReplicas('{args.clustername}', {srdp})" @@ -609,9 +984,18 @@ def do_use(): logger.info(f"exception selecting from {tname}, {exc}") pass if num_rows == 0: - logger.info(f"auxiliary table {tname} does not exist or empty, nothing to do") - - graceful_exit() + # Exiting 0 here reads as success, but with --usecollected an absent or + # empty auxiliary table means the collect never ran, ran against another + # host, or was truncated. The table is a NODE-LOCAL ReplacingMergeTree, so + # a load-balanced ClickHouse Service can collect on one replica and land + # here on the other. Fail loudly instead of reporting a clean bucket. + raise RuntimeError( + f"auxiliary table {tname} does not exist or is empty on {args.chhost}. " + "Run the collect phase first, and make sure every phase targets the SAME " + "replica: the table is node-local, so a load-balanced Service will not do." + ) + + check_samples_match_partitioning() def make_antijoin(calc_only=False, sample=None): after_condition = f"AND s3o.objpath > {args.useafter} " if args.useafter else "" @@ -622,11 +1006,12 @@ def make_antijoin(calc_only=False, sample=None): if not calc_only: sample_condition = f"CRC32(s3o.objpath) % {args.samples} = {sample} AND " + order_by = " ORDER BY s3o.objpath" if args.order_by_objpath else "" antijoin = f""" SELECT s3o.objpath, s3o.size as size, s3o.last_modified as last_modified FROM {tname} AS s3o LEFT ANTI JOIN {srdp} AS rdp ON (rdp.remote_path = s3o.objpath AND rdp.disk_name='{args.s3diskname}') WHERE {sample_condition} s3o.active=true {after_condition} {age_condition} - ORDER BY s3o.objpath {limit} SETTINGS final = 1""" + {order_by} {limit} SETTINGS final = 1""" if calc_only: countantijoin = f"SELECT COUNT(1), SUM(size) FROM ({antijoin}) q" @@ -662,50 +1047,94 @@ def make_antijoin(calc_only=False, sample=None): num_removed = 0 total_size = 0 - objs = [] - + if not args.dryrun_flag and args.deletebatchsize < 1: + raise ValueError("--deletebatchsize must be a positive integer") for sample in range(0, args.samples): antijoin = make_antijoin(sample=sample) logger.info(f"antijoin {antijoin}") with ch_client.query_row_block_stream(antijoin) as stream: for block in stream: - objects_to_remove = [] - object_to_remove = [] + selected_rows = [] for row in block: logger.debug( f"{'removing' if not args.dryrun_flag else 'would remove if no dryrun flag'} {row[0]} of size {row[1]}" ) + selected_rows.append(row) + + if args.dryrun_flag: + num_removed += len(selected_rows) + total_size += sum(row[1] for row in selected_rows) + continue + + for offset in range(0, len(selected_rows), args.deletebatchsize): + batch_rows = selected_rows[offset : offset + args.deletebatchsize] + errors = [] if args.use_remove_objects: - objects_to_remove.append(DeleteObject(row[0])) - else: - object_to_remove.append(row[0]) - objs.append([row[0], row[1], row[2], False]) - total_size += row[1] - if not args.dryrun_flag: - if args.use_remove_objects: - errors = minio_client.remove_objects( - args.s3bucket, objects_to_remove - ) + errors = remove_objects_reconnecting(batch_rows) for error in errors: logger.info(f"error occurred when deleting object via remove_objects {error}") + + failed_names = { + getattr(error, "object_name", None) or getattr(error, "name", None) + for error in errors + } + if None in failed_names: + # Do not tombstone any object for an uncorrelatable batch error. + successful_rows = [] + else: + successful_rows = [ + row for row in batch_rows if row[0] not in failed_names + ] else: - for object_path in object_to_remove: + successful_rows = [] + for row in batch_rows: try: - minio_client.remove_object( - args.s3bucket, object_path - ) + minio_client.remove_object(args.s3bucket, row[0]) + successful_rows.append(row) except Exception as error: - logger.info(f"error occurred when deleting object {object_path} via remove_object {error}") - - num_removed += len(objects_to_remove) + logger.info(f"error occurred when deleting object {row[0]} via remove_object {error}") + errors.append(error) + + if successful_rows: + tombstones = [ + [row[0], row[1], row[2], False] for row in successful_rows + ] + ch_client.insert( + tname, + tombstones, + column_names=["objpath", "size", "last_modified", "active"], + ) + num_removed += len(successful_rows) + total_size += sum(row[1] for row in successful_rows) + logger.info( + f"delete checkpoint: {num_removed} objects / {total_size} bytes removed so far" + ) - if not args.dryrun_flag: - ch_client.insert(tname, objs, column_names=["objpath", "size", "last_modified", "active"]) + if errors: + raise S3DeletionError( + f"{len(errors)} S3 deletion error(s); successful deletes were checkpointed" + ) + # "this attempt", not "this run": a resumed run leaves earlier attempts' + # deletions out of these counters, so the line understated one aps1 run by + # 16.61 TiB. The cumulative truth is the tombstone count in the aux table. logger.info( - f"{num_removed} objects of total size {total_size} {'are removed' if not args.dryrun_flag else 'would be removed but for dryrun flag'}" + f"{num_removed} objects of total size {total_size} " + f"{'are removed' if not args.dryrun_flag else 'would be removed but for dryrun flag'} " + "in this attempt" ) + if not args.dryrun_flag: + try: + cumulative = ch_client.query( + f"SELECT count(), sum(size) FROM {tname} FINAL WHERE active = false" + ).result_rows[0] + logger.info( + f"cumulative for this auxiliary table: {cumulative[0]} objects / " + f"{cumulative[1]} bytes tombstoned" + ) + except Exception as exc: # never fail a completed run over a status query + logger.info(f"could not read cumulative tombstone count: {exc}") if not args.keepdata_flag and not args.dryrun_flag: logger.info(f"truncating {tname}") @@ -713,15 +1142,22 @@ def make_antijoin(calc_only=False, sample=None): def main(): - connect_to_ch() - if not (args.usecollected_flag and args.dryrun_flag): - connect_to_s3() - if not args.usecollected_flag: - do_collect() - if not args.collectonly_flag: - do_use() - - graceful_exit() + try: + connect_to_ch() + if not (args.usecollected_flag and args.dryrun_flag): + connect_to_s3() + if not args.usecollected_flag: + do_collect() + if not args.collectonly_flag: + do_use() + + graceful_exit() + except UserVisibleError as exc: + if args.debug_flag: + logger.exception(str(exc)) + else: + logger.error(str(exc)) + sys.exit(1) if __name__ == "__main__": diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..afda1a7 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,24 @@ +import logging +import runpy +import sys +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] + + +@pytest.fixture +def s3gc_module(monkeypatch): + """Load the script with isolated command-line arguments and logging.""" + logger = logging.getLogger("s3gc_test") + existing_handlers = list(logger.handlers) + monkeypatch.setattr(sys, "argv", [str(ROOT / "s3gc.py")]) + module = runpy.run_path(str(ROOT / "s3gc.py"), run_name="s3gc_test") + yield module + + for handler in list(logger.handlers): + if handler not in existing_handlers: + logger.removeHandler(handler) + handler.close() diff --git a/tests/test_s3gc.py b/tests/test_s3gc.py new file mode 100644 index 0000000..bae2da6 --- /dev/null +++ b/tests/test_s3gc.py @@ -0,0 +1,756 @@ +from argparse import ArgumentError +import os +import subprocess +import sys +import types +from pathlib import Path + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] + + +class QueryResult: + def __init__(self, value): + self.result_rows = [(value,)] + + +class FakeStream: + def __init__(self, blocks): + self.blocks = blocks + + def __enter__(self): + return iter(self.blocks) + + def __exit__(self, *args): + return False + + +class FakeCH: + def __init__(self, cluster="cluster", replicas=2, blocks=()): + self.cluster = cluster + self.replicas = replicas + self.blocks = blocks + self.inserts = [] + self.stream_query = "" + + def query(self, query): + if "getMacro" in query: + return QueryResult(self.cluster) + if "clusterAllReplicas" in query and "system.one" in query: + return QueryResult(self.replicas) + raise AssertionError(query) + + def command(self, query): + if "COUNT(1)" in query: + return 1 + raise AssertionError(query) + + def query_row_block_stream(self, query): + self.stream_query = query + return FakeStream(self.blocks) + + def insert(self, table, rows, column_names): + self.inserts.append((table, rows, column_names)) + + +class DeleteError: + def __init__(self, name): + self.name = name + + +class FailingMinio: + def remove_objects(self, bucket, objects): + return iter([DeleteError("bad-object")]) + + +@pytest.fixture +def args_factory(): + def make_args(**overrides): + values = { + "clustername": "cluster", + "expected_replicas": 2, + "dryrun_flag": False, + "s3diskname": "s3", + "useafter": None, + "useage": 24, + "usetotal": None, + "samples": 1, + "deletebatchsize": 1000, + "order_by_objpath": False, + "interactive_flag": False, + "use_remove_objects": True, + "s3bucket": "bucket", + "keepdata_flag": True, + "silent_flag": True, + # S3 auth surface (static | aws | iam) + "s3auth": "static", + "s3profile": "", + "s3accesskey": "", + "s3secretkey": "", + "s3sessiontoken": "", + "s3region": "eu-central-1", + } + values.update(overrides) + return types.SimpleNamespace(**values) + + return make_args + + +def test_preflight_rejects_wrong_cluster(s3gc_module, args_factory, monkeypatch): + namespace = s3gc_module["preflight_cluster"].__globals__ + monkeypatch.setitem(namespace, "args", args_factory(clustername="expected")) + monkeypatch.setitem(namespace, "ch_client", FakeCH(cluster="actual")) + + with pytest.raises(RuntimeError, match="cluster preflight failed"): + s3gc_module["preflight_cluster"]() + + +def test_batch_errors_checkpoint_only_confirmed_deletes( + s3gc_module, args_factory, monkeypatch +): + namespace = s3gc_module["do_use"].__globals__ + client = FakeCH(blocks=[[("good-object", 10, "time"), ("bad-object", 20, "time")]]) + monkeypatch.setitem(namespace, "args", args_factory()) + monkeypatch.setitem(namespace, "ch_client", client) + monkeypatch.setitem(namespace, "minio_client", FailingMinio()) + + with pytest.raises(s3gc_module["S3DeletionError"]): + s3gc_module["do_use"]() + + assert client.inserts == [ + ( + "`s3objects_for_s3`", + [["good-object", 10, "time", False]], + ["objpath", "size", "last_modified", "active"], + ) + ] + + +def test_delete_batches_are_checkpointed_independently( + s3gc_module, args_factory, monkeypatch +): + namespace = s3gc_module["do_use"].__globals__ + client = FakeCH(blocks=[[("object-a", 10, "time"), ("object-b", 20, "time")]]) + + class SuccessfulMinio: + def remove_objects(self, bucket, objects): + return iter(()) + + monkeypatch.setitem(namespace, "args", args_factory(deletebatchsize=1)) + monkeypatch.setitem(namespace, "ch_client", client) + monkeypatch.setitem(namespace, "minio_client", SuccessfulMinio()) + s3gc_module["do_use"]() + + assert [insert[1] for insert in client.inserts] == [ + [["object-a", 10, "time", False]], + [["object-b", 20, "time", False]], + ] + + +def test_delete_entrypoint_requires_confirmation(): + result = subprocess.run( + ["sh", str(ROOT / "docker/kubernetes-entrypoint.sh")], + env={ + **os.environ, + "S3GC_PHASE": "delete", + "S3GC_CLUSTERNAME": "cluster", + "S3GC_EXPECTED_REPLICAS": "2", + }, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 64 + assert "Refusing delete" in result.stderr + + +def test_dev_automation_entrypoint_runs_collect_dry_run_and_delete(tmp_path): + calls_path = tmp_path / "calls" + fake_python = tmp_path / "python" + fake_python.write_text( + "#!/bin/sh\n" + "printf '%s\\n' \"$*\" >> \"$CALLS_PATH\"\n" + ) + fake_python.chmod(0o755) + + result = subprocess.run( + ["sh", str(ROOT / "docker/kubernetes-entrypoint.sh")], + env={ + **os.environ, + "PATH": f"{tmp_path}:{os.environ['PATH']}", + "CALLS_PATH": str(calls_path), + "S3GC_PHASE": "dev-automation", + "S3GC_DELETE_CONFIRMATION": "DELETE_ORPHANS", + "S3GC_CLUSTERNAME": "cluster", + "S3GC_EXPECTED_REPLICAS": "2", + }, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0 + assert calls_path.read_text().splitlines() == [ + "/app/s3gc.py --collectonly --keepdata --drop-collecttable", + "/app/s3gc.py --usecollected --dry-run", + "/app/s3gc.py --usecollected --keepdata --non-interactive", + ] + + +def test_dev_automation_entrypoint_stops_after_an_error(tmp_path): + calls_path = tmp_path / "calls" + fake_python = tmp_path / "python" + fake_python.write_text( + "#!/bin/sh\n" + "printf '%s\\n' \"$*\" >> \"$CALLS_PATH\"\n" + "case \"$*\" in *--dry-run) exit 42 ;; esac\n" + ) + fake_python.chmod(0o755) + + result = subprocess.run( + ["sh", str(ROOT / "docker/kubernetes-entrypoint.sh")], + env={ + **os.environ, + "PATH": f"{tmp_path}:{os.environ['PATH']}", + "CALLS_PATH": str(calls_path), + "S3GC_PHASE": "dev-automation", + "S3GC_DELETE_CONFIRMATION": "DELETE_ORPHANS", + "S3GC_CLUSTERNAME": "cluster", + "S3GC_EXPECTED_REPLICAS": "2", + }, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 42 + assert calls_path.read_text().splitlines() == [ + "/app/s3gc.py --collectonly --keepdata --drop-collecttable", + "/app/s3gc.py --usecollected --dry-run", + ] + + +@pytest.mark.parametrize( + ("replacement", "message"), + [ + (("PHASE=dry-run", "PHASE=delete"), "delete requires"), + (("PHASE=dry-run", "PHASE=dev-automation"), "dev-automation requires"), + (("ORDER_BY_OBJPATH=false", "ORDER_BY_OBJPATH=yes"), "ORDER_BY_OBJPATH"), + ], +) +def test_renderer_rejects_invalid_configuration(tmp_path, replacement, message): + source = (ROOT / "deploy/kubernetes/example.env").read_text() + config_path = tmp_path / "invalid.env" + config_path.write_text(source.replace(*replacement)) + + result = subprocess.run( + [sys.executable, str(ROOT / "deploy/kubernetes/render.py"), config_path], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 64 + assert message in result.stderr + + +def test_renderer_accepts_confirmed_dev_automation(tmp_path): + source = (ROOT / "deploy/kubernetes/example.env").read_text() + config_path = tmp_path / "dev-automation.env" + config_path.write_text( + source.replace("PHASE=dry-run", "PHASE=dev-automation").replace( + "DELETE_CONFIRMATION=", "DELETE_CONFIRMATION=DELETE_ORPHANS" + ) + ) + + result = subprocess.run( + [sys.executable, str(ROOT / "deploy/kubernetes/render.py"), config_path], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0 + assert 's3gc.altinity.com/phase: "dev-automation"' in result.stdout + + +def test_kubernetes_default_antijoin_does_not_globally_sort( + s3gc_module, args_factory, monkeypatch +): + namespace = s3gc_module["do_use"].__globals__ + client = FakeCH() + monkeypatch.setitem(namespace, "args", args_factory(dryrun_flag=True)) + monkeypatch.setitem(namespace, "ch_client", client) + + s3gc_module["do_use"]() + + assert "ORDER BY s3o.objpath" not in client.stream_query + + +def test_antijoin_ordering_is_an_explicit_opt_in(s3gc_module, args_factory, monkeypatch): + namespace = s3gc_module["do_use"].__globals__ + client = FakeCH() + monkeypatch.setitem( + namespace, "args", args_factory(dryrun_flag=True, order_by_objpath=True) + ) + monkeypatch.setitem(namespace, "ch_client", client) + + s3gc_module["do_use"]() + + assert "ORDER BY s3o.objpath" in client.stream_query + + +def test_delete_transport_failure_reconnects_once( + s3gc_module, args_factory, monkeypatch +): + namespace = s3gc_module["remove_objects_reconnecting"].__globals__ + attempts = [] + + class TransportFailingMinio: + def remove_objects(self, bucket, objects): + attempts.append("failed") + raise s3gc_module["urllib3"].exceptions.ReadTimeoutError( + None, "https://s3.example", "timed out" + ) + + class SuccessfulMinio: + def remove_objects(self, bucket, objects): + attempts.append("success") + return iter(()) + + def reconnect(): + namespace["minio_client"] = SuccessfulMinio() + + monkeypatch.setitem(namespace, "args", args_factory()) + monkeypatch.setitem(namespace, "minio_client", TransportFailingMinio()) + monkeypatch.setitem(namespace, "connect_to_s3", reconnect) + + assert s3gc_module["remove_objects_reconnecting"]([("object-a", 10, "time")]) == [] + assert attempts == ["failed", "success"] + + +# --------------------------------------------------------------------------- +# Regression tests for defects found in production (SUP-30408). +# --------------------------------------------------------------------------- + + +def _load_with_env(monkeypatch, **env): + """Load s3gc.py with the given S3GC_* environment, as a Kubernetes Job would.""" + import runpy + + for key, value in env.items(): + monkeypatch.setenv(key, value) + monkeypatch.setattr(sys, "argv", [str(Path(__file__).resolve().parents[1] / "s3gc.py")]) + return runpy.run_path( + str(Path(__file__).resolve().parents[1] / "s3gc.py"), run_name="s3gc_test" + ) + + +@pytest.mark.parametrize( + "value, expected", + [ + ("false", False), + ("False", False), + ("no", False), + ("off", False), + ("0", False), + ("", False), + ("true", True), + ("True", True), + ("yes", True), + ("on", True), + ("1", True), + ], +) +def test_coerce_bool_parses_env_spellings(s3gc_module, value, expected): + assert s3gc_module["coerce_bool"](value) is expected + + +def test_coerce_bool_passes_through_real_bools_and_none(s3gc_module): + assert s3gc_module["coerce_bool"](True) is True + assert s3gc_module["coerce_bool"](False) is False + assert s3gc_module["coerce_bool"](None) is False + + +def test_coerce_bool_rejects_nonsense(s3gc_module): + with pytest.raises(ValueError): + s3gc_module["coerce_bool"]("maybe") + + +@pytest.mark.parametrize( + "dest, env_name", + [ + ("s3secure_flag", "S3GC_S3SECURE_FLAG"), + ("dryrun_flag", "S3GC_DRYRUN_FLAG"), + ("keepdata_flag", "S3GC_KEEPDATA_FLAG"), + ("order_by_objpath", "S3GC_ORDER_BY_OBJPATH"), + ("verbose_flag", "S3GC_VERBOSE_FLAG"), + ], +) +def test_boolean_env_false_is_false(monkeypatch, dest, env_name): + """S3GC_*=false must not remain the truthy string 'false'.""" + module = _load_with_env(monkeypatch, **{env_name: "false"}) + assert getattr(module["args"], dest) is False + + +def test_bare_cli_flag_still_enables(monkeypatch): + """Coercion must not break `--dryrun` used as a bare flag.""" + import runpy + + root = Path(__file__).resolve().parents[1] + monkeypatch.setattr(sys, "argv", [str(root / "s3gc.py"), "--dryrun"]) + module = runpy.run_path(str(root / "s3gc.py"), run_name="s3gc_test") + assert module["args"].dryrun_flag is True + + +def test_removed_s3useiam_cli_flag_is_rejected(monkeypatch): + import runpy + + root = Path(__file__).resolve().parents[1] + monkeypatch.setattr(sys, "argv", [str(root / "s3gc.py"), "--s3useiam"]) + with pytest.raises(ArgumentError, match="Unrecognized arguments: --s3useiam"): + runpy.run_path(str(root / "s3gc.py"), run_name="s3gc_test") + + +def test_collect_age_filter_uses_total_seconds(s3gc_module, args_factory, monkeypatch): + """--age 24 must keep a 30-day-old object. + + The filter used timedelta.seconds (the sub-day remainder, 0..86399), so + computed age never exceeded 23 h and --age 24 collected nothing at all, + leaving an empty aux table and a dry-run that reported a clean bucket. + """ + import datetime + + namespace = s3gc_module["do_collect"].__globals__ + now = datetime.datetime.now(datetime.timezone.utc) + + class Obj: + def __init__(self, name, age): + self.object_name = name + self.size = 1 + self.last_modified = now - age + + old = Obj("thirty-days-old", datetime.timedelta(days=30, hours=5)) + fresh = Obj("one-hour-old", datetime.timedelta(hours=1)) + + class Minio: + def list_objects(self, bucket, prefix, recursive, start_after): + return iter([old, fresh]) + + class CH: + def __init__(self): + self.rows = [] + + def command(self, query): + return None + + def insert(self, table, rows, column_names): + self.rows.extend(rows) + + ch = CH() + monkeypatch.setitem( + namespace, + "args", + args_factory( + age=24, + collectbatchsize=10, + total=None, + collectafter="", + s3path="", + s3bucket="bucket", + createdatabase_flag=False, + drop_collecttable_flag=False, + ), + ) + monkeypatch.setitem(namespace, "minio_client", Minio()) + monkeypatch.setitem(namespace, "ch_client", ch) + monkeypatch.setitem(namespace, "tname", "`aux`") + + s3gc_module["do_collect"]() + + collected = [row[0] for row in ch.rows] + assert "thirty-days-old" in collected + assert "one-hour-old" not in collected + + +def test_usecollected_without_aux_table_fails_loudly( + s3gc_module, args_factory, monkeypatch +): + """An absent/empty aux table used to exit 0 — indistinguishable from success. + + That is exactly what a load-balanced CHHOST produces, because the aux table + is a node-local ReplacingMergeTree. + """ + namespace = s3gc_module["do_use"].__globals__ + + class EmptyCH(FakeCH): + def command(self, query): + return 0 + + monkeypatch.setitem(namespace, "args", args_factory(dryrun_flag=True, chhost="replica-1")) + monkeypatch.setitem(namespace, "ch_client", EmptyCH()) + monkeypatch.setitem(namespace, "tname", "`aux`") + + with pytest.raises(RuntimeError, match="does not exist or is empty"): + s3gc_module["do_use"]() + + +def test_samples_mismatch_warns(s3gc_module, args_factory, monkeypatch, caplog): + """--samples must match the aux table's PARTITION BY or pruning is lost.""" + namespace = s3gc_module["check_samples_match_partitioning"].__globals__ + + class PartitionedCH: + def query(self, query): + return QueryResult("CRC32(objpath) % 4") + + monkeypatch.setitem(namespace, "args", args_factory(samples=3)) + monkeypatch.setitem(namespace, "ch_client", PartitionedCH()) + monkeypatch.setitem(namespace, "tname", "`aux`") + + with caplog.at_level("WARNING"): + s3gc_module["check_samples_match_partitioning"]() + + assert "does not match" in caplog.text + + +def test_gcs_endpoint_disables_batch_delete(s3gc_module, args_factory, monkeypatch): + """GCS has no batch DeleteObjects; remove_objects() fails there.""" + namespace = s3gc_module["connect_to_s3"].__globals__ + parsed = args_factory( + s3ip="storage.googleapis.com", + s3port=443, + use_remove_objects=True, + s3secure_flag=True, + s3accesskey="k", + s3secretkey="s", + s3region="auto", + s3sslcertfile="", + s3_connect_timeout=15, + s3_read_timeout=120, + s3_retries=3, + ) + monkeypatch.setitem(namespace, "args", parsed) + monkeypatch.setitem(namespace, "Minio", lambda *a, **k: object()) + + s3gc_module["connect_to_s3"]() + + assert parsed.use_remove_objects is False + + +def test_renderer_omits_empty_image_pull_secret(tmp_path): + """A public image needs no pull secret; `- name: ""` would be meaningless.""" + config_path = tmp_path / "public.env" + config_path.write_text((ROOT / "deploy/kubernetes/example.env").read_text()) + + result = subprocess.run( + [sys.executable, str(ROOT / "deploy/kubernetes/render.py"), config_path], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0 + assert "imagePullSecrets" not in result.stdout + assert 'name: ""' not in result.stdout + + +def test_renderer_keeps_configured_image_pull_secret(tmp_path): + """A private mirror must still be able to set one.""" + source = (ROOT / "deploy/kubernetes/example.env").read_text() + config_path = tmp_path / "private.env" + config_path.write_text( + source.replace("IMAGE_PULL_SECRET=", "IMAGE_PULL_SECRET=my-mirror-pull") + ) + + result = subprocess.run( + [sys.executable, str(ROOT / "deploy/kubernetes/render.py"), config_path], + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 0 + assert "imagePullSecrets:" in result.stdout + assert "- name: \"my-mirror-pull\"" in result.stdout + + +# --------------------------------------------------------------------------- +# S3 authentication modes (merged from Altinity/s3gc PR #2, unified with iam). +# --------------------------------------------------------------------------- + + +def _resolve(s3gc_module, monkeypatch, **overrides): + namespace = s3gc_module["resolve_s3_credentials"].__globals__ + args = types.SimpleNamespace( + s3auth="static", s3profile="", + s3accesskey="", s3secretkey="", s3sessiontoken="", s3region="eu-central-1", + ) + for key, value in overrides.items(): + setattr(args, key, value) + monkeypatch.setitem(namespace, "args", args) + return s3gc_module["resolve_s3_credentials"]() + + +def test_static_mode_returns_supplied_keys(s3gc_module, monkeypatch): + result = _resolve(s3gc_module, monkeypatch, s3accesskey="AK", s3secretkey="SK") + assert result[0] == "AK" and result[1] == "SK" + assert result[2] is None # no session token + assert result[4] == "static" + + +def test_static_mode_carries_session_token(s3gc_module, monkeypatch): + result = _resolve( + s3gc_module, monkeypatch, s3accesskey="AK", s3secretkey="SK", s3sessiontoken="TOKEN" + ) + assert result[2] == "TOKEN" + + +def test_static_mode_without_keys_is_anonymous(s3gc_module, monkeypatch): + assert _resolve(s3gc_module, monkeypatch)[4] == "anonymous" + + +@pytest.mark.parametrize( + "overrides, message", + [ + ({"s3accesskey": "AK"}, "must be specified together"), + ({"s3secretkey": "SK"}, "must be specified together"), + ({"s3sessiontoken": "TOKEN"}, "requires s3accesskey"), + ], +) +def test_static_mode_rejects_incomplete_credentials(s3gc_module, monkeypatch, overrides, message): + with pytest.raises(ValueError, match=message): + _resolve(s3gc_module, monkeypatch, **overrides) + + +def test_iam_mode_defers_to_the_provider(s3gc_module, monkeypatch): + """iam returns no keys: MinIO gets the provider so it can refresh them.""" + result = _resolve(s3gc_module, monkeypatch, s3auth="iam") + assert result[:3] == (None, None, None) + assert result[4] == "iam" + + +def test_s3profile_implies_aws_mode(s3gc_module, monkeypatch): + calls = [] + namespace = s3gc_module["resolve_s3_credentials"].__globals__ + monkeypatch.setitem( + namespace, "resolve_aws_s3_credentials", + lambda: calls.append("aws") or (None, None, None, "eu-central-1", "aws"), + ) + assert _resolve(s3gc_module, monkeypatch, s3profile="sso")[4] == "aws" + assert calls == ["aws"] + + +def test_unknown_auth_mode_is_rejected(s3gc_module, monkeypatch): + with pytest.raises(ValueError, match="s3auth must be one of"): + _resolve(s3gc_module, monkeypatch, s3auth="magic") + + +@pytest.mark.parametrize( + "overrides", + [ + {"s3auth": "iam", "s3profile": "sso"}, # profile implies aws, conflicts with iam + ], +) +def test_contradictory_auth_settings_error(s3gc_module, monkeypatch, overrides): + """A contradiction must fail, not silently pick a winner and send the wrong identity.""" + with pytest.raises(ValueError, match="conflicts with"): + _resolve(s3gc_module, monkeypatch, **overrides) + + +def test_aws_mode_rejects_explicit_keys(s3gc_module, monkeypatch): + with pytest.raises(ValueError, match="cannot be combined with explicit"): + _resolve(s3gc_module, monkeypatch, s3auth="aws", s3accesskey="AK", s3secretkey="SK") + + +def test_aws_mode_without_boto3_is_user_visible(s3gc_module, monkeypatch): + """A missing optional dependency must not surface as a bare ImportError.""" + import builtins + + real_import = builtins.__import__ + + def fake_import(name, *rest): + if name == "boto3": + raise ImportError("no boto3") + return real_import(name, *rest) + + monkeypatch.setattr(builtins, "__import__", fake_import) + with pytest.raises(s3gc_module["UserVisibleError"], match="boto3 is required"): + _resolve(s3gc_module, monkeypatch, s3auth="aws") + + +def test_format_s3_list_error_names_the_permission(s3gc_module, args_factory, monkeypatch): + """The listing failure must tell the operator exactly what to grant.""" + namespace = s3gc_module["format_s3_list_error"].__globals__ + monkeypatch.setitem( + namespace, "args", args_factory(s3bucket="my-bucket", s3path="pre/fix/", s3profile="sso") + ) + + class Err: + code = "AccessDenied" + message = "denied" + + text = s3gc_module["format_s3_list_error"](Err()) + assert "s3:ListBucket" in text + assert "my-bucket" in text and "pre/fix/" in text + assert "even with --dry-run" in text + assert "--profile sso" in text + + +def test_iam_mode_keeps_the_hardened_transport(s3gc_module, args_factory, monkeypatch): + """The merge must not revert to a bare PoolManager: that hung a run for 2h19m.""" + namespace = s3gc_module["connect_to_s3"].__globals__ + captured = {} + monkeypatch.setitem(namespace, "Minio", lambda endpoint, **kw: captured.update(kw) or object()) + monkeypatch.setitem( + namespace, "args", + args_factory(s3auth="iam", s3ip="s3.eu-central-1.amazonaws.com", s3port=443, + s3secure_flag=True, s3sslcertfile="", s3_connect_timeout=15, + s3_read_timeout=120, s3_retries=3), + ) + s3gc_module["connect_to_s3"]() + + assert "credentials" in captured # provider, not frozen keys + assert "access_key" not in captured + timeout = captured["http_client"].connection_pool_kw["timeout"] + assert timeout.read_timeout == 120 and timeout.connect_timeout == 15 + + +@pytest.mark.parametrize( + ("replacement", "message"), + [ + (("S3AUTH=iam", "S3AUTH=magic"), "S3AUTH must be static, aws or iam"), + (("S3PROFILE=", "S3PROFILE=sso"), "S3PROFILE requires S3AUTH=aws"), + ], +) +def test_renderer_validates_auth_configuration(tmp_path, replacement, message): + source = (ROOT / "deploy/kubernetes/example.env").read_text() + config_path = tmp_path / "auth.env" + config_path.write_text(source.replace(*replacement)) + + result = subprocess.run( + [sys.executable, str(ROOT / "deploy/kubernetes/render.py"), config_path], + capture_output=True, text=True, check=False, + ) + + assert result.returncode == 64 + assert message in result.stderr + + +def test_renderer_wires_auth_env_into_the_job(tmp_path): + """PR #2's flags were unreachable from a Job until the template exposed them.""" + source = (ROOT / "deploy/kubernetes/example.env").read_text() + config_path = tmp_path / "aws.env" + config_path.write_text(source.replace("S3AUTH=iam", "S3AUTH=aws").replace("S3PROFILE=", "S3PROFILE=sso")) + + result = subprocess.run( + [sys.executable, str(ROOT / "deploy/kubernetes/render.py"), config_path], + capture_output=True, text=True, check=False, + ) + + assert result.returncode == 0 + assert 'name: S3GC_S3AUTH' in result.stdout + assert 'value: "aws"' in result.stdout + assert 'value: "sso"' in result.stdout