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

[Feature]: Improve service-to-service communication #6450

Closed
5 tasks
xetys opened this issue Oct 1, 2017 · 10 comments
Closed
5 tasks

[Feature]: Improve service-to-service communication #6450

xetys opened this issue Oct 1, 2017 · 10 comments

Comments

@xetys
Copy link
Member

xetys commented Oct 1, 2017

As UAA is out of beta and more oauth2 support is coming to microservice, I would like to propose some enhancements on the current state:

I would like to know if there is interest in these improvements. Possibly something for Hacktoberfest?

@gmarziou
Copy link
Contributor

gmarziou commented Oct 2, 2017

Looks good to me, I also had the need for a RestTemplate and as Spring Cloud supports it with feign clients, I think it makes sense to let developers choose the one they prefer.

@PierreBesson
Copy link
Contributor

I have wanted to do #2 for such a long time and there are many quirks with it. As with JWT you don't have a OAuth Context, you basically have to read headers from the servlet request, but as soon as you do a Feign client request to another microservice it opens a separate Hystrix thread and you lose the "context". So definitly an advanced topic here. The simple workaround is to pass the header in the feignclient like that:

    @RequestMapping(value = "/api/test",
        method = RequestMethod.GET,
        produces = MediaType.APPLICATION_JSON_VALUE)
    void test(@RequestHeader("Authorization") String token);

But this is not optimal.

@mraible
Copy link
Contributor

mraible commented Oct 2, 2017

In my blog post on securing microservices, I used a RequestInterceptor to pass along the header.

package com.example;

import com.stormpath.sdk.servlet.http.Resolver;
import com.stormpath.zuul.account.ForwardedAccountHeaderFilter;
import feign.RequestInterceptor;
import feign.RequestTemplate;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.web.context.request.RequestContextHolder;
import org.springframework.web.context.request.ServletRequestAttributes;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

public class ForwardedAccountRequestInterceptor implements RequestInterceptor {

    private static final Logger LOGGER = LoggerFactory.getLogger(ForwardedAccountRequestInterceptor.class);

    private final Resolver<String> valueResolver;

    public ForwardedAccountRequestInterceptor(Resolver<String> accountStringResolver) {
        this.valueResolver = accountStringResolver;
    }

    @Override
    public void apply(RequestTemplate template) {
        if (template.headers().containsKey(ForwardedAccountHeaderFilter.DEFAULT_HEADER_NAME)) {
            LOGGER.warn("The X-Forwarded-User has been already set");
        } else {
            LOGGER.debug("Constructing Header {} for Account", ForwardedAccountHeaderFilter.DEFAULT_HEADER_NAME);
            HttpServletRequest request = ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes()).getRequest();
            HttpServletResponse response = ((ServletRequestAttributes) RequestContextHolder.getRequestAttributes()).getResponse();
            template.header(ForwardedAccountHeaderFilter.DEFAULT_HEADER_NAME, valueResolver.get(request, response));
        }
    }
}

@cbornet
Copy link
Member

cbornet commented Oct 2, 2017

@PierreBesson @mraible there's a getCurrentUserJWT method that you can use to get the JWT and put it in a Feign interceptor.

@PierreBesson
Copy link
Contributor

Wow @mraible Thanks a lot ! This code should be in the default setup generated by JHipster !
Quoting your article

@HystrixCommand(fallbackMethod = "fallback", commandProperties = {
        @HystrixProperty(name="execution.isolation.strategy", value="SEMAPHORE")
})

I had known that this hystrix property was required but didn't know you could set it like that. IMO, this should be documented on our website.

Also we need to better document the different timeouts. There are many kinds of timeout that must be taken into account when doing microservices:

  • Hystrix timeout (hystrix.command.default.execution.isolation.thread.timeoutInMilliseconds)
  • Zuul timeout (zuul.host.connect-timeout-millis and zuul.host.connect.socket-timeout-millis)
  • Feign timeout (feign.client.config.feignClientName.connectTimeout and readTimeout). We should add default values for those properties as they are not super easy to find.
    @xetys Maybe you can add this to your checklist.
    Note that there used to be a Ribbon timeout as well but it is not taken into account in recent versions of Spring Cloud.

@gmarziou
Copy link
Contributor

gmarziou commented Oct 2, 2017

Similarly, I have used SecurityContextHolder.MODE_INHERITABLETHREADLOCAL to access JWT in async communication with other services, it worked in dev but failed in real world testing. So I removed it to rely on passing token via a method parameter.

@jdubois
Copy link
Member

jdubois commented Dec 4, 2017

  • this issue has been stalled for nearly 2 months
  • we have 44 opened issues!

-> @xetys would you be OK to close it? We just have too much work at the moment

@xetys
Copy link
Member Author

xetys commented Dec 7, 2017

hmm...I just had also almost no time the last weeks...I would love to do this soon, if that's oks

@jdubois jdubois added this to the 4.12.0 milestone Dec 8, 2017
@abhisheksharma85
Copy link

Can someone please guide me or share any resource on how to achieve microservice to microservice calls.

I have 3 microservices where sometime I need to call each other as well as a call to gateway to get user-details.

I am so lost

I m using jhipster with jwt

Looking for jhipster + FeignClient. I don't want to hard code any url or IP addresses.. I want to get host and ip using Eureka also want to avoid JWT for inter-service calls.

Even anyone can share some links to read that would be very helpful

Regards,
Abhi

@pascalgrimaud
Copy link
Member

@abhisheksharma85 :
read this ticket closely and this #3649 too
You have all answers there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants