Skip to content

AWS: Simplify enabling S3 remote signing #7080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielcweeks merged 1 commit into from
Mar 11, 2023
Merged

AWS: Simplify enabling S3 remote signing #7080

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 13 additions & 52 deletions aws/src/main/java/org/apache/iceberg/aws/AwsProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import org.apache.iceberg.aws.glue.GlueCatalog;
import org.apache.iceberg.aws.lakeformation.LakeFormationAwsClientFactory;
import org.apache.iceberg.aws.s3.S3FileIO;
import org.apache.iceberg.common.DynConstructors;
import org.apache.iceberg.aws.s3.signer.S3V4RestSignerClient;
import org.apache.iceberg.common.DynMethods;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
Expand All @@ -47,7 +47,6 @@
import software.amazon.awssdk.awscore.client.builder.AwsSyncClientBuilder;
import software.amazon.awssdk.core.client.builder.SdkClientBuilder;
import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
import software.amazon.awssdk.core.signer.Signer;
import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain;
import software.amazon.awssdk.services.dynamodb.DynamoDbClientBuilder;
Expand Down Expand Up @@ -269,7 +268,9 @@ public class AwsProperties implements Serializable {

public static final boolean S3_CHECKSUM_ENABLED_DEFAULT = false;

public static final String S3_SIGNER_IMPL = "s3.signer-impl";
public static final String S3_REMOTE_SIGNING_ENABLED = "s3.remote-signing-enabled";

public static final boolean S3_REMOTE_SIGNING_ENABLED_DEFAULT = false;

/** Configure the batch size used when deleting multiple files from a given S3 bucket */
public static final String S3FILEIO_DELETE_BATCH_SIZE = "s3.delete.batch-size";
Expand Down Expand Up @@ -712,7 +713,7 @@ public class AwsProperties implements Serializable {
private String dynamoDbTableName;
private String dynamoDbEndpoint;

private final String s3SignerImpl;
private final boolean s3RemoteSigningEnabled;
private final Map<String, String> allProperties;

private String restSigningRegion;
Expand Down Expand Up @@ -768,7 +769,7 @@ public AwsProperties() {
this.dynamoDbEndpoint = null;
this.dynamoDbTableName = DYNAMODB_TABLE_NAME_DEFAULT;

this.s3SignerImpl = null;
this.s3RemoteSigningEnabled = S3_REMOTE_SIGNING_ENABLED_DEFAULT;
this.allProperties = Maps.newHashMap();

this.restSigningName = REST_SIGNING_NAME_DEFAULT;
Expand Down Expand Up @@ -898,7 +899,9 @@ public AwsProperties(Map<String, String> properties) {
this.dynamoDbTableName =
PropertyUtil.propertyAsString(properties, DYNAMODB_TABLE_NAME, DYNAMODB_TABLE_NAME_DEFAULT);

this.s3SignerImpl = properties.get(S3_SIGNER_IMPL);
this.s3RemoteSigningEnabled =
PropertyUtil.propertyAsBoolean(
properties, S3_REMOTE_SIGNING_ENABLED, S3_REMOTE_SIGNING_ENABLED_DEFAULT);
this.allProperties = SerializableMap.copyOf(properties);

this.restSigningRegion = properties.get(REST_SIGNER_REGION);
Expand Down Expand Up @@ -1157,54 +1160,12 @@ public <T extends S3ClientBuilder> void applyS3ServiceConfigurations(T builder)
* </pre>
*/
public <T extends S3ClientBuilder> void applyS3SignerConfiguration(T builder) {
if (null != s3SignerImpl) {
if (s3RemoteSigningEnabled) {
builder.overrideConfiguration(
c -> c.putAdvancedOption(SdkAdvancedClientOption.SIGNER, loadS3SignerDynamically()));
}
}

private Signer loadS3SignerDynamically() {
// load the signer implementation dynamically
Object signer = null;
try {
signer =
DynMethods.builder("create")
.impl(s3SignerImpl, Map.class)
.buildStaticChecked()
.invoke(allProperties);
} catch (NoSuchMethodException e) {
LOG.warn(
"Cannot find static method create(Map<String, String> properties) for signer {}",
s3SignerImpl,
e);
c ->
c.putAdvancedOption(
SdkAdvancedClientOption.SIGNER, S3V4RestSignerClient.create(allProperties)));
}

if (null == signer) {
try {
signer = DynMethods.builder("create").impl(s3SignerImpl).buildChecked().invoke(null);
} catch (NoSuchMethodException e) {
LOG.warn("Cannot find static method create() for signer {}", s3SignerImpl, e);
}
}

if (null == signer) {
// try via default no-arg constructor
try {
signer = DynConstructors.builder().impl(s3SignerImpl).buildChecked().newInstance();
} catch (NoSuchMethodException e) {
LOG.warn("Cannot find no-arg constructor for signer {}", s3SignerImpl, e);
}
}

Preconditions.checkArgument(
null != signer, "Cannot instantiate custom signer: %s", s3SignerImpl);

Preconditions.checkArgument(
signer instanceof Signer,
"Custom signer %s must be an instance of %s",
s3SignerImpl,
Signer.class.getName());
return (Signer) signer;
}

/**
Expand Down
41 changes: 6 additions & 35 deletions aws/src/test/java/org/apache/iceberg/aws/TestAwsProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
import software.amazon.awssdk.auth.credentials.AwsSessionCredentials;
import software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider;
import software.amazon.awssdk.auth.signer.AwsS3V4Signer;
import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
import software.amazon.awssdk.core.signer.Signer;
import software.amazon.awssdk.http.SdkHttpClient;
Expand Down Expand Up @@ -302,7 +301,7 @@ public void testKryoSerialization() throws IOException {
@Test
public void testS3RemoteSignerWithoutUri() {
Map<String, String> properties =
ImmutableMap.of(AwsProperties.S3_SIGNER_IMPL, S3V4RestSignerClient.class.getName());
ImmutableMap.of(AwsProperties.S3_REMOTE_SIGNING_ENABLED, "true");
AwsProperties awsProperties = new AwsProperties(properties);

Assertions.assertThatThrownBy(
Expand All @@ -312,14 +311,11 @@ public void testS3RemoteSignerWithoutUri() {
}

@Test
public void testS3RemoteSigner() {
public void testS3RemoteSigningEnabled() {
String uri = "http://localhost:12345";
Map<String, String> properties =
ImmutableMap.of(
AwsProperties.S3_SIGNER_IMPL,
S3V4RestSignerClient.class.getName(),
CatalogProperties.URI,
uri);
AwsProperties.S3_REMOTE_SIGNING_ENABLED, "true", CatalogProperties.URI, uri);
AwsProperties awsProperties = new AwsProperties(properties);
S3ClientBuilder builder = S3Client.builder();

Expand All @@ -334,41 +330,16 @@ public void testS3RemoteSigner() {
}

@Test
public void testS3LocalSigner() {
public void testS3RemoteSigningDisabled() {
Map<String, String> properties =
ImmutableMap.of(AwsProperties.S3_SIGNER_IMPL, AwsS3V4Signer.class.getName());
ImmutableMap.of(AwsProperties.S3_REMOTE_SIGNING_ENABLED, "false");
AwsProperties awsProperties = new AwsProperties(properties);
S3ClientBuilder builder = S3Client.builder();

awsProperties.applyS3SignerConfiguration(builder);

Optional<Signer> signer =
builder.overrideConfiguration().advancedOption(SdkAdvancedClientOption.SIGNER);
Assertions.assertThat(signer).isPresent().get().isInstanceOf(AwsS3V4Signer.class);
}

@Test
public void testS3WrongSigner() {
Map<String, String> properties = ImmutableMap.of(AwsProperties.S3_SIGNER_IMPL, "WrongSigner");
AwsProperties awsProperties = new AwsProperties(properties);

Assertions.assertThatThrownBy(
() -> awsProperties.applyS3SignerConfiguration(S3Client.builder()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Cannot instantiate custom signer: WrongSigner");
}

@Test
public void testS3SignerWrongSubclass() {
// we're just passing some random class as a signer impl
Map<String, String> properties =
ImmutableMap.of(AwsProperties.S3_SIGNER_IMPL, AwsProperties.class.getName());
AwsProperties awsProperties = new AwsProperties(properties);

Assertions.assertThatThrownBy(
() -> awsProperties.applyS3SignerConfiguration(S3Client.builder()))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage(
"Custom signer org.apache.iceberg.aws.AwsProperties must be an instance of software.amazon.awssdk.core.signer.Signer");
Assertions.assertThat(signer).isNotPresent();
}
}