spring-cloud / spring-cloud-netflix

Integration with Netflix OSS components
http://cloud.spring.io/spring-cloud-netflix/
Apache License 2.0
4.87k stars 2.44k forks source link

getURI() in EurekaServiceInstance misses custom contextPath. #737

Closed thomasdarimont closed 4 years ago

thomasdarimont commented 8 years ago

I have several Spring Boot Apps with REST endpoints with custom server.contextPath's which use @EnableDiscoveryClient to register themselves in the eureka service registry.

The applications are registered with eureka at startup and later queried by a management application. I just found out that the URIs returned by

org.springframework.cloud.netflix.eureka.EurekaDiscoveryClient.EurekaServiceInstance.getUri()

only contain scheme,hostname and port but no custom context-path.

The URIs were returned by discoveryClient.getInstances(serviceId) where discoveryClient is an EurekaDiscoveryClient.

The URI one gets via

public URI getUri() {
  return DefaultServiceInstance.getUri(this);
}

looks like:

http://localhost:8761

The proper URI would be:

http://localhost:8761/mycontextpath/

I fixed tested this by locally shadowing the org.springframework.cloud.netflix.eureka.EurekaDiscoveryClient and replacing the body of the URI getUri() method with:

@Override
public URI getUri() {

  //HACK to get proper URI with context-path back
  //return DefaultServiceInstance.getUri(this);
  return URI.create(this.instance.getHomePageUrl());
}

With that adjustment the org.springframework.cloud.client.ServiceInstance that I got back returned the proper service URIs.

I think the op of the issue #500 has the same problem.

Another Problem was that I needed to explicitly declare a

@Bean
public EurekaInstanceConfig eurekaConfigBean() {
  return new EurekaInstanceConfigBean();
}

in order to get custom eureka instance settings in an applications.yml file taken into account at all but I guess that's for another issue ;-)

server:
  port: 9999
  context-path: /admin
...
eureka:
  instance:
    statusPageUrlPath: /admin/manage/info
    homePageUrlPath: /admin/
    healthCheckUrlPath: /admin/manage/health
    non-secure-port: ${server.port:9999}
    ...
spencergibb commented 8 years ago

We've not extended eureka to know about an instances context path yet.

cbelleza commented 8 years ago

Hi,

When will it be available to download?

spencergibb commented 8 years ago

@ofbizbrazil it hasn't been done, until it has, it won't be available to download.

cbelleza commented 8 years ago

@spencergibb,

My applications have context-path at uri, is there any workaround until this is done?

spencergibb commented 8 years ago

https://github.com/spring-cloud/spring-cloud-netflix/issues/737#issue-123043546

cbelleza commented 8 years ago

Hi @spencergibb, of course i saw that comment, but my doubt is how to apply this 'locally shadowing' mentioned in there.

I guess the spring package was patched to overwrite the class EurekaDiscoveryClient. Do you know how to do it?

thomasdarimont commented 8 years ago

Hi @ofbizbrazil,

(as a quick hack) just copy the org.springframework.cloud.netflix.eureka.EurekaDiscoveryClient in your local source folder (e.g. src/main/java) with the same package structure and change the method in question as described. Your classes take precedence over classes from additional libraries.

Cheers, Thomas

cbelleza commented 8 years ago

That's I was afraid of, doing a patch into my project to handle the class EurekaDiscoveryClient.

thomasdarimont commented 8 years ago

Yep - same here - I just used this to verify what a potential solution might be... ;-)

spencergibb commented 8 years ago

What's the use case? Why do you need this?

cbelleza commented 8 years ago

Nowadays we are limiting an application to only use a root context, and if I want to change the context-path to a different name, EurekaDiscoveryClient simply does work and nobody will know about it.

So, in my opinion that's a critical issue, taking in mind Spring Netflix only works for a specific URI.

spencergibb commented 8 years ago

I guess I think it's strange that you would change the context of a service and not expect clients to change. This has been marked as Help Wanted. Pull requests are welcome.

cbelleza commented 8 years ago

My micro-service uses a custom context-path and the clients which uses it really works fine.

The issue here is EurekaDiscoveryClient does support context-path when the method getUri() builds the URI. There should be the methods getContextPath or getHomePageUrl in EurekaServiceInstance to allow this resource.

sfat commented 8 years ago

@spencergibb I wanted to do a fix on this fella the other day When I modified the EurekaDiscoveryClient.getUri to what @thomasdarimont suggested, DiscoveryClientConfigServiceAutoConfigurationTests fails on onWhenRequested because DiscoveryClientConfigServiceBootstrapConfiguration sets the homePage as follows

private String getHomePage(ServiceInstance server) {
        return server.getUri().toString() + "/";
 }

