CloudMap support for spring aws applications

[hariohmprasath]

:loudspeaker: Type of change

• [ ] Bugfix
• [x] New feature
• [ ] Enhancement
• [ ] Refactoring

:scroll: Description

This pull request adds support for integrating spring boot application with AWS cloudmap. Here is an example of how an simple yaml file integration looks like for both cloudmap registration and discovery

aws:
    cloudmap:
        discovery:
            discoveryList:
            -   healthCheckProtocol: http
                healthCheckResourcePath: /health
                healthCheckThreshold: 5
                service: test-service
                serviceNameSpace: test-namespace
            failFast: true
        enabled: true
        region: us-east-1
        registry:
            description: Namespace for sample cloudmap registry service
            port: 80
            service: service-service
            serviceNameSpace: service-namespace

You can also use annotations to register the spring boot applications to cloudmap

@CloudMapRegistry(
		serviceNameSpace = "dark1-namespace",
		service = "dark1-service",
		port = 80
)
public class TestController {}

Limitation: This pull requests support cloudmap discovery on both EC2 and Fargate based ECS instances, but for service registry currently it only supported for Fargate instance as it uses theECS_CONTAINER_METADATA_URI_V4 URL.

:bulb: Motivation and Context

Adds support for cloudmap integration with spring boot applications for both service registry and discovery

#5

:green_heart: How did you test it?

I executed the following tests:

• Ran unit test cases and made sure nothing failed
• Deployed the sample spring boot app checked in part of this pull request in ECS fargate instance. The application could successfully discover and register the application to AWS cloudmap.

:pencil: Checklist

• [x] I reviewed submitted code
• [x] I added tests to verify changes
• [ ] I updated reference documentation to reflect the change
• [x] All tests passing
• [x] No breaking changes

:crystal_ball: Next steps

• Complete code review
• Add support for cloudmap registration from EC2 instances

[hariohmprasath]

@hariohmprasath thank you so much for the PR! I am really excited about this one TBH and I don't know why. I have submitted some comments to help you organize the modules much better. Please, don't forget to add yourself as an author for the classes that have been touched. @eddumelendez Thanks a lot for all the support and helping with the review. Really appreciate it. I have responded to your comments and I am happy to answer any further questions you may have.

I'm not an expert in cloudmap and left one question to be clear about it. Also, it would be nice to add some javadocs to know more the class responsibility. Sure I have added more javadocs. Once the changes are approved, I'm planning to create a blog post with end to end deployment scripts using AWS CDK (which can build a ECS cluster and deploying this application) so it would be easier for anyone to incorporate this in future.

[hariohmprasath]

Hi @spencergibb, @eddumelendez, @MatejNedic, @jkuipers,

Hope everyone had a wonderful weekend. I have incorporated everyone's feedback, just want to check whether we are fine in merging this pull request or do we have any feedback that you would like to incorporate. Happy to do so.

Let me know, appreciate all the love and support from this community. Thanks.

[hariohmprasath]

Thanks for the integration with AWS Cloud Map. I added a few comments.

Thanks @vanekjar for all the feedback. I have incorporated the changes. By the way @vanekjar is part of the AWS cloudmap development team, thanks for accepting the request for review and validating the correctness of the integration.

[hariohmprasath]

Hi @eddumelendez & @vanekjar, I have taken care of all the requested changes, so following up on this PR to see whether this can get merged to the repository?

Thanks Hari

[eddumelendez]

@hariohmprasath sorry for the delay. I will review on Thursday but still see things to improve. I think we should see at how https://github.com/spring-cloud/spring-cloud-consul/ has been implemented. More details on Thursday.

[hariohmprasath]

@hariohmprasath sorry for the delay. I will review on Thursday but still see things to improve. I think we should see at how https://github.com/spring-cloud/spring-cloud-consul/ has been implemented. More details on Thursday.

Thanks, appreciate the help on this

[eddumelendez]

@hariohmprasath please make sure this PR only contains your changes. it is hard to review.

[hariohmprasath]

@hariohmprasath please make sure this PR only contains your changes. it is hard to review.

@eddumelendez this PR only contains changes related to cloudmap integration. We got lot of good feedback from this community and everything related to this feature has been incorporated part of this PR.

Thanks Hari

[eddumelendez]

check the commits tab, please

[hariohmprasath]

