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

Use a newer Maven version for CD workflow #36

Closed
jglick opened this issue Sep 26, 2024 · 13 comments · Fixed by #37
Closed

Use a newer Maven version for CD workflow #36

jglick opened this issue Sep 26, 2024 · 13 comments · Fixed by #37
Labels
bug Something isn't working

Comments

@jglick
Copy link
Collaborator

jglick commented Sep 26, 2024

uses: actions/setup-java@v4
picks up some oldish Maven version 3.8.8, which is not new enough to handle Java 17 bytecode. Analysis in jenkinsci/jenkins-test-harness#847. See actions/setup-java#685 for possible fix.

@basil
Copy link

basil commented Sep 27, 2024

See actions/setup-java#685 for possible fix.

See jenkins-infra/jenkins-security-scan#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
Copy link
Collaborator Author

jglick commented Sep 30, 2024

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
Copy link

basil commented Sep 30, 2024

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

@jglick
Copy link
Collaborator Author

jglick commented Sep 30, 2024

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
Copy link

basil commented Sep 30, 2024

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

@daniel-beck
Copy link

daniel-beck commented Oct 1, 2024

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

@basil
Copy link

basil commented Oct 2, 2024

Lazy consensus decision

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

@timja timja added the bug Something isn't working label Oct 2, 2024
@timja
Copy link
Member

timja commented Oct 2, 2024

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?

@jonesbusy
Copy link

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
Copy link
Member

timja commented Oct 3, 2024

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

@jonesbusy
Copy link

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
Copy link
Member

timja commented Oct 3, 2024

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
Copy link
Collaborator Author

jglick commented Oct 3, 2024

#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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants