Skip to content

Commit f2353a5

Browse files
mosiac1mosiac1
committed
Make RemoteS3Connection an interface, replacing Map with RemoteS3Facade
Update RemoteS3Facade default bindings to not set the optional default but the value. RemoteS3Connection no longer has a function to get a Map of configs, but gets a function to get an instantiated RemoteS3Facade. Add a StaticRemoteS3Connection record to replace the previous one.
1 parent 15ccf77 commit f2353a5

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

0 commit comments

Comments
 (0)