Introducing Transparent Proxy Builder #851

isevdalinosap · 2025-07-03T13:20:46Z

Context

This PR adds support for consuming systems in Cloud SDK applications leveraging the Transparent Proxy for new destination features and on-premise connectivity.

Feature scope:

Implement TransparentProxyDestination class

Definition of Done

Functionality scope stated & covered
Tests cover the scope above
~~Error handling created / updated & covered by the tests above~~
~~Documentation updated~~
~~Release notes updated~~

cla-assistant · 2025-07-03T13:21:00Z

All committers have signed the CLA.

newtork · 2025-07-04T08:45:18Z

.../test/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestinationTest.java

+                .chainVarSubjectToken("chainVarSubjectToken")
+                .chainVarSubjectTokenType("chainVarSubjectTokenType")
+                .chainVarSamlProviderDestinationName("chainVarSamlProviderDestName")
+                .property(DestinationProperty.URI, "https://example.com")


(Question)

I don't understand this. I thought destinations referenced via "Transparent Proxy" would rely on relative URIs not absolute addresses.

It is confusing to me that the following parameters are not required by API design:

destination name

namespace

(relative) URI

I believe from transparent proxy point of view only destination name is required. The url is designed to be defaulted and the namespace is embedded into the url.

newtork

For further review purpose please find the official documentation on Transparent Proxy
https://help.sap.com/docs/connectivity/sap-btp-connectivity-cf/using-transparent-proxy

Please find our current explanation in SAP Cloud SDK
https://sap.github.io/cloud-sdk/docs/java/environments/kubernetes-kyma#executing-requests

MatKuhr

Thanks for the contribution!

Overall, the approach makes sense to me. I noted some open questions that require discussions. I'll create a meeting to go over them together.

MatKuhr · 2025-07-07T07:59:31Z

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

+    private static final String REDIRECT_URI_HEADER_KEY = "x-redirect-uri";
+    private static final String CODE_VERIFIER_HEADER_KEY = "x-code-verifier";


For some of these headers I'm not sure what exactly they do and if or how they fit into Cloud SDK

All those headers give different possibilities for consuming destinations using the transparent proxy.

MatKuhr · 2025-07-07T08:01:04Z

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

+    @Override
+    public Collection<Header> getHeaders( @Nonnull URI requestUri )
+    {
+        return customHeaders;


We have to evaluate how this should interact with static header providers. Probably, those need to be loaded and evaluated.

MatKuhr · 2025-07-07T08:02:55Z

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

+        this.customHeaders =
+            customHeaders != null ? ImmutableList.<Header> builder().addAll(customHeaders).build() : ImmutableList.of();
+
+        cachedProxyConfiguration = destinationPropertyFactory.getProxyConfiguration(baseProperties);


I'm not sure if we should set the transparent proxy as a proxy in the HTTP client, or if we should leave it empty.

MatKuhr · 2025-07-07T08:04:13Z

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

+    public Option<KeyStore> getKeyStore()
+    {
+        return null;
+    }
+
+    @Nullable
+    @Override
+    public Option<String> getKeyStorePassword()
+    {
+        return null;
+    }
+
+    @Override
+    public boolean isTrustingAllCertificates()
+    {
+        return false;
+    }
+
+    @Nullable
+    @Override
+    public Option<BasicCredentials> getBasicCredentials()
+    {
+        return null;
+    }
+
+    @Nullable
+    @Override
+    public AuthenticationType getAuthenticationType()
+    {
+        return null;
+    }
+


These values will likely depend on how connections to the transparent proxy will be secured, and thus might need to be implemented.

The transparent proxy is currently deployed as trusted component in trusted environment only and therefore requires no authentication/certificates. Because of this I don't think implementing those methods is needed/necessary.

I think it's up to the user how to authenticate the connection to TP. Currently, TP is only running in K8S, where this is typically done with sidecars. So in that case, users are unlikely to need this functionality. However, TP is also coming to CF, and users may decide to authenticate differently, or want to use some of these methods for other purposes.

For now, I think it's okay to not implement actual logic, leaving these cases unsupported until users request it.

However, returning null when the method is declared Option<..> should never be done, so instead, please return Option.none() and change to @Nonnull

Changed all the methods in question to return Option.none() and be annotated with @Nonnull.

MatKuhr · 2025-07-07T08:04:38Z

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

+     * Builder class to allow for easy creation of an immutable {@code TransparentProxyDestination} instance.
+     */
+    @Accessors( fluent = true, chain = true )
+    public static class Builder


As Alex pointed out, the builder should enforce required properties

I believe from transparent proxy point of view only destination name is required. The url is designed to be defaulted and the namespace is embedded into the url. I have added enforcement of the destination name.

MatKuhr · 2025-07-07T08:06:28Z

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

+                this.property(DestinationProperty.URI, TRANSPARENT_PROXY_GATEWAY);
+            }
+
+            TenantAccessor.tryGetCurrentTenant().onSuccess(tenant -> {


I think we have two options on how to handle tenancy:

Determine the tenant upon building the destination

Determine the tenant when using the destination

Currently, strategy (1) is applied, but I think (2) better fits the Cloud SDK APIs.

In the context of the transparent proxy all other header parameters make sense only if the tenant can be determined and basically all transparent proxy operations require tenant. So in this case constructing a TransparentProxyDestination without known tenant seems illogical to me.

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

MatKuhr · 2025-07-07T08:09:39Z

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

@@ -0,0 +1,604 @@
+package com.sap.cloud.sdk.cloudplatform.connectivity;


Since this functionality is still specific to the destination service, I think this would be more suitable under connectivity-destination-service module.

Isn't it more appropriate for the class to reside inside com.sap.cloud.sdk.cloudplatform.connectivity because TransparentProxyDestination implements HttpDestination and is similar to class DefaultHttpDestination, both of which reside inside com.sap.cloud.sdk.cloudplatform.connectivity?

Not quite, the modules are split based on generic API vs. destination service specific logic. Since TP conceptually only works with a destination service, it should be in the destination service module

Agreed, moved all the logic into connectivity-destination-service module.

MatKuhr

Overall looks really good! I added some minor suggestions, mostly on test code. But I think we've covered most of the challenging bits 😉

One thing we have to keep in mind: One destination will use one HTTP client. If the tenant is added dynamically per request, the same client will be shared across all tenants.

This is important also for any endpoints that set tenant- or user-specific cookies. For such cases users should build a destination object per request and manually set auth tokens. That way, separate HTTP clients will be used.

.../src/main/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestination.java

.../test/java/com/sap/cloud/sdk/cloudplatform/connectivity/TransparentProxyDestinationTest.java

MatKuhr

Changes LGTM!

Can you please add a release note entry here: https://github.com/SAP/cloud-sdk-java/blob/main/release_notes.md?plain=1#L13

it should briefly state the new feature and link to the docs page (even if the docs page doesn't exist yet, please put the expected URL)

isevdalinosap force-pushed the transparentProxyBuilder branch 6 times, most recently from 7f3c14b to b4355df Compare July 3, 2025 19:55

newtork reviewed Jul 4, 2025

View reviewed changes

MatKuhr reviewed Jul 7, 2025

View reviewed changes

isevdalinosap force-pushed the transparentProxyBuilder branch 5 times, most recently from 9b14007 to 06cb15c Compare July 10, 2025 14:40

isevdalinosap force-pushed the transparentProxyBuilder branch 9 times, most recently from 59e0a6a to 54726e2 Compare July 20, 2025 20:12

MatKuhr reviewed Jul 28, 2025

View reviewed changes

isevdalinosap force-pushed the transparentProxyBuilder branch 4 times, most recently from 3477825 to ea07c40 Compare July 30, 2025 14:53

MatKuhr previously approved these changes Aug 4, 2025

View reviewed changes

Introducing Transparent Proxy Builder

121f01f

isevdalinosap dismissed MatKuhr’s stale review via 121f01f August 6, 2025 11:35

isevdalinosap force-pushed the transparentProxyBuilder branch from ea07c40 to 121f01f Compare August 6, 2025 11:35

Merge branch 'main' into transparentProxyBuilder

5456771

MatKuhr approved these changes Aug 11, 2025

View reviewed changes

MatKuhr enabled auto-merge (squash) August 11, 2025 13:55

MatKuhr merged commit 18a98eb into SAP:main Aug 11, 2025
14 checks passed

		private static final String REDIRECT_URI_HEADER_KEY = "x-redirect-uri";
		private static final String CODE_VERIFIER_HEADER_KEY = "x-code-verifier";

		@@ -0,0 +1,604 @@
		package com.sap.cloud.sdk.cloudplatform.connectivity;

Introducing Transparent Proxy Builder #851

Introducing Transparent Proxy Builder #851

Uh oh!

Conversation

isevdalinosap commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Feature scope:

Definition of Done

Uh oh!

cla-assistant bot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

newtork Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

newtork left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatKuhr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

isevdalinosap Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatKuhr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MatKuhr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

isevdalinosap commented Jul 3, 2025 •

edited

Loading

cla-assistant bot commented Jul 3, 2025 •

edited

Loading

newtork Jul 4, 2025 •

edited

Loading

newtork left a comment •

edited

Loading

isevdalinosap Jul 8, 2025 •

edited

Loading