So, I think if it is decided to change the EurekaDiscoveryClient.getUri, to modify in spring-cloud-config the homePage

kk-manh commented 8 years ago

Hi, I ran into the same problem and looks like the current implementation assumes only one eureka client application per JVM (web)container. Is this a restriction being imposed ? If so , then it needs to be documented else it needs to be fixed to include a configured context-path as part of the look ups

cbelleza commented 8 years ago

is ther any news about this matter?

dilgerma commented 8 years ago

Is this still the case? I´m running in to this problem at the moment using the default ribbon configuration.

cjemison commented 7 years ago

I am curious. Has there been any update to this issue? Thanks!

spencergibb commented 7 years ago

No update. It's not high on our priorities since it can be done with properties. Pull requests welcome.

cbos commented 7 years ago

Use case for this issue: We have a java application deployed on Tomcat with a context root. The urls look like '/myapp/url_path_in_the_application. Now we have also deploy the application in on Docker. Now the urls are the same, but without the context root '/myapp', so only '/url_path_in_the_application'.

We are looking for a way that we can register the old version with context root '/myapp' and the new version with '/' as context root. For the client the invocation is easy base_url + '/url_path_in_the_application'. Base_url is created based on the registration in Eureka.

TateHuang commented 7 years ago

@cbos the exact same problem,any suggestions for now?

cbos commented 7 years ago

We fixed the issue ourselves, as we did register the services ourselves to Eureka. So we added the context path as setting to the metadata. In this specific situation we did not use the default eureka client, so we were able to read that context path setting and use that in the request as well. In the default setup it is hard to fix it yourself.

cecchisandrone commented 7 years ago

@cbos How did you register your service manually with custom context-path?

cbos commented 7 years ago

We have our own client which registers the service to Eureka. With that we add teh context-path in the metadata.

rax215 commented 7 years ago

@cbos could you please provide any code snippet you used in your client to register context path?

silentFred commented 7 years ago

Hi @spencergibb,

Thank you for your insights! So in my case we are running multiple services in a single weblogic container and they all need different context paths. The port number is also fixed, so that's not an option.

You mentioned that this can be changed with properties? I have looked into the home page url properties and tried every combination I could think of with no luck.

kasibkismath commented 7 years ago

Hi @spencergibb,

I too face the same issue as @silentFred . It would be great if you could provide us some insights on this issue.

I have tried the following as of now and it has failed:

  1. turbine.istance-url-suffix. together with turbine.cluster-config
  2. Setting eureka.instance.metadataMap.management.context-path and eureka.instance.home-page-url-path the same values in the microservice's application property.

The context path is not added to the hystrix stream url, it's still http://host:port/hystrix.stream, it isn't http://host:port/context-path/hystrix.stream

As @silentFred addressed, the host and the port are same for all the microservice while the context paths are different.

Note: The microservice, turbine-app are deployed in weblogic 12c and the eureka server is running in tomcat.

spencergibb commented 7 years ago

@kasibkismath you are the first to mention turbine, so I'm not sure if it's the same issue.

kasibkismath commented 7 years ago

@spencergibb, since I have the context path added to each microservice's weblogic.xml, the hystrix dashboard is accessible through http://host:port/context-path/hystrix.stream, whereas nextflix turbine listens for http://host:port/hystrix.stream - without the context path, by default.

Therefore, I should make netflix turbine listen with the microservices context path or else the microservices should use the root context path, the latter wouldn't be possible with weblogic because all the microservices will be running on the same host and port but the context path is different.

Hope you get my question right.

spencergibb commented 7 years ago

@kasibkismath I think I understand, it is just different than the current issue.

silentFred commented 7 years ago

Agree. In general though, it has proven very difficult to use all the spring cloud components on services that have context roots. We also have yet to figure out how we will get turbine and co. to work

kasibkismath commented 7 years ago

@spencergibb, yes, it's a different issue but the underlying issue is due to the context path not being added. @silentFred hope that the context path issue would be fixed soon, so that all comes together.

rax215 commented 7 years ago

looked at @thomasdarimont Hack!!! wondering if that can be included in source code and published as new jar files

thomasdarimont commented 7 years ago

...btw. was asked about that a month ago on twitter ...turns out one also needs to take care about the context-path when using ribbon. A PoC for this can be found here.

spencergibb commented 7 years ago

Part of the issue is this goes beyond just eureka and would have to be changed in other discovery clients as well.

silentFred commented 7 years ago

