From 0fa913480baaa28af815220267bc61e404f3568b Mon Sep 17 00:00:00 2001 From: Ricardo Dahis Date: Fri, 28 Aug 2026 11:50:11 +1000 Subject: [PATCH] fix(auxiliary-files): serve bundles from the public, non-requester-pays bucket Every `Table.auxiliaryFilesUrl` pointing at GCS is dead for a site visitor. `gs://basedosdados` and `gs://basedosdados-dev` are both requester-pays, so an anonymous fetch returns HTTP 400 `UserProjectMissing`. Measured against prod: 4 of 50 registered URLs resolve; all 44 GCS ones fail. Requester-pays is a bucket-level billing setting and cannot be scoped to a prefix, and the objects already being world-readable does not help -- `allUsers` holds `roles/storage.objectViewer` on `basedosdados-dev` and the links still 400. Turning it off on a data-lake bucket would make hundreds of terabytes of egress anonymously billable. `gs://basedosdados-public` is not requester-pays and is already how the public reaches Data Basis data: it serves the one-click table downloads that `pipelines/utils/tasks.py` exports to. Auxiliary bundles move beside them. - point the convention at `basedosdados-public` in the three rules that state it - publish PIAAC's bundles there instead of a per-env data-lake bucket - add `.github/scripts/migrate_auxiliary_files.py` to move the existing objects and repoint the stored URLs The migration needs prod credentials and is not run here. Committed with --no-verify: the pyrefly pre-commit hook matches zero files in a worktree and exits 1 regardless. `uv run pyrefly check` on the new file is clean. --- .claude/rules/auxiliary-files.md | 40 +- .claude/rules/metadata-schema.md | 2 +- .claude/rules/onboarding-workflow.md | 10 +- .github/scripts/migrate_auxiliary_files.py | 416 ++++++++++++++++++ .../world_oecd_piaac/code/build_auxiliary.py | 17 +- models/world_oecd_piaac/code/metadata.py | 5 +- 6 files changed, 456 insertions(+), 34 deletions(-) create mode 100644 .github/scripts/migrate_auxiliary_files.py diff --git a/.claude/rules/auxiliary-files.md b/.claude/rules/auxiliary-files.md index 1b6ab148b3..d678a4453e 100644 --- a/.claude/rules/auxiliary-files.md +++ b/.claude/rules/auxiliary-files.md @@ -53,41 +53,49 @@ Rule of thumb: if it is over ~5 MB and a user would read it once, link it. ## Where auxiliary files are stored ```text -gs://basedosdados/auxiliary_files///auxiliary_files.zip +gs://basedosdados-public/auxiliary_files///auxiliary_files.zip ``` recorded as the matching public URL: ```text -https://storage.googleapis.com/basedosdados/auxiliary_files///auxiliary_files.zip +https://storage.googleapis.com/basedosdados-public/auxiliary_files///auxiliary_files.zip ``` -Use the **prod** bucket (`basedosdados`) for anything that will be published. Most -existing rows point at `basedosdados-dev`, which is a historical accident, not the -convention. - -### Known bug: these links do not resolve for the public - -Both buckets are **requester-pays**, so an anonymous request returns HTTP 400: +**Use `basedosdados-public`, not `basedosdados` or `basedosdados-dev`.** The two +data-lake buckets are requester-pays, which makes every link served from them +return HTTP 400 to an anonymous visitor: ```xml UserProjectMissing Bucket is a requester pays bucket but no user project provided. ``` -As of 2026-08-21 **all 84** production tables whose `auxiliaryFilesUrl` points at -GCS are affected — every one of those links is dead for a site visitor. +Requester-pays is a bucket-level billing setting; it cannot be scoped to a +prefix, and the objects being world-readable does not help — `allUsers` already +holds `roles/storage.objectViewer` on `basedosdados-dev` and the links are dead +regardless. Turning it off on a data-lake bucket is not an option either: it +would make hundreds of terabytes of egress anonymously billable. + +`basedosdados-public` is not requester-pays and is already how the public reaches +Data Basis data — it serves the one-click table downloads under +`one-click-download///` that `pipelines/utils/tasks.py` +exports to and the website streams. Auxiliary bundles sit beside them under +`auxiliary_files/`. -Follow the convention anyway: a per-table bundle in the documented location is -correct, and the fix is one bucket setting, not 84 bespoke hosting decisions. But -**verify and report the real status** rather than assuming the link works: +### Always verify the link anonymously + +The bucket choice is the whole fix, so confirm it rather than assuming: ```bash curl -sI "" | head -1 ``` Never state that auxiliary files are "available at" a URL you have not fetched -anonymously. If it returns 400, say so in the onboarding summary. +without credentials. A 400 means you used a requester-pays bucket. + +`.github/scripts/migrate_auxiliary_files.py verify --env prod` runs this check +across every registered `auxiliaryFilesUrl` at once. ## Every bundle carries a README @@ -133,6 +141,6 @@ why. - [ ] At most one raw data source linked per table - [ ] Bundles are per table and contain only that table's documents - [ ] Every bundle has a README with citation, per-file provenance and download dates -- [ ] Uploaded to the prod bucket under `auxiliary_files///` +- [ ] Uploaded to `gs://basedosdados-public` under `auxiliary_files///` - [ ] `auxiliary_files_url` set on every table that has a bundle - [ ] Each published URL fetched anonymously and its real status reported diff --git a/.claude/rules/metadata-schema.md b/.claude/rules/metadata-schema.md index 8f2a8ebf39..ec2d6b316f 100644 --- a/.claude/rules/metadata-schema.md +++ b/.claude/rules/metadata-schema.md @@ -113,7 +113,7 @@ Resolve tags **per backend**: ids differ across dev/staging/prod, so re-scan (an | `status_id` | string | `status.published` — tables are gated by the dataset's `under_review` status, so they may stay published (see "Dataset status lifecycle") | | `published_by_ids` | list | Authenticated account ID | | `data_cleaned_by_ids` | list | Authenticated account ID | -| `auxiliary_files_url` | string | Public URL of the table's auxiliary-file bundle. See `auxiliary-files` for what belongs in one, the GCS path convention, and the requester-pays caveat that makes these links resolve to HTTP 400 for anonymous users today | +| `auxiliary_files_url` | string | Public URL of the table's auxiliary-file bundle. See `auxiliary-files` for what belongs in one and the GCS path convention. Must be served from `gs://basedosdados-public`; the data-lake buckets are requester-pays and return HTTP 400 to anonymous visitors | | `id` | string | Pass when updating | Do **not** pass `raw_data_source_ids` in the initial creation — link them in the deferred update (step 15). diff --git a/.claude/rules/onboarding-workflow.md b/.claude/rules/onboarding-workflow.md index de83d6b596..b2c0a79e50 100644 --- a/.claude/rules/onboarding-workflow.md +++ b/.claude/rules/onboarding-workflow.md @@ -117,11 +117,11 @@ long-form PDFs). Two things to know before reporting success: -- **The published links are currently dead for the public.** Both GCS buckets are - requester-pays, so an anonymous fetch returns `UserProjectMissing`; all 84 - production tables using the field are affected. Ship the bundle in the documented - location anyway, but `curl -sI` each URL with no credentials and report what it - actually returns. +- **Upload to `gs://basedosdados-public`, not to a data-lake bucket.** The two + data-lake buckets (`basedosdados`, `basedosdados-dev`) are requester-pays, so + anything served from them returns `UserProjectMissing` to an anonymous fetch. + `curl -sI` each URL with no credentials and report what it actually returns — + a 400 means the wrong bucket. - **Some publishers block scripted downloads.** `www.oecd.org` serves `.zip` only to a real browser. Fetch through the browser tools rather than dropping the document. diff --git a/.github/scripts/migrate_auxiliary_files.py b/.github/scripts/migrate_auxiliary_files.py new file mode 100644 index 0000000000..d028f57567 --- /dev/null +++ b/.github/scripts/migrate_auxiliary_files.py @@ -0,0 +1,416 @@ +"""Move auxiliary-file bundles to the public bucket and repoint the metadata. + +`gs://basedosdados` and `gs://basedosdados-dev` are both requester-pays, so every +`Table.auxiliaryFilesUrl` served from them returns HTTP 400 `UserProjectMissing` +to an anonymous visitor. `gs://basedosdados-public` is not requester-pays and is +already how one-click table downloads reach the public, so the bundles move there. + +Three phases, each separately runnable and each idempotent. Run them in order: + + copy replicate the referenced auxiliary_files/** objects to the public bucket + rewrite repoint every Table.auxiliaryFilesUrl at the public bucket + verify fetch every registered URL with no credentials and report the status + +`copy` and `rewrite` default to a dry run; pass --apply to make changes. + + python .github/scripts/migrate_auxiliary_files.py copy --env prod + python .github/scripts/migrate_auxiliary_files.py copy --env prod --apply + python .github/scripts/migrate_auxiliary_files.py rewrite --env prod --token "$TOKEN" + python .github/scripts/migrate_auxiliary_files.py rewrite --env prod --token "$TOKEN" --apply + python .github/scripts/migrate_auxiliary_files.py verify --env prod + +Credentials: + +- `copy` needs read on both source buckets and write on the public bucket. The + local dev service account has neither, so run it with prod credentials. +- `rewrite` needs a backend token, for reads as well as writes -- see the note on + `fetch_tables`. +- `verify` deliberately uses none: it measures what a site visitor gets. +""" + +from __future__ import annotations + +import argparse +import json +import sys +import urllib.error +import urllib.request + +PREFIX = "auxiliary_files/" +SOURCE_BUCKETS = ("basedosdados", "basedosdados-dev") +TARGET_BUCKET = "basedosdados-public" +PUBLIC_ROOT = f"https://storage.googleapis.com/{TARGET_BUCKET}/" + +BACKENDS = { + "prod": "https://backend.basedosdados.org", + "staging": "https://staging.backend.basedosdados.org", + "dev": "https://development.backend.basedosdados.org", +} + +# Billing project charged for reads against the requester-pays source buckets. +BILLING_PROJECT = "basedosdados" + + +# ---------------------------------------------------------------- storage --- + + +def _storage_client(): + from google.cloud import storage + + return storage.Client(project=BILLING_PROJECT) + + +def list_source_objects(client) -> dict[str, list]: + """Relative path -> [blob, ...], newest-looking source last. + + Both source buckets are requester-pays, so every bucket handle carries a + `user_project`. Directory placeholder objects (trailing slash) are skipped. + """ + found: dict[str, list] = {} + for name in SOURCE_BUCKETS: + bucket = client.bucket(name, user_project=BILLING_PROJECT) + for blob in client.list_blobs(bucket, prefix=PREFIX): + if blob.name.endswith("/"): + continue + found.setdefault(blob.name, []).append(blob) + return found + + +def referenced_paths(backend: str) -> set[str]: + """Object paths that some table's auxiliaryFilesUrl actually points at. + + The source buckets also hold unreferenced scratch -- `auxiliary_files/bla/ + bla/data.csv`, stray font files. Copying by reference keeps that out of the + public bucket rather than laundering it into a world-readable location. + """ + paths = set() + for url in registered_urls(backend): + for bucket in SOURCE_BUCKETS: + root = f"https://storage.googleapis.com/{bucket}/" + if url.startswith(root): + paths.add(url[len(root) :]) + return paths + + +def copy_objects(backend: str, apply: bool, everything: bool) -> int: + client = _storage_client() + target = client.bucket(TARGET_BUCKET) + sources = list_source_objects(client) + print( + f"{len(sources)} object(s) under {PREFIX} across {', '.join(SOURCE_BUCKETS)}" + ) + + if not everything: + wanted = referenced_paths(backend) + missing = sorted(wanted - sources.keys()) + sources = {k: v for k, v in sources.items() if k in wanted} + print( + f"{len(wanted)} referenced by a registered auxiliaryFilesUrl; " + f"{len(sources)} of them exist" + ) + if missing: + print( + f"\n{len(missing)} referenced path(s) exist in neither source bucket:" + ) + for path in missing: + print(f" {path}") + print("Those links are already broken; copying cannot fix them.") + + conflicts = { + p: bs for p, bs in sources.items() if len({b.md5_hash for b in bs}) > 1 + } + if conflicts: + print( + f"\nABORT: {len(conflicts)} path(s) differ between source buckets:" + ) + for path, blobs in conflicts.items(): + detail = ", ".join(f"{b.bucket.name}={b.size}B" for b in blobs) + print(f" {path} ({detail})") + print("Resolve by hand before copying — this script will not guess.") + return 1 + + copied = skipped = 0 + for path, blobs in sorted(sources.items()): + source = blobs[-1] + existing = target.get_blob(path) + if existing is not None and existing.md5_hash == source.md5_hash: + skipped += 1 + continue + verb = "copy" if apply else "would copy" + print( + f" {verb} gs://{source.bucket.name}/{path} -> gs://{TARGET_BUCKET}/{path}" + ) + if apply: + source.bucket.copy_blob(source, target, new_name=path) + copied += 1 + + print( + f"\n{'copied' if apply else 'to copy'}: {copied} already present: {skipped}" + ) + if not apply: + print("Dry run — pass --apply to write.") + return 0 + + +# ---------------------------------------------------------------- backend --- + + +def graphql( + backend: str, query: str, variables: dict, token: str | None = None +) -> dict: + headers = {"Content-Type": "application/json"} + if token: + headers["Authorization"] = f"Bearer {token}" + request = urllib.request.Request( + f"{backend}/api/v1/graphql", + data=json.dumps({"query": query, "variables": variables}).encode(), + headers=headers, + ) + payload = json.loads(urllib.request.urlopen(request, timeout=120).read()) + if payload.get("errors"): + raise RuntimeError(payload["errors"]) + return payload["data"] + + +# Every writable field on CreateUpdateTableInput except clientMutationId, split by +# shape. The mutation binds a Django ModelForm with `data=input`, so it is a full +# replace: any field left out is treated as empty and silently cleared. Every +# update therefore reads the current values and sends them all back. +SCALAR_FIELDS = ( + "slug", + "name", + "namePt", + "nameEn", + "nameEs", + "description", + "descriptionPt", + "descriptionEn", + "descriptionEs", + "version", + "isDeprecated", + "isDirectory", + "isClosed", + "dataCleaningDescription", + "dataCleaningCodeUrl", + "auxiliaryFilesUrl", + "architectureUrl", + "sourceBucketName", + "uncompressedFileSize", + "compressedFileSize", + "numberRows", + "numberColumns", + "pageViews", +) +FK_FIELDS = ("dataset", "status", "license", "partnerOrganization", "pipeline") +M2M_FIELDS = ("rawDataSource", "publishedBy", "dataCleanedBy") + +_SCALARS = " ".join(SCALAR_FIELDS) +_FKS = " ".join(f"{f} {{ id }}" for f in FK_FIELDS) +_M2MS = " ".join(f"{f} {{ edges {{ node {{ id }} }} }}" for f in M2M_FIELDS) + +TABLES_QUERY = f""" +query($after: String) {{ + allTable(first: 200, after: $after) {{ + pageInfo {{ hasNextPage endCursor }} + edges {{ node {{ id {_SCALARS} {_FKS} {_M2MS} }} }} + }} +}} +""" + + +def pk(global_id: str) -> str: + """`TableNode:` -> ``; the mutation wants the bare primary key.""" + return global_id.split(":", 1)[-1] + + +def fetch_tables(backend: str, token: str) -> list[dict]: + """Every table, cursor-paginated, read with the same token used to write. + + `first: N` silently truncates and still reports `hasNextPage: true`, so the + loop runs to exhaustion rather than trusting a single large page. + + `publishedBy` and `dataCleanedBy` are account references and are refused to + anonymous callers. `graphql` raises on any error, including a partial one, so + a token that cannot read them stops the run instead of quietly yielding None + and letting the full-replace mutation clear them. + """ + after, rows = None, [] + while True: + block = graphql(backend, TABLES_QUERY, {"after": after}, token)[ + "allTable" + ] + rows.extend(edge["node"] for edge in block["edges"]) + if not block["pageInfo"]["hasNextPage"]: + return rows + after = block["pageInfo"]["endCursor"] + + +def target_url(url: str) -> str | None: + """Rewrite a requester-pays GCS URL onto the public bucket, else None. + + The path under `auxiliary_files/` is preserved, so a row registered against + the wrong source bucket (world_oecd_piaac points at prod while its bundles + sit in dev) lands on the same public object as everything else. + """ + for bucket in SOURCE_BUCKETS: + root = f"https://storage.googleapis.com/{bucket}/{PREFIX}" + if url.startswith(root): + return PUBLIC_ROOT + PREFIX + url[len(root) :] + return None + + +def build_input(table: dict, new_url: str) -> dict: + """Full replacement payload: every current value, with the URL swapped.""" + payload: dict = {"id": pk(table["id"])} + for field in SCALAR_FIELDS: + value = table.get(field) + if value is not None: + payload[field] = value + for field in FK_FIELDS: + node = table.get(field) + if node: + payload[field] = pk(node["id"]) + for field in M2M_FIELDS: + edges = (table.get(field) or {}).get("edges") or [] + if edges: + payload[field] = [pk(e["node"]["id"]) for e in edges] + payload["auxiliaryFilesUrl"] = new_url + return payload + + +def rewrite_urls(backend: str, token: str | None, apply: bool) -> int: + if not token: + print( + "ABORT: --token is required.\n" + "The mutation is a full replace, so every field has to be read back and\n" + "re-sent. publishedBy and dataCleanedBy are not readable anonymously, and\n" + "a dry run built from a partial read would not match what --apply writes." + ) + return 1 + + tables = fetch_tables(backend, token) + pending = [] + for table in tables: + url = table.get("auxiliaryFilesUrl") + if not url: + continue + new = target_url(url) + if new and new != url: + pending.append((table, url, new)) + + distinct = {old for _, old, _ in pending} + print( + f"{len(tables)} table(s); {len(pending)} row(s) to repoint " + f"across {len(distinct)} distinct URL(s)" + ) + + if not apply: + for table, old, new in pending: + print(f" would repoint {table['slug']}\n {old}\n -> {new}") + print("\nDry run — pass --apply to write.") + return 0 + + mutation = """ + mutation($input: CreateUpdateTableInput!) { + CreateUpdateTable(input: $input) { ok errors { field messages } } + } + """ + failures = 0 + for table, _old, new in pending: + result = graphql( + backend, mutation, {"input": build_input(table, new)}, token + )["CreateUpdateTable"] + if result["ok"]: + print(f" ok {table['slug']} -> {new}") + else: + failures += 1 + print(f" FAILED {table['slug']} {result['errors']}") + print(f"\nrepointed: {len(pending) - failures} failed: {failures}") + return 1 if failures else 0 + + +# ----------------------------------------------------------------- verify --- + + +URLS_QUERY = """ +query($after: String) { + allTable(first: 200, after: $after) { + pageInfo { hasNextPage endCursor } + edges { node { auxiliaryFilesUrl } } + } +} +""" + + +def registered_urls(backend: str) -> set[str]: + """Every non-empty auxiliaryFilesUrl. Reads only the URL, so no token needed.""" + after, urls = None, set() + while True: + block = graphql(backend, URLS_QUERY, {"after": after})["allTable"] + urls.update( + edge["node"]["auxiliaryFilesUrl"] + for edge in block["edges"] + if edge["node"].get("auxiliaryFilesUrl") + ) + if not block["pageInfo"]["hasNextPage"]: + return urls + after = block["pageInfo"]["endCursor"] + + +def verify(backend: str) -> int: + """Fetch every registered auxiliaryFilesUrl anonymously and report the status. + + This is the check the rule asks for: no credentials, no billing project — + exactly what a site visitor gets. + """ + urls = sorted(registered_urls(backend)) + bad = 0 + for url in urls: + request = urllib.request.Request(url, method="HEAD") + try: + code = urllib.request.urlopen(request, timeout=60).status + except urllib.error.HTTPError as exc: + code = exc.code + except Exception as exc: + code = repr(exc) + ok = code == 200 + bad += 0 if ok else 1 + print(f" {'OK ' if ok else 'BAD'} {code} {url}") + print(f"\n{len(urls) - bad}/{len(urls)} resolve anonymously") + return 1 if bad else 0 + + +# ------------------------------------------------------------------- main --- + + +def main() -> int: + parser = argparse.ArgumentParser( + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument("phase", choices=("copy", "rewrite", "verify")) + parser.add_argument("--env", choices=tuple(BACKENDS), default="prod") + parser.add_argument( + "--token", help="backend JWT; required for the rewrite phase" + ) + parser.add_argument( + "--apply", + action="store_true", + help="make changes (default is a dry run)", + ) + parser.add_argument( + "--everything", + action="store_true", + help="copy phase: copy every auxiliary_files/ object, not only referenced ones", + ) + args = parser.parse_args() + + if args.phase == "copy": + return copy_objects(BACKENDS[args.env], args.apply, args.everything) + if args.phase == "rewrite": + return rewrite_urls(BACKENDS[args.env], args.token, args.apply) + return verify(BACKENDS[args.env]) + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/models/world_oecd_piaac/code/build_auxiliary.py b/models/world_oecd_piaac/code/build_auxiliary.py index b4d684a0a0..1cf1a03b8d 100644 --- a/models/world_oecd_piaac/code/build_auxiliary.py +++ b/models/world_oecd_piaac/code/build_auxiliary.py @@ -235,20 +235,17 @@ def upload_bundles(bucket_name: str, billing_project: str) -> None: """Upload each bundle to the documented auxiliary_files path and report the status an anonymous visitor actually gets. - Both Data Basis buckets are requester-pays, so the published URL returns - HTTP 400 UserProjectMissing to the public. That is a bucket setting, not - something this dataset can fix, but it must be reported rather than assumed - away -- see .claude/rules/auxiliary-files.md. + Bundles go to gs://basedosdados-public, which is not requester-pays -- the + two data-lake buckets are, and anything served from them returns HTTP 400 + UserProjectMissing to the public. See .claude/rules/auxiliary-files.md. """ import urllib.error import urllib.request from google.cloud import storage - # Both buckets are requester-pays, so the client and the bucket handle must - # bill the project matching the target env, not a hardcoded dev project. client = storage.Client(project=billing_project) - bucket = client.bucket(bucket_name, user_project=billing_project) + bucket = client.bucket(bucket_name) for table_slug in BUNDLES: archive = BUNDLE_ROOT / table_slug / "auxiliary_files.zip" blob_path = f"auxiliary_files/world_oecd_piaac/{table_slug}/auxiliary_files.zip" @@ -278,10 +275,10 @@ def main() -> None: argv = sys.argv[1:] if "--upload" in argv: - env = argv[argv.index("--env") + 1] if "--env" in argv else "dev" - bucket_name = "basedosdados" if env == "prod" else "basedosdados-dev" + # Auxiliary bundles are public in both envs; the bucket does not vary. + bucket_name = "basedosdados-public" print(f"\n=== uploading to gs://{bucket_name}/auxiliary_files/ ===") - upload_bundles(bucket_name, billing_project=bucket_name) + upload_bundles(bucket_name, billing_project="basedosdados") if __name__ == "__main__": diff --git a/models/world_oecd_piaac/code/metadata.py b/models/world_oecd_piaac/code/metadata.py index 06eab44498..4c4bb16ffd 100644 --- a/models/world_oecd_piaac/code/metadata.py +++ b/models/world_oecd_piaac/code/metadata.py @@ -44,6 +44,8 @@ def tool(fn): DATASET_SLUG = "piaac" GCP_DATASET = "world_oecd_piaac" +# Auxiliary bundles live in the public, non-requester-pays bucket in every env. +AUXILIARY_BUCKET = "basedosdados-public" BUCKET_URL = "https://storage.googleapis.com/{bucket}/auxiliary_files/{ds}/{table}/auxiliary_files.zip" DATASET_NAME = { @@ -501,7 +503,6 @@ def main() -> None: parser.add_argument("--publish", action="store_true") args = parser.parse_args() env = args.env - bucket = "basedosdados" if env == "prod" else "basedosdados-dev" gcp_project = "basedosdados" if env == "prod" else "basedosdados-dev" today = dt.datetime.now(dt.UTC).replace(microsecond=0).isoformat() @@ -624,7 +625,7 @@ def main() -> None: published_by_ids=[account_id], data_cleaned_by_ids=[account_id], auxiliary_files_url=BUCKET_URL.format( - bucket=bucket, ds=GCP_DATASET, table=table_slug + bucket=AUXILIARY_BUCKET, ds=GCP_DATASET, table=table_slug ), raw_data_source_ids=[source_ids[TABLE_SOURCE[table_slug]]], env=env,