ci: Add CI jobs to detect proto/OpenAPI breaking changes#2229
ci: Add CI jobs to detect proto/OpenAPI breaking changes#2229thossain-nv wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-06-05 01:00:49 UTC | Commit: 79ed891 |
| uses: oasdiff/oasdiff-action/breaking@v0.0.51 | ||
| with: | ||
| # Compare the exact base and head commits from this PR. | ||
| base: '${{ github.event.pull_request.base.sha }}:rest-api/openapi/spec.yaml' |
There was a problem hiding this comment.
Our pipeline use copy-pr-bot when we create a pull request. The bot create a push event on pull-request, so github.event.pull_request.base.sha is not populated. we can change to push-safe ref, e.g. origin/main:rest-api/openapi/spec.yaml
There was a problem hiding this comment.
Thanks Larry, updated.
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
| - name: Check for Breaking Changes | ||
| # If this action is not approved, the following command can be used instead: | ||
| # buf breaking crates/rpc/proto --against 'https://github.com/NVIDIA/infra-controller.git#branch=main,subdir=crates/rpc/proto' | ||
| uses: bufbuild/buf-breaking-action@v1 |
There was a problem hiding this comment.
The third-party GitHub Actions should pin to immutable commit SHAs instead of mutable tags like @v1 or @v0.0.51. This keeps CI reproducible and reduces supply-chain drift.
It looks like the action isn’t on the allow list yet. I can help file a request to get it approved.
There was a problem hiding this comment.
bug breaking supports github-actions error format, I think we can skip the action.
| - name: Check OpenAPI breaking changes | ||
| # If this action is not approved, the following command can be used instead: | ||
| # oasdiff breaking <(git show origin/main:rest-api/openapi/spec.yaml) rest-api/openapi/spec.yaml --fail-on ERR | ||
| uses: oasdiff/oasdiff-action/breaking@v0.0.51 |
There was a problem hiding this comment.
Same here: it would be better pin this action by commit SHA instead of a mutable tag.
Just checked this action is already on our allow list.
There was a problem hiding this comment.
is v0.0.51 a mutable tag though?
There was a problem hiding this comment.
ah i see the guidance from github here now says to use sha for 3rd party tags: https://docs.github.com/en/actions/reference/security/secure-use#using-third-party-actions. We should mark in a comment above the version to know what the sha pins to.
There was a problem hiding this comment.
Pinned to specific commit.
There was a problem hiding this comment.
LGTM other than needing to adjust 3rd party actions to use sha ref over tag ref. See https://docs.github.com/en/actions/reference/security/secure-use#using-third-party-actions
69044a1 to
6d2641d
Compare
|
🌿 Preview your docs: https://nvidia-preview-pull-request-2229.docs.buildwithfern.com/infra-controller |
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
6d2641d to
3a43dd0
Compare
Description
We want to initially run these jobs for informational purposes, and later decide which of them should block PR merging.
Note
The actions being used may not be NVIDIA approved. If we can't get them approved, I've added alternative commands
Type of Change
Related Issues (Optional)
None
Breaking Changes
Testing
Additional Notes
None