jglick commented 1 month ago

https://github.com/jenkins-infra/github-reusable-workflows/blob/4f9dc4e44f4acd0c2a353b22f0b061e3ae6e7fee/.github/workflows/maven-cd.yml#L84 picks up some oldish Maven version 3.8.8, which is not new enough to handle Java 17 bytecode. Analysis in https://github.com/jenkinsci/jenkins-test-harness/pull/847. See https://github.com/actions/setup-java/issues/685 for possible fix.

basil commented 1 month ago

See https://github.com/actions/setup-java/issues/685 for possible fix.

See https://github.com/jenkins-infra/jenkins-security-scan/pull/32 (a different fix for the same issue) for comparison. I am not implying that one solution is better or worse than another, but rather I am mentioning the alternative for completeness.

jglick commented 1 month ago

The action mentioned there looks more convenient. On the other hand the maven-cd workflow is sensitive to supply-chain attacks, so if there is no official actions/* option to pick a Maven version, it seems safer to just download it explicitly in this workflow. CC @daniel-beck @MarkEWaite @jonesbusy for visibility

basil commented 1 month ago

Neither proposed solution seems to do checksum verification of the downloaded Maven binary, which is concerning to me.

jglick commented 1 month ago

Does it matter, if the tarball is downloaded via HTTPS from an official mirror? Admittedly it puts less burden on mirrors to use HTTP for the download.

basil commented 1 month ago

Only if the underlying storage is unreliable, but I have seen that several times throughout my career.

daniel-beck commented 1 month ago

I'd have pinned upstream hash, but doing it ourselves makes sense to me. No outsized review burden for dependency updates.

basil commented 1 month ago

Lazy consensus decision

We would like this task to be implemented in both github-reusable-workflows and jenkins-security-scan as in https://github.com/actions/setup-java/issues/685 but with additional checksum verification of the downloaded Maven binary.

This reduces the surface area of supply chain attacks and the review burden for dependency updates, satisfying https://github.com/jenkins-infra/github-reusable-workflows/issues/36#issuecomment-2383881689 and https://github.com/jenkins-infra/github-reusable-workflows/issues/36#issuecomment-2384927072.
This eliminates fragility due to unstable storage, satisfying https://github.com/jenkins-infra/github-reusable-workflows/issues/36#issuecomment-2383889901 and https://github.com/jenkins-infra/github-reusable-workflows/issues/36#issuecomment-2384157917.
This does not call Maven actions on Gradle-based plugins, satisfying https://github.com/jenkins-infra/jenkins-security-scan/pull/32#discussion_r1782710872.
This decision stands despite the fact that other solutions were described as "more convenient" in https://github.com/jenkins-infra/github-reusable-workflows/issues/36#issuecomment-2383881689, implied to be acceptable given a pinned upstream hash in https://github.com/jenkins-infra/github-reusable-workflows/issues/36#issuecomment-2384927072, and (even) recommended in https://github.com/jenkins-infra/jenkins-security-scan/pull/32#discussion_r1782710872.

timja commented 1 month ago

FWIW I have also requested the maven version to be updated on the runners in https://github.com/actions/runner-images/issues/10715, maybe we can wait a little?

jonesbusy commented 1 month ago

FWIW I have also requested the maven version to be updated on the runners in actions/runner-images#10715, maybe we can wait a little?

Not sure if it will happen anytime soon. From what I see is that Maven 3.9.0 cannot publish to GH Maven packages due to authentication: https://github.com/orgs/community/discussions/49001

It only works by changing the resolver transport mvn ... -Dmaven.resolver.transport=wagon ... deploy

timja commented 1 month ago

Yes but that was resolved in 3.9.1 so hopefully a non issue now

jonesbusy commented 1 month ago

I'm not 100% sure.

Last time I was testing with 3.9.9 a SNAPSHOT deploy I got

authentication failed for https://maven.pkg.github.com/jonesbusy/oras-java/land/oras/java/oras-java-sdk/0.0.1-SNAPSHOT/maven-metadata.xml, status: 401 Unauthorized -> [Help 1]

Like the authorization is completly ignored

My only solution was to fallback on -Dmaven.resolver.transport=wagon

timja commented 1 month ago

I tried now and it worked fine: https://github.com/timja-org/maven-test-deploy/packages/2272360

❯ mvn --version
Apache Maven 3.9.9 (8e8579a9e76f7d015ee5ec7bfcdc97d260186937)
Maven home: /opt/homebrew/Cellar/maven/3.9.9/libexec
Java version: 17, vendor: Azul Systems, Inc., runtime: /Library/Java/JavaVirtualMachines/zulu-17.jdk/Contents/Home
Default locale: en_GB, platform encoding: UTF-8
OS name: "mac os x", version: "15.0", arch: "aarch64", family: "mac"

maven-test-deploy [🌱 main][?][📦 v1.0-SNAPSHOT][☕ v17]
❯ mvn deploy
[INFO] Scanning for projects...
[INFO]
[INFO] --------------------< com.github.timja:deploy-test >--------------------
[INFO] Building deploy-test 1.0-SNAPSHOT
[INFO]   from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------
[INFO]
[INFO] --- resources:3.3.1:resources (default-resources) @ deploy-test ---
[INFO] skip non existing resourceDirectory /Users/timja/code/timja/maven-test-deploy/src/main/resources
[INFO]
[INFO] --- compiler:3.13.0:compile (default-compile) @ deploy-test ---
[INFO] Nothing to compile - all classes are up to date.
[INFO]
[INFO] --- resources:3.3.1:testResources (default-testResources) @ deploy-test ---
[INFO] skip non existing resourceDirectory /Users/timja/code/timja/maven-test-deploy/src/test/resources
[INFO]
[INFO] --- compiler:3.13.0:testCompile (default-testCompile) @ deploy-test ---
[INFO] Nothing to compile - all classes are up to date.
[INFO]
[INFO] --- surefire:3.3.0:test (default-test) @ deploy-test ---
[INFO] Using auto detected provider org.apache.maven.surefire.junitplatform.JUnitPlatformProvider
[INFO]
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running com.github.timja.AppTest
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.016 s -- in com.github.timja.AppTest
[INFO]
[INFO] Results:
[INFO]
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO]
[INFO] --- jar:3.4.2:jar (default-jar) @ deploy-test ---
[INFO] Building jar: /Users/timja/code/timja/maven-test-deploy/target/deploy-test-1.0-SNAPSHOT.jar
[INFO]
[INFO] --- install:3.1.2:install (default-install) @ deploy-test ---
[INFO] Installing /Users/timja/code/timja/maven-test-deploy/pom.xml to /Users/timja/.m2/repository/com/github/timja/deploy-test/1.0-SNAPSHOT/deploy-test-1.0-SNAPSHOT.pom
[INFO] Installing /Users/timja/code/timja/maven-test-deploy/target/deploy-test-1.0-SNAPSHOT.jar to /Users/timja/.m2/repository/com/github/timja/deploy-test/1.0-SNAPSHOT/deploy-test-1.0-SNAPSHOT.jar
[INFO]
[INFO] --- deploy:3.1.2:deploy (default-deploy) @ deploy-test ---
Downloading from github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/1.0-SNAPSHOT/maven-metadata.xml
Uploading to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/1.0-SNAPSHOT/deploy-test-1.0-20241003.091323-1.pom
Uploaded to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/1.0-SNAPSHOT/deploy-test-1.0-20241003.091323-1.pom (3.4 kB at 972 B/s)
Uploading to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/1.0-SNAPSHOT/deploy-test-1.0-20241003.091323-1.jar
Uploaded to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/1.0-SNAPSHOT/deploy-test-1.0-20241003.091323-1.jar (2.9 kB at 875 B/s)
Downloading from github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/maven-metadata.xml
Downloaded from github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/maven-metadata.xml (239 B at 326 B/s)
Uploading to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/1.0-SNAPSHOT/maven-metadata.xml
Uploaded to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/1.0-SNAPSHOT/maven-metadata.xml (771 B at 1.3 kB/s)
Uploading to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/maven-metadata.xml
Uploaded to github: https://maven.pkg.github.com/timja-org/maven-test-deploy/com/github/timja/deploy-test/maven-metadata.xml (319 B at 521 B/s)
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  10.187 s
[INFO] Finished at: 2024-10-03T10:13:33+01:00
[INFO] ------------------------------------------------------------------------

jglick commented 1 month ago

37 seems to work. I do not think we need to wait for the official runner to update; anyway we may prefer going forward to retain tighter control over the version.

jenkins-infra / github-reusable-workflows

Use a newer Maven version for CD workflow #36

Lazy consensus decision

37 seems to work. I do not think we need to wait for the official runner to update; anyway we may prefer going forward to retain tighter control over the version.