From 60b8b409f52ccc42af2cd18eb959f4264bfdf705 Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Mon, 30 Mar 2026 14:42:02 +0100 Subject: [PATCH 1/3] fixup role Signed-off-by: Joe Isaacs --- .github/workflows/compat-gen-upload.yml | 9 +++- vortex-test/compat-gen/scripts/compat.py | 68 ++++++++++++++++++++---- 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/.github/workflows/compat-gen-upload.yml b/.github/workflows/compat-gen-upload.yml index f9e28a85a6d..da17ec9a28c 100644 --- a/.github/workflows/compat-gen-upload.yml +++ b/.github/workflows/compat-gen-upload.yml @@ -28,6 +28,7 @@ jobs: && format('runs-on={0}/runner=amd64-medium/image=ubuntu24-full-x64-pre-v2/tag=compat-gen-dry-run', github.run_id) || 'ubuntu-latest' }} permissions: + id-token: write contents: read outputs: version: ${{ steps.detect.outputs.version }} @@ -41,6 +42,12 @@ jobs: fetch-depth: 0 - uses: ./.github/actions/setup-prebuild + - name: Configure AWS credentials + uses: aws-actions/configure-aws-credentials@v6 + with: + role-to-assume: "arn:aws:iam::245040174862:role/GitHubCompatUploadRole" + aws-region: us-east-1 + - name: Detect version id: detect run: | @@ -87,7 +94,7 @@ jobs: - name: Configure AWS credentials uses: aws-actions/configure-aws-credentials@v6 with: - role-to-assume: "arn:aws:iam::245040174862:role/GitHubBenchmarkRole" + role-to-assume: "arn:aws:iam::245040174862:role/GitHubCompatUploadRole" aws-region: us-east-1 - name: Upload fixtures for v${{ needs.dry-run.outputs.version }} diff --git a/vortex-test/compat-gen/scripts/compat.py b/vortex-test/compat-gen/scripts/compat.py index 9f5d874b840..797087b51ad 100644 --- a/vortex-test/compat-gen/scripts/compat.py +++ b/vortex-test/compat-gen/scripts/compat.py @@ -211,6 +211,50 @@ def display_name(self) -> str: return f"s3://{self.bucket}" +class DryRunStore(Store): + """Wrapper that validates write credentials without actually uploading. + + Reads are delegated to the inner store. Writes use ``aws s3 cp --dryrun`` + (for S3 stores) so that IAM permissions are verified, but no data is + written. For local stores, writes are silently skipped. + """ + + def __init__(self, inner: Store): + self.inner = inner + + def read(self, key: str) -> bytes | None: + return self.inner.read(key) + + def write(self, key: str, data: bytes) -> None: + if isinstance(self.inner, S3Store): + with tempfile.NamedTemporaryFile(delete=False) as f: + f.write(data) + tmp_path = f.name + try: + self.write_file(key, Path(tmp_path)) + finally: + os.unlink(tmp_path) + else: + _info(f" dry-run: would write {key}") + + def write_file(self, key: str, local_path: Path) -> None: + if isinstance(self.inner, S3Store): + dest = f"s3://{self.inner.bucket}/{key}" + _info(f" {local_path.name} -> {dest} (dry-run)") + subprocess.run( + ["aws", "s3", "cp", str(local_path), dest, "--dryrun"], + check=True, + ) + else: + _info(f" dry-run: would write {key}") + + def list_versions(self) -> list[str]: + return self.inner.list_versions() + + def display_name(self) -> str: + return self.inner.display_name() + + def _parse_store(spec: str) -> Store: """Parse a store specification into a Store instance.""" if spec.startswith("s3://"): @@ -424,16 +468,19 @@ def _publish_full( _info(f" removed fixtures: {', '.join(sorted(removed))}") if not added and not removed: _info(f" same {len(new_names)} fixtures as existing") - _info(" target paths:") - for entry in manifest["fixtures"]: - _info(f" {store.display_name()}/v{version}/arrays/{entry['name']}") - _info(f" {store.display_name()}/v{version}/arrays/manifest.json") - _info(f" {store.display_name()}/versions.json") if version not in versions: updated_versions = sorted(versions + [version], key=_version_sort_key) _info(f" versions.json would update: {versions} -> {updated_versions}") else: _info(f" versions.json unchanged: {versions}") + + dry_store = DryRunStore(store) + _info(f"dry-run uploading {len(manifest['fixtures'])} fixtures (verifying credentials)...") + _parallel_upload( + dry_store, + [(f"v{version}/arrays/{e['name']}", output / e["name"]) for e in manifest["fixtures"]], + ) + _info("dry-run upload succeeded — credentials are valid") return if not args.yes: @@ -511,10 +558,13 @@ def _publish_update( return if args.dry_run: - _info(f"dry run — would upload {len(new_fixtures)} new fixture(s):") - for name in new_fixtures: - _info(f" {store.display_name()}/{prefix}/{name}") - _info(f" {store.display_name()}/{prefix}/manifest.json (updated)") + dry_store = DryRunStore(store) + _info(f"dry-run uploading {len(new_fixtures)} new fixture(s) (verifying credentials)...") + _parallel_upload( + dry_store, + [(f"{prefix}/{name}", output / name) for name in new_fixtures], + ) + _info("dry-run upload succeeded — credentials are valid") return if not args.yes: From 28b4ee46318b220c51360c97a2fe803f7bef7d1b Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Mon, 30 Mar 2026 14:49:57 +0100 Subject: [PATCH 2/3] fixup role Signed-off-by: Joe Isaacs --- vortex-test/compat-gen/scripts/compat.py | 111 +++++++---------------- 1 file changed, 34 insertions(+), 77 deletions(-) diff --git a/vortex-test/compat-gen/scripts/compat.py b/vortex-test/compat-gen/scripts/compat.py index 797087b51ad..1f74aa7f78c 100644 --- a/vortex-test/compat-gen/scripts/compat.py +++ b/vortex-test/compat-gen/scripts/compat.py @@ -226,16 +226,13 @@ def read(self, key: str) -> bytes | None: return self.inner.read(key) def write(self, key: str, data: bytes) -> None: - if isinstance(self.inner, S3Store): - with tempfile.NamedTemporaryFile(delete=False) as f: - f.write(data) - tmp_path = f.name - try: - self.write_file(key, Path(tmp_path)) - finally: - os.unlink(tmp_path) - else: - _info(f" dry-run: would write {key}") + with tempfile.NamedTemporaryFile(delete=False) as f: + f.write(data) + tmp_path = f.name + try: + self.write_file(key, Path(tmp_path)) + finally: + os.unlink(tmp_path) def write_file(self, key: str, local_path: Path) -> None: if isinstance(self.inner, S3Store): @@ -453,59 +450,22 @@ def _publish_full( manifest = _merge_manifest(store, fixtures_json, version, prev) manifest_json = json.dumps(manifest, indent=2) + "\n" - if args.dry_run: - _info(f"dry run — would publish to {store.display_name()}") - existing = _read_manifest(store, version) - if existing: - existing.pop("_prefix", None) - existing_names = {e["name"] for e in existing["fixtures"]} - new_names = {e["name"] for e in manifest["fixtures"]} - added = new_names - existing_names - removed = existing_names - new_names - if added: - _info(f" new fixtures: {', '.join(sorted(added))}") - if removed: - _info(f" removed fixtures: {', '.join(sorted(removed))}") - if not added and not removed: - _info(f" same {len(new_names)} fixtures as existing") - if version not in versions: - updated_versions = sorted(versions + [version], key=_version_sort_key) - _info(f" versions.json would update: {versions} -> {updated_versions}") - else: - _info(f" versions.json unchanged: {versions}") - - dry_store = DryRunStore(store) - _info(f"dry-run uploading {len(manifest['fixtures'])} fixtures (verifying credentials)...") - _parallel_upload( - dry_store, - [(f"v{version}/arrays/{e['name']}", output / e["name"]) for e in manifest["fixtures"]], - ) - _info("dry-run upload succeeded — credentials are valid") - return - - if not args.yes: - _info(f"\nabout to upload {len(manifest['fixtures'])} fixtures for v{version} to {store.display_name()}") - answer = input("proceed? [y/N] ").strip().lower() - if answer not in ("y", "yes"): - _info("aborted") - sys.exit(1) + write_store = _write_store( + store, args, + f"\nabout to upload {len(manifest['fixtures'])} fixtures for v{version} to {store.display_name()}", + ) - _info(f"uploading {len(manifest['fixtures'])} fixtures to {store.display_name()}...") _parallel_upload( - store, + write_store, [(f"v{version}/arrays/{e['name']}", output / e["name"]) for e in manifest["fixtures"]], ) - store.write(f"v{version}/arrays/manifest.json", manifest_json.encode()) - _info(" uploaded manifest.json") + write_store.write(f"v{version}/arrays/manifest.json", manifest_json.encode()) if version not in versions: versions.append(version) versions.sort(key=_version_sort_key) - store.write("versions.json", (json.dumps(versions, indent=2) + "\n").encode()) - _info(" updated versions.json") - - _info(f"\ndone: {len(manifest['fixtures'])} fixtures for v{version} published to {store.display_name()}") + write_store.write("versions.json", (json.dumps(versions, indent=2) + "\n").encode()) def _publish_update( @@ -557,27 +517,14 @@ def _publish_update( _info("no new fixtures to add") return - if args.dry_run: - dry_store = DryRunStore(store) - _info(f"dry-run uploading {len(new_fixtures)} new fixture(s) (verifying credentials)...") - _parallel_upload( - dry_store, - [(f"{prefix}/{name}", output / name) for name in new_fixtures], - ) - _info("dry-run upload succeeded — credentials are valid") - return - - if not args.yes: - _info(f"\nabout to upload {len(new_fixtures)} new fixture(s) for v{version} to {store.display_name()}") - answer = input("proceed? [y/N] ").strip().lower() - if answer not in ("y", "yes"): - _info("aborted") - sys.exit(1) + write_store = _write_store( + store, args, + f"\nabout to upload {len(new_fixtures)} new fixture(s) for v{version} to {store.display_name()}", + ) - # Upload only new fixture files. new_fixture_names = set(new_fixtures) _parallel_upload( - store, + write_store, [(f"{prefix}/{name}", output / name) for name in new_fixtures], ) @@ -592,10 +539,7 @@ def _publish_update( "generated_at": datetime.now(UTC).isoformat(), "fixtures": new_entries, } - store.write(f"{prefix}/manifest.json", (json.dumps(updated_manifest, indent=2) + "\n").encode()) - _info(" updated manifest.json") - - _info(f"\ndone: added {len(new_fixtures)} new fixture(s) to v{version} in {store.display_name()}") + write_store.write(f"{prefix}/manifest.json", (json.dumps(updated_manifest, indent=2) + "\n").encode()) def cmd_check(args: argparse.Namespace) -> None: @@ -798,6 +742,19 @@ def cmd_validate_manifest(args: argparse.Namespace) -> None: # --------------------------------------------------------------------------- +def _write_store(store: Store, args: argparse.Namespace, prompt: str) -> Store: + """Return a DryRunStore wrapper or the real store, with optional confirmation.""" + if args.dry_run: + return DryRunStore(store) + if not args.yes: + _info(prompt) + answer = input("proceed? [y/N] ").strip().lower() + if answer not in ("y", "yes"): + _info("aborted") + sys.exit(1) + return store + + def _parallel_upload(store: Store, items: list[tuple[str, Path]], max_workers: int = 8) -> None: """Upload files to the store in parallel.""" with ThreadPoolExecutor(max_workers=max_workers) as pool: @@ -994,7 +951,7 @@ def main() -> None: p.add_argument( "--dry-run", action="store_true", - help="Generate and show manifest, but don't upload", + help="Validate credentials and show what would be uploaded, without writing", ) p.add_argument( "--force", From 871bfb8389e5ea2211bd931560e8c2ca2bd07a8e Mon Sep 17 00:00:00 2001 From: Joe Isaacs Date: Mon, 30 Mar 2026 14:51:47 +0100 Subject: [PATCH 3/3] fixup role Signed-off-by: Joe Isaacs --- vortex-test/compat-gen/scripts/compat.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/vortex-test/compat-gen/scripts/compat.py b/vortex-test/compat-gen/scripts/compat.py index 1f74aa7f78c..e94b509747d 100644 --- a/vortex-test/compat-gen/scripts/compat.py +++ b/vortex-test/compat-gen/scripts/compat.py @@ -451,7 +451,8 @@ def _publish_full( manifest_json = json.dumps(manifest, indent=2) + "\n" write_store = _write_store( - store, args, + store, + args, f"\nabout to upload {len(manifest['fixtures'])} fixtures for v{version} to {store.display_name()}", ) @@ -518,7 +519,8 @@ def _publish_update( return write_store = _write_store( - store, args, + store, + args, f"\nabout to upload {len(new_fixtures)} new fixture(s) for v{version} to {store.display_name()}", )