search

openanalytics / containerproxy

Manage HTTP proxy routes into Docker containers
Apache License 2.0
43 stars 66 forks source link

Bump spring boot starter 2.7.2 #74

Closed janhh closed 1 year ago

janhh commented 2 years ago

Bump spring boot starter 2.7.2, lose up circular dependencies, remove WebSecurityConfigurerAdapter.

I've made an initial attempt to upgrade spring boot starter, partly because this is required to complete PR #69.

Tests are passing with mvn test -Dtest=\!TestAppRecovery,\!TestIntegrationOnKube.

LEDfan commented 2 years ago

Hi @janhh

Thanks for the effort, really appreciated! Upgrading Spring Boot is always a bit tricky, but it appears you already have fixed quite some stuff. There might be a less verbose way to fix the circular dependencies, e.g. such as here: https://github.com/openanalytics/containerproxy/blob/master/src/main/java/eu/openanalytics/containerproxy/auth/impl/KeycloakAuthenticationBackend.java#L97

Currently we are still developing a few new features for the next release, once I have some time, I'll merge your PR and perform some extensive testing we have to do. I'll of course merge your other PRs regarding PKCE as well.

janhh commented 2 years ago

Thank you, @LEDfan .

It might work with the @ Lazy annotation. I did an early but not so successful attempt with @ Lazy first. But maybe the explicit approach can make it somewhat easier to see where the circular dependencies appear if there will a design change later.

The changes I did in WebSecurityConfig took me some guesswork, so I think it'll be good if that class gets carefully reviewed by someone else in particular.

I rebased #68 and #69 on top of #74 successfully and it appears to be working fine with ShinyProxy 2.6.1. I am changing this draft to a pull request. 👍

LEDfan commented 1 year ago

Thanks, Spring has been upgrade to 2.7.4 in ShinyProxy 3.0.0!