check the commits tab, please

There were other changes that were happening in this branch in parallel while this pull request was open. So I have to rebase my changes with the target branch to make it compatible for getting this merged, which has led to other commits getting in the way of this one. If you remember I infact created a discussion to ask a few questions around this.

What do you suggest? I am open to make things easier for you to review so I can take care of the comments. Let me know your thoughts on this. Thanks.

[eddumelendez]

after the rebase is done only your commits should be displayed. looks like in the process some commits from other authors keep in your branch. there is no need to update the PR with changes happening in the main branch but this time I will appreciate to do it and only keep your commits, please

[hariohmprasath]

after the rebase is done only your commits should be displayed. looks like in the process some commits from other authors keep in your branch. there is no need to update the PR with changes happening in the main branch but this time I will appreciate to do it and only keep your commits, please

Are you fine in reviewing it this time? or do you want me to change the PR. Sorry I am trying to understand your response. Appreciate your quick responses. Thanks

[eddumelendez]

@hariohmprasath please, just make sure the branch contains only your commits related to cloudmap integration

[hariohmprasath]

please, just make sure the branch contains only your commits related to cloudmap integration

Hi @eddumelendez, I might need some help on this. Only way I can think of doing this is creating a new fork/branch and merging the cloudmap changes that we have in this pull request on to the new branch. Do you want me to that?

Or do we have a way of doing what you have in this current pull request? I know we are almost there with a lot of reviews done and feedback incorporated frankly I am not an expert with GIT so can you please help with this? Any pointer to how I can do this on the current one will be really helpful.

Thanks in advance

• Hari

[hariohmprasath]

Hi @eddumelendez & @spencergibb , Thanks to @juho9000 I finally cleaned up this pull request with just my commits. Will you be able to review it now? If you feedback drop a comment, I can address them.

Thanks Hari

[hariohmprasath]

Hi @eddumelendez Will you be able to review this pull request? Currently it just contains my changes.

[juho9000]

Is there still something that needs to be done to get this merged? @eddumelendez

[juho9000]

It's been a long time since the required changes were submitted, any chance of reviewing? Eagerly waiting for these changes to be merged! Pinging @eddumelendez @maciejwalkowiak

[hariohmprasath]

Hi @eddumelendez and @maciejwalkowiak, This PR can serve as a substitute for customers using eurkera and moving into AWS. If you have any suggestions or recommendations on the PR please let me know, happy to work on it. Thanks.

[cyky]

Is there more work needed to get this merged @maciejwalkowiak and @eddumelendez? It would be really helpful to get this feature merged soon.

[willome]

Any news for this PR ?

[migroskub]

Any news?

[hariohmprasath]

Hi @eddumelendez & @maciejwalkowiak, Can you please respond to this PR? I see a lot of interest around this. If you still feel this is not the right place to get it merged, that's fine too, but please respond so we can either close this PR or merge it to the main branch.

Hari

[eddumelendez]

@hariohmprasath first of all, apologies for the long absence. We are still interested in this feature and I left a comment to something similar to spring-cloud-consul-discovery-module which will fit with what CloudMap provides.

[kdhrubo]

We also need this feature urgently. When can this merged to a release. Is this compatible with with Spring Boot 2.6.x?

[eddumelendez]

@kdhrubo currently, @hariohmprasath is working on some changes to make it work. This a contribution for an external member of the team coding of his spare time and we don't have a target date for this feature.

[kdhrubo]

@eddumelendez - how can i help to get this merged? I can help test this in my dev environment.

Also will this also support service to service load balancing with spring cloud load balancer and feign?

[eddumelendez]

Indeed, our suggestion is this one which as far as I remember use the load balancer. Not sure about feign tho, IMHO there is no development needed on this side for that

[kdhrubo]

Sounds good @eddumelendez . Do you need testing help? Given that dev is completed - any reviews pending?

[hariohmprasath]

Hi @kdhrubo, Based on @eddumelendez suggestion. I rewrote this integration using spring discovery. You can find a first draft of the PR here:

https://github.com/hariohmprasath/spring-cloud-aws/commit/4339b0a6d32bf8c7e5248f93fb6f0d3dc8edae90

I have reached out @eddumelendez and team for some early peer review before I wrap up the testcases and submit a PR (more polished one :) ) thanks again for your patience.