[SPARK-30797][SQL] Set tradition user/group/other permission to ACL entries when setting up ACLs in truncate table
### What changes were proposed in this pull request? This is a follow-up to the PR #26956. In #26956, the patch proposed to preserve path permission when truncating table. When setting up original ACLs, we need to set user/group/other permission as ACL entries too, otherwise if the path doesn't have default user/group/other ACL entries, ACL API will complain an error `Invalid ACL: the user, group and other entries are required.`. In short this change makes sure: 1. Permissions for user/group/other are always kept into ACLs to work with ACL API. 2. Other custom ACLs are still kept after TRUNCATE TABLE (#26956 did this). ### Why are the changes needed? Without this fix, `TRUNCATE TABLE` will get an error when setting up ACLs if there is no default default user/group/other ACL entries. ### Does this PR introduce any user-facing change? No ### How was this patch tested? Update unit test. Manual test on dev Spark cluster. Set ACLs for a table path without default user/group/other ACL entries: ``` hdfs dfs -setfacl --set 'user:liangchi:rwx,user::rwx,group::r--,other::r--' /user/hive/warehouse/test.db/test_truncate_table hdfs dfs -getfacl /user/hive/warehouse/test.db/test_truncate_table # file: /user/hive/warehouse/test.db/test_truncate_table # owner: liangchi # group: supergroup user::rwx user:liangchi:rwx group::r-- mask::rwx other::r-- ``` Then run `sql("truncate table test.test_truncate_table")`, it works by normally truncating the table and preserve ACLs. Closes #27548 from viirya/fix-truncate-table-permission. Lead-authored-by: Liang-Chi Hsieh <liangchi@uber.com> Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
This commit is contained in:
parent
aa0d13683c
commit
5b76367a9d
|
@ -19,12 +19,13 @@ package org.apache.spark.sql.execution.command
|
|||
|
||||
import java.net.{URI, URISyntaxException}
|
||||
|
||||
import scala.collection.JavaConverters._
|
||||
import scala.collection.mutable.ArrayBuffer
|
||||
import scala.util.Try
|
||||
import scala.util.control.NonFatal
|
||||
|
||||
import org.apache.hadoop.fs.{FileContext, FsConstants, Path}
|
||||
import org.apache.hadoop.fs.permission.{AclEntry, FsPermission}
|
||||
import org.apache.hadoop.fs.permission.{AclEntry, AclEntryScope, AclEntryType, FsAction, FsPermission}
|
||||
|
||||
import org.apache.spark.sql.{AnalysisException, Row, SparkSession}
|
||||
import org.apache.spark.sql.catalyst.TableIdentifier
|
||||
|
@ -538,12 +539,27 @@ case class TruncateTableCommand(
|
|||
}
|
||||
}
|
||||
optAcls.foreach { acls =>
|
||||
val aclEntries = acls.asScala.filter(_.getName != null).asJava
|
||||
|
||||
// If the path doesn't have default ACLs, `setAcl` API will throw an error
|
||||
// as it expects user/group/other permissions must be in ACL entries.
|
||||
// So we need to add tradition user/group/other permission
|
||||
// in the form of ACL.
|
||||
optPermission.map { permission =>
|
||||
aclEntries.add(newAclEntry(AclEntryScope.ACCESS,
|
||||
AclEntryType.USER, permission.getUserAction()))
|
||||
aclEntries.add(newAclEntry(AclEntryScope.ACCESS,
|
||||
AclEntryType.GROUP, permission.getGroupAction()))
|
||||
aclEntries.add(newAclEntry(AclEntryScope.ACCESS,
|
||||
AclEntryType.OTHER, permission.getOtherAction()))
|
||||
}
|
||||
|
||||
try {
|
||||
fs.setAcl(path, acls)
|
||||
fs.setAcl(path, aclEntries)
|
||||
} catch {
|
||||
case NonFatal(e) =>
|
||||
throw new SecurityException(
|
||||
s"Failed to set original ACL $acls back to " +
|
||||
s"Failed to set original ACL $aclEntries back to " +
|
||||
s"the created path: $path. Exception: ${e.getMessage}")
|
||||
}
|
||||
}
|
||||
|
@ -574,6 +590,16 @@ case class TruncateTableCommand(
|
|||
}
|
||||
Seq.empty[Row]
|
||||
}
|
||||
|
||||
private def newAclEntry(
|
||||
scope: AclEntryScope,
|
||||
aclType: AclEntryType,
|
||||
permission: FsAction): AclEntry = {
|
||||
new AclEntry.Builder()
|
||||
.setScope(scope)
|
||||
.setType(aclType)
|
||||
.setPermission(permission).build()
|
||||
}
|
||||
}
|
||||
|
||||
abstract class DescribeCommandBase extends RunnableCommand {
|
||||
|
|
|
@ -2042,6 +2042,7 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
|
|||
// Set ACL to table path.
|
||||
val customAcl = new java.util.ArrayList[AclEntry]()
|
||||
customAcl.add(new AclEntry.Builder()
|
||||
.setName("test")
|
||||
.setType(AclEntryType.USER)
|
||||
.setScope(AclEntryScope.ACCESS)
|
||||
.setPermission(FsAction.READ).build())
|
||||
|
@ -2061,8 +2062,26 @@ abstract class DDLSuite extends QueryTest with SQLTestUtils {
|
|||
if (ignore) {
|
||||
assert(aclEntries.size() == 0)
|
||||
} else {
|
||||
assert(aclEntries.size() == 1)
|
||||
assert(aclEntries.size() == 4)
|
||||
assert(aclEntries.get(0) == customAcl.get(0))
|
||||
|
||||
// Setting ACLs will also set user/group/other permissions
|
||||
// as ACL entries.
|
||||
val user = new AclEntry.Builder()
|
||||
.setType(AclEntryType.USER)
|
||||
.setScope(AclEntryScope.ACCESS)
|
||||
.setPermission(FsAction.ALL).build()
|
||||
val group = new AclEntry.Builder()
|
||||
.setType(AclEntryType.GROUP)
|
||||
.setScope(AclEntryScope.ACCESS)
|
||||
.setPermission(FsAction.ALL).build()
|
||||
val other = new AclEntry.Builder()
|
||||
.setType(AclEntryType.OTHER)
|
||||
.setScope(AclEntryScope.ACCESS)
|
||||
.setPermission(FsAction.ALL).build()
|
||||
assert(aclEntries.get(1) == user)
|
||||
assert(aclEntries.get(2) == group)
|
||||
assert(aclEntries.get(3) == other)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue