Merge branch 'abozigian/secure-metadata-rq' into abozigian/1-3-1-rc-7

Netflix · Apr 8, 2024 · 02d0570 · 02d0570
2 parents a7530cd + a0a4385
commit 02d0570
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 31 deletions.
diff --git a/build.gradle b/build.gradle
@@ -146,7 +146,7 @@ configure(javaProjects) {
             dependency("org.codehaus.gpars:gpars:1.2.1")
             /**es 5.4.1 dependencies*/
             dependency("org.elasticsearch.client:transport:5.4.1")
-            dependency("net.snowflake:snowflake-jdbc:3.4.2")
+            dependency("net.snowflake:snowflake-jdbc:3.15.0")
             dependency("com.esotericsoftware.kryo:kryo:2.22")
             dependency("org.apache.iceberg:iceberg-spark-runtime:${iceberg_version}")
             dependency("com.datastax.cassandra:cassandra-driver-core:3.7.2")

diff --git a/metacat-client/src/main/java/com/netflix/metacat/client/api/MetacatV1.java b/metacat-client/src/main/java/com/netflix/metacat/client/api/MetacatV1.java
@@ -557,10 +557,11 @@ TableDto updateMView(
     /**
      * Update table.
      *
-     * @param catalogName  catalog name
-     * @param databaseName database name
-     * @param tableName    table name
-     * @param table        table
+     * @param catalogName           catalog name
+     * @param databaseName          database name
+     * @param tableName             table name
+     * @param includeSecureMetadata consider updates to secure metadata if true
+     * @param table                 table
      * @return table
      */
     @PUT
@@ -574,6 +575,9 @@ TableDto updateTable(
             String databaseName,
         @PathParam("table-name")
             String tableName,
+        @DefaultValue("false")
+        @QueryParam("includeSecureMetadata")
+        Boolean includeSecureMetadata,
         TableDto table
     );
 }
diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/api/v1/MetacatV1.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/api/v1/MetacatV1.java
@@ -168,16 +168,18 @@ void renameTable(
     /**
      * Update table.
      *
-     * @param catalogName  catalog name
-     * @param databaseName database name
-     * @param tableName    table name
-     * @param table        table
+     * @param catalogName           catalog name
+     * @param databaseName          database name
+     * @param tableName             table name
+     * @param includeSecureMetadata consider updates to secure metadata if true
+     * @param table                 table
      * @return table
      */
     TableDto updateTable(
         final String catalogName,
         final String databaseName,
         final String tableName,
+        final boolean includeSecureMetadata,
         final TableDto table
     );
 

diff --git a/...nctional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatFunctionalSpec.groovy b/...nctional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatFunctionalSpec.groovy
@@ -680,7 +680,7 @@ class MetacatFunctionalSpec extends Specification {
         table.serde.uri = updatedDataUri
 
         and:
-        api.updateTable(catalog.name, databaseName, tableName, table)
+        api.updateTable(catalog.name, databaseName, tableName, false, table)
         table = api.getTable(catalog.name, databaseName, tableName, true, true, true)
 
         then: 'saving should merge or insert metadata'
@@ -696,7 +696,7 @@ class MetacatFunctionalSpec extends Specification {
         table.serde.uri = origDataUri
         table.definitionMetadata = null
         table.dataMetadata = null
-        api.updateTable(catalog.name, databaseName, tableName, table)
+        api.updateTable(catalog.name, databaseName, tableName, false, table)
         table = api.getTable(catalog.name, databaseName, tableName, true, true, true)
 
         then: 'the old data metadata should be back'

diff --git a/...at-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatSmokeSpec.groovy b/...at-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatSmokeSpec.groovy
@@ -242,7 +242,7 @@ class MetacatSmokeSpec extends Specification {
             }
             api.createTable(catalogName, databaseName, tableName, tableDto)
             tableDto.getSerde().setUri(updatedUri)
-            api.updateTable(catalogName, databaseName, tableName, tableDto)
+            api.updateTable(catalogName, databaseName, tableName, false, tableDto)
             error == null
         } catch (Exception e) {
             e.class == error
@@ -516,7 +516,7 @@ class MetacatSmokeSpec extends Specification {
         when:
         // Updating a non-iceberg table with iceberg metadata should fail
         createTable(catalogName, databaseName, tableName)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         thrown(MetacatBadRequestException)
         when:
@@ -527,22 +527,22 @@ class MetacatSmokeSpec extends Specification {
         def updatedUri = tableDto.getDataUri() + 'updated'
         tableDto.getMetadata().putAll(metadata1)
         tableDto.getSerde().setUri(updatedUri)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         noExceptionThrown()
         when:
         tagApi.setTableTags(catalogName, databaseName, tableName, ['do_not_drop','iceberg_migration_do_not_modify'] as Set)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         thrown(RetryableException)
         when:
         tagApi.setTableTags(catalogName, databaseName, tableName, ['do_not_drop'] as Set)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         noExceptionThrown()
         when:
         tagApi.removeTableTags(catalogName, databaseName, tableName, true, [] as Set)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         noExceptionThrown()
         when:
@@ -571,7 +571,7 @@ class MetacatSmokeSpec extends Specification {
         def metadataLocation2 = '/tmp/data/metadata/00002-2d6c1951-31d5-4bea-8edd-e35746b172f3.metadata.json'
         def metadata2 = [table_type: 'ICEBERG', metadata_location: metadataLocation2, previous_metadata_location: metadataLocation1, 'partition_spec': 'invalid']
         tableDto.getMetadata().putAll(metadata2)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         updatedTable = api.getTable(catalogName, databaseName, tableName, true, false, false)
         then:
         noExceptionThrown()
@@ -580,7 +580,7 @@ class MetacatSmokeSpec extends Specification {
         !updatedTable.getMetadata().containsKey('metadata_content')
         updatedTable.getFields().get(1).getSource_type().equals('string')
         when:
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         def tableWithDetails = api.getTable(catalogName, databaseName, tableName, true, false, false, true)
         then:
         tableWithDetails.getMetadata().get('metadata_content') != null
@@ -589,7 +589,7 @@ class MetacatSmokeSpec extends Specification {
         api.renameTable(catalogName, databaseName, tableName, renamedTableName)
         def t = api.getTable(catalogName, databaseName, renamedTableName, true, false, false)
         t.getMetadata().put('previous_metadata_location', metadataLocation1)
-        api.updateTable(catalogName, databaseName, renamedTableName, t)
+        api.updateTable(catalogName, databaseName, renamedTableName, false, t)
         then:
         noExceptionThrown()
         api.getTable(catalogName, databaseName, renamedTableName, true, false, false) != null
@@ -600,20 +600,20 @@ class MetacatSmokeSpec extends Specification {
         api.getTable(catalogName, databaseName, tableName, true, false, false) != null
         when:
         tableDto.getMetadata().putAll(metadata1)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         thrown(MetacatPreconditionFailedException)
         when:
         metadata1.put('previous_metadata_location', '/tmp/data/metadata/filenotfound')
         tableDto.getMetadata().putAll(metadata1)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         thrown(MetacatBadRequestException)
         when:
         // Failure to get table after a successful update shouldn't fail
         def updatedInvalidMetadata = [table_type: 'ICEBERG', metadata_location: icebergMetadataLocation, previous_metadata_location: updatedTable.getMetadata().get('metadata_location')]
         tableDto.getMetadata().putAll(updatedInvalidMetadata)
-        api.updateTable(catalogName, databaseName, tableName, tableDto)
+        api.updateTable(catalogName, databaseName, tableName, false, tableDto)
         then:
         noExceptionThrown()
         when:
@@ -1268,7 +1268,7 @@ class MetacatSmokeSpec extends Specification {
         then:
         thrown(MetacatNotFoundException)
         when:
-        api.updateTable('invalid', 'invalid', 'invalid', new TableDto(name: QualifiedName.ofTable('invalid', 'invalid', 'invalid')))
+        api.updateTable('invalid', 'invalid', 'invalid', false, new TableDto(name: QualifiedName.ofTable('invalid', 'invalid', 'invalid')))
         then:
         thrown(MetacatNotFoundException)
         when:
@@ -1320,7 +1320,7 @@ class MetacatSmokeSpec extends Specification {
         then:
         thrown(MetacatNotFoundException)
         when:
-        api.updateTable(catalogName, 'invalid', 'invalid', new TableDto(name: QualifiedName.ofTable('invalid', 'invalid', 'invalid')))
+        api.updateTable(catalogName, 'invalid', 'invalid', false, new TableDto(name: QualifiedName.ofTable('invalid', 'invalid', 'invalid')))
         then:
         thrown(MetacatNotFoundException)
         when:
@@ -1365,7 +1365,7 @@ class MetacatSmokeSpec extends Specification {
         then:
         thrown(MetacatNotFoundException)
         when:
-        api.updateTable(catalogName, 'invalid', 'invalid', new TableDto(name: QualifiedName.ofTable('invalid', 'invalid', 'invalid')))
+        api.updateTable(catalogName, 'invalid', 'invalid', false, new TableDto(name: QualifiedName.ofTable('invalid', 'invalid', 'invalid')))
         then:
         thrown(MetacatNotFoundException)
         when:

diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/MetacatController.java b/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/MetacatController.java
@@ -1234,6 +1234,8 @@ public TableDto updateTable(
         @PathVariable("database-name") final String databaseName,
         @ApiParam(value = "The name of the table", required = true)
         @PathVariable("table-name") final String tableName,
+        @ApiParam(value = "To consider updates to secure metadata if true", required = false)
+        @RequestParam(name = "includeSecureMetadata", defaultValue = "false") final boolean includeSecureMetadata,
         @ApiParam(value = "The table information", required = true)
         @RequestBody final TableDto table
     ) {

diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/PartitionController.java b/metacat-main/src/main/java/com/netflix/metacat/main/api/v1/PartitionController.java
@@ -1087,7 +1087,7 @@ public PartitionsSaveResponseDto savePartitions(
                         dto.setName(name);
                         dto.setDefinitionMetadata(partitionsSaveRequestDto.getDefinitionMetadata());
                         dto.setDataMetadata(partitionsSaveRequestDto.getDataMetadata());
-                        this.v1.updateTable(catalogName, databaseName, tableName, dto);
+                        this.v1.updateTable(catalogName, databaseName, tableName, false, dto);
                     }
                 }
                 return result;

diff --git a/metacat-thrift/src/main/java/com/netflix/metacat/thrift/CatalogThriftHiveMetastore.java b/metacat-thrift/src/main/java/com/netflix/metacat/thrift/CatalogThriftHiveMetastore.java
@@ -441,7 +441,7 @@ public void alter_table_with_environment_context(
                     v1.renameTable(catalogName, oldName.getDatabaseName(), oldName.getTableName(),
                         newName.getTableName());
                 }
-                v1.updateTable(catalogName, dbname, newName.getTableName(), dto);
+                v1.updateTable(catalogName, dbname, newName.getTableName(), false, dto);
                 return null;
             });
     }

diff --git a/...t-thrift/src/test/groovy/com/netflix/metacat/thrift/CatalogThriftHiveMetastoreSpec.groovy b/...t-thrift/src/test/groovy/com/netflix/metacat/thrift/CatalogThriftHiveMetastoreSpec.groovy
@@ -626,7 +626,7 @@ class CatalogThriftHiveMetastoreSpec extends Specification {
         } else {
             0 * metacatV1.renameTable(_, _, _, _)
         }
-        1 * metacatV1.updateTable(_, _, _, _)
+        1 * metacatV1.updateTable(_, _, _, _, _)
         registry.clock() >> clock
         registry.timer(_) >> timer
         timer.record(_, _) >> {}
@@ -659,7 +659,7 @@ class CatalogThriftHiveMetastoreSpec extends Specification {
         } else {
             0 * metacatV1.renameTable(_, _, _, _)
         }
-        1 * metacatV1.updateTable(_, _, _, _)
+        1 * metacatV1.updateTable(_, _, _, _, _)
         registry.clock() >> clock
         registry.timer(_) >> timer
         timer.record(_, _) >> {}
@@ -692,7 +692,7 @@ class CatalogThriftHiveMetastoreSpec extends Specification {
         } else {
             0 * metacatV1.renameTable(_, _, _, _)
         }
-        1 * metacatV1.updateTable(_, _, _, _)
+        1 * metacatV1.updateTable(_, _, _, _, _)
         registry.clock() >> clock
         registry.timer(_) >> timer
         timer.record(_, _) >> {}