Skip to content

Gradle wrapper validation all the time #807

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 10, 2025
Merged

Gradle wrapper validation all the time #807

merged 2 commits into from
Feb 10, 2025

Conversation

trask
Copy link
Member

@trask trask commented Feb 9, 2025

Related to #803, this ensures that the OSSF "binary artifacts" verification for gradle wrappers passes

@trask trask requested a review from a team as a code owner February 9, 2025 21:54
marandaneto
marandaneto reviewed Feb 10, 2025
View reviewed changes
.github/workflows/gradle-wrapper-validation.yml Outdated
Comment on lines 4 to 5
push:
pull_request:
Copy link
Member

@marandaneto marandaneto Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should we keep the paths tho? You don't need to run if the wrapper hasn't changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, the OSSF "binary artifacts" check only goes green if it is run on every PR that is merged to main (it looks at CI runs for PRs that are merged to main and verifies that gradle wrapper validation was performed)

Copy link
Member

Choose a reason for hiding this comment

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

gotcha, feels unnecessary and wasted resources but if there's no way around it, LGTM (its a quick check anyway)

Copy link
Contributor

Choose a reason for hiding this comment

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

There might also be some trickery (that I'm not specifically aware of) that could convince a gradle wrapper to run from a different location, so this is probably still nice to have.

breedx-splk
breedx-splk approved these changes Feb 10, 2025
View reviewed changes
Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

Tempted to reject outright with the name change from yaml -> yml 😬 but we're already inconsistent in our use of file suffixes for workflows...... 🤷🏻

@breedx-splk
Copy link
Contributor

Whoops, I think merging #801 broke this. Sorry.

@trask
Copy link
Member Author

trask commented Feb 10, 2025

Tempted to reject outright with the name change from yaml -> yml 😬 but we're already inconsistent in our use of file suffixes for workflows...... 🤷🏻

yeah, the other "Java" repos are using .yml, sorry

@breedx-splk
Copy link
Contributor

Tempted to reject outright with the name change from yaml -> yml 😬 but we're already inconsistent in our use of file suffixes for workflows...... 🤷🏻

yeah, the other "Java" repos are using .yml, sorry

Yeah, well, they're wrong. 😁 Some 8x3 lifers over there..... 🤣

@breedx-splk breedx-splk merged commit 51d1674 into open-telemetry:main Feb 10, 2025
4 checks passed
@trask
Copy link
Member Author

trask commented Feb 10, 2025

8x3 lifers

😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marandaneto marandaneto marandaneto approved these changes

@breedx-splk breedx-splk breedx-splk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@trask @breedx-splk @marandaneto