Skip to content
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.

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

Store AWS region in AwsStorageConfigurationInfo #455

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ private StorageConfigInfo getStorageInfo(Map<String, String> internalProperties)
.setUserArn(awsConfig.getUserARN())
.setStorageType(StorageConfigInfo.StorageTypeEnum.S3)
.setAllowedLocations(awsConfig.getAllowedLocations())
.setRegion(awsConfig.getRegion())
.build();
}
if (configInfo instanceof AzureStorageConfigurationInfo) {
Expand Down Expand Up @@ -244,7 +245,8 @@ public Builder setStorageConfigurationInfo(
PolarisStorageConfigurationInfo.StorageType.S3,
new ArrayList<>(allowedLocations),
awsConfigModel.getRoleArn(),
awsConfigModel.getExternalId());
awsConfigModel.getExternalId(),
awsConfigModel.getRegion());
awsConfig.validateArn(awsConfigModel.getRoleArn());
config = awsConfig;
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,30 @@ public class AwsStorageConfigurationInfo extends PolarisStorageConfigurationInfo
@JsonProperty(value = "userARN")
private @Nullable String userARN = null;

/** User ARN for the service principal */
@JsonProperty(value = "region")
private @Nullable String region = null;

@JsonCreator
public AwsStorageConfigurationInfo(
@JsonProperty(value = "storageType", required = true) @NotNull StorageType storageType,
@JsonProperty(value = "allowedLocations", required = true) @NotNull
List<String> allowedLocations,
@JsonProperty(value = "roleARN", required = true) @NotNull String roleARN) {
this(storageType, allowedLocations, roleARN, null);
@JsonProperty(value = "roleARN", required = true) @NotNull String roleARN,
@JsonProperty(value = "region", required = false) @NotNull String region) {
this(storageType, allowedLocations, roleARN, null, region);
}

public AwsStorageConfigurationInfo(
@NotNull StorageType storageType,
@NotNull List<String> allowedLocations,
@NotNull String roleARN,
@Nullable String externalId) {
@Nullable String externalId,
@Nullable String region) {
super(storageType, allowedLocations);
this.roleARN = roleARN;
this.externalId = externalId;
this.region = region;
validateMaxAllowedLocations(MAX_ALLOWED_LOCATIONS);
}

Expand Down Expand Up @@ -107,6 +114,14 @@ public void setUserARN(@Nullable String userARN) {
this.userARN = userARN;
}

public @Nullable String getRegion() {
return region;
}

public void setRegion(@Nullable String region) {
this.region = region;
}

@Override
public String toString() {
return MoreObjects.toStringHelper(this)
Expand All @@ -116,6 +131,7 @@ public String toString() {
.add("userARN", userARN)
.add("externalId", externalId)
.add("allowedLocation", getAllowedLocations())
.add("region", region)
.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public void testValidateAccessToLocations() {
"s3://bucket/path/to/warehouse",
"s3://bucket/anotherpath/to/warehouse",
"s3://bucket2/warehouse/"),
"arn:aws:iam::012345678901:role/jdoe"),
"arn:aws:iam::012345678901:role/jdoe",
"us-east-2"),
Set.of(PolarisStorageActions.READ),
Set.of(
"s3://bucket/path/to/warehouse/namespace/table",
Expand Down Expand Up @@ -136,7 +137,8 @@ public void testValidateAccessToLocationsNoAllowedLocations() {
new AwsStorageConfigurationInfo(
PolarisStorageConfigurationInfo.StorageType.S3,
List.of(),
"arn:aws:iam::012345678901:role/jdoe"),
"arn:aws:iam::012345678901:role/jdoe",
"us-east-2"),
Set.of(PolarisStorageActions.READ),
Set.of(
"s3://bucket/path/to/warehouse/namespace/table",
Expand Down Expand Up @@ -169,7 +171,8 @@ public void testValidateAccessToLocationsWithPrefixOfAllowedLocation() {
new AwsStorageConfigurationInfo(
PolarisStorageConfigurationInfo.StorageType.S3,
List.of("s3://bucket/path/to/warehouse"),
"arn:aws:iam::012345678901:role/jdoe"),
"arn:aws:iam::012345678901:role/jdoe",
"us-east-2"),
Set.of(PolarisStorageActions.READ),
// trying to read a prefix under the allowed location
Set.of("s3://bucket/path/to"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ public void testGetSubscopedCreds() {
PolarisStorageConfigurationInfo.StorageType.S3,
List.of(warehouseDir),
roleARN,
externalId),
externalId,
null),
true,
Set.of(warehouseDir + "/namespace/table"),
Set.of(warehouseDir + "/namespace/table"));
Expand Down Expand Up @@ -217,7 +218,11 @@ public void testGetSubscopedCredsInlinePolicy(String awsPartition) {
.getSubscopedCreds(
Mockito.mock(PolarisDiagnostics.class),
new AwsStorageConfigurationInfo(
storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), roleARN, externalId),
storageType,
List.of(s3Path(bucket, warehouseKeyPrefix)),
roleARN,
externalId,
"us-east-2"),
true,
Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)),
Set.of(s3Path(bucket, firstPath)));
Expand Down Expand Up @@ -310,7 +315,8 @@ public void testGetSubscopedCredsInlinePolicyWithoutList() {
PolarisStorageConfigurationInfo.StorageType.S3,
List.of(s3Path(bucket, warehouseKeyPrefix)),
roleARN,
externalId),
externalId,
"us-east-2"),
false, /* allowList = false*/
Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)),
Set.of(s3Path(bucket, firstPath)));
Expand Down Expand Up @@ -398,7 +404,11 @@ public void testGetSubscopedCredsInlinePolicyWithoutWrites() {
.getSubscopedCreds(
Mockito.mock(PolarisDiagnostics.class),
new AwsStorageConfigurationInfo(
storageType, List.of(s3Path(bucket, warehouseKeyPrefix)), roleARN, externalId),
storageType,
List.of(s3Path(bucket, warehouseKeyPrefix)),
roleARN,
externalId,
"us-east-2"),
true, /* allowList = true */
Set.of(s3Path(bucket, firstPath), s3Path(bucket, secondPath)),
Set.of());
Expand Down Expand Up @@ -459,7 +469,8 @@ public void testGetSubscopedCredsInlinePolicyWithEmptyReadAndWrite() {
PolarisStorageConfigurationInfo.StorageType.S3,
List.of(s3Path(bucket, warehouseKeyPrefix)),
roleARN,
externalId),
externalId,
"us-east-2"),
true, /* allowList = true */
Set.of(),
Set.of());
Expand Down
4 changes: 4 additions & 0 deletions spec/polaris-management-service.yml
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,10 @@ components:
type: string
description: the aws user arn used to assume the aws role
example: "arn:aws:iam::123456789001:user/abc1-b-self1234"
region:
type: string
description: the aws region where data is stored
example: "us-east-2"
required:
- roleArn

Expand Down
Loading