Skip to content

AWS: add skip name validation config for glue namespace #5041

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
jackye1995 merged 9 commits into from
Jun 15, 2022
Merged

AWS: add skip name validation config for glue namespace #5041

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
bf70cbd
add skip name validation for glue namesapce
xingfanx Jun 14, 2022
70f2e4e
add skip name validation for glue table name
xingfanx Jun 14, 2022
bec2e90
add unittest for glue table name skip validation
xingfanx Jun 14, 2022
c8b70a8
add documentation
xingfanx Jun 14, 2022
8c0eec8
style
xingfanx Jun 14, 2022
43f5e18
style fix
xingfanx Jun 14, 2022
455b509
style fix
xingfanx Jun 15, 2022
8d8eea5
style and doc fix
xingfanx Jun 15, 2022
0b138d7
style fix indent
xingfanx Jun 15, 2022
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
20 changes: 18 additions & 2 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 @@ -103,6 +103,12 @@ public class AwsProperties implements Serializable {
public static final String GLUE_CATALOG_SKIP_ARCHIVE = "glue.skip-archive";
public static final boolean GLUE_CATALOG_SKIP_ARCHIVE_DEFAULT = false;

/**
* If Glue should skip name validations
Comment thread
xingfanx marked this conversation as resolved.
*/
public static final String GLUE_CATALOG_SKIP_NAME_VALIDATION = "glue.skip-archive";
Comment thread
xingfanx marked this conversation as resolved.
Outdated
public static final boolean GLUE_CATALOG_SKIP_NAME_VALIDATION_DEFAULT = false;

/**
* If set, GlueCatalog will use Lake Formation for access control.
* For more credential vending details, see: https://docs.aws.amazon.com/lake-formation/latest/dg/api-overview.html.
Expand Down Expand Up @@ -375,6 +381,7 @@ public class AwsProperties implements Serializable {

private String glueCatalogId;
private boolean glueCatalogSkipArchive;
private boolean glueCatalogSkipNameValidation;
private boolean glueLakeFormationEnabled;

private String dynamoDbTableName;
Expand All @@ -399,8 +406,8 @@ public AwsProperties() {

this.glueCatalogId = null;
this.glueCatalogSkipArchive = GLUE_CATALOG_SKIP_ARCHIVE_DEFAULT;
this.glueCatalogSkipNameValidation = GLUE_CATALOG_SKIP_NAME_VALIDATION_DEFAULT;
this.glueLakeFormationEnabled = GLUE_LAKEFORMATION_ENABLED_DEFAULT;

Comment thread
jackye1995 marked this conversation as resolved.
this.dynamoDbTableName = DYNAMODB_TABLE_NAME_DEFAULT;
}

Expand All @@ -417,6 +424,8 @@ public AwsProperties(Map<String, String> properties) {
this.glueCatalogId = properties.get(GLUE_CATALOG_ID);
this.glueCatalogSkipArchive = PropertyUtil.propertyAsBoolean(properties,
AwsProperties.GLUE_CATALOG_SKIP_ARCHIVE, AwsProperties.GLUE_CATALOG_SKIP_ARCHIVE_DEFAULT);
this.glueCatalogSkipNameValidation = PropertyUtil.propertyAsBoolean(properties,
AwsProperties.GLUE_CATALOG_SKIP_NAME_VALIDATION, AwsProperties.GLUE_CATALOG_SKIP_NAME_VALIDATION_DEFAULT);
this.glueLakeFormationEnabled = PropertyUtil.propertyAsBoolean(properties,
GLUE_LAKEFORMATION_ENABLED,
GLUE_LAKEFORMATION_ENABLED_DEFAULT);
Expand Down Expand Up @@ -512,11 +521,18 @@ public void setGlueCatalogId(String id) {
public boolean glueCatalogSkipArchive() {
return glueCatalogSkipArchive;
}

public void setGlueCatalogSkipArchive(boolean skipArchive) {
this.glueCatalogSkipArchive = skipArchive;
}

public boolean glueCatalogSkipNameValidation() {
return glueCatalogSkipNameValidation;
}

public void setGlueCatalogSkipNameValidation(boolean glueCatalogSkipNameValidation) {
this.glueCatalogSkipNameValidation = glueCatalogSkipNameValidation;
}

public boolean glueLakeFormationEnabled() {
return glueLakeFormationEnabled;
}
Expand Down
35 changes: 20 additions & 15 deletions aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ protected TableOperations newTableOps(TableIdentifier tableIdentifier) {
Map<String, String> tableSpecificCatalogProperties = ImmutableMap.<String, String>builder()
.putAll(catalogProperties)
.put(AwsProperties.LAKE_FORMATION_DB_NAME,
IcebergToGlueConverter.getDatabaseName(tableIdentifier))
IcebergToGlueConverter.getDatabaseName(tableIdentifier, awsProperties.glueCatalogSkipNameValidation()))
.put(AwsProperties.LAKE_FORMATION_TABLE_NAME,
Comment thread
xingfanx marked this conversation as resolved.
IcebergToGlueConverter.getTableName(tableIdentifier))
.build();
Expand All @@ -224,7 +224,7 @@ protected TableOperations newTableOps(TableIdentifier tableIdentifier) {
protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
// check if value is set in database
GetDatabaseResponse response = glue.getDatabase(GetDatabaseRequest.builder()
.name(IcebergToGlueConverter.getDatabaseName(tableIdentifier))
.name(IcebergToGlueConverter.getDatabaseName(tableIdentifier, awsProperties.glueCatalogSkipNameValidation()))
.build());
String dbLocationUri = response.database().locationUri();
if (dbLocationUri != null) {
Expand All @@ -234,7 +234,7 @@ protected String defaultWarehouseLocation(TableIdentifier tableIdentifier) {
return String.format(
"%s/%s.db/%s",
warehousePath,
IcebergToGlueConverter.getDatabaseName(tableIdentifier),
IcebergToGlueConverter.getDatabaseName(tableIdentifier, awsProperties.glueCatalogSkipNameValidation()),
tableIdentifier.name());
}

Expand All @@ -247,7 +247,7 @@ public List<TableIdentifier> listTables(Namespace namespace) {
do {
GetTablesResponse response = glue.getTables(GetTablesRequest.builder()
.catalogId(awsProperties.glueCatalogId())
.databaseName(IcebergToGlueConverter.toDatabaseName(namespace))
.databaseName(IcebergToGlueConverter.toDatabaseName(namespace, awsProperties.glueCatalogSkipNameValidation()))
.nextToken(nextToken)
.build());
nextToken = response.nextToken();
Expand Down Expand Up @@ -276,7 +276,8 @@ public boolean dropTable(TableIdentifier identifier, boolean purge) {
TableMetadata lastMetadata = ops.current();
glue.deleteTable(DeleteTableRequest.builder()
.catalogId(awsProperties.glueCatalogId())
.databaseName(IcebergToGlueConverter.getDatabaseName(identifier))
.databaseName(IcebergToGlueConverter.getDatabaseName(identifier,
Comment thread
jackye1995 marked this conversation as resolved.
Outdated
awsProperties.glueCatalogSkipNameValidation()))
.name(identifier.name())
.build());
LOG.info("Successfully dropped table {} from Glue", identifier);
Expand Down Expand Up @@ -309,9 +310,9 @@ public void renameTable(TableIdentifier from, TableIdentifier to) {
}
// keep metadata
Table fromTable = null;
String fromTableDbName = IcebergToGlueConverter.getDatabaseName(from);
String fromTableDbName = IcebergToGlueConverter.getDatabaseName(from, awsProperties.glueCatalogSkipNameValidation());
String fromTableName = IcebergToGlueConverter.getTableName(from);
String toTableDbName = IcebergToGlueConverter.getDatabaseName(to);
String toTableDbName = IcebergToGlueConverter.getDatabaseName(to, awsProperties.glueCatalogSkipNameValidation());
String toTableName = IcebergToGlueConverter.getTableName(to);
try {
GetTableResponse response = glue.getTable(GetTableRequest.builder()
Expand Down Expand Up @@ -359,7 +360,8 @@ public void createNamespace(Namespace namespace, Map<String, String> metadata) {
try {
glue.createDatabase(CreateDatabaseRequest.builder()
.catalogId(awsProperties.glueCatalogId())
.databaseInput(IcebergToGlueConverter.toDatabaseInput(namespace, metadata))
.databaseInput(IcebergToGlueConverter.toDatabaseInput(namespace, metadata,
awsProperties.glueCatalogSkipNameValidation()))
.build());
LOG.info("Created namespace: {}", namespace);
} catch (software.amazon.awssdk.services.glue.model.AlreadyExistsException e) {
Expand Down Expand Up @@ -400,7 +402,8 @@ public List<Namespace> listNamespaces(Namespace namespace) throws NoSuchNamespac

@Override
public Map<String, String> loadNamespaceMetadata(Namespace namespace) throws NoSuchNamespaceException {
String databaseName = IcebergToGlueConverter.toDatabaseName(namespace);
String databaseName = IcebergToGlueConverter.toDatabaseName(namespace,
awsProperties.glueCatalogSkipNameValidation());
try {
Database database = glue.getDatabase(GetDatabaseRequest.builder()
.catalogId(awsProperties.glueCatalogId())
Expand Down Expand Up @@ -434,7 +437,7 @@ public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyExcept

GetTablesResponse response = glue.getTables(GetTablesRequest.builder()
.catalogId(awsProperties.glueCatalogId())
.databaseName(IcebergToGlueConverter.toDatabaseName(namespace))
.databaseName(IcebergToGlueConverter.toDatabaseName(namespace, awsProperties.glueCatalogSkipNameValidation()))
.build());

if (response.hasTableList() && !response.tableList().isEmpty()) {
Expand All @@ -450,7 +453,7 @@ public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyExcept

glue.deleteDatabase(DeleteDatabaseRequest.builder()
.catalogId(awsProperties.glueCatalogId())
.name(IcebergToGlueConverter.toDatabaseName(namespace))
.name(IcebergToGlueConverter.toDatabaseName(namespace, awsProperties.glueCatalogSkipNameValidation()))
Comment thread
xingfanx marked this conversation as resolved.
.build());
LOG.info("Dropped namespace: {}", namespace);
// Always successful, otherwise exception is thrown
Expand All @@ -464,8 +467,9 @@ public boolean setProperties(Namespace namespace, Map<String, String> properties
newProperties.putAll(properties);
glue.updateDatabase(UpdateDatabaseRequest.builder()
.catalogId(awsProperties.glueCatalogId())
.name(IcebergToGlueConverter.toDatabaseName(namespace))
.databaseInput(IcebergToGlueConverter.toDatabaseInput(namespace, newProperties))
.name(IcebergToGlueConverter.toDatabaseName(namespace, awsProperties.glueCatalogSkipNameValidation()))
.databaseInput(IcebergToGlueConverter.toDatabaseInput(namespace, newProperties,
Comment thread
xingfanx marked this conversation as resolved.
Outdated
awsProperties.glueCatalogSkipNameValidation()))
.build());
LOG.debug("Successfully set properties {} for {}", properties.keySet(), namespace);
// Always successful, otherwise exception is thrown
Expand All @@ -481,8 +485,9 @@ public boolean removeProperties(Namespace namespace, Set<String> properties) thr

glue.updateDatabase(UpdateDatabaseRequest.builder()
.catalogId(awsProperties.glueCatalogId())
.name(IcebergToGlueConverter.toDatabaseName(namespace))
.databaseInput(IcebergToGlueConverter.toDatabaseInput(namespace, metadata))
.name(IcebergToGlueConverter.toDatabaseName(namespace, awsProperties.glueCatalogSkipNameValidation()))
.databaseInput(IcebergToGlueConverter.toDatabaseInput(namespace, metadata,
Comment thread
xingfanx marked this conversation as resolved.
Outdated
awsProperties.glueCatalogSkipNameValidation()))
.build());
LOG.debug("Successfully removed properties {} from {}", properties, namespace);
// Always successful, otherwise exception is thrown
Expand Down
3 changes: 2 additions & 1 deletion aws/src/main/java/org/apache/iceberg/aws/glue/GlueTableOperations.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class GlueTableOperations extends BaseMetastoreTableOperations {
FileIO fileIO, TableIdentifier tableIdentifier) {
this.glue = glue;
this.awsProperties = awsProperties;
this.databaseName = IcebergToGlueConverter.getDatabaseName(tableIdentifier);
this.databaseName = IcebergToGlueConverter.getDatabaseName(tableIdentifier,
Comment thread
xingfanx marked this conversation as resolved.
Outdated
awsProperties.glueCatalogSkipNameValidation());
this.tableName = IcebergToGlueConverter.getTableName(tableIdentifier);
this.fullTableName = String.format("%s.%s.%s", catalogName, databaseName, tableName);
this.commitLockEntityId = String.format("%s.%s", databaseName, tableName);
Expand Down
30 changes: 20 additions & 10 deletions aws/src/main/java/org/apache/iceberg/aws/glue/IcebergToGlueConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,31 +105,41 @@ static void validateNamespace(Namespace namespace) {

/**
* Validate and convert Iceberg namespace to Glue database name
* @param namespace Iceberg namespace
*
* @param namespace Iceberg namespace
* @param shouldSkipNameValidation should skip name validation
* @return database name
*/
static String toDatabaseName(Namespace namespace) {
validateNamespace(namespace);
static String toDatabaseName(Namespace namespace, boolean shouldSkipNameValidation) {
Comment thread
xingfanx marked this conversation as resolved.
Outdated
if (!shouldSkipNameValidation) {
validateNamespace(namespace);
}
Comment thread
xingfanx marked this conversation as resolved.
return namespace.level(0);
}

/**
* Validate and get Glue database name from Iceberg TableIdentifier
* @param tableIdentifier Iceberg table identifier
*
* @param tableIdentifier Iceberg table identifier
* @param shouldSkipNameValidation should skip name validation
* @return database name
*/
static String getDatabaseName(TableIdentifier tableIdentifier) {
return toDatabaseName(tableIdentifier.namespace());
static String getDatabaseName(TableIdentifier tableIdentifier, boolean shouldSkipNameValidation) {
return toDatabaseName(tableIdentifier.namespace(), shouldSkipNameValidation);
}

/**
* Validate and convert Iceberg name to Glue DatabaseInput
* @param namespace Iceberg namespace
* @param metadata metadata map
*
* @param namespace Iceberg namespace
* @param metadata metadata map
* @param shouldSkipNameValidation should skip name validation
* @return Glue DatabaseInput
*/
static DatabaseInput toDatabaseInput(Namespace namespace, Map<String, String> metadata) {
DatabaseInput.Builder builder = DatabaseInput.builder().name(toDatabaseName(namespace));
static DatabaseInput toDatabaseInput(Namespace namespace, Map<String, String> metadata,
Comment thread
xingfanx marked this conversation as resolved.
Outdated
boolean shouldSkipNameValidation) {
DatabaseInput.Builder builder = DatabaseInput.builder().name(toDatabaseName(namespace,
shouldSkipNameValidation));
Map<String, String> parameters = Maps.newHashMap();
metadata.forEach((k, v) -> {
if (GLUE_DB_DESCRIPTION_KEY.equals(k)) {
Expand Down
28 changes: 22 additions & 6 deletions aws/src/test/java/org/apache/iceberg/aws/glue/TestIcebergToGlueConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ public class TestIcebergToGlueConverter {

@Test
public void testToDatabaseName() {
Assert.assertEquals("db", IcebergToGlueConverter.toDatabaseName(Namespace.of("db")));
Assert.assertEquals("db", IcebergToGlueConverter.toDatabaseName(Namespace.of("db"),
false));
}

@Test
Expand All @@ -64,7 +65,18 @@ public void testToDatabaseNameFailure() {
AssertHelpers.assertThrows("bad namespace name",
ValidationException.class,
"Cannot convert namespace",
() -> IcebergToGlueConverter.toDatabaseName(name)
() -> IcebergToGlueConverter.toDatabaseName(name, false)
);
}
}

@Test
public void testSkipNamespaceValidation() {
List<Namespace> acceptableNames = Lists.newArrayList(
Namespace.of("db-1"),
Namespace.of("db-1-1-1"));
for (Namespace name : acceptableNames) {
Assert.assertEquals(name.toString(), IcebergToGlueConverter.toDatabaseName(name, true)
);
}
}
Expand All @@ -75,7 +87,8 @@ public void testToDatabaseInput() {
IcebergToGlueConverter.GLUE_DB_DESCRIPTION_KEY, "description",
IcebergToGlueConverter.GLUE_DB_LOCATION_KEY, "s3://location",
"key", "val");
DatabaseInput databaseInput = IcebergToGlueConverter.toDatabaseInput(Namespace.of("ns"), properties);
DatabaseInput databaseInput = IcebergToGlueConverter.toDatabaseInput(Namespace.of("ns"), properties,
false);
Assert.assertEquals("Location should be set", "s3://location", databaseInput.locationUri());
Assert.assertEquals("Description should be set", "description", databaseInput.description());
Assert.assertEquals("Parameters should be set", ImmutableMap.of("key", "val"), databaseInput.parameters());
Expand All @@ -89,15 +102,17 @@ public void testToDatabaseInputNoParameter() {
.parameters(ImmutableMap.of())
.build();
Namespace namespace = Namespace.of("db");
Assert.assertEquals(input, IcebergToGlueConverter.toDatabaseInput(namespace, ImmutableMap.of()));
Assert.assertEquals(input, IcebergToGlueConverter.toDatabaseInput(namespace, ImmutableMap.of(),
false));
}

@Test
public void testToDatabaseInputEmptyLocation() {
Map<String, String> properties = ImmutableMap.of(
IcebergToGlueConverter.GLUE_DB_DESCRIPTION_KEY, "description",
"key", "val");
DatabaseInput databaseInput = IcebergToGlueConverter.toDatabaseInput(Namespace.of("ns"), properties);
DatabaseInput databaseInput = IcebergToGlueConverter.toDatabaseInput(Namespace.of("ns"), properties,
false);
Assert.assertNull("Location should not be set", databaseInput.locationUri());
Assert.assertEquals("Description should be set", "description", databaseInput.description());
Assert.assertEquals("Parameters should be set", ImmutableMap.of("key", "val"), databaseInput.parameters());
Expand All @@ -109,7 +124,8 @@ public void testToDatabaseInputEmptyDescription() {
Map<String, String> properties = ImmutableMap.of(
IcebergToGlueConverter.GLUE_DB_LOCATION_KEY, "s3://location",
"key", "val");
DatabaseInput databaseInput = IcebergToGlueConverter.toDatabaseInput(Namespace.of("ns"), properties);
DatabaseInput databaseInput = IcebergToGlueConverter.toDatabaseInput(Namespace.of("ns"), properties,
false);
Assert.assertEquals("Location should be set", "s3://location", databaseInput.locationUri());
Assert.assertNull("Description should not be set", databaseInput.description());
Assert.assertEquals("Parameters should be set", ImmutableMap.of("key", "val"), databaseInput.parameters());
Expand Down