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

#292 Migrated to Spring Boot 3 using OpenRewrite #293

Merged

Conversation

jimbethancourt
Copy link
Contributor

@jimbethancourt jimbethancourt commented Jun 29, 2023

Please note:

  • Galapagos is now on Java 17 and Spring Boot 3
  • SecurityConfig has been commented out since Keycloak is deprecated and not supported in Spring Boot 3 / Spring 6. Unfortunately I don't know how to implement OAuth 2 at this time either.

I can't seem to figure out how to run it locally - I get a 401 error when I try to run it from my IDE with the demo,democonf,actuator profiles:

Could not access or read /service_accounts workaround endpoint: Server returned 401 for /service_accounts

OpenRewrite configuration used to perform migration:

<plugin>
	<groupId>org.openrewrite.maven</groupId>
	<artifactId>rewrite-maven-plugin</artifactId>
	<version>5.2.4</version>
	<configuration>
		<activeRecipes>
			<recipe>org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_0</recipe>
		</activeRecipes>
	</configuration>
	<dependencies>
		<dependency>
			<groupId>org.openrewrite.recipe</groupId>
			<artifactId>rewrite-spring</artifactId>
			<version>5.0.2</version>
		</dependency>
	</dependencies>
</plugin>

Please note:
- Galapagos is now on Java 17
- KafkaSenderImpl no longer needs KafkaFutureDecoupler since kafkaTemlate.send() now returns a CompletableFuture
- KafkaSenderImplTest now performs an assertFalse() as a result
- SecurityConfig has been commented out since Keycloak is deprecated and not supported in Spring Boot 3 / Spring 6. Unfortunately I don't know how to implement OAuth 2 at this time either.

I can't seem to figure out how to run it locally - I get a 401 error when I try to run it from my IDE with the demo,democonf,actuator profiles:

Could not access or read /service_accounts workaround endpoint: Server returned 401 for /service_accounts

OpenRewrite configuration used to perform migration:
```xml
<plugin>
	<groupId>org.openrewrite.maven</groupId>
	<artifactId>rewrite-maven-plugin</artifactId>
	<version>5.2.4</version>
	<configuration>
		<activeRecipes>
			<recipe>org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_0</recipe>
		</activeRecipes>
	</configuration>
	<dependencies>
		<dependency>
			<groupId>org.openrewrite.recipe</groupId>
			<artifactId>rewrite-spring</artifactId>
			<version>5.0.2</version>
		</dependency>
	</dependencies>
</plugin>
```
@jimbethancourt
Copy link
Contributor Author

Creating a separate work branch since SecurityConfig needs to be re-implemented may be the best approach

@albrechtflo-hg albrechtflo-hg added good first issue Good for newcomers enhancement New feature or request and removed good first issue Good for newcomers labels Jul 3, 2023
@albrechtflo-hg
Copy link
Member

Hi Jim, thanks for your contribution!

The KafkaFutureDecoupler had not only the purpose of converting the Future, but also to really decouple the Futures from the Core Kafka Thread. Without this, concatenated calls like myKafkaRepo.save(o).thenCompose(x -> doSomethingElseWithKafka()) lead to deadlocks. So I think we will have to keep it, unfortunately. (We can also test if it is really needed, or if the Threading is more clean now inside the Core library, but that was hard to reproduce when we had these issues...)

I will have a look in replacing the Keycloak lib in Spring Security. I already did that in another project, so that should not be much of an issue.

The service_accounts workaround endpoint from Confluent is, fortunately, no longer required (since few weeks now). You can set ccloud.serviceAccountIdCompatMode to false (in the cluster-specific config), which should avoid these calls.

@jimbethancourt
Copy link
Contributor Author

Hi Florian,
It's my pleasure! I think I see what you mean about the KafkaFutureDecoupler. That was one of the few manual changes I needed to make and can probably be undone. I'm not sure offhand how to best approach it - there was enough of a change that it didn't compile correctly after I applied the OpenRewrite recipe.

I'm relieved to hear that you've done the migration off of Keycloak before.

Thanks for letting me know about the service_accounts workaround as well!

I'm not sure exactly how much help I can be going forward. Unfortunately I'm busy the next few weeks focusing on other things, but hopefully I can try out any changes you make.

Thanks,
Jim

albrechtflo-hg and others added 4 commits July 11, 2023 12:08
Also, minor fixes in Spring Boot 3 context, and cleanups on the way.
…oaded method to KafkaFutureDecoupler to accept new return type of kafkaTemplate.send()
@jimbethancourt
Copy link
Contributor Author

@albrechtflo-hg I've reverted changes to KafkaSenderImpl and calling classes and added an overloaded method to KafkaFutureDecoupler
The only change that needs to happen now is to re-implement OAuth2 😃

@albrechtflo-hg albrechtflo-hg merged commit d31a895 into HermesGermany:next_minor Jul 20, 2023
3 checks passed
@albrechtflo-hg
Copy link
Member

OK, it was not as simple as expected to replace Keycloak; mainly because Galapagos uses roles configured in Keycloak clients for authorization, and because the UI used lots of stuff from the Keycloak lib.

There are now some new properties to configure the required realm names to use from the JWT, and a whole new OAuth2 configuration file, replacing the keycloak.json. Added is also a migration guide for Galapagos 2.8, which will contain this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants