From cd67f6b220719cd019e29bf45e2753a37415c25f Mon Sep 17 00:00:00 2001 From: Scott Todd Date: Wed, 1 Apr 2026 14:32:38 -0700 Subject: [PATCH 1/4] Add dynamic configure-aws-credentials with release bucket support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- .../action.yml | 35 +++ .../build_portable_linux_artifacts.yml | 9 +- .../build_portable_linux_python_packages.yml | 8 +- .github/workflows/build_windows_artifacts.yml | 10 +- .../build_windows_python_packages.yml | 9 +- ...ti_arch_build_portable_linux_artifacts.yml | 11 +- .../multi_arch_build_windows_artifacts.yml | 12 +- .github/workflows/multi_arch_ci_linux.yml | 13 +- .github/workflows/multi_arch_ci_windows.yml | 13 +- build_tools/_therock_utils/s3_buckets.py | 198 +++++++++++++++++ .../_therock_utils/workflow_outputs.py | 99 +-------- .../write_artifacts_bucket_info.py | 45 ++++ build_tools/tests/s3_buckets_test.py | 210 ++++++++++++++++++ build_tools/tests/workflow_outputs_test.py | 154 ------------- docs/development/s3_buckets.md | 26 +-- 15 files changed, 525 insertions(+), 327 deletions(-) create mode 100644 .github/actions/configure_aws_artifacts_credentials/action.yml create mode 100644 build_tools/_therock_utils/s3_buckets.py create mode 100644 build_tools/github_actions/write_artifacts_bucket_info.py create mode 100644 build_tools/tests/s3_buckets_test.py diff --git a/.github/actions/configure_aws_artifacts_credentials/action.yml b/.github/actions/configure_aws_artifacts_credentials/action.yml new file mode 100644 index 00000000000..23a3cc7f3ae --- /dev/null +++ b/.github/actions/configure_aws_artifacts_credentials/action.yml @@ -0,0 +1,35 @@ +# Copyright Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +name: "Configure AWS credentials for artifact uploads" + +description: >- + Determine the IAM role for the artifacts S3 bucket and assume it via OIDC. + * Forks and external repos skip OIDC and use runner base credentials. + * Release builds (dev/nightly/prerelease) use 'therock-{release_type}' roles. + * Other CI runs use the 'therock-ci' role. + See the "Authentication" section in docs/development/s3_buckets.md. + +inputs: + release_type: + description: 'Release type: "" for CI, or "dev", "nightly", "prerelease".' + default: "" + +runs: + using: "composite" + steps: + - name: Determine IAM role + if: ${{ always() }} + id: iam + shell: bash + run: | + python build_tools/github_actions/write_artifacts_bucket_info.py \ + --release-type "${{ inputs.release_type }}" + + - name: Configure AWS Credentials + if: ${{ always() && steps.iam.outputs.iam_role != '' }} + uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 + with: + aws-region: ${{ steps.iam.outputs.aws_region }} + role-to-assume: ${{ steps.iam.outputs.iam_role }} + special-characters-workaround: ${{ runner.os == 'Windows' }} diff --git a/.github/workflows/build_portable_linux_artifacts.yml b/.github/workflows/build_portable_linux_artifacts.yml index 2c2e71e7924..b57d164a64f 100644 --- a/.github/workflows/build_portable_linux_artifacts.yml +++ b/.github/workflows/build_portable_linux_artifacts.yml @@ -165,14 +165,9 @@ jobs: python3 build_tools/resource_info.py --finalize python3 build_tools/analyze_build_times.py --build-dir build - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - name: Configure AWS Credentials - if: ${{ always() && github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci + if: ${{ always() }} + uses: ./.github/actions/configure_aws_artifacts_credentials - name: Post Build Upload if: always() diff --git a/.github/workflows/build_portable_linux_python_packages.yml b/.github/workflows/build_portable_linux_python_packages.yml index d90e0245582..bd9a80fa6d6 100644 --- a/.github/workflows/build_portable_linux_python_packages.yml +++ b/.github/workflows/build_portable_linux_python_packages.yml @@ -110,14 +110,8 @@ jobs: --dest-dir="${{ env.PACKAGES_DIR }}" \ --version="${{ inputs.package_version }}" - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - name: Configure AWS Credentials - if: ${{ github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci + uses: ./.github/actions/configure_aws_artifacts_credentials # NOTE: we use `github.run_id` and NOT `env.ARTIFACT_RUN_ID` here! # This ensures that if they are different we _download_ artifacts from the diff --git a/.github/workflows/build_windows_artifacts.yml b/.github/workflows/build_windows_artifacts.yml index 822502cbc3d..83c782eccf4 100644 --- a/.github/workflows/build_windows_artifacts.yml +++ b/.github/workflows/build_windows_artifacts.yml @@ -175,15 +175,9 @@ jobs: $fsout | % {$_.Used/=1GB;$_.Free/=1GB;$_} | Write-Host get-disk | Select-object @{Name="Size(GB)";Expression={$_.Size/1GB}} | Write-Host - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - name: Configure AWS Credentials - if: ${{ always() && github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci - special-characters-workaround: true + if: ${{ always() }} + uses: ./.github/actions/configure_aws_artifacts_credentials - name: Post Build Upload if: always() diff --git a/.github/workflows/build_windows_python_packages.yml b/.github/workflows/build_windows_python_packages.yml index 89b98358128..7cb16899fb3 100644 --- a/.github/workflows/build_windows_python_packages.yml +++ b/.github/workflows/build_windows_python_packages.yml @@ -119,15 +119,8 @@ jobs: --dest-dir="${{ env.PACKAGES_DIR }}" \ --version="${{ inputs.package_version }}" - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - name: Configure AWS Credentials - if: ${{ github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci - special-characters-workaround: true + uses: ./.github/actions/configure_aws_artifacts_credentials # NOTE: we use `github.run_id` and NOT `env.ARTIFACT_RUN_ID` here! # This ensures that if they are different we _download_ artifacts from the diff --git a/.github/workflows/multi_arch_build_portable_linux_artifacts.yml b/.github/workflows/multi_arch_build_portable_linux_artifacts.yml index 9d53988bf6f..b48fd3652f9 100644 --- a/.github/workflows/multi_arch_build_portable_linux_artifacts.yml +++ b/.github/workflows/multi_arch_build_portable_linux_artifacts.yml @@ -137,14 +137,9 @@ jobs: echo "Artifacts:" ls -lh "${BUILD_DIR}"/artifacts/*.tar.xz 2>/dev/null || echo "No artifacts found" - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - - name: Configure AWS Credentials - if: ${{ always() && github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci + - name: Configure AWS credentials for artifact uploads + if: ${{ always() }} + uses: ./.github/actions/configure_aws_artifacts_credentials # TODO(#3336): Attempt upload `always()`? # * If some part of the build failed, partial artifacts may be useful for debugging. diff --git a/.github/workflows/multi_arch_build_windows_artifacts.yml b/.github/workflows/multi_arch_build_windows_artifacts.yml index f5cb2f55b1b..5e6fbcc6ba0 100644 --- a/.github/workflows/multi_arch_build_windows_artifacts.yml +++ b/.github/workflows/multi_arch_build_windows_artifacts.yml @@ -164,15 +164,9 @@ jobs: echo "Artifacts:" ls -lh "${BUILD_DIR}"/artifacts/*.tar.xz 2>/dev/null || echo "No artifacts found" - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - - name: Configure AWS Credentials - if: ${{ always() && github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci - special-characters-workaround: true + - name: Configure AWS credentials for artifact uploads + if: ${{ always() }} + uses: ./.github/actions/configure_aws_artifacts_credentials # TODO(#3336): Attempt upload `always()`? # * If some part of the build failed, partial artifacts may be useful for debugging. diff --git a/.github/workflows/multi_arch_ci_linux.yml b/.github/workflows/multi_arch_ci_linux.yml index 14ab13002c7..4b8429e1c32 100644 --- a/.github/workflows/multi_arch_ci_linux.yml +++ b/.github/workflows/multi_arch_ci_linux.yml @@ -44,17 +44,8 @@ jobs: - name: Install artifact manager dependencies run: pip install boto3 - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - # - # This is just needed for write access to the current run's bucket, - # all possible source run buckets have public read access. - - name: Configure AWS Credentials - if: ${{ github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci + - name: Configure AWS credentials for artifact uploads + uses: ./.github/actions/configure_aws_artifacts_credentials - name: Copy prebuilt stage artifacts env: diff --git a/.github/workflows/multi_arch_ci_windows.yml b/.github/workflows/multi_arch_ci_windows.yml index f6dedcb2a63..b2100c88e83 100644 --- a/.github/workflows/multi_arch_ci_windows.yml +++ b/.github/workflows/multi_arch_ci_windows.yml @@ -49,17 +49,8 @@ jobs: - name: Install artifact manager dependencies run: pip install boto3 - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - # - # This is just needed for write access to the current run's bucket, - # all possible source run buckets have public read access. - - name: Configure AWS Credentials - if: ${{ github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci + - name: Configure AWS credentials for artifact uploads + uses: ./.github/actions/configure_aws_artifacts_credentials - name: Copy prebuilt stage artifacts env: diff --git a/build_tools/_therock_utils/s3_buckets.py b/build_tools/_therock_utils/s3_buckets.py new file mode 100644 index 00000000000..af9ae935a4b --- /dev/null +++ b/build_tools/_therock_utils/s3_buckets.py @@ -0,0 +1,198 @@ +# Copyright Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +"""Inventory of S3 buckets used by CI/CD systems and related functions. + +See docs/development/s3_buckets.md. +""" + +from dataclasses import dataclass, field +import json +import os +import sys + + +def _log(*args, **kwargs): + """Log to stdout with flush for CI visibility.""" + print(*args, **kwargs) + sys.stdout.flush() + + +@dataclass(frozen=True) +class S3BucketConfig: + """Metadata for a single bucket in S3""" + + name: str + """S3 bucket name (e.g. 'therock-ci-artifacts')""" + + region: str = field(default="us-east-2") + """Region in S3 (e.g. 'us-east-2')""" + + iam_role: str | None = field(default=None) + """IAM role name that grants write access to this bucket (e.g. 'therock-ci'), if any""" + + iam_namespace: str | None = field(default="arn:aws:iam::692859939525:role") + """Namespace for write_access_iam_role (e.g. 'arn:aws:iam::692859939525:role')""" + + @property + def write_access_iam_role(self) -> str | None: + """IAM role granting write access to the bucket""" + if not self.iam_role: + return None + if not self.iam_namespace: + raise ValueError( + f"Bucket {self.name!r} has iam_role={self.iam_role!r} but no iam_namespace" + ) + return f"{self.iam_namespace}/{self.iam_role}" + + +s3_bucket_configs = [ + # CI (self-hosted runners include credentials for therock-ci-artifacts-external) + S3BucketConfig("therock-ci-artifacts", iam_role="therock-ci"), + S3BucketConfig("therock-ci-artifacts-external", iam_role=None), + # Release type "dev" + S3BucketConfig("therock-dev-artifacts", iam_role="therock-dev"), + S3BucketConfig("therock-dev-packages", iam_role="therock-dev"), + S3BucketConfig("therock-dev-python", iam_role="therock-dev"), + S3BucketConfig("therock-dev-tarball", iam_role="therock-dev"), + # Release type "nightly" + S3BucketConfig("therock-nightly-artifacts", iam_role="therock-nightly"), + S3BucketConfig("therock-nightly-packages", iam_role="therock-nightly"), + S3BucketConfig("therock-nightly-python", iam_role="therock-nightly"), + S3BucketConfig("therock-nightly-tarball", iam_role="therock-nightly"), + # Release type "prerelease" + S3BucketConfig("therock-prerelease-artifacts", iam_role="therock-prerelease"), + S3BucketConfig("therock-prerelease-packages", iam_role="therock-prerelease"), + S3BucketConfig("therock-prerelease-python", iam_role="therock-prerelease"), + S3BucketConfig("therock-prerelease-tarball", iam_role="therock-prerelease"), + # Release type "release" (no automated credentials for uploading) + S3BucketConfig("therock-release-artifacts", iam_role=None), + S3BucketConfig("therock-release-packages", iam_role=None), + S3BucketConfig("therock-release-python", iam_role=None), + S3BucketConfig("therock-release-tarball", iam_role=None), +] + + +_BUCKET_CONFIGS_BY_NAME = {c.name: c for c in s3_bucket_configs} + +_ALLOWED_RELEASE_TYPES = {"dev", "nightly", "prerelease"} + +# Repositories allowed to use release_type. Only these repositories are trusted +# to assume release IAM roles that grant write access to release buckets. +_ALLOWED_RELEASE_REPOS = {"ROCm/TheRock", "ROCm/rockrel"} + + +def get_artifacts_bucket_config( + release_type: str, + repository: str, + is_pr_from_fork: bool, +) -> S3BucketConfig: + """Look up the artifacts bucket config for a repository. + + Args: + release_type: "" for CI builds, or "dev", "nightly", "prerelease". + repository: GitHub repository (e.g. "ROCm/TheRock"). + is_pr_from_fork: Whether this is a PR from a fork. + """ + if release_type: + if release_type not in _ALLOWED_RELEASE_TYPES: + raise ValueError( + f"release_type={release_type!r} is invalid, " + f"expected empty string or one of {_ALLOWED_RELEASE_TYPES}" + ) + if repository not in _ALLOWED_RELEASE_REPOS: + raise ValueError( + f"release_type={release_type!r} is set but " + f"repository {repository!r} is not one of " + f"{_ALLOWED_RELEASE_REPOS}" + ) + bucket_name = f"therock-{release_type}-artifacts" + else: + if is_pr_from_fork or repository != "ROCm/TheRock": + bucket_name = "therock-ci-artifacts-external" + else: + bucket_name = "therock-ci-artifacts" + return _BUCKET_CONFIGS_BY_NAME[bucket_name] + + +def _is_current_run_pr_from_fork() -> bool: + """Check if the current workflow run is a pull request from a fork. + + Reads the GitHub event payload to check the .fork property on the + head repo, matching the behavior of the GitHub Actions expression + ``github.event.pull_request.head.repo.fork``. + + Returns False for non-pull_request events or if the event payload + is not available (e.g. local development). + """ + event_name = os.environ.get("GITHUB_EVENT_NAME", "") + if event_name != "pull_request": + return False + + event_path = os.environ.get("GITHUB_EVENT_PATH") + if not event_path: + return False + + with open(event_path) as f: + event = json.load(f) + + return bool( + event.get("pull_request", {}).get("head", {}).get("repo", {}).get("fork", False) + ) + + +def get_artifacts_bucket_config_for_workflow_run( + github_repository: str, + release_type: str | None = None, + workflow_run_id: str | None = None, + workflow_run: dict | None = None, +) -> S3BucketConfig: + """Look up the artifacts bucket config for a workflow run. + + Combines environment-based inputs (RELEASE_TYPE, event payload) with + optional workflow run metadata from the GitHub API to determine the + correct artifacts bucket. + + Args: + github_repository: GitHub repository (e.g. "ROCm/TheRock"). + release_type: Release type override. If None, reads RELEASE_TYPE + from the environment (default: empty string = CI build). + workflow_run_id: If set and ``workflow_run`` is None, fetches the + workflow run from the GitHub API for fork detection. + workflow_run: Optional workflow run dict from GitHub API. If + provided, used directly for fork detection (no API call). + """ + _log("Retrieving bucket info for workflow run...") + _log(f" github_repository: {github_repository}") + + if release_type is None: + release_type = os.environ.get("RELEASE_TYPE", "") + if release_type: + _log(f" release_type: {release_type}") + + # Fetch workflow_run from API if not provided but workflow_run_id is set. + # Deferred import: github_actions is an optional dependency not available in + # all environments (e.g. local dev without the GHA support package installed). + if workflow_run is None and workflow_run_id is not None: + from github_actions.github_actions_api import gha_query_workflow_run_by_id + + workflow_run = gha_query_workflow_run_by_id(github_repository, workflow_run_id) + + # Extract metadata from workflow_run if available + if workflow_run is not None: + _log(f" workflow_run_id: {workflow_run['id']}") + head_github_repository = workflow_run["head_repository"]["full_name"] + is_pr_from_fork = head_github_repository != github_repository + _log(f" head_github_repository: {head_github_repository}") + _log(f" is_pr_from_fork: {is_pr_from_fork}") + else: + is_pr_from_fork = _is_current_run_pr_from_fork() + _log(f" is_pr_from_fork: {is_pr_from_fork}") + + config = get_artifacts_bucket_config( + release_type=release_type, + repository=github_repository, + is_pr_from_fork=is_pr_from_fork, + ) + _log(f" bucket: {config.name}") + return config diff --git a/build_tools/_therock_utils/workflow_outputs.py b/build_tools/_therock_utils/workflow_outputs.py index 117922eb8de..f2fd7ea80b1 100644 --- a/build_tools/_therock_utils/workflow_outputs.py +++ b/build_tools/_therock_utils/workflow_outputs.py @@ -51,12 +51,7 @@ sys.path.insert(0, os.fspath(Path(__file__).parent.parent)) from _therock_utils.storage_location import StorageLocation - - -def _log(*args, **kwargs): - """Log to stdout with flush for CI visibility.""" - print(*args, **kwargs) - sys.stdout.flush() +from _therock_utils.s3_buckets import get_artifacts_bucket_config_for_workflow_run # --------------------------------------------------------------------------- @@ -294,39 +289,6 @@ def for_local( ) -# --------------------------------------------------------------------------- -# Bucket selection logic -# --------------------------------------------------------------------------- - - -def _is_current_run_pr_from_fork() -> bool: - """Check if the current workflow run is a pull request from a fork. - - Reads the GitHub event payload to check the .fork property on the - head repo, matching the behavior of the GitHub Actions expression - ``github.event.pull_request.head.repo.fork``. - - Returns False for non-pull_request events or if the event payload - is not available (e.g. local development). - """ - import json - - event_name = os.environ.get("GITHUB_EVENT_NAME", "") - if event_name != "pull_request": - return False - - event_path = os.environ.get("GITHUB_EVENT_PATH") - if not event_path: - return False - - with open(event_path) as f: - event = json.load(f) - - return bool( - event.get("pull_request", {}).get("head", {}).get("repo", {}).get("fork", False) - ) - - def _retrieve_bucket_info( github_repository: str | None = None, workflow_run_id: str | None = None, @@ -342,57 +304,18 @@ def _retrieve_bucket_info( - external_repo: ``''`` for ROCm/TheRock, or ``'{owner}-{repo}/'`` - bucket: S3 bucket name """ - _log("Retrieving bucket info...") - - if github_repository: - _log(f" (explicit) github_repository: {github_repository}") - else: + if not github_repository: github_repository = os.environ.get("GITHUB_REPOSITORY", "ROCm/TheRock") - _log(f" (implicit) github_repository: {github_repository}") - - # Fetch workflow_run from API if not provided but workflow_run_id is set. - # Deferred import: github_actions is an optional dependency not available in - # all environments (e.g. local dev without the GHA support package installed). - if workflow_run is None and workflow_run_id is not None: - from github_actions.github_actions_api import gha_query_workflow_run_by_id - - workflow_run = gha_query_workflow_run_by_id(github_repository, workflow_run_id) - - # Extract metadata from workflow_run if available - if workflow_run is not None: - _log(f" workflow_run_id : {workflow_run['id']}") - head_github_repository = workflow_run["head_repository"]["full_name"] - is_pr_from_fork = head_github_repository != github_repository - _log(f" head_github_repository : {head_github_repository}") - _log(f" is_pr_from_fork : {is_pr_from_fork}") - else: - is_pr_from_fork = _is_current_run_pr_from_fork() - _log(f" is_pr_from_fork : {is_pr_from_fork}") + artifact_bucket_config = get_artifacts_bucket_config_for_workflow_run( + github_repository=github_repository, + workflow_run_id=workflow_run_id, + workflow_run=workflow_run, + ) owner, repo_name = github_repository.split("/") external_repo = ( - "" - if repo_name == "TheRock" and owner == "ROCm" and not is_pr_from_fork - else f"{owner}-{repo_name}/" + f"{owner}-{repo_name}/" + if artifact_bucket_config.name == "therock-ci-artifacts-external" + else "" ) - - release_type = os.environ.get("RELEASE_TYPE") - if release_type: - _VALID_RELEASE_TYPES = {"dev", "nightly", "prerelease"} - if release_type not in _VALID_RELEASE_TYPES: - raise ValueError( - f"Invalid RELEASE_TYPE={release_type!r}, " - f"expected one of {sorted(_VALID_RELEASE_TYPES)}" - ) - _log(f" (implicit) RELEASE_TYPE: {release_type}") - bucket = f"therock-{release_type}-artifacts" - else: - if external_repo == "": - bucket = "therock-ci-artifacts" - else: - bucket = "therock-ci-artifacts-external" - - _log("Retrieved bucket info:") - _log(f" external_repo: {external_repo}") - _log(f" bucket : {bucket}") - return (external_repo, bucket) + return (external_repo, artifact_bucket_config.name) diff --git a/build_tools/github_actions/write_artifacts_bucket_info.py b/build_tools/github_actions/write_artifacts_bucket_info.py new file mode 100644 index 00000000000..a6725a914fe --- /dev/null +++ b/build_tools/github_actions/write_artifacts_bucket_info.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python3 +# Copyright Advanced Micro Devices, Inc. +# SPDX-License-Identifier: MIT + +"""Writes `bucket`, `iam_role`, and `aws_region` for the artifacts S3 bucket to GITHUB_OUTPUT. + +Used by .github/actions/configure_aws_artifacts_credentials/action.yml. +""" + +import argparse +import os + +from _therock_utils.s3_buckets import get_artifacts_bucket_config_for_workflow_run +from github_actions_api import gha_set_output + + +def main(argv: list[str] | None = None): + parser = argparse.ArgumentParser( + description="Determine IAM role ARN and region for the artifacts S3 bucket" + ) + parser.add_argument( + "--release-type", + type=str, + default="", + help='Release type: "" for CI, or "dev", "nightly", "prerelease".', + ) + args = parser.parse_args(argv) + + repository = os.environ.get("GITHUB_REPOSITORY", "ROCm/TheRock") + config = get_artifacts_bucket_config_for_workflow_run( + github_repository=repository, + release_type=args.release_type, + ) + + gha_set_output( + { + "bucket": config.name, + "iam_role": config.write_access_iam_role or "", + "aws_region": config.region, + } + ) + + +if __name__ == "__main__": + main() diff --git a/build_tools/tests/s3_buckets_test.py b/build_tools/tests/s3_buckets_test.py new file mode 100644 index 00000000000..803d6a8e441 --- /dev/null +++ b/build_tools/tests/s3_buckets_test.py @@ -0,0 +1,210 @@ +#!/usr/bin/env python +"""Unit tests for s3_buckets.py.""" + +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +sys.path.insert(0, os.fspath(Path(__file__).parent.parent)) + +from _therock_utils.s3_buckets import ( + get_artifacts_bucket_config, + get_artifacts_bucket_config_for_workflow_run, +) + + +# --------------------------------------------------------------------------- +# get_artifacts_bucket_config +# --------------------------------------------------------------------------- + + +class TestGetArtifactsBucketConfig(unittest.TestCase): + def test_ci_rocm_therock(self): + config = get_artifacts_bucket_config( + release_type="", repository="ROCm/TheRock", is_pr_from_fork=False + ) + self.assertEqual(config.name, "therock-ci-artifacts") + + def test_ci_fork_pr(self): + config = get_artifacts_bucket_config( + release_type="", repository="ROCm/TheRock", is_pr_from_fork=True + ) + self.assertEqual(config.name, "therock-ci-artifacts-external") + + def test_ci_external_repo(self): + config = get_artifacts_bucket_config( + release_type="", repository="ROCm/rocm-libraries", is_pr_from_fork=False + ) + self.assertEqual(config.name, "therock-ci-artifacts-external") + + def test_release_type_dev(self): + config = get_artifacts_bucket_config( + release_type="dev", repository="ROCm/TheRock", is_pr_from_fork=False + ) + self.assertEqual(config.name, "therock-dev-artifacts") + self.assertEqual(config.iam_role, "therock-dev") + + def test_release_type_from_rockrel(self): + config = get_artifacts_bucket_config( + release_type="nightly", repository="ROCm/rockrel", is_pr_from_fork=False + ) + self.assertEqual(config.name, "therock-nightly-artifacts") + + def test_release_type_invalid_raises(self): + with self.assertRaises(ValueError) as cm: + get_artifacts_bucket_config( + release_type="bogus", + repository="ROCm/TheRock", + is_pr_from_fork=False, + ) + self.assertIn("bogus", str(cm.exception)) + + def test_release_type_disallowed_repo_raises(self): + with self.assertRaises(ValueError) as cm: + get_artifacts_bucket_config( + release_type="dev", + repository="ROCm/rocm-libraries", + is_pr_from_fork=False, + ) + self.assertIn("ROCm/rocm-libraries", str(cm.exception)) + + +# --------------------------------------------------------------------------- +# get_artifacts_bucket_config_for_workflow_run +# --------------------------------------------------------------------------- + + +class TestGetArtifactsBucketConfigForWorkflowRun(unittest.TestCase): + """Test the workflow-run-aware wrapper.""" + + def setUp(self): + self.api_patcher = mock.patch( + "github_actions.github_actions_api.gha_query_workflow_run_by_id" + ) + self.mock_api = self.api_patcher.start() + + self.env_patcher = mock.patch.dict(os.environ) + self.env_patcher.start() + os.environ.pop("GITHUB_REPOSITORY", None) + os.environ.pop("GITHUB_EVENT_NAME", None) + os.environ.pop("GITHUB_EVENT_PATH", None) + os.environ.pop("RELEASE_TYPE", None) + + def tearDown(self): + self.env_patcher.stop() + self.api_patcher.stop() + + def test_default_ci(self): + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock" + ) + self.assertEqual(config.name, "therock-ci-artifacts") + + def test_explicit_release_type(self): + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock", release_type="nightly" + ) + self.assertEqual(config.name, "therock-nightly-artifacts") + + def test_release_type_from_env(self): + os.environ["RELEASE_TYPE"] = "dev" + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock" + ) + self.assertEqual(config.name, "therock-dev-artifacts") + + def test_explicit_release_type_overrides_env(self): + os.environ["RELEASE_TYPE"] = "dev" + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock", release_type="nightly" + ) + self.assertEqual(config.name, "therock-nightly-artifacts") + + def test_workflow_run_same_repo(self): + fake_run = { + "id": 12345, + "head_repository": {"full_name": "ROCm/TheRock"}, + } + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock", workflow_run=fake_run + ) + self.assertEqual(config.name, "therock-ci-artifacts") + + def test_workflow_run_from_fork(self): + fake_run = { + "id": 12345, + "head_repository": {"full_name": "SomeUser/TheRock"}, + } + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock", workflow_run=fake_run + ) + self.assertEqual(config.name, "therock-ci-artifacts-external") + + def test_workflow_run_id_triggers_api_call(self): + self.mock_api.return_value = { + "id": 12345, + "head_repository": {"full_name": "ROCm/TheRock"}, + } + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock", workflow_run_id="12345" + ) + self.mock_api.assert_called_once_with("ROCm/TheRock", "12345") + self.assertEqual(config.name, "therock-ci-artifacts") + + def _write_event(self, event: dict) -> str: + f = tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) + json.dump(event, f) + f.close() + return f.name + + def test_fork_pr_from_event_payload(self): + """Fork PR detected via event payload (no workflow_run dict).""" + event_path = self._write_event( + {"pull_request": {"head": {"repo": {"fork": True}}}} + ) + try: + os.environ["GITHUB_EVENT_NAME"] = "pull_request" + os.environ["GITHUB_EVENT_PATH"] = event_path + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock" + ) + self.assertEqual(config.name, "therock-ci-artifacts-external") + finally: + os.unlink(event_path) + + def test_same_repo_pr_from_event_payload(self): + """Same-repo PR detected via event payload (no workflow_run dict).""" + event_path = self._write_event( + {"pull_request": {"head": {"repo": {"fork": False}}}} + ) + try: + os.environ["GITHUB_EVENT_NAME"] = "pull_request" + os.environ["GITHUB_EVENT_PATH"] = event_path + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock" + ) + self.assertEqual(config.name, "therock-ci-artifacts") + finally: + os.unlink(event_path) + + def test_workflow_run_id_ignored_when_workflow_run_provided(self): + """workflow_run takes priority over workflow_run_id.""" + fake_run = { + "id": 12345, + "head_repository": {"full_name": "ROCm/TheRock"}, + } + config = get_artifacts_bucket_config_for_workflow_run( + github_repository="ROCm/TheRock", + workflow_run=fake_run, + workflow_run_id="99999", + ) + self.mock_api.assert_not_called() + self.assertEqual(config.name, "therock-ci-artifacts") + + +if __name__ == "__main__": + unittest.main() diff --git a/build_tools/tests/workflow_outputs_test.py b/build_tools/tests/workflow_outputs_test.py index 0ed29f2111b..196dd1ecfdf 100644 --- a/build_tools/tests/workflow_outputs_test.py +++ b/build_tools/tests/workflow_outputs_test.py @@ -352,159 +352,5 @@ def test_lookup_ignored_when_workflow_run_provided(self, mock_retrieve): ) -# --------------------------------------------------------------------------- -# _retrieve_bucket_info -# --------------------------------------------------------------------------- - - -class TestRetrieveBucketInfo(unittest.TestCase): - """Test _retrieve_bucket_info with mocked environment.""" - - def setUp(self): - # Patch where the name is defined, not where it's imported. The import - # in workflow_outputs.py is deferred (inside _retrieve_bucket_info), so - # patching the definition site is both correct and necessary here. - self.api_patcher = mock.patch( - "github_actions.github_actions_api.gha_query_workflow_run_by_id" - ) - self.mock_api = self.api_patcher.start() - - # Isolate from ambient env vars that _retrieve_bucket_info reads. - # mock.patch.dict records the original state; individual tests add - # specific vars via @mock.patch.dict decorators on top. - self.env_patcher = mock.patch.dict(os.environ) - self.env_patcher.start() - os.environ.pop("GITHUB_REPOSITORY", None) - os.environ.pop("GITHUB_EVENT_NAME", None) - os.environ.pop("GITHUB_EVENT_PATH", None) - os.environ.pop("RELEASE_TYPE", None) - - def tearDown(self): - self.env_patcher.stop() - self.api_patcher.stop() - - def _call(self, **kwargs): - from _therock_utils.workflow_outputs import _retrieve_bucket_info - - return _retrieve_bucket_info(**kwargs) - - def test_no_env_defaults_to_rocm_therock(self): - """When GITHUB_REPOSITORY is not set, defaults to ROCm/TheRock.""" - external_repo, bucket = self._call() - self.assertEqual(external_repo, "") - self.assertEqual(bucket, "therock-ci-artifacts") - - @mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": "ROCm/TheRock"}, clear=False) - def test_rocm_therock_default(self): - external_repo, bucket = self._call() - self.assertEqual(external_repo, "") - self.assertEqual(bucket, "therock-ci-artifacts") - - @mock.patch.dict(os.environ, {"GITHUB_REPOSITORY": "ROCm/TheRock"}, clear=False) - def test_rocm_therock_explicit(self): - external_repo, bucket = self._call(github_repository="ROCm/TheRock") - self.assertEqual(external_repo, "") - self.assertEqual(bucket, "therock-ci-artifacts") - - def test_fork_pr(self): - """Fork PR: head repo .fork is true in event payload.""" - import json - import tempfile - - event = {"pull_request": {"head": {"repo": {"fork": True}}}} - with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: - json.dump(event, f) - event_path = f.name - - try: - with mock.patch.dict( - os.environ, - { - "GITHUB_REPOSITORY": "ROCm/TheRock", - "GITHUB_EVENT_NAME": "pull_request", - "GITHUB_EVENT_PATH": event_path, - }, - clear=False, - ): - external_repo, bucket = self._call() - self.assertEqual(external_repo, "ROCm-TheRock/") - self.assertEqual(bucket, "therock-ci-artifacts-external") - finally: - os.unlink(event_path) - - @mock.patch.dict( - os.environ, - {"GITHUB_REPOSITORY": "ROCm/TheRock", "RELEASE_TYPE": "nightly"}, - clear=False, - ) - def test_release_type_nightly(self): - external_repo, bucket = self._call() - self.assertEqual(external_repo, "") - self.assertEqual(bucket, "therock-nightly-artifacts") - - @mock.patch.dict( - os.environ, - {"GITHUB_REPOSITORY": "ROCm/TheRock", "RELEASE_TYPE": "prerelease"}, - clear=False, - ) - def test_release_type_prerelease(self): - external_repo, bucket = self._call() - self.assertEqual(external_repo, "") - self.assertEqual(bucket, "therock-prerelease-artifacts") - - @mock.patch.dict( - os.environ, - {"GITHUB_REPOSITORY": "ROCm/TheRock", "RELEASE_TYPE": "bogus"}, - clear=False, - ) - def test_release_type_invalid_raises(self): - with self.assertRaises(ValueError) as cm: - self._call() - self.assertIn("bogus", str(cm.exception)) - - def test_with_workflow_run_recent(self): - """Workflow run from TheRock should use therock-ci-artifacts.""" - fake_run = { - "id": 12345, - "head_repository": {"full_name": "ROCm/TheRock"}, - } - external_repo, bucket = self._call( - github_repository="ROCm/TheRock", - workflow_run=fake_run, - ) - self.assertEqual(external_repo, "") - self.assertEqual(bucket, "therock-ci-artifacts") - - def test_with_workflow_run_from_fork(self): - """Workflow run from a fork should use external bucket. - - The external_repo prefix uses the base repo (github_repository), not - the head repo. When head != base, is_pr_from_fork is True. - """ - fake_run = { - "id": 12345, - "head_repository": {"full_name": "SomeUser/TheRock"}, - } - external_repo, bucket = self._call( - github_repository="ROCm/TheRock", - workflow_run=fake_run, - ) - self.assertEqual(external_repo, "ROCm-TheRock/") - self.assertEqual(bucket, "therock-ci-artifacts-external") - - def test_workflow_run_id_triggers_api_call(self): - """When workflow_run_id is provided without workflow_run, API is called.""" - self.mock_api.return_value = { - "id": 12345, - "head_repository": {"full_name": "ROCm/TheRock"}, - } - external_repo, bucket = self._call( - github_repository="ROCm/TheRock", - workflow_run_id="12345", - ) - self.mock_api.assert_called_once_with("ROCm/TheRock", "12345") - self.assertEqual(bucket, "therock-ci-artifacts") - - if __name__ == "__main__": unittest.main() diff --git a/docs/development/s3_buckets.md b/docs/development/s3_buckets.md index 1b4311c09f7..02a24245efe 100644 --- a/docs/development/s3_buckets.md +++ b/docs/development/s3_buckets.md @@ -25,12 +25,13 @@ using OIDC is needed. This requires `id-token: write` in the job's `permissions` The full ARN pattern is `arn:aws:iam::692859939525:role/therock-{ci,dev,nightly,prerelease}`. -```yaml -# This covers the common case for "CI" workflows that run on PRs from -# ROCm repositories and PRs from forks. Other workflows, such as release -# workflows also used by https://github.com/ROCm/rockrel, may use different -# roles and slightly different usage patterns. +Use the +[`configure_aws_artifacts_credentials`](/.github/actions/configure_aws_artifacts_credentials/action.yml) +composite action to set up credentials. It determines the correct IAM role and +bucket from the repository, event type, and optional `release_type` input +by using [`build_tools/_therock_utils/s3_buckets.py`](/build_tools/_therock_utils/s3_buckets.py): +```yaml jobs: build: runs-on: azure-linux-scale-rocm @@ -44,17 +45,8 @@ jobs: # ... build steps ... # Credentials are short-lived — assume the role close to when it's needed. - - # Assume the therock-ci OIDC role in ROCm/TheRock. Other repos - # fall back to runner base credentials (therock-ci-artifacts-external). - name: Configure AWS Credentials - if: ${{ github.repository == 'ROCm/TheRock' && !github.event.pull_request.head.repo.fork }} - uses: aws-actions/configure-aws-credentials@8df5847569e6427dd6c4fb1cf565c83acfa8afa7 # v6.0.0 - with: - aws-region: us-east-2 - role-to-assume: arn:aws:iam::692859939525:role/therock-ci - # Windows only — retry until secret key has no special characters: - special-characters-workaround: true + uses: ./.github/actions/configure_aws_artifacts_credentials # ... upload steps that use the credentials ... ``` @@ -68,7 +60,9 @@ jobs: - **Windows** jobs must pass `special-characters-workaround: true` to `aws-actions/configure-aws-credentials`. This retries credential fetching until the secret access key contains no special characters, which some - Windows environments cannot tolerate. + Windows environments cannot tolerate. (The + `configure_aws_artifacts_credentials` composite action mentioned above + handles this automatically) ## Bucket inventory From f895a762de80b600bf6ee4162e7d8e2cd00d5c91 Mon Sep 17 00:00:00 2001 From: Scott Todd Date: Tue, 7 Apr 2026 16:42:29 -0700 Subject: [PATCH 2/4] Add syspath to fix import --- build_tools/github_actions/write_artifacts_bucket_info.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/build_tools/github_actions/write_artifacts_bucket_info.py b/build_tools/github_actions/write_artifacts_bucket_info.py index a6725a914fe..573ab012b67 100644 --- a/build_tools/github_actions/write_artifacts_bucket_info.py +++ b/build_tools/github_actions/write_artifacts_bucket_info.py @@ -9,6 +9,10 @@ import argparse import os +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) from _therock_utils.s3_buckets import get_artifacts_bucket_config_for_workflow_run from github_actions_api import gha_set_output From eba89b4546a3b96ef9a780a0c40848aac968a80a Mon Sep 17 00:00:00 2001 From: Scott Todd Date: Wed, 8 Apr 2026 09:01:30 -0700 Subject: [PATCH 3/4] Split iam_namespace into iam_account and fstring in property --- build_tools/_therock_utils/s3_buckets.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/build_tools/_therock_utils/s3_buckets.py b/build_tools/_therock_utils/s3_buckets.py index af9ae935a4b..61d82aafcc6 100644 --- a/build_tools/_therock_utils/s3_buckets.py +++ b/build_tools/_therock_utils/s3_buckets.py @@ -28,22 +28,22 @@ class S3BucketConfig: region: str = field(default="us-east-2") """Region in S3 (e.g. 'us-east-2')""" + iam_account: str | None = field(default="692859939525") + """IAM account for write_access_iam_role""" + iam_role: str | None = field(default=None) """IAM role name that grants write access to this bucket (e.g. 'therock-ci'), if any""" - iam_namespace: str | None = field(default="arn:aws:iam::692859939525:role") - """Namespace for write_access_iam_role (e.g. 'arn:aws:iam::692859939525:role')""" - @property def write_access_iam_role(self) -> str | None: """IAM role granting write access to the bucket""" if not self.iam_role: return None - if not self.iam_namespace: + if not self.iam_account: raise ValueError( - f"Bucket {self.name!r} has iam_role={self.iam_role!r} but no iam_namespace" + f"Bucket {self.name!r} has iam_role={self.iam_role!r} but no iam_account" ) - return f"{self.iam_namespace}/{self.iam_role}" + return f"arn:aws:iam::{self.iam_account}:role/{self.iam_role}" s3_bucket_configs = [ From a2f90048d70d5a80d086957910836bd014108ec0 Mon Sep 17 00:00:00 2001 From: Scott Todd Date: Wed, 8 Apr 2026 09:15:32 -0700 Subject: [PATCH 4/4] Use delete=False for tempfile in tests (Windows compat) NamedTemporaryFile(delete=True) holds an exclusive lock on Windows, preventing the code under test from opening the file. Use delete=False with try/finally cleanup, matching the pattern in configure_multi_arch_ci_test.py. Co-Authored-By: Claude Opus 4.6 (1M context) --- build_tools/tests/s3_buckets_test.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/build_tools/tests/s3_buckets_test.py b/build_tools/tests/s3_buckets_test.py index 803d6a8e441..d7b286f9257 100644 --- a/build_tools/tests/s3_buckets_test.py +++ b/build_tools/tests/s3_buckets_test.py @@ -156,10 +156,15 @@ def test_workflow_run_id_triggers_api_call(self): self.assertEqual(config.name, "therock-ci-artifacts") def _write_event(self, event: dict) -> str: - f = tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) - json.dump(event, f) - f.close() - return f.name + """Write a synthetic GitHub event payload to a temp file. + + Returns the path. Caller must os.unlink() after use. + Uses delete=False because NamedTemporaryFile(delete=True) holds an + exclusive lock on Windows, preventing the code under test from reading. + """ + with tempfile.NamedTemporaryFile(mode="w", suffix=".json", delete=False) as f: + json.dump(event, f) + return f.name def test_fork_pr_from_event_payload(self): """Fork PR detected via event payload (no workflow_run dict)."""