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

GUACAMOLE-1325: Migrate from javax to jakarta namespace. #972

Draft
wants to merge 2 commits into
base: next
Choose a base branch
from

Conversation

necouchman
Copy link
Contributor

Okay, well, here we go. This PR takes care of the migration from the javax.* namespaces to the new jakarta.* counterparts, updates dependencies, and updates the licenses. I've got it all in a single commit, now, because I couldn't find very clean places to break up the commit - dependency management at this level reminds me of the cat-in-bathtub gif.

The code does compile, I'm able to install it into Tomcat 10.1, and run it, log in (JDBC), and start Guacamole session. It's worth noting that this will be a big change in terms of compatibility:

  • Right now I'm compiling and running with Java 11.x. I have not gone back to test 8.x, yet - I will, here, soon - but there's a reasonable chance that 8.x will no longer work with some of the dependency updates.
  • There were a couple of dependencies that I could not fully update because they required Java 17, and I wasn't willing to push quite to that level of breakage.
  • This completely removes the Websocket support for Jetty8, Jetty9, and Tomcat 7, relying on the standard WebSocket support that should be built in to anything recent.
  • I have not tested with anything besides Tomcat 10.x, yet. I intend to try Tomcat 11, JBoss, and Wildfly, at least.
  • There is likely a decent amount of cleanup to be done, still, both in pom.xml files, and in the doc/licenses directory. I suspect I've left a lot of unused cruft around in getting things updated.
  • I have not tested all of the extensions, yet - just JDBC. I'll try to get to several of them, soon, but there will likely be some I cannot test.
  • I'm unsure about the future of the RADIUS module - I haven't even tried to compile that one, yet, and, given the age of the jRADIUS library, I'm not sure it can live on as it is. We'll can either find a different RADIUS library, write our own (ick, but not too difficult), drop RADIUS support altogether, or see if there are other options for maintaining compatibility with class loading and the deep magic of Java.

I'm going to put this in Draft mode for now - I welcome comments, requests for changes, and testing.

@necouchman
Copy link
Contributor Author

  • Right now I'm compiling and running with Java 11.x. I have not gone back to test 8.x, yet - I will, here, soon - but there's a reasonable chance that 8.x will no longer work with some of the dependency updates.

Well, turns out Tomcat 10 requires Java 11, so that's settled.

@z0rb
Copy link

z0rb commented May 14, 2024

Hi! Thanks a lot for starting this! I've been trying to find time to try it out.
I assume that the maven snapshots didn't land in a maven repository (yet). Should I fork this and build it separately/internally?
The build is failing, should I take anything into account while building?

@necouchman
Copy link
Contributor Author

@z0rb The code has not been merged, so there will be no artifacts or builds with this change, yet. Yes, you will need to clone/patch locally and build yourself to test it out.

@z0rb
Copy link

z0rb commented May 15, 2024

Looks good on Java 17 and Spring-Boot 3.2.5!
Had no problem with guacamole-common (only module I'm using), most migration effort was from Spring Security.

Thanks again! Hope this makes it into main once the open questions @necouchman mentioned are resolved.

@necouchman
Copy link
Contributor Author

Looks good on Java 17 and Spring-Boot 3.2.5! Had no problem with guacamole-common (only module I'm using), most migration effort was from Spring Security.

Thanks again! Hope this makes it into main once the open questions @necouchman mentioned are resolved.

Thank you for testing! It will not go into main, it will go into next, since it is targeted for a future major release (2.0 or something similar). 1.6.0 will be a feature release.

@kilaketia
Copy link

Thanks for the Tomcat10 patch, I've deployed it and the only issue I have is with the OpenID Connect extension that throw this error :

The "openid" authentication provider has been skipped due to an internal error. If this is unexpected or you are the developer of this authentication provider, you may wish to enable debug-level logging: jakarta.ws.rs.ext.RuntimeDelegate: org.glassfish.jersey.internal.RuntimeDelegateImpl not a subtype

Mind you, I've only used guacamole-history-recording-storage / guacamole-auth-sso-openid / guacamole-auth-totp / guacamole-auth-jdbc.

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