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

Support ConnectionsDetails and ServiceConnections for Spring Boot 3.1 #793

Closed
eddumelendez opened this issue May 2, 2023 · 15 comments · Fixed by #1075
Closed

Support ConnectionsDetails and ServiceConnections for Spring Boot 3.1 #793

eddumelendez opened this issue May 2, 2023 · 15 comments · Fixed by #1075
Labels
component: core Core functionality related issue type: feature Integration with a new AWS service or bigger change in existing integration
Milestone

Comments

@eddumelendez
Copy link
Contributor

eddumelendez commented May 2, 2023

Type: Feature

First of all, congrats for 3.0.0 release! 🎉

Is your feature request related to a problem? Please describe.
I'm pretty sure you are already having this in mind but just want to sync. This is not a problem but a nice feature when used with upcoming spring boot 3.1 version which can auto-configure spring cloud aws properties (endpoint, access key, secret key, region) when using LocalStackContainer and it can enable Dev Mode too

Describe the solution you'd like
Update existing autoconfigurations to use ConnectionDetails for the properties I mentioned above. Add a new module, spring-cloud-aws-testcontainers which will add a ContainerConnectionsDetailsFactory.

Describe alternatives you've considered
Unless, spring cloud aws provides initial support for ConnectionDetails there is no much to be done.

Additional context
I can contribute this feature but would like to know the timeline for this 😁

@maciejwalkowiak
Copy link
Contributor

Thanks @eddumelendez! There is no way to make ServiceConnections contribute to environment properties instead of changing autoconfigurations to rely on ConnectionDetails? This would be ideal as we could likely continue to maintain single Springs Cloud AWS version for both Spring Boot 3.0 and 3.1

@eddumelendez
Copy link
Contributor Author

@maciejwalkowiak
Copy link
Contributor

maciejwalkowiak commented May 4, 2023

It still feels suboptimal. I don't get why Spring Boot does not make it easier to extend for 3rd party libraries. To have an option to pass something like a service connection configurer:

@TestConfiguration(proxyBeanMethods = false)
public class MyContainersConfiguration {

    @Bean
    @ServiceConnection(LocalstackServiceConnectionConfigurer.class)
    public LocalstackContainer localstackContainer() {
        return new LocalstackContainer();
    }

}

class LocalstackServiceConnectionConfigurer implements ServiceConnectionConfigurer { // i made up this interface
    void apply(LocalstackContainer container, DynamicPropertyRegistry properties) {
       // set properties
    }
}

Probably there is a reason why such thing does not exist, or maybe its something to discuss with the Spring Boot team. I am trying to come up with reasonable option where we don't end up in maintenance hell with multiple versions of Spring Cloud AWS.

@bsideup
Copy link

bsideup commented Jun 6, 2023

That's something that @philwebb and I have discussed while prototyping ideas around the service accounts. The problem is that ServiceConnection isn't based on properties, but objects. You can, however, do the following:

    @Bean
    public LocalstackContainer localstackContainer(DynamicPropertyRegistry properties) {
        var localstack = new LocalstackContainer();
        // populate properties from the container
        return localstack;
    }

@philwebb
Copy link

philwebb commented Jun 6, 2023

Some documentation for this is at https://docs.spring.io/spring-boot/docs/current/reference/htmlsingle/#features.testing.testcontainers.at-development-time.dynamic-properties

@maciejwalkowiak
Copy link
Contributor

@bsideup @philwebb thanks, this looks great! I think it wasn't available at the time I wrote my made-up sample.

@MatejNedic MatejNedic assigned MatejNedic and unassigned MatejNedic Aug 9, 2023
@alikian
Copy link

alikian commented Aug 15, 2023

@maciejwalkowiak I was thinking something more, in application-test.yaml having something like this:

spring:
   cloud:
       localstack: 
            enabled: true
            imageName: localstack/localstack:2.2.0
            cloudformation: test-cloudformation.yaml

And this will:

  • Start localstack container using testcontainer
  • Override endpoint based on started container
  • Override AWS Access Key ID and Secret Key based on started container
  • Override region based on started container
  • Create AWS resources using cloudformation provided

Something like this:
https://github.com/alikian/localstack-loader

@dominik-kovacs
Copy link
Contributor

How could I help to speed this up? @MatejNedic @maciejwalkowiak

@MatejNedic
Copy link
Member

@maciejwalkowiak should we target this for 3.1.0 ?

@maciejwalkowiak
Copy link
Contributor

I think the date for 3.1 will be dictated by Spring Boot and Spring Cloud release, but yes it would be nice to have in 3.1. I guess this shouldn't be much work the question is who has time ;)

@dominik-kovacs
Copy link
Contributor

@maciejwalkowiak @MatejNedic I could try

@maciejwalkowiak
Copy link
Contributor

@poklakni go for it. PRs welcome!

@maciejwalkowiak maciejwalkowiak added this to the 3.1.0 milestone Nov 6, 2023
@dominik-kovacs
Copy link
Contributor

dominik-kovacs commented Nov 8, 2023

I have a little problem with file placement.

I created AwsConnectionDetails, PropertiesAwsConnectionDetails, and LocalStackContainerConnectionDetailsFactory.

AwsConnectionDetails and PropertiesAwsConnectionDetails depend on spring-boot-autoconfigure therefore I assume it should be placed in spring-cloud-aws-autoconfigure module.

But then where should I create LocalStackContainerConnectionDetailsFactory? If I place it to newly created spring-cloud-aws-testcontainers module I would have to add spring-cloud-aws-autoconfigure which does not seem right.

Right now I have all 3 classes in spring-cloud-aws-autoconfigure module which does not seem right either.

This is what I have so far https://github.com/poklakni/spring-cloud-aws/commit/d13267d0cba7910e4d387e80aa3412d4034f0558

@eddumelendez
Copy link
Contributor Author

AwsConnectionDetails and PropertiesAwsConnectionDetails depend on spring-boot-autoconfigure therefore I assume it should be placed in spring-cloud-aws-autoconfigure module.

👍🏽

But then where should I create LocalStackContainerConnectionDetailsFactory? If I place it to newly created spring-cloud-aws-testcontainers module I would have to add spring-cloud-aws-autoconfigure which does not seem right.

When I thought about this, I have the same idea about having spring-cloud-aws-testcontainers, which will bring spring-boot-testcontainers as a transitive dependency and depend on spring-cloud-aws-autoconfigure but marked as an optional dependency.

If your concern is about circular dependency between spring-cloud-aws-autoconfigure and spring-cloud-aws-testcontainers because of moving the IT to the new service connection support. Then, I would say not to move those IT, LocalStackContainerConnectionDetailsFactory should be tested on its own module. Those IT in spring-cloud-aws-autoconfigure could be in another module but it's not my call and probably out of scope of this issue.

@maciejwalkowiak maciejwalkowiak modified the milestones: 3.1.0, 3.1 Dec 9, 2023
@maciejwalkowiak maciejwalkowiak modified the milestones: 3.1.x, 3.2 Jan 15, 2024
@maciejwalkowiak maciejwalkowiak added component: core Core functionality related issue type: feature Integration with a new AWS service or bigger change in existing integration labels Mar 10, 2024
@maciejwalkowiak maciejwalkowiak modified the milestones: 3.2.0 RC1, 3.2.0 M1 Mar 18, 2024
@herder
Copy link

herder commented Mar 18, 2024

Woop, happy news! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: core Core functionality related issue type: feature Integration with a new AWS service or bigger change in existing integration
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants