Handle RemoteS3Facade dynamic creation in RemoteS3ConnectionController

trino-aws-proxy-spi/src/main/java/io/trino/aws/proxy/spi/remote/RemoteS3Connection.java

@@ -13,23 +13,47 @@
  */
 package io.trino.aws.proxy.spi.remote;
 
-import com.google.common.collect.ImmutableMap;
 import io.trino.aws.proxy.spi.credentials.Credential;
 
-import java.util.Map;
 import java.util.Optional;
 
 import static java.util.Objects.requireNonNull;
 
-public record RemoteS3Connection(
-        Credential remoteCredential,
-        Optional<RemoteSessionRole> remoteSessionRole,
-        Optional<Map<String, String>> remoteS3FacadeConfiguration)
+public interface RemoteS3Connection
 {
-    public RemoteS3Connection
+    Credential remoteCredential();
+
+    default Optional<RemoteSessionRole> remoteSessionRole()
     {
-        requireNonNull(remoteCredential, "remoteCredential is null");
-        requireNonNull(remoteSessionRole, "remoteSessionRole is null");
-        remoteS3FacadeConfiguration = remoteS3FacadeConfiguration.map(ImmutableMap::copyOf);
+        return Optional.empty();
+    }
+
+    default Optional<RemoteS3Facade> remoteS3Facade()
+    {
+        return Optional.empty();
+    }
+
+    record StaticRemoteS3Connection(
+            Credential remoteCredential,
+            Optional<RemoteSessionRole> remoteSessionRole,
+            Optional<RemoteS3Facade> remoteS3Facade)
+            implements RemoteS3Connection
+    {
+        public StaticRemoteS3Connection
+        {
+            requireNonNull(remoteCredential, "remoteCredential is null");
+            requireNonNull(remoteSessionRole, "remoteSessionRole is null");
+            requireNonNull(remoteS3Facade, "remoteS3Facade is null");
+        }
+
+        public StaticRemoteS3Connection(Credential remoteCredential)
+        {
+            this(remoteCredential, Optional.empty(), Optional.empty());
+        }
+
+        public StaticRemoteS3Connection(Credential remoteCredential, RemoteSessionRole remoteSessionRole)
+        {
+            this(remoteCredential, Optional.of(remoteSessionRole), Optional.empty());
+        }
     }
 }

trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/TrinoAwsProxyPluginValidatorModule.java

@@ -19,8 +19,12 @@
 import io.trino.aws.proxy.spi.credentials.CredentialsProvider;
 import io.trino.aws.proxy.spi.plugin.config.AssumedRoleProviderConfig;
 import io.trino.aws.proxy.spi.plugin.config.CredentialsProviderConfig;
+import io.trino.aws.proxy.spi.plugin.config.RemoteS3Config;
 import io.trino.aws.proxy.spi.plugin.config.RemoteS3ConnectionProviderConfig;
 import io.trino.aws.proxy.spi.remote.RemoteS3ConnectionProvider;
+import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
+
+import java.util.Optional;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
@@ -36,7 +40,9 @@ public TrinoAwsProxyPluginValidator(
                 AssumedRoleProviderConfig assumedRoleProviderConfig,
                 AssumedRoleProvider assumedRoleProvider,
                 RemoteS3ConnectionProvider remoteS3ConnectionProvider,
-                RemoteS3ConnectionProviderConfig remoteS3ConnectionProviderConfig)
+                RemoteS3ConnectionProviderConfig remoteS3ConnectionProviderConfig,
+                Optional<RemoteS3Facade> remoteS3Facade,
+                RemoteS3Config remoteS3Config)
         {
             boolean credentialsProviderIsNoop = credentialsProvider.equals(CredentialsProvider.NOOP);
             boolean credentialsProviderIsConfigured = credentialsProviderConfig.getPluginIdentifier().isPresent();
@@ -58,6 +64,11 @@ public TrinoAwsProxyPluginValidator(
                     "%s of type \"%s\" is not registered",
                     RemoteS3ConnectionProvider.class.getSimpleName(),
                     remoteS3ConnectionProviderConfig.getPluginIdentifier().orElse("<empty>"));
+
+            if (remoteS3Facade.isEmpty()) {
+                throw new IllegalArgumentException("%s of type \"%s\" is not registered"
+                        .formatted(RemoteS3Facade.class.getSimpleName(), remoteS3Config.getPluginIdentifier().orElseThrow()));
+            }
         }
     }
 

trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/TrinoAwsProxyServerModule.java

@@ -80,13 +80,6 @@ public class TrinoAwsProxyServerModule
 {
     private static final Logger log = Logger.get(TrinoAwsProxyServerModule.class);
 
-    @Provides
-    @Singleton
-    public RemoteUriFacade remoteUriFacade(RemoteS3Facade remoteS3Facade)
-    {
-        return remoteS3Facade;
-    }
-
     @Override
     protected void setup(Binder binder)
     {
@@ -116,6 +109,7 @@ protected void setup(Binder binder)
         httpServerBinder.enableLegacyUriCompliance();
         httpServerBinder.enableCaseSensitiveHeaderCache();
 
+        // RemoteS3ConnectionProvider binder
         configBinder(binder).bindConfig(RemoteS3ConnectionProviderConfig.class);
         newOptionalBinder(binder, RemoteS3ConnectionProvider.class).setDefault().toProvider(() -> {
             log.info("Using default %s NOOP implementation", RemoteS3ConnectionProvider.class.getSimpleName());
@@ -141,8 +135,7 @@ protected void setup(Binder binder)
         install(new OpaS3SecurityModule());
         install(new HttpCredentialsModule());
 
-        // RemoteS3 binder
-        newOptionalBinder(binder, RemoteS3Facade.class);
+        configBinder(binder).bindConfig(RemoteS3Config.class);
         // RemoteS3 provided implementation
         install(conditionalModule(
                 RemoteS3Config.class,
@@ -172,6 +165,13 @@ public XmlMapper newXmlMapper()
         return xmlMapper;
     }
 
+    @Provides
+    @Singleton
+    public RemoteUriFacade remoteUriFacade(RemoteS3Facade remoteS3Facade)
+    {
+        return remoteS3Facade;
+    }
+
     public static void bindResourceAtPath(JaxrsBinder jaxrsBinder, Class<?> resourceClass, String resourcePrefix)
     {
         jaxrsBinder.bind(resourceClass);

trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/remote/DefaultRemoteS3Module.java

@@ -41,11 +41,11 @@ protected void setup(Binder binder)
                 DefaultRemoteS3Config.class,
                 DefaultRemoteS3Config::getVirtualHostStyle,
                 innerBinder -> newOptionalBinder(innerBinder, RemoteS3Facade.class)
-                        .setDefault()
+                        .setBinding()
                         .to(VirtualHostStyleRemoteS3Facade.class)
                         .in(Scopes.SINGLETON),
                 innerBinder -> newOptionalBinder(innerBinder, RemoteS3Facade.class)
-                        .setDefault()
+                        .setBinding()
                         .to(PathStyleRemoteS3Facade.class)
                         .in(Scopes.SINGLETON)));
     }

trino-aws-proxy/src/main/java/io/trino/aws/proxy/server/remote/RemoteS3ConnectionController.java

@@ -14,8 +14,6 @@
 package io.trino.aws.proxy.server.remote;
 
 import com.google.inject.Inject;
-import com.google.inject.Injector;
-import io.airlift.bootstrap.Bootstrap;
 import io.airlift.log.Logger;
 import io.trino.aws.proxy.spi.credentials.Credential;
 import io.trino.aws.proxy.spi.credentials.Identity;
@@ -140,12 +138,7 @@ public <T> Optional<T> withRemoteConnection(SigningMetadata signingMetadata, Opt
     {
         return remoteS3ConnectionProvider.remoteConnection(signingMetadata, identity, request)
                 .flatMap(remoteConnection -> {
-                    RemoteS3Facade contextRemoteS3Facade = remoteConnection.remoteS3FacadeConfiguration().map(config -> {
-                        // TODO: This should respect the plugin installed for the RemoteS3Facade somehow
-                        Injector subInjector = new Bootstrap(new DefaultRemoteS3Module()).doNotInitializeLogging().quiet().setRequiredConfigurationProperties(config).initialize();
-                        return subInjector.getInstance(RemoteS3Facade.class);
-                    }).orElse(defaultS3Facade);
-
+                    RemoteS3Facade contextRemoteS3Facade = remoteConnection.remoteS3Facade().orElse(defaultS3Facade);
                     return remoteConnection.remoteSessionRole()
                             .map(remoteSessionRole ->
                                     internalRemoteSession(remoteSessionRole, remoteConnection.remoteCredential())

trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/TestProxiedEmulatedAndRemoteAssumedRoleRequests.java

@@ -21,7 +21,7 @@
 import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
 import io.trino.aws.proxy.spi.credentials.Credential;
 import io.trino.aws.proxy.spi.credentials.IdentityCredential;
-import io.trino.aws.proxy.spi.remote.RemoteS3Connection;
+import io.trino.aws.proxy.spi.remote.RemoteS3Connection.StaticRemoteS3Connection;
 import io.trino.aws.proxy.spi.remote.RemoteSessionRole;
 import software.amazon.awssdk.services.s3.S3Client;
 
@@ -50,6 +50,6 @@ public TestProxiedEmulatedAndRemoteAssumedRoleRequests(
         Credential policyUserCredential = s3Container.policyUserCredential();
         RemoteSessionRole remoteSessionRole = new RemoteSessionRole("us-east-1", "minio-doesnt-care", Optional.empty(), Optional.empty());
         IdentityCredential identityCredential = new IdentityCredential(CREDENTIAL, TESTING_IDENTITY_CREDENTIAL.identity());
-        credentialsController.addCredentials(identityCredential, new RemoteS3Connection(policyUserCredential, Optional.of(remoteSessionRole), Optional.empty()));
+        credentialsController.addCredentials(identityCredential, new StaticRemoteS3Connection(policyUserCredential, remoteSessionRole));
     }
 }

trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/TestProxiedRequestsToVirtualHostEndpoint.java

@@ -14,20 +14,33 @@
 package io.trino.aws.proxy.server;
 
 import com.google.inject.Inject;
+import io.trino.aws.proxy.server.remote.DefaultRemoteS3Config;
+import io.trino.aws.proxy.server.remote.VirtualHostStyleRemoteS3Facade;
+import io.trino.aws.proxy.server.testing.TestingRemoteS3Facade;
 import io.trino.aws.proxy.server.testing.TestingS3RequestRewriteController;
+import io.trino.aws.proxy.server.testing.containers.S3Container;
 import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
 import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTest;
 import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTestCommonModules.WithConfiguredBuckets;
-import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTestCommonModules.WithVirtualHostAddressing;
 import software.amazon.awssdk.services.s3.S3Client;
 
-@TrinoAwsProxyTest(filters = {WithConfiguredBuckets.class, WithVirtualHostAddressing.class})
+import static io.trino.aws.proxy.server.testing.TestingUtil.LOCALHOST_DOMAIN;
+
+@TrinoAwsProxyTest(filters = WithConfiguredBuckets.class)
 public class TestProxiedRequestsToVirtualHostEndpoint
         extends AbstractTestProxiedRequests
 {
     @Inject
-    public TestProxiedRequestsToVirtualHostEndpoint(S3Client s3Client, @ForS3Container S3Client storageClient, TestingS3RequestRewriteController requestRewriteController)
+    public TestProxiedRequestsToVirtualHostEndpoint(S3Client s3Client, @ForS3Container S3Client storageClient, TestingS3RequestRewriteController requestRewriteController,
+            TestingRemoteS3Facade testingRemoteS3Facade, S3Container s3Container)
     {
         super(s3Client, storageClient, requestRewriteController);
+
+        testingRemoteS3Facade.setDelegate(new VirtualHostStyleRemoteS3Facade(new DefaultRemoteS3Config()
+                .setDomain(LOCALHOST_DOMAIN)
+                .setPort(s3Container.containerHost().getPort())
+                .setHttps(false)
+                .setVirtualHostStyle(true)
+                .setHostnameTemplate("${bucket}.${domain}")));
     }
 }

trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/TestRemoteSessionProxiedRequests.java

@@ -23,7 +23,7 @@
 import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTestCommonModules.WithConfiguredBuckets;
 import io.trino.aws.proxy.spi.credentials.Credential;
 import io.trino.aws.proxy.spi.credentials.IdentityCredential;
-import io.trino.aws.proxy.spi.remote.RemoteS3Connection;
+import io.trino.aws.proxy.spi.remote.RemoteS3Connection.StaticRemoteS3Connection;
 import io.trino.aws.proxy.spi.remote.RemoteSessionRole;
 import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
 import software.amazon.awssdk.services.s3.S3Client;
@@ -52,7 +52,7 @@ private static S3Client buildClient(TestingHttpServer httpServer, TrinoAwsProxyC
         RemoteSessionRole remoteSessionRole = new RemoteSessionRole("us-east-1", "minio-doesnt-care", Optional.empty(), Optional.empty());
         IdentityCredential identityCredential = new IdentityCredential(new Credential(UUID.randomUUID().toString(), UUID.randomUUID().toString()),
                 TESTING_IDENTITY_CREDENTIAL.identity());
-        testingCredentialsRolesProvider.addCredentials(identityCredential, new RemoteS3Connection(policyUserCredential, Optional.of(remoteSessionRole), Optional.empty()));
+        testingCredentialsRolesProvider.addCredentials(identityCredential, new StaticRemoteS3Connection(policyUserCredential, remoteSessionRole));
         AwsBasicCredentials awsBasicCredentials = AwsBasicCredentials.create(identityCredential.emulated().accessKey(), identityCredential.emulated().secretKey());
         return clientBuilder(httpServer.getBaseUrl(), Optional.of(trinoAwsProxyConfig.getS3Path()))
                 .credentialsProvider(() -> awsBasicCredentials)

trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/testing/TestingRemoteS3Facade.java

@@ -14,7 +14,9 @@
 package io.trino.aws.proxy.server.testing;
 
 import com.google.inject.Inject;
-import io.trino.aws.proxy.server.testing.TestingUtil.ForTesting;
+import io.trino.aws.proxy.server.remote.DefaultRemoteS3Config;
+import io.trino.aws.proxy.server.remote.PathStyleRemoteS3Facade;
+import io.trino.aws.proxy.server.testing.containers.S3Container;
 import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
 import jakarta.ws.rs.core.UriBuilder;
 
@@ -28,12 +30,15 @@ public class TestingRemoteS3Facade
 {
     private final AtomicReference<RemoteS3Facade> delegate = new AtomicReference<>();
 
-    public TestingRemoteS3Facade() {}
-
     @Inject
-    public TestingRemoteS3Facade(@ForTesting RemoteS3Facade delegate)
+    public TestingRemoteS3Facade(S3Container s3Container)
     {
-        setDelegate(delegate);
+        delegate.set(new PathStyleRemoteS3Facade(new DefaultRemoteS3Config()
+                .setDomain(s3Container.containerHost().getHost())
+                .setPort(s3Container.containerHost().getPort())
+                .setHttps(false)
+                .setVirtualHostStyle(false)
+                .setHostnameTemplate("${domain}")));
     }
 
     @Override

trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/testing/TestingTrinoAwsProxyServer.java

@@ -41,14 +41,12 @@
 import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
 import io.trino.aws.proxy.spi.credentials.Credential;
 import io.trino.aws.proxy.spi.credentials.IdentityCredential;
-import io.trino.aws.proxy.spi.remote.RemoteS3Connection;
-import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
+import io.trino.aws.proxy.spi.remote.RemoteS3Connection.StaticRemoteS3Connection;
 
 import java.io.Closeable;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 
 import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder;
 import static io.trino.aws.proxy.server.testing.TestingUtil.TESTING_IDENTITY_CREDENTIAL;
@@ -120,14 +118,9 @@ public Builder withS3Container()
                 binder.bind(Credential.class).annotatedWith(ForTesting.class).toInstance(TESTING_IDENTITY_CREDENTIAL.emulated());
                 binder.bind(Credential.class).annotatedWith(ForS3Container.class).toInstance(TESTING_REMOTE_CREDENTIAL);
                 newOptionalBinder(binder, Key.get(new TypeLiteral<List<String>>() {}, ForS3Container.class)).setDefault().toInstance(ImmutableList.of());
-
-                newOptionalBinder(binder, Key.get(RemoteS3Facade.class, ForTesting.class))
-                        .setDefault()
-                        .to(ContainerS3Facade.PathStyleContainerS3Facade.class)
-                        .in(Scopes.SINGLETON);
             });
 
-            addModule(remoteS3Module("testing", TestingRemoteS3Facade.class, (binder) -> binder.bind(TestingRemoteS3Facade.class).in(Scopes.SINGLETON)));
+            addModule(remoteS3Module("testing", TestingRemoteS3Facade.class, binder -> binder.bind(TestingRemoteS3Facade.class).in(Scopes.SINGLETON)));
             withProperty("remote-s3.type", "testing");
 
             return this;
@@ -244,7 +237,7 @@ static class TestingCredentialsInitializer
         TestingCredentialsInitializer(TestingCredentialsRolesProvider credentialsController)
         {
             credentialsController.addCredentials(TESTING_IDENTITY_CREDENTIAL);
-            credentialsController.setDefaultRemoteConnection(new RemoteS3Connection(TESTING_REMOTE_CREDENTIAL, Optional.empty(), Optional.empty()));
+            credentialsController.setDefaultRemoteConnection(new StaticRemoteS3Connection(TESTING_REMOTE_CREDENTIAL));
         }
     }
 

trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/testing/harness/TrinoAwsProxyTestCommonModules.java

@@ -15,13 +15,10 @@
 
 import com.google.common.collect.ImmutableList;
 import com.google.inject.Key;
-import com.google.inject.Scopes;
 import com.google.inject.TypeLiteral;
-import io.trino.aws.proxy.server.testing.ContainerS3Facade;
 import io.trino.aws.proxy.server.testing.TestingTrinoAwsProxyServer;
 import io.trino.aws.proxy.server.testing.TestingUtil.ForTesting;
 import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
-import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
 
 import java.util.List;
 
@@ -46,20 +43,6 @@ public TestingTrinoAwsProxyServer.Builder filter(TestingTrinoAwsProxyServer.Buil
         }
     }
 
-    public static class WithVirtualHostAddressing
-            implements BuilderFilter
-    {
-        @Override
-        public TestingTrinoAwsProxyServer.Builder filter(TestingTrinoAwsProxyServer.Builder builder)
-        {
-            return builder.addModule(binder ->
-                    newOptionalBinder(binder, Key.get(RemoteS3Facade.class, ForTesting.class))
-                            .setBinding()
-                            .to(ContainerS3Facade.VirtualHostStyleContainerS3Facade.class)
-                            .in(Scopes.SINGLETON));
-        }
-    }
-
     public static class WithVirtualHostEnabledProxy
             implements BuilderFilter
     {