Improve FileBasedCredentialsProvider binding and test

Use bound Identity type to deserialize credentials
trinodb · Aug 1, 2024 · d8873c7 · d8873c7
1 parent 653e641
commit d8873c7
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 37 deletions.
diff --git a/.../src/main/java/io/trino/aws/proxy/server/credentials/file/FileBasedCredentialsModule.java b/.../src/main/java/io/trino/aws/proxy/server/credentials/file/FileBasedCredentialsModule.java
@@ -13,34 +13,27 @@
  */
 package io.trino.aws.proxy.server.credentials.file;
 
-import com.fasterxml.jackson.databind.Module;
-import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.google.inject.Binder;
 import io.airlift.configuration.AbstractConfigurationAwareModule;
-import io.trino.aws.proxy.spi.credentials.Identity;
 
-import static com.google.inject.multibindings.Multibinder.newSetBinder;
 import static io.airlift.configuration.ConfigBinder.configBinder;
 import static io.trino.aws.proxy.spi.plugin.TrinoAwsProxyServerBinding.credentialsProviderModule;
 
 public class FileBasedCredentialsModule
  extends AbstractConfigurationAwareModule
 {
+ // set as config value for "credentials-provider.type"
+ public static final String FILE_BASED_CREDENTIALS_IDENTIFIER = "file";
+
  @Override
  protected void setup(Binder binder)
  {
  install(credentialsProviderModule(
- "file",
+ FILE_BASED_CREDENTIALS_IDENTIFIER,
  FileBasedCredentialsProvider.class,
  innerBinder -> {
  configBinder(innerBinder).bindConfig(FileBasedCredentialsProviderConfig.class);
  innerBinder.bind(FileBasedCredentialsProvider.class);
  }));
  }
-
- public static void bindFileBasedCredentialsIdentity(Binder binder, Class<? extends Identity> type)
- {
- newSetBinder(binder, Module.class).addBinding()
- .toInstance(new SimpleModule().addAbstractTypeMapping(Identity.class, type));
- }
 }
diff --git a/...rc/main/java/io/trino/aws/proxy/server/credentials/file/FileBasedCredentialsProvider.java b/...rc/main/java/io/trino/aws/proxy/server/credentials/file/FileBasedCredentialsProvider.java
@@ -15,9 +15,11 @@
 
 import com.fasterxml.jackson.core.type.TypeReference;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.google.inject.Inject;
 import io.trino.aws.proxy.spi.credentials.Credentials;
 import io.trino.aws.proxy.spi.credentials.CredentialsProvider;
+import io.trino.aws.proxy.spi.credentials.Identity;
 
 import java.io.File;
 import java.io.FileInputStream;
@@ -38,9 +40,10 @@ public class FileBasedCredentialsProvider
  private final Map<String, Credentials> credentialsStore;
 
  @Inject
- public FileBasedCredentialsProvider(FileBasedCredentialsProviderConfig config, ObjectMapper objectMapper)
+ public FileBasedCredentialsProvider(FileBasedCredentialsProviderConfig config, ObjectMapper objectMapper, Class<? extends Identity> identityClass)
  {
  requireNonNull(config, "Config is null");
+ objectMapper = objectMapper.registerModule(new SimpleModule().addAbstractTypeMapping(Identity.class, identityClass));
  this.credentialsStore = buildCredentialsMap(config.getCredentialsFile(), objectMapper);
  }
 

diff --git a/...est/java/io/trino/aws/proxy/server/credentials/file/TestFileBasedCredentialsProvider.java b/...est/java/io/trino/aws/proxy/server/credentials/file/TestFileBasedCredentialsProvider.java
@@ -13,44 +13,59 @@
  */
 package io.trino.aws.proxy.server.credentials.file;
 
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.module.SimpleModule;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.io.Resources;
-import io.airlift.json.ObjectMapperProvider;
+import com.google.inject.Inject;
 import io.trino.aws.proxy.server.testing.TestingIdentity;
+import io.trino.aws.proxy.server.testing.TestingTrinoAwsProxyServer;
+import io.trino.aws.proxy.server.testing.harness.BuilderFilter;
+import io.trino.aws.proxy.server.testing.harness.TrinoAwsProxyTest;
 import io.trino.aws.proxy.spi.credentials.Credential;
 import io.trino.aws.proxy.spi.credentials.Credentials;
-import io.trino.aws.proxy.spi.credentials.Identity;
-import org.junit.jupiter.api.BeforeAll;
+import io.trino.aws.proxy.spi.credentials.CredentialsProvider;
 import org.junit.jupiter.api.Test;
 
 import java.io.File;
 import java.util.Optional;
 
+import static io.trino.aws.proxy.server.credentials.file.FileBasedCredentialsModule.FILE_BASED_CREDENTIALS_IDENTIFIER;
+import static io.trino.aws.proxy.spi.plugin.TrinoAwsProxyServerBinding.bindIdentityType;
+import static java.util.Objects.requireNonNull;
 import static org.assertj.core.api.Assertions.assertThat;
 