@spencergibb yep! We ended up abusing the homePageUrl config to cater for contexts in Zuul and other clients that use ribbon. Not all that pretty =(

With some inspiration from @thomasdarimont I managed to put together something that worked for me:

http://webler.co.za/blog/spring-cloud-web-context-root-aware-zuul-eureka-service-calls-code-bit/ http://webler.co.za/blog/spring-cloud-web-context-aware-feign-clients-using-eureka-service-discovery-code-bit/

spencergibb commented 7 years ago

It's all or nothing and doing "all" is a big effort.

kasibkismath commented 7 years ago

@silentFred when I go to the link I get 403 error.

rax215 commented 7 years ago

@silentFred Can you please check for the Autowired block in http://webler.co.za/blog/spring-cloud-web-context-root-aware-zuul-eureka-service-calls-code-bit/ ? I am getting error in that

silentFred commented 7 years ago

@rax215 @kasibkismath the pages load fine for me...


@Bean
@Primary
public RibbonRoutingFilter ribbonRoutingFilter(ProxyRequestHelper helper,
                                               RibbonCommandFactory<?>; ribbonCommandFactory,
                                               LoadBalancerClient loadBalancerClient) {

    return new ContextAwareRibbonRoutingFilter(helper, ribbonCommandFactory, loadBalancerClient);
}
rax215 commented 7 years ago

@silentFred Sorry for the confusion, I meant Autowired block of "ContextAwareRibbonRoutingFilter" class.

public ContextAwareRibbonRoutingFilter(
            ProxyRequestHelper helper,
            RibbonCommandFactory<?>; ribbonCommandFactory,
            LoadBalancerClient loadBalancerClient) {

        super(helper, ribbonCommandFactory);

The semicolon after RibbonCommandFactory<?>;, is it correct? and the attributes in super() are throwing error for me.

silentFred commented 7 years ago

@rax215 It looks like a typo. Remove that semicolon. Not too sure why super() is throwing errors

fahimfarookme commented 7 years ago

@kasibkismath for the context-path with Turbine issue you have raised, PFB my finding and the fix worked for me.

PoC here.

kasibkismath commented 7 years ago

@fahimfarookme it works perfectly as expected.

There are few gotchas as you have mentioned, however, what is the overall impact since we are extending EurekaInstanceDiscovery, which is a class of spring-cloud-netflix-turbine, considering the fact that at a later point of time we will be upgrading netflix-turbine as well.

fahimfarookme commented 7 years ago

@kasibkismath This solution is only for the Turbine issue you have raised. This is the standard way of providing custom discovery mechanism for Turbine. Will have upgradability concern only for Turbine, however safer than extending Eureka server or client related classes - if everything else works fine for you.

Pytry commented 7 years ago

Is anyone already working on this issue? I don't want to step on toes.

dsushil2000 commented 6 years ago

I resolved this error with following server: contextPath: /caching

eureka: instance: prefer-ip-address: true leaseRenewalIntervalInSeconds: 10

appname: TestHZApp

instanceId: TestHZApp:${spring.application.instance-id:${random.value}}

metadata-map:

zone: primary # This is needed for the load balancer

profile: ${spring.profiles.active}

version: ${info.project.version}

   management:
     context-path: ${server.contextPath}
silentFred commented 6 years ago

I don't know if this code will still work on later version but give this a shot: http://webler.co.za/blog/spring-cloud-web-context-root-aware-zuul-eureka-service-calls-code-bit/

On Sat, 30 Dec 2017 at 01:38 dsushil2000 notifications@github.com wrote:

I have requirement to publish Boot Microservices with context path. How we can solve the problem? Eureka does not register the App with Context Path

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/spring-cloud/spring-cloud-netflix/issues/737#issuecomment-354513011, or mute the thread https://github.com/notifications/unsubscribe-auth/ABenf5974A6boiGaumxxXYYpi0jTqIILks5tFXfegaJpZM4G4gMt .

jrauschenbusch commented 6 years ago

I think that there is still a lot of interest in this feature and I also want to present a problem that we are currently facing.

Example: Dynamic microservice architecture using TLS/SSL secured endpoints only with no need for an wildcard certificate.

Solutions:

Downside of context-path based routing with current Spring Cloud (Zuul, Eureka) implementation:

=> Support for context paths would really help here

Downside SANs: One has to add a new SAN to the certificate all the time a new microservice is added

Pytry commented 6 years ago

@jrauschenbusch For me, this behavior seems like a huge design bug with Spring Cloud in general (not just Eureka). One of the founding design concepts of Java Servlets, and web development in general, was the ability to be deployed anywhere under any context, and have the application behave the same. It shouldn't matter if we "make war" instead of jar, deploy as root or with a custom context, nor even if the different instances have different contexts. This was the main appeal of Servlets, and the reason why so many companies (many on windows) decided to adopt it.