Added feature to fail rename or delete of a table if it is tagged not…

… to be renamed or deleted. (#389)

metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/Config.java

@@ -450,5 +450,19 @@ public interface Config {
  * @return True if it is.
  */
  boolean isTableAliasEnabled();
+
+ /**
+ * Set of tags that disable table delete.
+ *
+ * @return set of tags
+ */
+ Set<String> getNoTableDeleteOnTags();
+
+ /**
+ * Set of tags that disable table rename.
+ *
+ * @return set of tags
+ */
+ Set<String> getNoTableRenameOnTags();
 }
 

metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/DefaultConfigImpl.java

@@ -531,4 +531,14 @@ public String getIcebergPartitionUriScheme() {
  public boolean isTableAliasEnabled() {
  return this.metacatProperties.getAliasServiceProperties().isEnabled();
  }
+
+ @Override
+ public Set<String> getNoTableDeleteOnTags() {
+ return this.metacatProperties.getTable().getDelete().getNoDeleteOnTagsSet();
+ }
+
+ @Override
+ public Set<String> getNoTableRenameOnTags() {
+ return this.metacatProperties.getTable().getRename().getNoRenameOnTagsSet();
+ }
 }

metacat-common-server/src/main/java/com/netflix/metacat/common/server/properties/Table.java

@@ -17,7 +17,13 @@
  */
 package com.netflix.metacat.common.server.properties;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
+import com.google.common.base.Splitter;
 import lombok.NonNull;
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.HashSet;
+import java.util.Set;
 
 /**
  * Table related properties.
@@ -30,6 +36,8 @@ public class Table {
 
  @NonNull
  private Delete delete = new Delete();
+ @NonNull
+ private Rename rename = new Rename();
 
  /**
  * Delete related properties.
@@ -42,6 +50,27 @@ public static class Delete {
 
  @NonNull
  private Cascade cascade = new Cascade();
+ private String noDeleteOnTags;
+ private Set<String> noDeleteOnTagsSet;
+
+ /**
+ * Get the tags that disable table deletes.
+ *
+ * @return Set of tags
+ */
+ @JsonIgnore
+ public Set<String> getNoDeleteOnTagsSet() {
+ if (noDeleteOnTagsSet == null) {
+ if (StringUtils.isNotBlank(noDeleteOnTags)) {
+ noDeleteOnTagsSet = new HashSet<>(Splitter.on(',')
+ .omitEmptyStrings()
+ .splitToList(noDeleteOnTags));
+ } else {
+ noDeleteOnTagsSet = new HashSet<>();
+ }
+ }
+ return noDeleteOnTagsSet;
+ }
 
  /**
  * Cascade related properties.
@@ -67,4 +96,36 @@ public static class Views {
  }
  }
  }
+
+ /**
+ * Rename related properties.
+ *
+ * @author amajumdar
+ * @since 1.3.0
+ */
+ @lombok.Data
+ public static class Rename {
+
+ private String noRenameOnTags;
+ private Set<String> noRenameOnTagsSet;
+
+ /**
+ * Get the tags that disable table renames.
+ *
+ * @return Set of tags
+ */
+ @JsonIgnore
+ public Set<String> getNoRenameOnTagsSet() {
+ if (noRenameOnTagsSet == null) {
+ if (StringUtils.isNotBlank(noRenameOnTags)) {
+ noRenameOnTagsSet = new HashSet<>(Splitter.on(',')
+ .omitEmptyStrings()
+ .splitToList(noRenameOnTags));
+ } else {
+ noRenameOnTagsSet = new HashSet<>();
+ }
+ }
+ return noRenameOnTagsSet;
+ }
+ }
 }

metacat-functional-tests/metacat-test-cluster/docker-compose.yml

@@ -64,7 +64,9 @@ services:
  -Dmetacat.authorization.enabled=true
  -Dmetacat.authorization.createAcl.createAclStr=embedded-fast-hive-metastore/fsmoke_acl:metacat-prod
  -Dmetacat.authorization.deleteAcl.deleteAclStr=embedded-fast-hive-metastore/fsmoke_acl:metacat-prod
- -Dmetacat.service.tables.error.list.partitions.threshold=100'
+ -Dmetacat.service.tables.error.list.partitions.threshold=100
+ -Dmetacat.table.delete.noDeleteOnTags=do_not_drop
+ -Dmetacat.table.rename.noRenameOnTags=do_not_rename'
  labels:
  - "com.netflix.metacat.oss.test"
  - "com.netflix.metacat.oss.test.war"

metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatSmokeSpec.groovy

