ci: use tidbx images for next-gen integration#1995
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
ti-chi-bot
Bot
added
contribution
📝 WalkthroughWalkthroughThe integration workflow expands branch triggers to include ChangesNext-Gen TiKV Integration Workflow
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ti-chi-bot
Bot
added
the
size/M
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/integration.yml (1)
201-215: ⚡ Quick winConsider pinning actions to SHA hashes for supply chain security.
The static analysis tool flagged
google-github-actions/auth@v3anddocker/login-action@v3as unpinned. SHA-pinned references (e.g.,google-github-actions/auth@<commit-sha>) prevent tag mutations from silently changing action behavior.However, I note that existing actions in this file (
actions/checkout@v2,actions/setup-go@v4,shrink/actions-docker-extract@v1) also use version tags rather than SHA hashes, so this is consistent with current practice. A comprehensive pinning effort could be addressed separately.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/integration.yml around lines 201 - 215, The workflow uses unpinned action versions (google-github-actions/auth@v3 and docker/login-action@v3); replace each tag with the corresponding commit SHA to pin the action (e.g., google-github-actions/auth@<commit-sha> and docker/login-action@<commit-sha>) by looking up the canonical commit in each action's GitHub repo and updating the uses: fields for the "Authenticate to Google Cloud" (auth) step and the "Login to GAR" step so the workflow references the exact commit SHA instead of the v3 tag.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/integration.yml:
- Around line 201-215: The workflow uses unpinned action versions
(google-github-actions/auth@v3 and docker/login-action@v3); replace each tag
with the corresponding commit SHA to pin the action (e.g.,
google-github-actions/auth@<commit-sha> and docker/login-action@<commit-sha>) by
looking up the canonical commit in each action's GitHub repo and updating the
uses: fields for the "Authenticate to Google Cloud" (auth) step and the "Login
to GAR" step so the workflow references the exact commit SHA instead of the v3
tag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ccd89ac-7384-4d43-882f-74912dffde5d
📒 Files selected for processing (1)
.github/workflows/integration.yml
wuhuizuo
force-pushed
the
feat/nextgen-tidbx-gar-auth
branch
from
c4acd8b to
68e22e0
Compare
ti-chi-bot
Bot
added
dco-signoff: yes
wuhuizuo
force-pushed
the
feat/nextgen-tidbx-gar-auth
branch
from
68e22e0 to
dab0fe2
Compare
|
Updated this PR:
Current tag rules are:
|
wuhuizuo
force-pushed
the
feat/nextgen-tidbx-gar-auth
branch
from
dab0fe2 to
026b825
Compare
|
Follow-up update:
|
Signed-off-by: wuhuizuo <wuhuizuo@126.com>
wuhuizuo
force-pushed
the
feat/nextgen-tidbx-gar-auth
branch
from
026b825 to
7d2bab5
Compare
|
/test ? |
|
@wuhuizuo: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test ? |
|
@wuhuizuo: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/integration.yml (1)
177-177: ⚡ Quick winPin actions to commit SHAs for supply-chain security.
The
google-github-actions/authanddocker/login-actionare referenced by mutable tags (@v3). For jobs withid-token: writepermission, pinning to commit SHAs prevents a compromised tag from obtaining OIDC tokens. While the existing workflow uses tag references, these new actions handle sensitive credentials.Proposed fix (example SHAs - verify current releases)
- name: Authenticate to Google Cloud id: auth - uses: google-github-actions/auth@v3 + uses: google-github-actions/auth@71f986410dfbc7added4569a411d84f104ca3d5 # v3.0.0 with: project_id: pingcap-testing-account - name: Login to GAR - uses: docker/login-action@v3 + uses: docker/login-action@74a5d142397b4f367a81961eba4e8cd7edddf772 # v3.4.0 with: registry: us-docker.pkg.devAlso applies to: 185-185
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/integration.yml at line 177, The workflow uses mutable action tags (google-github-actions/auth@v3 and docker/login-action@v3) which is a supply-chain risk; update the action references to specific commit SHAs (replace the `@v3` references for the steps that call google-github-actions/auth and docker/login-action with the corresponding verified commit SHA for each action), verify the SHA against the official repo release you intend to use, and ensure the steps that request id-token: write continue to function after pinning.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/integration.yml:
- Line 138: The job currently runs for forked PRs and checks only for the
'skip-integration-tests' label; update the job's if condition to also require
the PR originate from the same repository by combining the existing
contains(...) check with a same-repo check such as
github.event.pull_request.head.repo.full_name == github.repository (e.g. if: ${{
github.event.pull_request.head.repo.full_name == github.repository &&
!contains(github.event.pull_request.labels.*.name, 'skip-integration-tests')
}}), so the job runs only for same-repository PRs and avoids OIDC auth failures
for forks.
---
Nitpick comments:
In @.github/workflows/integration.yml:
- Line 177: The workflow uses mutable action tags (google-github-actions/auth@v3
and docker/login-action@v3) which is a supply-chain risk; update the action
references to specific commit SHAs (replace the `@v3` references for the steps
that call google-github-actions/auth and docker/login-action with the
corresponding verified commit SHA for each action), verify the SHA against the
official repo release you intend to use, and ensure the steps that request
id-token: write continue to function after pinning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 629642fd-9ea3-4dc1-80fc-9ec799409aa9
📒 Files selected for processing (1)
.github/workflows/integration.yml
| @@ -137,6 +137,9 @@ jobs: | |||
| integration-next-gen-tikv: | |||
| if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-integration-tests') }} | |||
There was a problem hiding this comment.
Consider gating the job for same-repository PRs only.
Per PR objectives, this job should request cloud credentials "only for same-repository PRs." Currently, the job runs for fork PRs but will fail at the auth step since GitHub doesn't grant OIDC tokens to forks. Adding an explicit condition avoids confusing failures:
Proposed fix
integration-next-gen-tikv:
- if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-integration-tests') }}
+ if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-integration-tests') && (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) }}
runs-on: ubuntu-latest📝 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.
| if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-integration-tests') }} | |
| if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-integration-tests') && (github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository) }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/integration.yml at line 138, The job currently runs for
forked PRs and checks only for the 'skip-integration-tests' label; update the
job's if condition to also require the PR originate from the same repository by
combining the existing contains(...) check with a same-repo check such as
github.event.pull_request.head.repo.full_name == github.repository (e.g. if: ${{
github.event.pull_request.head.repo.full_name == github.repository &&
!contains(github.event.pull_request.labels.*.name, 'skip-integration-tests')
}}), so the job runs only for same-repository PRs and avoids OIDC auth failures
for forks.
|
/ok-to-test |
|
/test ? |
|
/test ? |
|
@wuhuizuo: The following commands are available to trigger optional jobs: Use DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/test pull-integration-nextgen |
|
/retest |
|
@wuhuizuo: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Summary
pingcap-testing-account/tidbxrelease-nextgen-*uses the branch name whilemain/masteruse the fixed next-gen tagsImage mapping
us-docker.pkg.dev/pingcap-testing-account/tidbx/tikv/pd/image:<tag>us-docker.pkg.dev/pingcap-testing-account/tidbx/tikv/tikv/image:<tag>Tag rules
release-nextgen-*-><tag>is the target branch namemain/master-> PDmaster-nextgen, TiKVcloud-engine-nextgenNotes
projects/890604261603/locations/global/workloadIdentityPools/github-tikv/providers/github-oidcand thetidbx-gar-reader@pingcap-testing-account.iam.gserviceaccount.combinding are in placeSummary by CodeRabbit