This is an automated email from the ASF dual-hosted git repository. abukor pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 4bed2723490135dc1afa2a609f9d56fc6e82098a Author: Attila Bukor <[email protected]> AuthorDate: Sat Jul 25 22:11:36 2020 +0200 KUDU-3090 Allow restoring tables without owner fad779cf7 introduced backing up and restoring table ownership as part of the backup tool. 6f0e858ba restricted changing ownership to users with ALL + delegate admin privileges which is higher than what we required for running restore. To support running the restore job without having to grant additional privileges, this commit adds a new "restoreOwner" flag to the restore job which is true by default, but if set to false, the job creates tables as the logged in user which still requires the same privileges as before. Change-Id: I22d0b78858f60fafd3c116c7e4b3371839440d6c Reviewed-on: http://gerrit.cloudera.org:8080/16239 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> Reviewed-by: Grant Henke <[email protected]> --- .../scala/org/apache/kudu/backup/TableMetadata.scala | 8 ++++++-- .../scala/org/apache/kudu/backup/KuduRestore.scala | 9 +++++++-- .../main/scala/org/apache/kudu/backup/Options.scala | 12 +++++++++++- .../scala/org/apache/kudu/backup/TestKuduBackup.scala | 19 +++++++++++++++++-- .../scala/org/apache/kudu/backup/TestOptions.scala | 1 + .../org/apache/kudu/spark/kudu/KuduTestSuite.scala | 2 ++ 6 files changed, 44 insertions(+), 7 deletions(-) diff --git a/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala b/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala index 99b8769..21e2349 100644 --- a/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala +++ b/java/kudu-backup-common/src/main/scala/org/apache/kudu/backup/TableMetadata.scala @@ -327,10 +327,14 @@ object TableMetadata { } } - def getCreateTableOptionsWithoutRangePartitions(metadata: TableMetadataPB): CreateTableOptions = { + def getCreateTableOptionsWithoutRangePartitions( + metadata: TableMetadataPB, + restoreOwner: Boolean): CreateTableOptions = { val schema = getKuduSchema(metadata) val options = new CreateTableOptions() - options.setOwner(metadata.getTableOwner) + if (restoreOwner) { + options.setOwner(metadata.getTableOwner) + } options.setNumReplicas(metadata.getNumReplicas) metadata.getPartitions.getHashPartitionsList.asScala.foreach { hp => options diff --git a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala index b2e8b6e..4aad8e2 100644 --- a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala +++ b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/KuduRestore.scala @@ -75,7 +75,11 @@ object KuduRestore { log.info(s"Creating restore table $restoreName") // We use the last schema in the restore path when creating the table to // ensure the table is created in its final state. - createTableRangePartitionByRangePartition(restoreName, lastMetadata, context) + createTableRangePartitionByRangePartition( + restoreName, + lastMetadata, + options.restoreOwner, + context) } } val backupSchema = BackupUtils.dataSchema(TableMetadata.getKuduSchema(metadata)) @@ -214,10 +218,11 @@ object KuduRestore { private def createTableRangePartitionByRangePartition( restoreName: String, metadata: TableMetadataPB, + restoreOwner: Boolean, context: KuduContext): Unit = { // Create the table with the first range partition (or none if there are none). val schema = TableMetadata.getKuduSchema(metadata) - val options = TableMetadata.getCreateTableOptionsWithoutRangePartitions(metadata) + val options = TableMetadata.getCreateTableOptionsWithoutRangePartitions(metadata, restoreOwner) val bounds = TableMetadata.getRangeBoundPartialRows(metadata) bounds.headOption.foreach(bound => { val (lower, upper) = bound diff --git a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala index 8edab5d..2c9762a 100644 --- a/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala +++ b/java/kudu-backup/src/main/scala/org/apache/kudu/backup/Options.scala @@ -190,12 +190,14 @@ case class RestoreOptions( createTables: Boolean = RestoreOptions.DefaultCreateTables, timestampMs: Long = System.currentTimeMillis(), failOnFirstError: Boolean = RestoreOptions.DefaultFailOnFirstError, - numParallelRestores: Int = RestoreOptions.DefaultNumParallelRestores) + numParallelRestores: Int = RestoreOptions.DefaultNumParallelRestores, + restoreOwner: Boolean = RestoreOptions.DefaultRestoreOwner) object RestoreOptions { val DefaultCreateTables: Boolean = true val DefaultFailOnFirstError = false val DefaultNumParallelRestores = 1 + val DefaultRestoreOwner: Boolean = true val ClassName: String = KuduRestore.getClass.getCanonicalName.dropRight(1) // Remove trailing `$` val ProgramName: String = "spark-submit --class " + ClassName + " [spark-options] " + @@ -246,6 +248,14 @@ object RestoreOptions { .hidden() .optional() + opt[Boolean]("restoreOwner") + .action((v, o) => o.copy(restoreOwner = v)) + .text( + "If true, it restores table ownership when creating new tables, otherwise creates " + + "tables as the logged in user. Only used when createTables is true. Default: " + + DefaultRestoreOwner) + .optional() + help("help").text("prints this usage text") arg[String]("<table>...") diff --git a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala index 9fb9265..fec45e8 100644 --- a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala +++ b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestKuduBackup.scala @@ -27,6 +27,7 @@ import org.apache.kudu.client._ import org.apache.kudu.ColumnSchema import org.apache.kudu.Schema import org.apache.kudu.Type +import org.apache.kudu.client import org.apache.kudu.ColumnSchema.ColumnSchemaBuilder import org.apache.kudu.spark.kudu.SparkListenerUtil.withJobDescriptionCollector import org.apache.kudu.spark.kudu.SparkListenerUtil.withJobTaskCounter @@ -412,6 +413,16 @@ class TestKuduBackup extends KuduTestSuite { } @Test + def testBackupAndRestoreNoRestoreOwner(): Unit = { + val rowCount = 100 + insertRows(table, rowCount) + + backupAndValidateTable(tableName, rowCount, false) + assertTrue(runRestore(createRestoreOptions(Seq(tableName)).copy(restoreOwner = false))) + validateTablesMatch(tableName, s"$tableName-restore", false) + } + + @Test def testColumnAlterHandling(): Unit = { // Create a basic table. val tableName = "testColumnAlterHandling" @@ -744,10 +755,14 @@ class TestKuduBackup extends KuduTestSuite { KuduRestore.run(options, ss) } - def validateTablesMatch(tableA: String, tableB: String): Unit = { + def validateTablesMatch(tableA: String, tableB: String, ownersMatch: Boolean = true): Unit = { val tA = kuduClient.openTable(tableA) val tB = kuduClient.openTable(tableB) - assertEquals(tA.getOwner, tB.getOwner) + if (ownersMatch) { + assertEquals(tA.getOwner, tB.getOwner) + } else { + assertNotEquals(tA.getOwner, tB.getOwner) + } assertNotEquals("", tA.getOwner); assertEquals(tA.getNumReplicas, tB.getNumReplicas) assertTrue(schemasMatch(tA.getSchema, tB.getSchema)) diff --git a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestOptions.scala b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestOptions.scala index a844f39..41e7f66 100644 --- a/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestOptions.scala +++ b/java/kudu-backup/src/test/scala/org/apache/kudu/backup/TestOptions.scala @@ -56,6 +56,7 @@ class TestOptions extends KuduTestSuite { | --tableSuffix <value> If set, the suffix to add to the restored table names. Only used when createTables is true. | --timestampMs <value> A UNIX timestamp in milliseconds that defines the latest time to use when selecting restore candidates. Default: `System.currentTimeMillis()` | --failOnFirstError Whether to fail the restore job as soon as a single table restore fails. Default: false + | --restoreOwner <value> If true, it restores table ownership when creating new tables, otherwise creates tables as the logged in user. Only used when createTables is true. Default: true | --help prints this usage text | <table>... A list of tables to be restored.""".stripMargin assertEquals(expectedStr, RestoreOptions.parser.renderTwoColumnsUsage) diff --git a/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala b/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala index d775977..b502bc9 100644 --- a/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala +++ b/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala @@ -52,6 +52,7 @@ trait KuduTestSuite extends JUnitSuite { var kuduContext: KuduContext = _ val tableName: String = "test" + val owner: String = "testuser" val simpleTableName: String = "simple-test" lazy val schema: Schema = { @@ -115,6 +116,7 @@ trait KuduTestSuite extends JUnitSuite { .setRangePartitionColumns(List("key").asJava) .addRangePartition(bottom, middle) .addRangePartition(middle, top) + .setOwner(owner) .setNumReplicas(1) }
