Added unit-test codecov upload#1092
Conversation
AdamSaleh
force-pushed
the
codecov_upload
branch
3 times, most recently
from
781e926 to
54f6b6b
Compare
| fi | ||
| curl -Os https://cli.codecov.io/latest/linux/codecov | ||
| chmod +x codecov | ||
| CODECOV_TOKEN="$(cat "${CODECOV_TOKEN_FILE}")" ./codecov upload-process --file cover.out |
There was a problem hiding this comment.
should we also specify the flag "unit-tests" so we can distinguish between test types in a future once we start uploading e2e test coverage too?
e.g.
./codecov upload-process \
--token "${CODECOV_TOKEN}" \
--flag unit-tests \
--file coverage.out
see the jira ticket for more details
There was a problem hiding this comment.
Hm, fair point. Will add.
AdamSaleh
force-pushed
the
codecov_upload
branch
4 times, most recently
from
685a077 to
7345417
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a guarded Codecov upload step to the OpenShift CI presubmit unit test script: downloads a pinned Codecov CLI, verifies its PGP signature and SHA256, and uploads ChangesCodecov integration in unit test CI
sequenceDiagram
participant presubmit_script as presubmit_script
participant codecov_cli as codecov_v11_2_8
participant codecov_service as Codecov
presubmit_script->>codecov_cli: curl download binary
presubmit_script->>codecov_cli: verify SHA256 checksum and PGP signature
presubmit_script->>codecov_cli: ./codecov upload-process cover.out --flag unit-tests
codecov_cli->>codecov_service: upload coverage using token
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![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
🤖 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 `@scripts/openshiftci-presubmit-unittests.sh`:
- Around line 39-41: Replace the unpinned fetch of the Codecov CLI (the curl of
https://cli.codecov.io/latest/linux/codecov and subsequent chmod +x and
execution via CODECOV_TOKEN="$(cat "${CODECOV_TOKEN_FILE}")" ./codecov
upload-process) with a pinned-version workflow: define a CODECOV_CLI_VERSION,
download the matching release binary URL (not "latest") and its published
checksum/signature, verify the downloaded binary against the checksum or
signature before marking executable, and fail the job if verification fails;
keep the same upload invocation (./codecov upload-process --flag unit-tests
--file cover.out) after verification. Ensure the script exits non-zero on
checksum/signature mismatch so the untrusted binary is never executed.
🪄 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: Enterprise
Run ID: 7f9af1b9-747c-450d-967a-db51fa652e80
📒 Files selected for processing (1)
scripts/openshiftci-presubmit-unittests.sh
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/openshiftci-presubmit-unittests.sh (1)
33-44:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winPrevent executing Codecov CLI when its SHA256 checksum verification fails
set +einside the subshell means a non-zerosha256sum --check --statuswon’t stop the script, sochmod +x codecovand./codecov upload-process ...can still run. Gate installation/execution on successful checksum verification.Suggested fix
( set +e CODECOV_TOKEN_FILE="/var/run/codecov-token/CODECOV_TOKEN" @@ CODECOV_VERSION="v11.2.8" CODECOV_SHA="8930c4bb30254a42f3d8c340706b1be340885e20c0df5160a24efa2e030e662b" curl -OSs --fail-with-body "https://cli.codecov.io/${CODECOV_VERSION}/linux/codecov" - echo "${CODECOV_SHA} codecov" | sha256sum --check --status + if ! echo "${CODECOV_SHA} codecov" | sha256sum --check --status; then + echo "Codecov CLI checksum verification failed; skipping upload" + exit 1 + fi chmod +x codecov CODECOV_TOKEN="$(cat "${CODECOV_TOKEN_FILE}")" ./codecov upload-process --flag unit-tests --file cover.out ) || echo "Coverage upload to codecov.io failed, continuing"🤖 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 `@scripts/openshiftci-presubmit-unittests.sh` around lines 33 - 44, The script currently ignores a failed SHA256 check because set +e allows execution to continue; change the flow so that the sha256 verification (using CODECOV_SHA and the sha256sum --check --status invocation) must succeed before making the downloaded file executable and running CODECOV (chmod +x codecov and CODECOV_TOKEN=... ./codecov upload-process). Implement a conditional or explicit exit on failure around the sha256sum check (e.g., run the check and if it fails exit non-zero) so that chmod +x codecov and the CODECOV_TOKEN export and ./codecov upload-process are only executed when the checksum passes.
🤖 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.
Outside diff comments:
In `@scripts/openshiftci-presubmit-unittests.sh`:
- Around line 33-44: The script currently ignores a failed SHA256 check because
set +e allows execution to continue; change the flow so that the sha256
verification (using CODECOV_SHA and the sha256sum --check --status invocation)
must succeed before making the downloaded file executable and running CODECOV
(chmod +x codecov and CODECOV_TOKEN=... ./codecov upload-process). Implement a
conditional or explicit exit on failure around the sha256sum check (e.g., run
the check and if it fails exit non-zero) so that chmod +x codecov and the
CODECOV_TOKEN export and ./codecov upload-process are only executed when the
checksum passes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: c7538288-9a6e-4e98-b819-de4c5cd02ccc
📒 Files selected for processing (1)
scripts/openshiftci-presubmit-unittests.sh
There was a problem hiding this comment.
Can we follow the official documentation for binary integrity check as well https://docs.codecov.com/docs/the-codecov-cli#integrity-checking-the-binary? while keeping the version pin intact
from my limited knowledge on the subject, my understanding is sha256sum does file integrity check but does verify the source of the file.
|
Good idea, ammended |
There was a problem hiding this comment.
Seems like the token is not mounted, is this expected?
+ set +e
+ CODECOV_TOKEN_FILE=/var/run/codecov-token/CODECOV_TOKEN
+ [[ ! -f /var/run/codecov-token/CODECOV_TOKEN ]]
+ echo 'Codecov token not found at /var/run/codecov-token/CODECOV_TOKEN, skipping upload'
Codecov token not found at /var/run/codecov-token/CODECOV_TOKEN, skipping upload
+ exit 0
There was a problem hiding this comment.
It works now! Had to upload it directly through bash, but it works - https://app.codecov.io/github/redhat-developer/gitops-operator/commit/a25055be57a7a7f1b49924435b35c066410b3920
|
/retest |
|
/test ci/prow/unit |
|
/test unit |
AdamSaleh
force-pushed
the
codecov_upload
branch
5 times, most recently
from
2227840 to
b486458
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: varshab1210 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 |
|
/lgtm |
|
/test v4.14-ci-index-gitops-operator-bundle |
|
/test v4.14-kuttl-parallel |
|
/test v4.19-kuttl-parallel |
1 similar comment
|
/test v4.19-kuttl-parallel |
|
@AdamSaleh: 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. |
Signed-off-by: Adam Saleh <adam@asaleh.net>
Pin the codecov CLI download to a specific version (v11.2.8) and verify the SHA256 checksum before executing, addressing the supply-chain risk of fetching an unpinned "latest" binary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Adam Saleh <adam@asaleh.net>
|
New changes are detected. LGTM label has been removed. |
5 participants
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
We should be tracking our code coverage, this should enable it in such away that it won't cause test to fail if upload fails.