Skip to content

Commit cf4c81a

Browse files
mosiac1mosiac1
committed
Update RemoteS3Facade bindings, make RemoteS3Connection an interface
1 parent faea9c2 commit cf4c81a

File tree

11 files changed

+91
-69
lines changed

11 files changed

+91
-69
lines changed

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

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,47 @@
1313
*/
1414
package io.trino.aws.proxy.spi.remote;
1515

16-
import com.google.common.collect.ImmutableMap;
1716
import io.trino.aws.proxy.spi.credentials.Credential;
1817

19-
import java.util.Map;
2018
import java.util.Optional;
2119

2220
import static java.util.Objects.requireNonNull;
2321

24-
public record RemoteS3Connection(
25-
Credential remoteCredential,
26-
Optional<RemoteSessionRole> remoteSessionRole,
27-
Optional<Map<String, String>> remoteS3FacadeConfiguration)
22+
public interface RemoteS3Connection
2823
{
29-
public RemoteS3Connection
24+
Credential remoteCredential();
25+
26+
default Optional<RemoteSessionRole> remoteSessionRole()
3027
{
31-
requireNonNull(remoteCredential, "remoteCredential is null");
32-
requireNonNull(remoteSessionRole, "remoteSessionRole is null");
33-
remoteS3FacadeConfiguration = remoteS3FacadeConfiguration.map(ImmutableMap::copyOf);
28+
return Optional.empty();
29+
}
30+
31+
default Optional<RemoteS3Facade> remoteS3Facade()
32+
{
33+
return Optional.empty();
34+
}
35+
36+
record StaticRemoteS3Connection(
37+
Credential remoteCredential,
38+
Optional<RemoteSessionRole> remoteSessionRole,
39+
Optional<RemoteS3Facade> remoteS3Facade)
40+
implements RemoteS3Connection
41+
{
42+
public StaticRemoteS3Connection
43+
{
44+
requireNonNull(remoteCredential, "remoteCredential is null");
45+
requireNonNull(remoteSessionRole, "remoteSessionRole is null");
46+
requireNonNull(remoteS3Facade, "remoteS3Facade is null");
47+
}
48+
49+
public StaticRemoteS3Connection(Credential remoteCredential)
50+
{
51+
this(remoteCredential, Optional.empty(), Optional.empty());
52+
}
53+
54+
public StaticRemoteS3Connection(Credential remoteCredential, RemoteSessionRole remoteSessionRole)
55+
{
56+
this(remoteCredential, Optional.of(remoteSessionRole), Optional.empty());
57+
}
3458
}
3559
}

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,12 @@
1919
import io.trino.aws.proxy.spi.credentials.CredentialsProvider;
2020
import io.trino.aws.proxy.spi.plugin.config.AssumedRoleProviderConfig;
2121
import io.trino.aws.proxy.spi.plugin.config.CredentialsProviderConfig;
22+
import io.trino.aws.proxy.spi.plugin.config.RemoteS3Config;
2223
import io.trino.aws.proxy.spi.plugin.config.RemoteS3ConnectionProviderConfig;
2324
import io.trino.aws.proxy.spi.remote.RemoteS3ConnectionProvider;
25+
import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
26+
27+
import java.util.Optional;
2428

2529
import static com.google.common.base.Preconditions.checkArgument;
2630

