From 6740bb496bdb0dfd371949a0586ca8435c993e72 Mon Sep 17 00:00:00 2001 From: clee2000 <44682903+clee2000@users.noreply.github.com> Date: Wed, 27 Nov 2024 16:04:39 -0800 Subject: [PATCH] [oidc] Clang tidy binary upload to use OIDC instead of secrets (#5980) The role is defined in https://github.com/pytorch-labs/pytorch-gha-infra/pull/551 This action has only been run 6 times, and has failed 4 of those 6 runs. The most recent run failed due to lacking credentials, however it doesn't appear that those changes did any functionality change for the files related to these workflows. The last successful runs where last year Also * add a python script for checking if its safe to upload the new binary (overwriting with a new file that has a different sha256 will break lintrunner init if the config is not updated. It will also require people to pull the new config) * linux build - only start the container and copy the file, do not upload the image * upload the artifact to GHA artifacts as well * Move sanity check s3 python script + all uploads into reusable action to reduce code duplication --- Testing: Merge the role definition, add my branch in the console, run the workflow (clean up: remove my branch in the console) --- .github/actions/clang-tidy-upload/action.yml | 76 ++++++++++++++++++++ .github/workflows/clang-tidy-linux.yml | 38 +++------- .github/workflows/clang-tidy-macos.yml | 72 ++++++++----------- tools/clang-tidy-checks/check_s3.py | 58 +++++++++++++++ 4 files changed, 175 insertions(+), 69 deletions(-) create mode 100644 .github/actions/clang-tidy-upload/action.yml create mode 100644 tools/clang-tidy-checks/check_s3.py diff --git a/.github/actions/clang-tidy-upload/action.yml b/.github/actions/clang-tidy-upload/action.yml new file mode 100644 index 0000000000..e211d324ae --- /dev/null +++ b/.github/actions/clang-tidy-upload/action.yml @@ -0,0 +1,76 @@ +name: Sanity check and upload clang-tidy and clang-format binaries + +description: Sanity check and upload clang-tidy and clang-format binaries to s3 and GHA + +# Used by clang-tidy-macos and clang-tidy-linux workflows since the sanity +# checks and uploads are pretty much the same for them, so this helps reduce +# code duplication + +inputs: + platform: + description: 'The platform to upload the binaries for' + required: true + version: + description: 'The version of the binaries' + required: true + upload-to-s3: + description: 'Whether to upload the binaries to s3' + required: false + default: false + +runs: + using: composite + steps: + - name: Check if binaries have changed + shell: bash + run: | + set -ex + python3 tools/clang-tidy-checks/check_s3.py \ + --s3-key "${{ inputs.platform }}/${{ inputs.version }}/clang-tidy" \ + --local-file clang-tidy + python3 tools/clang-tidy-checks/check_s3.py \ + --s3-key "${{ inputs.platform }}/${{ inputs.version }}/clang-format" \ + --local-file clang-format + + - name: configure aws credentials + id: aws-credentials + uses: aws-actions/configure-aws-credentials@v4 + if: ${{ fromJSON(inputs.upload-to-s3) }} + with: + role-to-assume: arn:aws:iam::308535385114:role/gha_workflow_clang_tidy_upload + aws-region: us-east-1 + output-credentials: true + + - uses: seemethere/upload-artifact-s3@v5 + name: Publish clang-tidy binary + if: ${{ fromJSON(inputs.upload-to-s3) }} + with: + if-no-files-found: error + s3-prefix: ${{ inputs.platform }}/${{ inputs.version }} + s3-bucket: oss-clang-format + path: clang-tidy + + - uses: seemethere/upload-artifact-s3@v5 + name: Publish clang-format binary + if: ${{ fromJSON(inputs.upload-to-s3) }} + with: + if-no-files-found: error + s3-prefix: ${{ inputs.platform }}/${{ inputs.version }} + s3-bucket: oss-clang-format + path: clang-format + + - name: Upload clang-tidy to GHA + uses: actions/upload-artifact@v4 + with: + name: ${{ inputs.platform }}-${{ inputs.version }}-clang-tidy + retention-days: 14 + if-no-files-found: warn + path: clang-tidy + + - name: Upload clang-format to GHA + uses: actions/upload-artifact@v4 + with: + name: ${{ inputs.platform }}-${{ inputs.version }}-clang-format + retention-days: 14 + if-no-files-found: warn + path: clang-format diff --git a/.github/workflows/clang-tidy-linux.yml b/.github/workflows/clang-tidy-linux.yml index 15260b86b9..75c8c74d2e 100644 --- a/.github/workflows/clang-tidy-linux.yml +++ b/.github/workflows/clang-tidy-linux.yml @@ -14,22 +14,21 @@ on: - '!tools/clang-tidy-checks/README.md' - '.github/workflows/clang-tidy-linux.yml' +permissions: + id-token: write + jobs: build: runs-on: linux.12xlarge steps: - name: Checkout uses: actions/checkout@v4 - - name: Build docker image + + - name: Build docker image and extract binary run: | set -ex echo ${{ secrets.GITHUB_TOKEN }} | docker login ghcr.io -u "$GITHUB_ACTOR" --password-stdin docker build ./tools/clang-tidy-checks --tag ghcr.io/pytorch/cilint-clang-tidy:"$GITHUB_SHA" -f tools/clang-tidy-checks/Dockerfile.cilint-clang-tidy - - name: Publish docker image and extract binary - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - run: | - set -ex - docker push ghcr.io/pytorch/cilint-clang-tidy:"$GITHUB_SHA" # Copying files directly from a docker image is not supported # As a workaround, we create a temporary container, copy the binary, and remove it @@ -37,28 +36,13 @@ jobs: docker cp "$image_id":/clang-tidy-checks/build/bin/clang-tidy ./clang-tidy docker cp "$image_id":/clang-tidy-checks/build/bin/clang-format ./clang-format docker rm -v "$image_id" - - uses: driazati/upload-artifact-s3@50adbe4ef0b6d9221df25c18c5fc528dfcb7c3f8 - name: Publish clang-tidy binary - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - with: - name: clang-tidy - if-no-files-found: error - s3-prefix: linux64/19.1.4 - s3-bucket: oss-clang-format - path: clang-tidy - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - - uses: driazati/upload-artifact-s3@50adbe4ef0b6d9221df25c18c5fc528dfcb7c3f8 - name: Publish clang-format binary - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + + - name: Sanity check and upload + uses: ./.github/actions/clang-tidy-upload with: - name: clang-format - if-no-files-found: error - s3-prefix: linux64/19.1.4 - s3-bucket: oss-clang-format - path: clang-format - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + platform: linux64 + version: 19.1.4 + upload-to-s3: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} diff --git a/.github/workflows/clang-tidy-macos.yml b/.github/workflows/clang-tidy-macos.yml index d69d37b873..fb64b53efb 100644 --- a/.github/workflows/clang-tidy-macos.yml +++ b/.github/workflows/clang-tidy-macos.yml @@ -16,6 +16,9 @@ on: - '!tools/clang-tidy-checks/README.md' - '.github/workflows/clang-tidy-macos.yml' +permissions: + id-token: write + jobs: build-Intel: runs-on: macos-13 @@ -35,28 +38,21 @@ jobs: export PATH ./setup.sh - - uses: driazati/upload-artifact-s3@50adbe4ef0b6d9221df25c18c5fc528dfcb7c3f8 - name: Publish clang-tidy binary - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - with: - name: clang-tidy - if-no-files-found: error - s3-prefix: macos-i386/19.1.4 - s3-bucket: oss-clang-format - path: tools/clang-tidy-checks/llvm-project/build/bin/clang-tidy - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - - uses: driazati/upload-artifact-s3@50adbe4ef0b6d9221df25c18c5fc528dfcb7c3f8 - name: Publish clang-format binary - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + + - name: Move files for s3 upload + run: | + set -ex + mv tools/clang-tidy-checks/llvm-project/build/bin/clang-tidy \ + tools/clang-tidy-checks/llvm-project/build/bin/clang-format \ + . + + - name: Sanity check and upload + uses: ./.github/actions/clang-tidy-upload with: - name: clang-format - if-no-files-found: error - s3-prefix: macos-i386/19.1.4 - s3-bucket: oss-clang-format - path: tools/clang-tidy-checks/llvm-project/build/bin/clang-format - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + platform: macos-i386 + version: 19.1.4 + upload-to-s3: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + build-M1: runs-on: macos-m1-stable steps: @@ -75,28 +71,20 @@ jobs: export PATH ./setup.sh - - uses: driazati/upload-artifact-s3@50adbe4ef0b6d9221df25c18c5fc528dfcb7c3f8 - name: Publish clang-tidy binary - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} - with: - name: clang-tidy - if-no-files-found: error - s3-prefix: macos-arm/19.1.4 - s3-bucket: oss-clang-format - path: tools/clang-tidy-checks/llvm-project/build/bin/clang-tidy - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} - - uses: driazati/upload-artifact-s3@50adbe4ef0b6d9221df25c18c5fc528dfcb7c3f8 - name: Publish clang-format binary - if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + + - name: Move files for s3 upload + run: | + set -ex + mv tools/clang-tidy-checks/llvm-project/build/bin/clang-tidy \ + tools/clang-tidy-checks/llvm-project/build/bin/clang-format \ + . + + - name: Sanity check and upload + uses: ./.github/actions/clang-tidy-upload with: - name: clang-format - if-no-files-found: error - s3-prefix: macos-arm/19.1.4 - s3-bucket: oss-clang-format - path: tools/clang-tidy-checks/llvm-project/build/bin/clang-format - aws-access-key-id: ${{ secrets.AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.AWS_SECRET_ACCESS_KEY }} + platform: macos-arm64 + version: 19.1.4 + upload-to-s3: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} concurrency: group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} diff --git a/tools/clang-tidy-checks/check_s3.py b/tools/clang-tidy-checks/check_s3.py new file mode 100644 index 0000000000..52c1384d3a --- /dev/null +++ b/tools/clang-tidy-checks/check_s3.py @@ -0,0 +1,58 @@ +""" +This script checks if a file on s3 doesn't exist or matches the local file. If +this script returns a non zero exit code, it means that the file you are trying +to upload will overwrite an existing file on s3. If the s3 path is used in +lintrunner on pytorch/pytorch, this will break `lintrunner init`. If this +returns with exit code 0, then it is safe to upload to s3 for usage with +lintrunner in pytorch/pytorch. If you upload a new file, remember to add the +s3 path and hash to the lintrunner s3 init config: +https://github.com/pytorch/pytorch/blob/915625307eeda338fef00c984e223c5774c00a2b/tools/linter/adapters/s3_init_config.json#L1 +""" +import hashlib +from urllib.request import Request, urlopen +import argparse +from urllib.error import HTTPError + + +def download_s3_file(s3_key): + url = f"https://oss-clang-format.s3.us-east-2.amazonaws.com/{s3_key}" + req = Request(url) + try: + with urlopen(req) as response: + # File exists, return the contents + return response.read() + except HTTPError as e: + if "The specified key does not exist" in e.read().decode(): + # Acceptable error, file can be uploaded safely without overwriting + print(f"Cannot find the file on s3") + return + raise + + +def hash_file(file): + with open(file, "rb") as f: + return hashlib.sha256(f.read()).hexdigest() + + +def check_s3_file(s3_key, local_file): + s3_file = download_s3_file(s3_key) + local_hash = hash_file(local_file) + if not s3_file: + print(f"Hash of local file: {local_hash}") + return + s3_hash = hashlib.sha256(s3_file).hexdigest() + if local_hash != s3_hash: + raise RuntimeError(f"Hash mismatch for {local_file}: {local_hash} != {s3_hash}") + print(f"Hashes for local file and remote file match: {local_hash}") + + +def parse_args(): + parser = argparse.ArgumentParser() + parser.add_argument("--s3-key", required=True) + parser.add_argument("--local-file", required=True) + return parser.parse_args() + + +if __name__ == "__main__": + args = parse_args() + check_s3_file(args.s3_key, args.local_file)