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/Build: Add Gradle Build Scans #475

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

snazy
Copy link
Member

@snazy snazy commented Nov 25, 2024

Gradle Build Scans, free of use, collect a lot of information about a Gradle build, including the actual output of failed test. This becomes quite convenient when inspecting test failures in CI and a lot of other information about Gradle builds.

Example build scan

@jbonofre
Copy link
Member

jbonofre commented Nov 25, 2024

@snazy what about using ASF Develocity ? It's hosted here: https://ge.apache.org/ and "hosted"/approved by the ASF.

I would prefer to use ASF hosted platform here.

@jbonofre
Copy link
Member

For context: https://infra.apache.org/gradle.html

@snazy snazy force-pushed the gradle-build-scans branch from 4885d3b to 68dbd22 Compare November 25, 2024 10:49
@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

Good point, updated the PR.

@snazy snazy force-pushed the gradle-build-scans branch 8 times, most recently from 6b1b532 to 215632b Compare November 25, 2024 11:46
@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

Well, Apache's instance rejects unauthenticated build scans, which is always the case for PR CI runs (no secrets in those GH workflows). So I think it's fine to let CI runs that don't have the access-key secret publish to Gradle's infra, and those that have it publish to Apache's infra.

@adutra
Copy link
Contributor

adutra commented Nov 25, 2024

Well, Apache's instance rejects unauthenticated build scans, which is always the case for PR CI runs (no secrets in those GH workflows). So I think it's fine to let CI runs that don't have the access-key secret publish to Gradle's infra, and those that have it publish to Apache's infra.

Why can't we configure our PR CI runs to authenticate? The section named "GitHub Actions" in in this page says:

For GitHub Actions builds in the Apache GitHub organization, an access token is stored in the organizational secret GE_ACCESS_TOKEN.

@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

Why can't we configure our PR CI runs to authenticate?

Security thing - to not expose secrets in such CI runs.

@jbonofre
Copy link
Member

Two comments:

  • we need approval from PPMC and security team to export data in a third party instance
  • as other projects (like Apache Beam or Pekko) are using ge.apache.org, I wonder why we would be different 😄

@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

* as other projects (like Apache Beam or Pekko) are using ge.apache.org, I wonder why we would be different 😄

That still requires all GH workflow runs to have access to secrets, which is a big concern IMHO.

@jbonofre
Copy link
Member

@snazy For GitHub Actions builds in the Apache GitHub organization, an access token is stored in the organizational secret GE_ACCESS_TOKEN. For security, GitHub Actions do not pass secrets to runners running workflows triggered from forked repositories.
I don't see the issue here as the GE_ACCESS_TOKEN is setup by INFRA.
For local build, users can use their Apache ID.

@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

@jbonofre GH actions run from forks do NOT have any secrets, that's how GH workflows work.

@jbonofre
Copy link
Member

@snazy yes, that's expected. That's why I don't the security question :) If an user forks Polaris, if he's a Apache committer, he can use his Apache account, else he doesn't push.

Sorry, I failed to see the point 😄

@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

Yes, you failed the point. See the warning box under the discussion of pull_request_target vs the pull_request trigger. Apache Committer or not does not matter here.

@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

TL;DR both scenarios, pull_request_target for CI against apache/polaris and using branches in the apache/polaris repo with pull_request are both huge security risks - see this post.

@jbonofre
Copy link
Member

My point about Apache Committer is for local build (not related to GH Action).

I think I understand your point about PR, but I was talking about "regular" GH Action build.
For PRs, are you proposing to systematically publish build scans ?

@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

For PRs, are you proposing to systematically publish build scans ?

Yes, that's basically the whole point - I want to inspect build scans for PRs, not just test results, but more than that.

@jbonofre
Copy link
Member

@snazy so like this one: https://ge.apache.org/s/7hwlge7hwhch4
It's a build on a PR (apache/beam#32400) on Beam (action https://github.com/apache/beam/actions/runs/12012626739).

So it seems to work on Apache Beam.

@snazy
Copy link
Member Author

snazy commented Nov 25, 2024

@jbonofre that uses the insecure pull_request_target, right?

@jbonofre
Copy link
Member

@snazy I'm checking this.

Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Dec 26, 2024
Gradle Build Scans, free of use, collect a lot of information about a Gradle build, including the actual output of failed test. This becomes quite convenient when inspecting test failures in CI and a lot of other information about Gradle builds.

[Example build scan](https://scans.gradle.com/s/jpuykotf4hac6)
@snazy snazy force-pushed the gradle-build-scans branch from 215632b to ed1054d Compare January 3, 2025 13:25
@snazy snazy removed the Stale label Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants