Repository navigation
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 SummarySummary by CodeRabbit
WalkthroughAdds two manually triggered workflows. One validates a pull request before dispatching a private smoke workflow. The other validates a public CI run and its Windows Arm64 artifacts before dispatching private validation. Both verify the returned private workflow run. ChangesCross-repository PR smoke
Private WoA validation
Priority: ⬇️ Low Change: Feature Merge Risk: 🟡 Moderate · up to Protect the private-dispatch credential and fix both run checks before merging; either manual broker can report failure after starting its private run.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/woa-private-dispatch.yml (1)
67-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Derive the CUDA build version from
ci/versions.ymlatpublic_sha. Do not hard-code13.4.2.Two values are fixed to
13.4.2:
- the producer job name
Build win-arm64, CUDA 13.4.2 / py3.13;- the artifact name
cuda-bindings-python313-cuda13.4.2-win-arm64-<sha>.In
.github/workflows/ci.yml, both values come from.cuda.build.version(Lines 224-242 and 444-467). After the next CUDA version bump, this broker will reject every valid public run until someone edits this file. The failure is safe, but the broker becomes unusable.Fix: read
ci/versions.ymlatpublic_shawithgh api repos/NVIDIA/cuda-python/contents/ci/versions.yml?ref=$public_sha. Then build the job name and the artifact names from.cuda.build.version.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
3bdf7831-9285-4563-bf65-3d0b14907b12
📒 Files selected for processing (2)
.github/workflows/woa-crossrepo-smoke.yml.github/workflows/woa-private-dispatch.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| dispatch-private-validation: | ||
| needs: resolve-public-build | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
| permissions: {} | ||
| steps: | ||
| - name: Create private dispatch token | ||
| id: private-app-token | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3 | ||
| with: | ||
| client-id: ${{ vars.CUDA_PYTHON_WOA_XREPO_CI_PRIVATE_APP_CLIENT_ID }} | ||
| private-key: ${{ secrets.CUDA_PYTHON_WOA_XREPO_CI_PRIVATE_APP_PRIVATE_KEY }} | ||
| owner: NVIDIA-dev | ||
| repositories: cuda-python-private | ||
| permission-actions: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
important: Put the private-dispatch App key behind a protected environment.
dispatch-private-validation has no environment:. As a result, CUDA_PYTHON_WOA_XREPO_CI_PRIVATE_APP_PRIVATE_KEY is a repository-level or organization-level secret. A user with write access can push a branch that changes this workflow and dispatch it from that branch.
The modified workflow can then mint an actions: write token for NVIDIA-dev/cuda-python-private. That token can:
- dispatch arbitrary workflows, including GB10 jobs;
- cancel runs;
- delete runs, logs, and artifacts.
All checks in resolve-public-build run from the dispatched ref, so a modified branch can bypass all of them. An if: github.ref == 'refs/heads/main' guard does not help for the same reason.
Fix:
- Move the secret, and optionally the client ID variable, to an environment.
- Set that environment's deployment branch policy to
mainonly. - Reference the environment from this job.
dispatch-private-validation:
needs: resolve-public-build
runs-on: ubuntu-latest
timeout-minutes: 10
+ environment: woa-private-dispatch
permissions: {}Apply the same change to the dispatch job in woa-crossrepo-smoke.yml. That job uses the same credential pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| dispatch-private-validation: | |
| needs: resolve-public-build | |
| runs-on: ubuntu-latest | |
| timeout-minutes: 10 | |
| permissions: {} | |
| steps: | |
| - name: Create private dispatch token | |
| id: private-app-token | |
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3 | |
| with: | |
| client-id: ${{ vars.CUDA_PYTHON_WOA_XREPO_CI_PRIVATE_APP_CLIENT_ID }} | |
| private-key: ${{ secrets.CUDA_PYTHON_WOA_XREPO_CI_PRIVATE_APP_PRIVATE_KEY }} | |
| owner: NVIDIA-dev | |
| repositories: cuda-python-private | |
| permission-actions: write | |
| dispatch-private-validation: | |
| needs: resolve-public-build | |
| runs-on: ubuntu-latest | |
| timeout-minutes: 10 | |
| environment: woa-private-dispatch | |
| permissions: {} | |
| steps: | |
| - name: Create private dispatch token | |
| id: private-app-token | |
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3 | |
| with: | |
| client-id: ${{ vars.CUDA_PYTHON_WOA_XREPO_CI_PRIVATE_APP_CLIENT_ID }} | |
| private-key: ${{ secrets.CUDA_PYTHON_WOA_XREPO_CI_PRIVATE_APP_PRIVATE_KEY }} | |
| owner: NVIDIA-dev | |
| repositories: cuda-python-private | |
| permission-actions: write |
Source: Path instructions
|
|
The PR needs manual trigger to run smoke test and dispatch-validate test, if I understand correctly. Are we able to test the manual trigger when the PR is open, or we need to merge the PR to allow manual trigger? |
|
The PR introduces a two-phase design: a lightweight smoke check between the public and private repos to cheaply validate cross-repo connectivity/config, followed by a dispatch-validate step that verifies a merged main build and triggers real GB10 run. This lets connectivity/config issues surface cheaply in the first phase, before the costlier validation phase. LGTM. I just have a few questions. |
There's a way, as pointed out here: https://github.com/NVIDIA-dev/cuda-python-private/pull/615#issuecomment-5797975839 But there's still a gap: |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
f257626e-4b8d-488c-bad5-26dc8c1e75d6
📒 Files selected for processing (2)
.github/workflows/woa-transport-check.yml.github/workflows/woa-validation-dispatch.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
| "repos/NVIDIA-dev/cuda-python-private/actions/runs/$private_run_id") | ||
| jq -e ' | ||
| .repository.id == 809898190 and | ||
| .path == ".github/workflows/ctk-next-woa-transport-check.yml" and |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
important: Include the dispatch ref in the workflow-path check.
When the private dispatch returns a run, the workflow-runs API can report .path as .github/workflows/ctk-next-woa-transport-check.yml@ctk-next. The current jq -e comparison omits @ctk-next, so set -euo pipefail fails the broker after dispatch. This blocks the manually triggered connectivity smoke, so this is a major workflow failure rather than a harmless discrepancy.
Suggested fix
--- "a/.github/workflows/woa-transport-check.yml"
+++ "b/.github/workflows/woa-transport-check.yml"
@@ -99,7 +99,7 @@
"repos/NVIDIA-dev/cuda-python-private/actions/runs/$private_run_id")
jq -e '
.repository.id == 809898190 and
- .path == ".github/workflows/ctk-next-woa-transport-check.yml" and
+ .path == ".github/workflows/ctk-next-woa-transport-check.yml@ctk-next" and
.event == "workflow_dispatch" and
.head_branch == "ctk-next"
' <<< "$run" >/dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .path == ".github/workflows/ctk-next-woa-transport-check.yml" and | |
| .path == ".github/workflows/ctk-next-woa-transport-check.yml@ctk-next" and |
| "repos/NVIDIA-dev/cuda-python-private/actions/runs/$private_run_id") | ||
| jq -e ' | ||
| .repository.id == 809898190 and | ||
| .path == ".github/workflows/ctk-next-woa-validation.yml" and |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
important: Match the ref-qualified private workflow path.
The dispatch uses ref: "ctk-next", so the workflow-runs API returns .path as .github/workflows/ctk-next-woa-validation.yml@ctk-next. The current comparison rejects that valid run after dispatch, causing the exact-run broker to fail every successful validation. No fallback accepts the ref-qualified path.
Suggested fix
--- "a/.github/workflows/woa-validation-dispatch.yml"
+++ "b/.github/workflows/woa-validation-dispatch.yml"
@@ -185,7 +185,7 @@
"repos/NVIDIA-dev/cuda-python-private/actions/runs/$private_run_id")
jq -e '
.repository.id == 809898190 and
- .path == ".github/workflows/ctk-next-woa-validation.yml" and
+ .path == ".github/workflows/ctk-next-woa-validation.yml@ctk-next" and
.event == "workflow_dispatch" and
.head_branch == "ctk-next"
' <<< "$run" >/dev/null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .path == ".github/workflows/ctk-next-woa-validation.yml" and | |
| .path == ".github/workflows/ctk-next-woa-validation.yml@ctk-next" and |
lijinf2
left a comment
lijinf2
left a comment
There was a problem hiding this comment.
LGTM.
.path mismatched issue raised by coderabbit seems not blocking merge. We can confirm whether that is an actual issue after merge.
So we will have two new rows under Github Action. One is "woa-transport-check", and the other is "woa-validation-dispath". Does "woa-validation-dispath" changes status (i.e. green or red) before the private run finished or after?
This branch has not been deployed
2 participants
Description
Tracks https://github.com/NVIDIA-dev/cuda-python-private/issues/584.
This is public PR A of the four-PR WoA cross-repository CI rollout. It adds two manually dispatched brokers and enables no automatic
maintrigger:mainCI run, its producer jobs, SHA ancestry, artifact names, IDs, and server digests before dispatching private validation.The private-dispatch App credential is referenced only by isolated jobs with
permissions: {}. Those jobs do not check out source or download artifacts.Depends on private foundation PR https://github.com/NVIDIA-dev/cuda-python-private/pull/650.
Rollout
After both foundation PRs merge:
WoA cross-repository transport smokeagainst a public draft PR and verifycuda-python WoA integration / transport smokecompletes neutral without a GB10 job.WoA exact public-main dispatchwith known eligible run37333876562and verify the canonical check plus the minimal GB10 import test.Validation
actionlintpasses for both added workflows.git diff --checkpasses.37333876562.Checklist