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

RibbonLoadBalancingHttpClient issue with zuul and secured service #1168

Closed francistb closed 7 years ago

francistb commented 8 years ago

This is similar to https://github.com/spring-cloud/spring-cloud-netflix/issues/1126, but using RibbonLoadBalancingHttpClient instead of RibbonLoadBalancerClient.

When using a zuul proxy + HttpClientRibbonCommandFactory and targeting a secured service, the request is sent to the secured port in plain http.

nissel commented 8 years ago

Does adding ribbon.IsSecure=true to your properties fix it?

francistb commented 8 years ago

It seems not. I'm getting false at https://github.com/spring-cloud/spring-cloud-netflix/blob/1.1.x/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java#L73 and the configOverride is null in https://github.com/spring-cloud/spring-cloud-netflix/blob/1.1.x/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClient.java#L121 (from https://github.com/Netflix/ribbon/blob/master/ribbon-loadbalancer/src/main/java/com/netflix/client/AbstractLoadBalancerAwareClient.java#L81).

Also, since we will be migrating from http service to https, some servers will only be available in http during the migration, so I can't simply force an entire service (or even all of them in this case) to be https.

francistb commented 8 years ago

I was probably wrong about the ribbon.IsSecure. It does seems to work, but I'm still having the issue I need to be able to rollout the services from http to https. So, need to able to not change all them at the same time.

I'm testing a potential fix (https://github.com/francistb/spring-cloud-netflix/commit/b9b2b857f05d86e4f797888173c4456e2c61e41f), which is similar to how gh-1126 was fixed.

spencergibb commented 8 years ago

@francistb there have been some fixes regarding this, can you check with Camden.RELEASE or Brixton.SR6?

francistb commented 7 years ago

Finally had some time to test this with Camden, and yes, I can confirm the issue has been fixed. Sorry for the delay.

Thanks !