update cf discovery defaults in service discovery

[srinivasa-vasu]

• updated reactive discovery classes to populate spring.cloud.cloudfoundry.discovery.default-server-port and spring.cloud.cloudfoundry.discovery.internal-domain if passed externally.
• updated internal domain url to use 8080 and external route url to 80 as the default if the external input is not passed
• removed protocol check as cf application endpoint url is not sending the protocol info in the response.
• updated the appropriate test case

[spencergibb]

Don't use stream apis please

[srinivasa-vasu]

sure, changed it

[spencergibb]

Why did you change this?

[srinivasa-vasu]

• getUrls().get(0) always returns the external URL. It's not picking up the apps.internal url when we use CloudFoundryAppServiceReactiveDiscoveryClient
• protocol information is never returned as part of the cf-client call. URL info always gets returned without the protocol.

[spencergibb]

@making or @scottfrederick wondering if you think these changes make sense.

[srinivasa-vasu]

@spencergibb is this good to go?

[spencergibb]

@srinivasa-vasu I'm waiting for comment from team members.

[making]

@srinivasa-vasu
Let me confirm as I'm a bit confused.
If I understand correctly, CloudFoundryNativeReactiveDiscoveryClient was not supposed to be used with C2C networking and CloudFoundryAppServiceReactiveDiscoveryClient was responsible of that.
Is your commit trying to make CloudFoundryNativeReactiveDiscoveryClient available in both C2C and regular routing (via GoRouter) and it this what you want to solve in this PR?

[srinivasa-vasu]

@making

• The existing logic doesn't use C2C for CloudFoundryAppServiceReactiveDiscoveryClient. mapApplicationInstanceToServiceInstance logic in base class is used to populate DefaultServiceInstance for CloudFoundryAppServiceReactiveDiscoveryClient as well which never uses the C2C internal domain as getUrl(0) always returns the external URL. The change is to use C2C for CloudFoundryAppServiceReactiveDiscoveryClient and hence getRouteURL is defined in CloudFoundryNativeReactiveDiscoveryClient to use the external route there and overridden in CloudFoundryAppServiceReactiveDiscoveryClient to use the internal route for C2C cases.
• port logic change is to honor the defautServerPort property in CloudFoundryAppServiceReactiveDiscoveryClient and SimpleDnsBasedReactiveDiscoveryClient for C2C cases. The default is 8080. Let's say if we explicitly the pass the value as 61001 with lb as the protocol to pass through the envoy TLS proxy port, then it will be processed.

Hope this helps.

[making]

@srinivasa-vasu Thanks for the clarification.
With current implementation, if more than 2 routes (e.g external url and internal url) are mapped to the app, first one is always picked up. This makes C2C unavailable even if CloudFoundryAppServiceReactiveDiscoveryClient is used because the logic is implemented in the parent class. The PR makes the logic to determine the route protected and override it to respect the internal domain in CloudFoundryAppServiceReactiveDiscoveryClient.

Is this correct? I think the change makes sense.

[srinivasa-vasu]

@making Yes, that's right + the default properties update.

[srinivasa-vasu]

@spencergibb can we merge this as @making seems to be fine with the changes done?