You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ab...@apache.org on 2020/07/29 11:59:35 UTC
[kudu] 01/02: KUDU-3090 Allow restoring tables without owner
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 <ab...@apache.org>
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 <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
.../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)
}