spring-cloud-commons SimpleServcieInstance should override getScheme()

I'm submitting a bugreport for

spring-cloud-commons
class org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties

Versions used:

spring-boot: 2.3.3
spring-cloud: Hoxton.SR7
spring-cloud-commons: 2.2.4.RELEASE

When using SimpleDiscoveryClient, non-HTTPS Urls do not work, they are always resolved as https:// URLs. We are using https in production, but http in Tests to avoid the need to setup certificates for localhost. There are workarounds, but I think this is a bug that should be fixed in spring-cloud-commons

e.g.

"spring.cloud.discovery.client.simple.instances.foo-service[0].uri= http://localhost:8889", "spring.cloud.discovery.client.simple.instances.foo-service[0].secure = false",

foo-service is always resolved as https://localhost:8889, because SimpleServcieInstance (inner class of SimpleServiceProperties) does not override getScheme().

LoadBalancerUriTools calls getScheme() and in case of SimpleServiceInstance always receives null and thus always sets Scheme to https.

Proposed fix for org.springframework.cloud.client.discovery.simple.SimpleDiscoveryProperties#SimpleServiceInstance

public String getScheme() { return this.isSecure() ? "https" : "http"; }

Sep 14 '20 07:09 dagerber

This should be done in DefaultServiceInstance as well.

Sep 14 '20 14:09 spencergibb

Has been fixed with https://github.com/spring-cloud/spring-cloud-commons/commit/72762d76ed9eccc5876e5bf099c2495c4c44ff3e

Dec 02 '20 10:12 dagerber

Sorry, thought this was solved, but it is not.

Dec 02 '20 15:12 dagerber

https://github.com/spring-cloud/spring-cloud-commons/pull/926

Mar 12 '21 09:03 dagerber

Closed via #926

Mar 16 '21 17:03 spencergibb

Reverted, at least commons, gateway and contract have behavior that depends on the null implementation that will need to be addressed in the future.

Mar 16 '21 20:03 spencergibb

Requires more discussion on impact in GW. Reverting the change and reopening.

Nov 08 '22 17:11 OlgaMaciaszek