+@TrinoAwsProxyTest(filters = TestFileBasedCredentialsProvider.Filter.class)
 public class TestFileBasedCredentialsProvider
 {
- private static FileBasedCredentialsProvider credentialsProvider;
+ private final CredentialsProvider credentialsProvider;
 
- @BeforeAll
- public static void setUpClass()
+ public static class Filter
+  implements BuilderFilter
  {
- File configFile = new File(Resources.getResource("credentials.json").getFile());
- FileBasedCredentialsProviderConfig config = new FileBasedCredentialsProviderConfig().setCredentialsFile(configFile);
- ObjectMapper objectMapper = new ObjectMapperProvider()
- .withModules(ImmutableSet.of(new SimpleModule().addAbstractTypeMapping(Identity.class, TestingIdentity.class))).get();
- credentialsProvider = new FileBasedCredentialsProvider(config, objectMapper);
+ @Override
+ public TestingTrinoAwsProxyServer.Builder filter(TestingTrinoAwsProxyServer.Builder builder)
+ {
+ File configFile = new File(Resources.getResource("credentials.json").getFile());
+
+ return builder.withoutTestingCredentialsRoleProviders()
+ .addModule(new FileBasedCredentialsModule())
+ .addModule(binder -> bindIdentityType(binder, TestingIdentity.class))
+ .withProperty("credentials-provider.type", FILE_BASED_CREDENTIALS_IDENTIFIER)
+ .withProperty("credentials-provider.credentials-file-path", configFile.getAbsolutePath());
+ }
+ }
+
+ @Inject
+ public TestFileBasedCredentialsProvider(CredentialsProvider credentialsProvider)
+ {
+ this.credentialsProvider = requireNonNull(credentialsProvider, "credentialsProvider is null");
  }
 
  @Test
  public void testValidCredentials()
  {
  Credential emulated = new Credential("test-emulated-access-key", "test-emulated-secret");
  Credential remote = new Credential("test-remote-access-key", "test-remote-secret");
- Credentials expected = new Credentials(emulated, Optional.of(remote), Optional.empty(), Optional.of(new TestingIdentity("test-username", ImmutableList.of())));
+ Credentials expected = new Credentials(emulated, Optional.of(remote), Optional.empty(), Optional.of(new TestingIdentity("test-username", ImmutableList.of(), "xyzpdq")));
  Optional<Credentials> actual = credentialsProvider.credentials("test-emulated-access-key", Optional.empty());
  assertThat(actual).contains(expected);
  }

diff --git a/trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/testing/TestingIdentity.java b/trino-aws-proxy/src/test/java/io/trino/aws/proxy/server/testing/TestingIdentity.java
@@ -19,18 +19,13 @@
 
 import static java.util.Objects.requireNonNull;
 
-public record TestingIdentity(String user, List<String> groups)
+public record TestingIdentity(String user, List<String> groups, String id)
  implements Identity
 {
  public TestingIdentity
  {
  requireNonNull(user, "username is null");
  requireNonNull(groups, "groups is null");
- }
-
- @Override
- public String user()
- {
- return user;
+ requireNonNull(id, "id is null");
  }
 }
diff --git a/...aws-proxy/src/test/java/io/trino/aws/proxy/server/testing/TestingTrinoAwsProxyServer.java b/...aws-proxy/src/test/java/io/trino/aws/proxy/server/testing/TestingTrinoAwsProxyServer.java
@@ -109,8 +109,6 @@ public Builder withS3Container()
  mockS3ContainerAdded = true;
 
  modules.add(binder -> {
- binder.bind(TestingCredentialsInitializer.class).asEagerSingleton();
-
  binder.bind(S3Container.class).asEagerSingleton();
  binder.bind(Credentials.class).annotatedWith(ForTesting.class).toInstance(TESTING_CREDENTIALS);
  newOptionalBinder(binder, Key.get(new TypeLiteral<List<String>>() {}, ForS3Container.class)).setDefault().toInstance(ImmutableList.of());
@@ -200,6 +198,10 @@ public Builder withoutTestingCredentialsRoleProviders()
  public TestingTrinoAwsProxyServer buildAndStart()
  {
  if (addTestingCredentialsRoleProviders) {
+ if (mockS3ContainerAdded) {
+ modules.add(binder -> binder.bind(TestingCredentialsInitializer.class).asEagerSingleton());
+ }
+
  addModule(credentialsProviderModule("testing", TestingCredentialsRolesProvider.class, (binder) -> binder.bind(TestingCredentialsRolesProvider.class).in(Scopes.SINGLETON)));
  withProperty("credentials-provider.type", "testing");
  addModule(assumedRoleProviderModule("testing", TestingCredentialsRolesProvider.class, (binder) -> binder.bind(TestingCredentialsRolesProvider.class).in(Scopes.SINGLETON)));

diff --git a/trino-aws-proxy/src/test/resources/credentials.json b/trino-aws-proxy/src/test/resources/credentials.json
@@ -9,7 +9,8 @@
  "secretKey": "test-remote-secret"
  },
  "identity": {
- "user": "test-username"
+ "user": "test-username",
+ "id": "xyzpdq"
  }
  }
 ]