Skip to content
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

ci: ignore cbmc prereleases #4328

Merged
merged 3 commits into from
Dec 15, 2023
Merged

ci: ignore cbmc prereleases #4328

merged 3 commits into from
Dec 15, 2023

Conversation

lrstewart
Copy link
Contributor

@lrstewart lrstewart commented Dec 15, 2023

Description of changes:

Some of our CBMC proofs are currently failing with a python error from the CBMC viewer about missing goal descriptions: https://github.com/aws/s2n-tls/actions/runs/7215710149/job/19660493232

The issue seems to be the latest CBMC release: https://github.com/diffblue/cbmc/releases The latest release is a pre-release and even comes with a warning about not using it in production. However, the logic in our github action that queries for the latest CBMC release doesn't check the "prerelease" field in the response. I've updated it to ignore prereleases to avoid errors like this without needing to pin the version.

Call-outs:

I preemptively made the same change for litani. The other dependencies use tags to indicate what "latest" means.

Testing:

The CBMC proofs now pass, and you can see that version 5.59.1 is being chosen instead of 6.0.0 here.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Dec 15, 2023
@lrstewart lrstewart marked this pull request as ready for review December 15, 2023 02:36
@@ -62,7 +62,7 @@ jobs:
run: |
# Search within 5 most recent releases for latest available package
CBMC_REL="https://api.github.com/repos/diffblue/cbmc/releases?page=1&per_page=5"
CBMC_DEB=$(curl -s $CBMC_REL --header 'authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' | jq -r '.[].assets[].browser_download_url' | grep -e 'ubuntu-20.04' | head -n 1)
CBMC_DEB=$(curl -s $CBMC_REL --header 'authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' | jq -r '.[]|select(.prerelease|not).assets[].browser_download_url' | grep -e 'ubuntu-20.04' | head -n 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #4329 I proposed to make even more use of jq. Just a suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think the current version with grep + head is more readable. I don't mind the chaining, and jq can get pretty dense.

Copy link
Contributor

@dougch dougch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@goatgoose goatgoose self-requested a review December 15, 2023 17:54
@lrstewart lrstewart enabled auto-merge (squash) December 15, 2023 19:17
@lrstewart lrstewart merged commit c0bcde3 into aws:main Dec 15, 2023
28 checks passed
@lrstewart lrstewart deleted the ci_cbmc branch December 15, 2023 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants