Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

Support Paths in Configuration.*Uri #26

Closed
borfig opened this issue Feb 28, 2021 · 5 comments
Closed

Support Paths in Configuration.*Uri #26

borfig opened this issue Feb 28, 2021 · 5 comments

Comments

@borfig
Copy link

borfig commented Feb 28, 2021

Is your feature request related to a problem? Please describe.
Due to regulation, there are scenarios when our mobile clients are in a closed network and are not allowed to connect directly to *.launchdarkly.com. However, it is allowed to deploy a proxy server in the DMZ for each external FQDN. [1]

Describe the solution you'd like
To reduce changes to the internal network to an absolute minimum, it is desired to proxy launchdarkly requests along with our own requests on a single FQDN. The LaunchDarkly client will be initialized with:

var config = LdConfiguration.Builder(settingsLaunchDarkly.MobileKey)
    .BaseUri(new Uri("https://proxy.example.internal/launchdarkly/api"))
    .EventsUri(new Uri("https://proxy.example.internal/launchdarkly/mobile"))
    .StreamUri(new Uri("https://proxy.example.internal/launchdarkly/stream"))
    .Build();

The proxy would strip the prefix and forward the request as-is to the correct LaunchDarkly server directly.
The LaunchDarkly client configuration must not strip the paths provided by the configuration, in order to make the above example usable.

Describe alternatives you've considered
If the *Uri Paths are discarded as in v1.2.0, we would need to deploy 3 additional proxy servers (or 3 virtual hosts) for the base, events, and stream URIs, in addition to our own existing one. Such a change also requires changing the internal DNS to add 3 additional records. The LaunchDarkly client will be initialized with:

var config = LdConfiguration.Builder(settingsLaunchDarkly.MobileKey)
    .BaseUri(new Uri("https://api.launchdarkly.example.internal/"))
    .EventsUri(new Uri("https://mobile.launchdarkly.example.internal/"))
    .StreamUri(new Uri("https://clientstream.launchdarkly.example.internal/"))
    .Build();

This is not ideal for us, and it generates unnecessary friction.

Additional context
For example, the line at [2] is an example of a logic that discards any .Path value, and should be replaced with .Path += path or equivalent.

[1] https://www.hysolate.com/blog/hysolate-in-a-closed-network-with-nginx/
[2] https://github.com/launchdarkly/xamarin-client-sdk/blob/625397741e51a90efe88cc8d430fe0636a5f2263/src/LaunchDarkly.XamarinSdk/FeatureFlagRequestor.cs#L72

@eli-darkly
Copy link
Contributor

eli-darkly commented Mar 5, 2021

Yes, we should fix that. It wasn't our intention to discard the base path, and I know that we do retain the base path in most other SDKs, though there may be others where we made a similar mistake to this.

I do have one question, not directly relevant to this issue but something I wanted to make sure wasn't a sign of some misunderstanding. In your example code, is there a particular reason that you're using the path /mobile for the events base URI? There's nothing mobile-specific about that host, even though one of the endpoints on the host is used for mobile events. Again, this probably isn't of any real significance but I just wanted to clarify.

@eli-darkly
Copy link
Contributor

Ah, strike that last part— I forgot that we do in fact use a hostname mobile.launchdarkly.com in this SDK. The underlying service is the same as events.launchdarkly.com, but that explains why you were calling it that. Not important.

@borfig
Copy link
Author

borfig commented Mar 5, 2021

Yes, you can replace /mobile with /events in the example above.

@eli-darkly
Copy link
Contributor

We'll release a patch for this as soon as possible. The patch itself is straightforward, but we hadn't done a Xamarin SDK release in a while and we found that the build tools have changed enough to break our release process, so we need to resolve that first. Thanks again for reporting the bug.

@eli-darkly
Copy link
Contributor

Fixed in the 1.2.1 release. Sorry this took a while.

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

No branches or pull requests

2 participants