@@ -764,8 +764,13 @@ class MetacatSmokeSpec extends Specification {
  given:
  def tableName = 'test_create_table_rename'
  def tableName1 = 'test_create_table_rename1'
+ def tableNameTagNoDelete = 'test_create_table_tag_no_delete'
+ def tableNameTagNoRename = 'test_create_table_tag_no_rename'
+ def tableNameTagNoRenamed = 'test_create_table_tag_no_renamed'
  createTable(catalogName, databaseName, tableName, 'TRUE')
  createTable(catalogName, databaseName, tableName1, 'TRUE')
+ createTable(catalogName, databaseName, tableNameTagNoDelete, 'TRUE')
+ createTable(catalogName, databaseName, tableNameTagNoRename, 'TRUE')
  api.deleteTable(catalogName, databaseName, tableName)
  when:
  api.renameTable(catalogName, databaseName, tableName1, tableName)
@@ -788,8 +793,30 @@ class MetacatSmokeSpec extends Specification {
  def table = api.getTable(catalogName, databaseName, tableName, true, true, false)
  table.getMetadata() != null && table.getMetadata().get('EXTERNAL').equalsIgnoreCase('TRUE')
  table.getDefinitionMetadata() != null
+ when:
+ tagApi.setTableTags(catalogName, databaseName, tableNameTagNoDelete, ['do_not_drop'] as Set)
+ api.deleteTable(catalogName, databaseName, tableNameTagNoDelete)
+ then:
+ thrown(MetacatBadRequestException)
+ when:
+ tagApi.removeTableTags(catalogName, databaseName, tableNameTagNoDelete, true, [] as Set)
+ api.deleteTable(catalogName, databaseName, tableNameTagNoDelete)
+ then:
+ noExceptionThrown()
+ when:
+ tagApi.setTableTags(catalogName, databaseName, tableNameTagNoRename, ['do_not_drop','do_not_rename'] as Set)
+ api.renameTable(catalogName, databaseName, tableNameTagNoRename, tableNameTagNoRenamed)
+ then:
+ thrown(MetacatBadRequestException)
+ when:
+ tagApi.removeTableTags(catalogName, databaseName, tableNameTagNoRename, false, ['do_not_rename'] as Set)
+ api.renameTable(catalogName, databaseName, tableNameTagNoRename, tableNameTagNoRenamed)
+ tagApi.removeTableTags(catalogName, databaseName, tableNameTagNoRenamed, true, [] as Set)
+ then:
+ noExceptionThrown()
  cleanup:
  api.deleteTable(catalogName, databaseName, tableName1)
+ api.deleteTable(catalogName, databaseName, tableNameTagNoRenamed)
  where:
  catalogName | databaseName
  'embedded-fast-hive-metastore' | 'hsmoke_db3'

metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java

@@ -57,6 +57,7 @@
 import com.netflix.spectator.api.Registry;
 import lombok.extern.slf4j.Slf4j;
 
+import javax.annotation.Nullable;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -175,17 +176,24 @@ private void setOwnerIfNull(final TableDto tableDto, final String user) {
  }
 
  private void tag(final QualifiedName name, final ObjectNode definitionMetadata) {
+ final Set<String> tags = getTableTags(definitionMetadata);
+ if (!tags.isEmpty()) {
+ log.info("Setting tags {} for table {}", tags, name);
+ final Set<String> result = tagService.setTags(name, tags, false);
+ }
+ }
+
+ private Set<String> getTableTags(@Nullable final ObjectNode definitionMetadata) {
+ final Set<String> tags = Sets.newHashSet();
  if (definitionMetadata != null && definitionMetadata.get(NAME_TAGS) != null) {
  final JsonNode tagsNode = definitionMetadata.get(NAME_TAGS);
- final Set<String> tags = Sets.newHashSet();
  if (tagsNode.isArray() && tagsNode.size() > 0) {
  for (JsonNode tagNode : tagsNode) {
  tags.add(tagNode.textValue());
  }
- log.info("Setting tags {} for table {}", tags, name);
- final Set<String> result = tagService.setTags(name, tags, false);
  }
  }
+ return tags;
  }
 
  /**
@@ -216,6 +224,13 @@ public TableDto deleteAndReturn(final QualifiedName name, final boolean isMView)
  handleException(name, true, "deleteAndReturn_get", e);
  }
 
+ // Fail if the table is tagged not to be deleted.
+ if (hasTags(tableDto, config.getNoTableDeleteOnTags())) {
+ throw new IllegalArgumentException(
+ String.format("Table %s cannot be deleted because it is tagged with %s.", name,
+ config.getNoTableDeleteOnTags()));
+ }
+
  // Try to delete the table even if get above fails
  try {
  connectorTableServiceProxy.delete(name);
@@ -239,6 +254,20 @@ public TableDto deleteAndReturn(final QualifiedName name, final boolean isMView)
  return tableDto;
  }
 
+ private boolean hasTags(@Nullable final TableDto tableDto, final Set<String> hasTags) {
+ if (!hasTags.isEmpty() && tableDto != null) {
+ final Set<String> tags = getTableTags(tableDto.getDefinitionMetadata());
+ if (!tags.isEmpty()) {
+ for (String t: hasTags) {
+ if (tags.contains(t)) {
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+ }
+
  /**
  * Returns true
  * 1. If the system is configured to delete deifnition metadata.
@@ -342,6 +371,14 @@ public void rename(
  .includeDefinitionMetadata(true)
  .includeDataMetadata(true)
  .build()).orElseThrow(() -> new TableNotFoundException(oldName));
+
+ // Fail if the table is tagged not to be renamed.
+ if (hasTags(oldTable, config.getNoTableRenameOnTags())) {
+ throw new IllegalArgumentException(
+ String.format("Table %s cannot be renamed because it is tagged with %s.", oldName,
+ config.getNoTableRenameOnTags()));
+ }
+
  if (oldTable != null) {
  //Ignore if the operation is not supported, so that we can at least go ahead and save the user metadata
  eventBus.post(new MetacatRenameTablePreEvent(oldName, metacatRequestContext, this, newName));

metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy

@@ -155,13 +155,15 @@ class TableServiceImplSpec extends Specification {
  when:
  service.deleteAndReturn(name, false)
  then:
+ 1 * config.getNoTableDeleteOnTags() >> []
  1 * config.canDeleteTableDefinitionMetadata() >> true
  1 * usermetadataService.deleteMetadata(_,_)
  0 * usermetadataService.softDeleteDataMetadata(_,_)
  noExceptionThrown()
  when:
  service.deleteAndReturn(name, false)
  then:
+ 1 * config.getNoTableDeleteOnTags() >> []
  1 * config.canDeleteTableDefinitionMetadata() >> false
  1 * config.getNamesEnabledForDefinitionMetadataDelete() >> [QualifiedName.fromString('a')]
  1 * usermetadataService.deleteMetadata(_,_)
@@ -170,6 +172,7 @@ class TableServiceImplSpec extends Specification {
  when:
  service.deleteAndReturn(name, false)
  then:
+ 1 * config.getNoTableDeleteOnTags() >> []
  1 * config.canDeleteTableDefinitionMetadata() >> false
  1 * config.getNamesEnabledForDefinitionMetadataDelete() >> [QualifiedName.fromString('a/b')]
  1 * usermetadataService.deleteMetadata(_,_)
@@ -178,6 +181,7 @@ class TableServiceImplSpec extends Specification {
  when:
  service.deleteAndReturn(name, false)
  then:
+ 1 * config.getNoTableDeleteOnTags() >> []
  1 * config.canDeleteTableDefinitionMetadata() >> false
  1 * config.getNamesEnabledForDefinitionMetadataDelete() >> [QualifiedName.fromString('a/b/c')]
  1 * usermetadataService.deleteMetadata(_,_)
@@ -186,6 +190,7 @@ class TableServiceImplSpec extends Specification {
  when:
  service.deleteAndReturn(name, false)
  then:
+ 1 * config.getNoTableDeleteOnTags() >> []
  1 * config.canDeleteTableDefinitionMetadata() >> false
  1 * config.getNamesEnabledForDefinitionMetadataDelete() >> []
  1 * config.canSoftDeleteDataMetadata() >> true
@@ -195,6 +200,7 @@ class TableServiceImplSpec extends Specification {
  when:
  service.deleteAndReturn(name, false)
  then:
+ 1 * config.getNoTableDeleteOnTags() >> []
  1 * config.canDeleteTableDefinitionMetadata() >> false
  1 * config.getNamesEnabledForDefinitionMetadataDelete() >> [QualifiedName.fromString('a/c')]
  1 * config.canSoftDeleteDataMetadata() >> true
@@ -204,6 +210,7 @@ class TableServiceImplSpec extends Specification {
  when:
  service.deleteAndReturn(name, false)
  then:
+ 1 * config.getNoTableDeleteOnTags() >> []
  1 * connectorTableService.get(_, _) >> { throw new InvalidMetadataException(name) }
  1 * config.canDeleteTableDefinitionMetadata() >> true
  1 * usermetadataService.deleteMetadata(_,_)