@@ -36,7 +40,9 @@ public TrinoAwsProxyPluginValidator(
3640
AssumedRoleProviderConfig assumedRoleProviderConfig,
3741
AssumedRoleProvider assumedRoleProvider,
3842
RemoteS3ConnectionProvider remoteS3ConnectionProvider,
39-
RemoteS3ConnectionProviderConfig remoteS3ConnectionProviderConfig)
43+
RemoteS3ConnectionProviderConfig remoteS3ConnectionProviderConfig,
44+
Optional<RemoteS3Facade> remoteS3Facade,
45+
RemoteS3Config remoteS3Config)
4046
{
4147
boolean credentialsProviderIsNoop = credentialsProvider.equals(CredentialsProvider.NOOP);
4248
boolean credentialsProviderIsConfigured = credentialsProviderConfig.getPluginIdentifier().isPresent();
@@ -58,6 +64,11 @@ public TrinoAwsProxyPluginValidator(
5864
"%s of type \"%s\" is not registered",
5965
RemoteS3ConnectionProvider.class.getSimpleName(),
6066
remoteS3ConnectionProviderConfig.getPluginIdentifier().orElse("<empty>"));
67+
68+
if (remoteS3Facade.isEmpty()) {
69+
throw new IllegalArgumentException("%s of type \"%s\" is not registered"
70+
.formatted(RemoteS3Facade.class.getSimpleName(), remoteS3Config.getPluginIdentifier().orElseThrow()));
71+
}
6172
}
6273
}
6374

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,6 @@ public class TrinoAwsProxyServerModule
8080
{
8181
private static final Logger log = Logger.get(TrinoAwsProxyServerModule.class);
8282

83-
@Provides
84-
@Singleton
85-
public RemoteUriFacade remoteUriFacade(RemoteS3Facade remoteS3Facade)
86-
{
87-
return remoteS3Facade;
88-
}
89-
9083
@Override
9184
protected void setup(Binder binder)
9285
{
@@ -116,6 +109,7 @@ protected void setup(Binder binder)
116109
httpServerBinder.enableLegacyUriCompliance();
117110
httpServerBinder.enableCaseSensitiveHeaderCache();
118111

112+
// RemoteS3ConnectionProvider binder
119113
configBinder(binder).bindConfig(RemoteS3ConnectionProviderConfig.class);
120114
newOptionalBinder(binder, RemoteS3ConnectionProvider.class).setDefault().toProvider(() -> {
121115
log.info("Using default %s NOOP implementation", RemoteS3ConnectionProvider.class.getSimpleName());
@@ -141,8 +135,7 @@ protected void setup(Binder binder)
141135
install(new OpaS3SecurityModule());
142136
install(new HttpCredentialsModule());
143137

144-
// RemoteS3 binder
145-
newOptionalBinder(binder, RemoteS3Facade.class);
138+
configBinder(binder).bindConfig(RemoteS3Config.class);
146139
// RemoteS3 provided implementation
147140
install(conditionalModule(
148141
RemoteS3Config.class,
@@ -172,6 +165,13 @@ public XmlMapper newXmlMapper()
172165
return xmlMapper;
173166
}
174167

168+
@Provides
169+
@Singleton
170+
public RemoteUriFacade remoteUriFacade(RemoteS3Facade remoteS3Facade)
171+
{
172+
return remoteS3Facade;
173+
}
174+
175175
public static void bindResourceAtPath(JaxrsBinder jaxrsBinder, Class<?> resourceClass, String resourcePrefix)
176176
{
177177
jaxrsBinder.bind(resourceClass);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ protected void setup(Binder binder)
4141
DefaultRemoteS3Config.class,
4242
DefaultRemoteS3Config::getVirtualHostStyle,
4343
innerBinder -> newOptionalBinder(innerBinder, RemoteS3Facade.class)
44-
.setDefault()
44+
.setBinding()
4545
.to(VirtualHostStyleRemoteS3Facade.class)
4646
.in(Scopes.SINGLETON),
4747
innerBinder -> newOptionalBinder(innerBinder, RemoteS3Facade.class)
48-
.setDefault()
48+
.setBinding()
4949
.to(PathStyleRemoteS3Facade.class)
5050
.in(Scopes.SINGLETON)));
5151
}

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

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
package io.trino.aws.proxy.server.remote;
1515

1616
import com.google.inject.Inject;
17-
import com.google.inject.Injector;
18-
import io.airlift.bootstrap.Bootstrap;
1917
import io.airlift.log.Logger;
2018
import io.trino.aws.proxy.spi.credentials.Credential;
2119
import io.trino.aws.proxy.spi.credentials.Identity;
@@ -140,12 +138,7 @@ public <T> Optional<T> withRemoteConnection(SigningMetadata signingMetadata, Opt
140138
{
141139
return remoteS3ConnectionProvider.remoteConnection(signingMetadata, identity, request)
142140
.flatMap(remoteConnection -> {
143-
RemoteS3Facade contextRemoteS3Facade = remoteConnection.remoteS3FacadeConfiguration().map(config -> {
144-
// TODO: This should respect the plugin installed for the RemoteS3Facade somehow
145-
Injector subInjector = new Bootstrap(new DefaultRemoteS3Module()).doNotInitializeLogging().quiet().setRequiredConfigurationProperties(config).initialize();
146-
return subInjector.getInstance(RemoteS3Facade.class);
147-
}).orElse(defaultS3Facade);
148-
141+
RemoteS3Facade contextRemoteS3Facade = remoteConnection.remoteS3Facade().orElse(defaultS3Facade);
149142
return remoteConnection.remoteSessionRole()
150143
.map(remoteSessionRole ->
151144
internalRemoteSession(remoteSessionRole, remoteConnection.remoteCredential())

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
2222
import io.trino.aws.proxy.spi.credentials.Credential;
2323
import io.trino.aws.proxy.spi.credentials.IdentityCredential;
24-
import io.trino.aws.proxy.spi.remote.RemoteS3Connection;
24+
import io.trino.aws.proxy.spi.remote.RemoteS3Connection.StaticRemoteS3Connection;
2525
import io.trino.aws.proxy.spi.remote.RemoteSessionRole;
2626
import software.amazon.awssdk.services.s3.S3Client;
2727

@@ -50,6 +50,6 @@ public TestProxiedEmulatedAndRemoteAssumedRoleRequests(
5050
Credential policyUserCredential = s3Container.policyUserCredential();
5151
RemoteSessionRole remoteSessionRole = new RemoteSessionRole("us-east-1", "minio-doesnt-care", Optional.empty(), Optional.empty());
5252
IdentityCredential identityCredential = new IdentityCredential(CREDENTIAL, TESTING_IDENTITY_CREDENTIAL.identity());
53-
credentialsController.addCredentials(identityCredential, new RemoteS3Connection(policyUserCredential, Optional.of(remoteSessionRole), Optional.empty()));
53+
credentialsController.addCredentials(identityCredential, new StaticRemoteS3Connection(policyUserCredential, remoteSessionRole));
5454
}
5555
}

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,33 @@
1414
package io.trino.aws.proxy.server;
1515

1616
import com.google.inject.Inject;
17+
import io.trino.aws.proxy.server.remote.DefaultRemoteS3Config;
18+
import io.trino.aws.proxy.server.remote.VirtualHostStyleRemoteS3Facade;
19+
import io.trino.aws.proxy.server.testing.TestingRemoteS3Facade;
1720
import io.trino.aws.proxy.server.testing.TestingS3RequestRewriteController;
21+
import io.trino.aws.proxy.server.testing.containers.S3Container;
1822
import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
1923
import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTest;
2024
import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTestCommonModules.WithConfiguredBuckets;
21-
import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTestCommonModules.WithVirtualHostAddressing;
2225
import software.amazon.awssdk.services.s3.S3Client;
2326

24-
@TrinoAwsProxyTest(filters = {WithConfiguredBuckets.class, WithVirtualHostAddressing.class})
27+
import static io.trino.aws.proxy.server.testing.TestingUtil.LOCALHOST_DOMAIN;
28+
29+
@TrinoAwsProxyTest(filters = WithConfiguredBuckets.class)
2530
public class TestProxiedRequestsToVirtualHostEndpoint
2631
extends AbstractTestProxiedRequests
2732
{
2833
@Inject
29-
public TestProxiedRequestsToVirtualHostEndpoint(S3Client s3Client, @ForS3Container S3Client storageClient, TestingS3RequestRewriteController requestRewriteController)
34+
public TestProxiedRequestsToVirtualHostEndpoint(S3Client s3Client, @ForS3Container S3Client storageClient, TestingS3RequestRewriteController requestRewriteController,
35+
TestingRemoteS3Facade testingRemoteS3Facade, S3Container s3Container)
3036
{
3137
super(s3Client, storageClient, requestRewriteController);
38+
39+
testingRemoteS3Facade.setDelegate(new VirtualHostStyleRemoteS3Facade(new DefaultRemoteS3Config()
40+
.setDomain(LOCALHOST_DOMAIN)
41+
.setPort(s3Container.containerHost().getPort())
42+
.setHttps(false)
43+
.setVirtualHostStyle(true)
44+
.setHostnameTemplate("${bucket}.${domain}")));
3245
}
3346
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTestCommonModules.WithConfiguredBuckets;
2424
import io.trino.aws.proxy.spi.credentials.Credential;
2525
import io.trino.aws.proxy.spi.credentials.IdentityCredential;
26-
import io.trino.aws.proxy.spi.remote.RemoteS3Connection;
26+
import io.trino.aws.proxy.spi.remote.RemoteS3Connection.StaticRemoteS3Connection;
2727
import io.trino.aws.proxy.spi.remote.RemoteSessionRole;
2828
import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
2929
import software.amazon.awssdk.services.s3.S3Client;
@@ -52,7 +52,7 @@ private static S3Client buildClient(TestingHttpServer httpServer, TrinoAwsProxyC
5252
RemoteSessionRole remoteSessionRole = new RemoteSessionRole("us-east-1", "minio-doesnt-care", Optional.empty(), Optional.empty());
5353
IdentityCredential identityCredential = new IdentityCredential(new Credential(UUID.randomUUID().toString(), UUID.randomUUID().toString()),
5454
TESTING_IDENTITY_CREDENTIAL.identity());
55-
testingCredentialsRolesProvider.addCredentials(identityCredential, new RemoteS3Connection(policyUserCredential, Optional.of(remoteSessionRole), Optional.empty()));
55+
testingCredentialsRolesProvider.addCredentials(identityCredential, new StaticRemoteS3Connection(policyUserCredential, remoteSessionRole));
5656
AwsBasicCredentials awsBasicCredentials = AwsBasicCredentials.create(identityCredential.emulated().accessKey(), identityCredential.emulated().secretKey());
5757
return clientBuilder(httpServer.getBaseUrl(), Optional.of(trinoAwsProxyConfig.getS3Path()))
5858
.credentialsProvider(() -> awsBasicCredentials)

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

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
package io.trino.aws.proxy.server.testing;
1515

1616
import com.google.inject.Inject;
17-
import io.trino.aws.proxy.server.testing.TestingUtil.ForTesting;
17+
import io.trino.aws.proxy.server.remote.DefaultRemoteS3Config;
18+
import io.trino.aws.proxy.server.remote.PathStyleRemoteS3Facade;
19+
import io.trino.aws.proxy.server.testing.containers.S3Container;
1820
import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
1921
import jakarta.ws.rs.core.UriBuilder;
2022

@@ -28,12 +30,15 @@ public class TestingRemoteS3Facade
2830
{
2931
private final AtomicReference<RemoteS3Facade> delegate = new AtomicReference<>();
3032

31-
public TestingRemoteS3Facade() {}
32-
3333
@Inject
34-
public TestingRemoteS3Facade(@ForTesting RemoteS3Facade delegate)
34+
public TestingRemoteS3Facade(S3Container s3Container)
3535
{
36-
setDelegate(delegate);
36+
delegate.set(new PathStyleRemoteS3Facade(new DefaultRemoteS3Config()
37+
.setDomain(s3Container.containerHost().getHost())
38+
.setPort(s3Container.containerHost().getPort())
39+
.setHttps(false)
40+
.setVirtualHostStyle(false)
41+
.setHostnameTemplate("${domain}")));
3742
}
3843

3944
@Override

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,12 @@
4141
import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
4242
import io.trino.aws.proxy.spi.credentials.Credential;
4343
import io.trino.aws.proxy.spi.credentials.IdentityCredential;
44-
import io.trino.aws.proxy.spi.remote.RemoteS3Connection;
45-
import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
44+
import io.trino.aws.proxy.spi.remote.RemoteS3Connection.StaticRemoteS3Connection;
4645

4746
import java.io.Closeable;
4847
import java.util.Collection;
4948
import java.util.List;
5049
import java.util.Map;
51-
import java.util.Optional;
5250

5351
import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder;
5452
import static io.trino.aws.proxy.server.testing.TestingUtil.TESTING_IDENTITY_CREDENTIAL;
@@ -120,14 +118,9 @@ public Builder withS3Container()
120118
binder.bind(Credential.class).annotatedWith(ForTesting.class).toInstance(TESTING_IDENTITY_CREDENTIAL.emulated());
121119
binder.bind(Credential.class).annotatedWith(ForS3Container.class).toInstance(TESTING_REMOTE_CREDENTIAL);
122120
newOptionalBinder(binder, Key.get(new TypeLiteral<List<String>>() {}, ForS3Container.class)).setDefault().toInstance(ImmutableList.of());
123-
124-
newOptionalBinder(binder, Key.get(RemoteS3Facade.class, ForTesting.class))
125-
.setDefault()
126-
.to(ContainerS3Facade.PathStyleContainerS3Facade.class)
127-
.in(Scopes.SINGLETON);
128121
});
129122

130-
addModule(remoteS3Module("testing", TestingRemoteS3Facade.class, (binder) -> binder.bind(TestingRemoteS3Facade.class).in(Scopes.SINGLETON)));
123+
addModule(remoteS3Module("testing", TestingRemoteS3Facade.class, binder -> binder.bind(TestingRemoteS3Facade.class).in(Scopes.SINGLETON)));
131124
withProperty("remote-s3.type", "testing");
132125

133126
return this;
@@ -244,7 +237,7 @@ static class TestingCredentialsInitializer
244237
TestingCredentialsInitializer(TestingCredentialsRolesProvider credentialsController)
245238
{
246239
credentialsController.addCredentials(TESTING_IDENTITY_CREDENTIAL);
247-
credentialsController.setDefaultRemoteConnection(new RemoteS3Connection(TESTING_REMOTE_CREDENTIAL, Optional.empty(), Optional.empty()));
240+
credentialsController.setDefaultRemoteConnection(new StaticRemoteS3Connection(TESTING_REMOTE_CREDENTIAL));
248241
}
249242
}
250243

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

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,10 @@
1515

1616
import com.google.common.collect.ImmutableList;
1717
import com.google.inject.Key;
18-
import com.google.inject.Scopes;
1918
import com.google.inject.TypeLiteral;
20-
import io.trino.aws.proxy.server.testing.ContainerS3Facade;
2119
import io.trino.aws.proxy.server.testing.TestingTrinoAwsProxyServer;
2220
import io.trino.aws.proxy.server.testing.TestingUtil.ForTesting;
2321
import io.trino.aws.proxy.server.testing.containers.S3Container.ForS3Container;
24-
import io.trino.aws.proxy.spi.remote.RemoteS3Facade;
2522

2623
import java.util.List;
2724

@@ -46,20 +43,6 @@ public TestingTrinoAwsProxyServer.Builder filter(TestingTrinoAwsProxyServer.Buil
4643
}
4744
}
4845

49-
public static class WithVirtualHostAddressing
50-
implements BuilderFilter
51-
{
52-
@Override
53-
public TestingTrinoAwsProxyServer.Builder filter(TestingTrinoAwsProxyServer.Builder builder)
54-
{
55-
return builder.addModule(binder ->
56-
newOptionalBinder(binder, Key.get(RemoteS3Facade.class, ForTesting.class))
57-
.setBinding()
58-
.to(ContainerS3Facade.VirtualHostStyleContainerS3Facade.class)
59-
.in(Scopes.SINGLETON));
60-
}
61-
}
62-
6346
public static class WithVirtualHostEnabledProxy
6447
implements BuilderFilter
6548
{

0 commit comments

Comments
 (0)