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

getURI() in EurekaServiceInstance misses custom contextPath. #737

Closed
thomasdarimont opened this issue Dec 18, 2015 · 56 comments
Closed

getURI() in EurekaServiceInstance misses custom contextPath. #737

thomasdarimont opened this issue Dec 18, 2015 · 56 comments
Labels

Comments

@thomasdarimont
Copy link

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
Copy link
Member

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

@cbelleza
Copy link

Hi,

When will it be available to download?

@spencergibb
Copy link
Member

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

@cbelleza
Copy link

@spencergibb,

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

@spencergibb
Copy link
Member

#737 (comment)

@cbelleza
Copy link

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
Copy link
Author

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
Copy link

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

@thomasdarimont
Copy link
Author

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

@spencergibb
Copy link
Member

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

@cbelleza
Copy link

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
Copy link
Member

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
Copy link

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
Copy link
Contributor

sfat commented Apr 20, 2016

@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
Copy link

kk-manh commented Apr 20, 2016

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
Copy link

is ther any news about this matter?

@dilgerma
Copy link

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

@cjemison
Copy link

cjemison commented Jan 6, 2017

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

@spencergibb
Copy link
Member

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

@cbos
Copy link

cbos commented Jan 20, 2017

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
Copy link

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

@cbos
Copy link

cbos commented Feb 22, 2017

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
Copy link

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

@cbos
Copy link

cbos commented Mar 17, 2017

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

@rax215
Copy link

rax215 commented May 19, 2017

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

@silentFred
Copy link

silentFred commented Jun 2, 2017

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.

thomasdarimont added a commit to thomasdarimont/service-discovery-example-eureka-hacks that referenced this issue Jun 7, 2017
@kasibkismath
Copy link

kasibkismath commented Jul 19, 2017

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.

@rax215
Copy link

rax215 commented Jul 19, 2017

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

@thomasdarimont
Copy link
Author

thomasdarimont commented Jul 19, 2017

...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
Copy link
Member

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

@silentFred
Copy link

@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
Copy link
Member

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

@kasibkismath
Copy link

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

@rax215
Copy link

rax215 commented Jul 20, 2017

@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
Copy link

silentFred commented Jul 20, 2017

@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
Copy link

rax215 commented Jul 20, 2017

@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
Copy link

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

@fahimfarookme
Copy link
Contributor

fahimfarookme commented Jul 20, 2017

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

  • The service discovery approach can be plugged-into Turbine with the following property.
    InstanceDiscovery.impl=....
    https://github.com/Netflix/Turbine/wiki/Plugging-in-your-own-InstanceDiscovery-(1.x)

  • Spring cloud's implementation for this is based on 'Eureka discovery' and implemented in org.springframework.cloud.netflix.turbine.EurekaInstanceDiscovery class.

  • So that I have extended the spring EurekaInstanceDiscovery class to provide context-path as well.

  • Had to use same package as Spring org.springframework.cloud.netflix.turbine for my subclass in order to override a package-private method (i.e. just to avoid overriding entire thing.)

PoC here.

@kasibkismath
Copy link

@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
Copy link
Contributor

@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
Copy link

Pytry commented Oct 5, 2017

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

@dsushil2000
Copy link

dsushil2000 commented Dec 29, 2017

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
Copy link

silentFred commented Dec 30, 2017 via email

@jrauschenbusch
Copy link

jrauschenbusch commented May 2, 2018

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:

  • Context-path based routing
  • Creating an certificate containing all SANs of all microservices (not really dynamic)

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

  • Zuul must be configured to map routes not only to a hostname, but also to context paths
  • Context paths must be added to all inter-service API calls (e.g. if before sub-domain based routing was used)

=> 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
Copy link

Pytry commented May 2, 2018

@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.

@ghost
Copy link

ghost commented Oct 10, 2018

Has there been any update on this issue? I am facing the same issues mentioned above.

@silentFred
Copy link

silentFred commented Oct 10, 2018 via email

@spencergibb
Copy link
Member

This is something we've avoided in the past and are not looking to implement this at this time.

@georglundesgaard
Copy link

georglundesgaard commented Dec 11, 2019

I just bumped into this bu^H^Hfeature today. Is there a reason why this couldn't be fixed by using the specified homePageUrl/homePageUrlPath in Eureka? Would this break some other functionality? Most of our services unfortunately have a context path and are not served on '/', so this makes it a little harder to use the default Eureka client (I would rather use the default instead of supporting my own).

@spencergibb
Copy link
Member

We've decided as a team not to support this.

@tarn2206
Copy link

tarn2206 commented Jun 8, 2021

You can pass the custom context-path into the instance metadata-map

eureka:
   instance:
      metadata-map:
         contextPath: ${server.servlet.context-path}

and access the value by calling

String url = instance.getUri() + instance.getMetadata().get("contextPath");

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

No branches or pull requests