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

fix: [Destinations] Allow for Custom Proxy Config in ON_PREMISE Destinations #327

Merged
merged 2 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,21 @@ private Try<HttpDestination> tryGetConnectivityDestination(

private Try<HttpDestination> toProxiedDestination( @Nonnull final HttpDestination base )
{
final DefaultHttpDestination.Builder builder =
DefaultHttpDestination
.fromDestination(base)
// be sure to use exactly this instance of connectivityResolver since it has a cache attached
.headerProviders(connectivityResolver);
// don't override the proxy URL if it has been set explicitly/manually already
if( base.getProxyConfiguration().isDefined() ) {
return Try.of(builder::buildInternal);
}
final Try<URI> proxyUrl = Try.of(connectivityResolver::getProxyUrl);
if( proxyUrl.isFailure() ) {
return Try
.failure(
new DestinationAccessException("Failed to resolve on-premise proxy URL.", proxyUrl.getCause()));
}
final DefaultHttpDestination.Builder builder =
DefaultHttpDestination
.fromDestination(base)
// be sure to use exactly this instance of connectivityResolver since it has a cache attached
.headerProviders(connectivityResolver);

return proxyUrl.map(builder::proxy).map(DefaultHttpDestination.Builder::buildInternal);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import org.apache.http.HttpHeaders;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

Expand Down Expand Up @@ -315,6 +316,28 @@ void testConnectivityWithProviderTenant()
assertThatThrownBy(() -> sut.getDestination(options)).isInstanceOf(DestinationAccessException.class);
}

@Test
@DisplayName( "An already existing proxy configuration should not be overridden." )
void testConnectivityWithExistingProxyConfig()
{
final DefaultHttpDestination destination =
DefaultHttpDestination
.builder("foo")
.proxy(URI.create("http://bar"))
.proxyType(ProxyType.ON_PREMISE)
.buildInternal();

final ServiceBindingDestinationOptions options =
ServiceBindingDestinationOptions
.forService(MegacliteServiceBindingAccessor.CONNECTIVITY_BINDING)
.withOption(ProxyOptions.destinationToBeProxied(destination))
.build();
final HttpDestination result = sut.getDestination(options);

assertThat(result.getProxyConfiguration()).isNotEmpty();
assertThat(result.getProxyConfiguration().get().getUri()).isEqualTo(URI.create("http://bar"));
}

@Test
void testGetMandateConfiguration()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ HttpDestination toProxiedDestination(
final String destinationName =
destinationToBeProxied.get(DestinationProperty.NAME).getOrElse("<unnamed-destination>");
final DefaultHttpDestination.Builder destinationBuilder =
DefaultHttpDestination.fromDestination(destinationToBeProxied).proxy(proxyUrl);
DefaultHttpDestination.fromDestination(destinationToBeProxied);

if( oAuth2Options.skipTokenRetrieval() ) {
log.debug("Skipping OAuth2 token retrieval for proxied destination '{}'.", destinationName);
Expand All @@ -293,7 +293,11 @@ HttpDestination toProxiedDestination(
destinationBuilder.keyStore(oAuth2Options.getClientKeyStore());
}

return destinationBuilder.buildInternal();
// don't override the proxy URL if it has been set explicitly/manually already
if( destinationToBeProxied.getProxyConfiguration().isDefined() ) {
return destinationBuilder.buildInternal();
}
return destinationBuilder.proxy(proxyUrl).buildInternal();
}

DestinationHeaderProvider createHeaderProvider(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.http.HttpHeaders;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.mockito.InOrder;
import org.mockito.Mockito;
Expand Down Expand Up @@ -404,6 +405,31 @@ void testProxiedDestination()
eq(TEST_SERVICE));
}

@Test
@DisplayName( "An already existing proxy configuration should not be overridden." )
void testConnectivityWithExistingProxyConfig()
{
final DefaultHttpDestination destination =
DefaultHttpDestination
.builder("foo")
.proxy(URI.create("http://bar"))
.proxyType(ProxyType.ON_PREMISE)
.buildInternal();
final HttpDestination result =
sut
.toProxiedDestination(
destination,
URI.create("yolo"),
URI.create("foo"),
credentials,
OnBehalfOf.TECHNICAL_USER_CURRENT_TENANT,
OAuth2Options.DEFAULT,
TEST_SERVICE);

assertThat(result.getProxyConfiguration()).isNotEmpty();
assertThat(result.getProxyConfiguration().get().getUri()).isEqualTo(URI.create("http://bar"));
}

@Test
void testSkipTokenRetrieval()
{
Expand Down
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

### 📈 Improvements

- Improve the `DefaultHttpDestination` builder API: For destinations with proxy type `ON_PREMISE` the proxy URL can now be customized by using the `proxy` method of the builder.
- Dependency Updates:
- SAP dependency updates:
- Update [thing](https://link-to-thing) from `a.b.c` to `x.z.y`
Expand Down
Loading