-
Notifications
You must be signed in to change notification settings - Fork 187
Cherry pick PR #8701: Add Smoke Tests for Raspberry Pi and RDK #9024
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 26.eap
Are you sure you want to change the base?
Conversation
CI: Add Smoke Tests for Raspberry Pi and RDK This change enables the execution of smoke tests for Raspberry Pi (raspi) and RDK platforms. It integrates these tests into the existing internal_tests GitHub Action workflow. Configuration for these platforms has been updated in the respective JSON files. The on_device_tests_gateway_client.py script has been modified to support these tests. Bug: 470180018 --------- Co-authored-by: Eric Zhang <ericquinzhang@google.com> (cherry picked from commit fa9672c)
|
Caution There were merge conflicts while cherry picking! Check out cherry-pick-26.eap-8701 and fix the conflicts before proceeding. Check the log at https://github.com/youtube/cobalt/actions/runs/21836044105 for details. |
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request cherry-picks changes to add smoke tests for Raspberry Pi and RDK platforms, involving updates to GitHub Actions workflows, platform configurations, and the test gateway client script. A critical security concern has been identified: a potential command injection vulnerability in the .github/actions/e2e_tests/action.yaml file. This vulnerability arises from the direct interpolation of GitHub Actions expressions into a bash script within a run block, and recommendations have been provided to mitigate this by using environment variables. Additionally, two significant functional issues require attention: a critical issue in on_device_tests_gateway_client.py where the new artifact path construction logic is likely to break existing E2E tests for all platforms other than RDK and Raspberry Pi, and a high severity issue in the Raspberry Pi configuration (evergreen-arm-hardfp-raspi.json) where E2E tests appear to be disabled, which contradicts the PR's objective.
| else: | ||
| bigstore_path = f'/bigstore/{args.cobalt_path}/{args.artifact_name}' | ||
| files.append(f'cobalt_path={bigstore_path}') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic for constructing bigstore_path for non-raspi/rdk platforms appears to be incorrect due to a related change in .github/actions/e2e_tests/action.yaml.
The GCS_ARTIFACTS_PATH environment variable is now set to .../${{ matrix.platform }}/${{ matrix.platform }}_qa. This code then constructs bigstore_path as /bigstore/{args.cobalt_path}/{args.artifact_name}, resulting in a path with an extra /${{ matrix.platform }}_qa segment: /bigstore/.../${{ matrix.platform }}/${{ matrix.platform }}_qa/${args.artifact_name}.
This will likely break E2E tests for all existing platforms (e.g., Android) as the artifact path will be incorrect. The original path was /bigstore/.../${{ matrix.platform }}/${args.artifact_name}.
Please revise the path construction logic to ensure it works for all platforms.
| --artifact_name '${{ inputs.artifact_name }}' \ | ||
| --device_family '${{ inputs.test_device_family }}' \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using GitHub Actions expressions ${{ ... }} directly in a run block is a security risk. If the input contains shell metacharacters (like single quotes or semicolons), it can lead to command injection. To remediate this, pass the inputs as environment variables to the step and use the environment variables in the shell script.
Note: You should also add these variables to the env section of this step:
ARTIFACT_NAME: ${{ inputs.artifact_name }}
DEVICE_FAMILY: ${{ inputs.test_device_family }} --artifact_name "$ARTIFACT_NAME" \
--device_family "$DEVICE_FAMILY" \| - name: Run Tests on ${{ matrix.platform }} Platform | ||
| env: | ||
| GCS_ARTIFACTS_PATH: /bigstore/${{ env.PROJECT_NAME }}-test-artifacts/${{ github.workflow }}/${{ github.run_number }}/${{ matrix.platform }}/${{ inputs.artifact_name }} | ||
| GCS_ARTIFACTS_PATH: ${{ env.PROJECT_NAME }}-test-artifacts/${{ github.workflow }}/${{ github.run_number }}/${{ matrix.platform }}/${{ matrix.platform }}_qa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To safely handle the new inputs in the run block and prevent command injection, they should be mapped to environment variables here.
GCS_ARTIFACTS_PATH: ${{ env.PROJECT_NAME }}-test-artifacts/${{ github.workflow }}/${{ github.run_number }}/${{ matrix.platform }}/${{ matrix.platform }}_qa
ARTIFACT_NAME: ${{ inputs.artifact_name }}
DEVICE_FAMILY: ${{ inputs.test_device_family }}Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Removed duplicate 'test_package' entry from configuration.
Removed duplicate 'test_package' entry.

Refer to the original PR: #8701
CI: Add Smoke Tests for Raspberry Pi and RDK
This change enables the execution of smoke tests for Raspberry Pi (raspi) and RDK platforms. It integrates these tests into the existing internal_tests GitHub Action workflow. Configuration for these platforms has been updated in the respective JSON files. The on_device_tests_gateway_client.py script has been modified to support these tests.
Bug: 470180018