This is an automated email from the ASF dual-hosted git repository.
chengpan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/master by this push:
new 3fefc7d48 [KYUUBI #5472][AUTHZ][FOLLOWUP] Check permanent view also
need support merge projection
3fefc7d48 is described below
commit 3fefc7d487ae9dce5a4fb802ff10d8fe2271b76b
Author: Angerszhuuuu <[email protected]>
AuthorDate: Fri Oct 27 10:03:50 2023 +0800
[KYUUBI #5472][AUTHZ][FOLLOWUP] Check permanent view also need support
merge projection
### _Why are the changes needed?_
To close #5472 .
Check permanent view also need support merge projection
For case `SELECT count(id) FROM $db1.$view1 WHERE id > 10` we only need to
check `id`, before this pr, we need to check `id`, `scope`.
### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including
negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run
test](https://kyuubi.readthedocs.io/en/master/contributing/code/testing.html#running-tests)
locally before make a pull request
### _Was this patch authored or co-authored using generative AI tooling?_
Closes #5542 from AngersZhuuuu/KYUUBI-5472-FOLLOWUP.
Closes #5472
fd63c5ab2 [Bowen Liang] Update
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
41e487c95 [Angerszhuuuu] Update
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
4f688c4c1 [Angerszhuuuu] Update PrivilegesBuilder.scala
cb9322ce3 [Angerszhuuuu] [KYUUBI #5472][AUTHZ][FOLLOWUP] Check permanent
view also need support merge projection
Lead-authored-by: Angerszhuuuu <[email protected]>
Co-authored-by: Bowen Liang <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
---
.../plugin/spark/authz/PrivilegesBuilder.scala | 17 ++++---
.../ranger/RuleApplyPermanentViewMarker.scala | 3 +-
.../spark/authz/util/PermanentViewMarker.scala | 3 +-
.../authz/ranger/RangerSparkExtensionSuite.scala | 59 +++++++++++++---------
4 files changed, 50 insertions(+), 32 deletions(-)
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
index a0ed5fb6a..0d4b53a5c 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegesBuilder.scala
@@ -63,9 +63,19 @@ object PrivilegesBuilder {
conditionList: Seq[NamedExpression] = Nil,
spark: SparkSession): Unit = {
+ def getOutputColumnNames(plan: LogicalPlan): Seq[String] = {
+ plan match {
+ case pvm: PermanentViewMarker
+ if pvm.isSubqueryExpressionPlaceHolder || pvm.output.isEmpty =>
+ pvm.visitColNames
+ case _ =>
+ plan.output.map(_.name)
+ }
+ }
+
def mergeProjection(table: Table, plan: LogicalPlan): Unit = {
if (projectionList.isEmpty) {
- privilegeObjects += PrivilegeObject(table, plan.output.map(_.name))
+ privilegeObjects += PrivilegeObject(table, getOutputColumnNames(plan))
} else {
val cols = (projectionList ++ conditionList).flatMap(collectLeaves)
.filter(plan.outputSet.contains).map(_.name).distinct
@@ -103,11 +113,6 @@ object PrivilegesBuilder {
val cols = conditionList ++ aggCols
buildQuery(a.child, privilegeObjects, projectionList, cols, spark)
- case pvm: PermanentViewMarker =>
- getScanSpec(pvm).tables(pvm, spark).foreach { table =>
- privilegeObjects += PrivilegeObject(table, pvm.visitColNames)
- }
-
case scan if isKnownScan(scan) && scan.resolved =>
getScanSpec(scan).tables(scan, spark).foreach(mergeProjection(_, scan))
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
index 909cd9e93..f12088f5f 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RuleApplyPermanentViewMarker.scala
@@ -43,7 +43,8 @@ class RuleApplyPermanentViewMarker extends Rule[LogicalPlan] {
PermanentViewMarker(
subquery.plan,
permanentView.desc,
- permanentView.output.map(_.name)))
+ permanentView.output.map(_.name),
+ true))
}
PermanentViewMarker(
resolvedSubquery,
diff --git
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
index d19f7a923..e997e46f8 100644
---
a/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/PermanentViewMarker.scala
@@ -24,7 +24,8 @@ import
org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, UnaryNode}
case class PermanentViewMarker(
child: LogicalPlan,
catalogTable: CatalogTable,
- visitColNames: Seq[String]) extends UnaryNode
+ visitColNames: Seq[String],
+ isSubqueryExpressionPlaceHolder: Boolean = false) extends UnaryNode
with WithInternalChild {
override def output: Seq[Attribute] = child.output
diff --git
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
index c2e886f02..07c8efeda 100644
---
a/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
+++
b/extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/RangerSparkExtensionSuite.scala
@@ -913,31 +913,42 @@ class HiveCatalogRangerSparkExtensionSuite extends
RangerSparkExtensionSuite {
|CREATE VIEW $db1.$view2
|AS
|SELECT count(*) as cnt, sum(id) as sum_id FROM $db1.$table1
- """.stripMargin))
- val e1 = intercept[AccessControlException](
- doAs(someone, sql(s"SELECT count(*) FROM $db1.$table1").show()))
- assert(e1.getMessage.contains(
- s"does not have [select] privilege on
[$db1/$table1/id,$db1/$table1/scope]"))
+ """.stripMargin))
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(*) FROM $db1.$table1").show()))(
+ s"does not have [select] privilege on
[$db1/$table1/id,$db1/$table1/scope]")
- val e2 = intercept[AccessControlException](
- doAs(someone, sql(s"SELECT count(*) FROM $db1.$view1").show()))
- assert(e2.getMessage.contains(
- s"does not have [select] privilege on
[$db1/$view1/id,$db1/$view1/scope]"))
-
- val e3 = intercept[AccessControlException](
- doAs(someone, sql(s"SELECT count(*) FROM $db1.$view2").show()))
- assert(e3.getMessage.contains(
- s"does not have [select] privilege on
[$db1/$view2/cnt,$db1/$view2/sum_id]"))
-
- val e4 = intercept[AccessControlException](
- doAs(someone, sql(s"SELECT count(*) FROM $db1.$view2 WHERE cnt >
10").show()))
- assert(e4.getMessage.contains(
- s"does not have [select] privilege on
[$db1/$view2/cnt,$db1/$view2/sum_id]"))
-
- val e5 = intercept[AccessControlException](
- doAs(someone, sql(s"SELECT count(cnt) FROM $db1.$view2 WHERE cnt >
10").show()))
- assert(e5.getMessage.contains(
- s"does not have [select] privilege on
[$db1/$view2/cnt,$db1/$view2/sum_id]"))
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(*) FROM $db1.$view1").show()))(
+ s"does not have [select] privilege on
[$db1/$view1/id,$db1/$view1/scope]")
+
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(*) FROM $db1.$view2").show()))(
+ s"does not have [select] privilege on
[$db1/$view2/cnt,$db1/$view2/sum_id]")
+
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(id) FROM $db1.$table1 WHERE id >
10").show()))(
+ s"does not have [select] privilege on [$db1/$table1/id]")
+
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(id) FROM $db1.$view1 WHERE id >
10").show()))(
+ s"does not have [select] privilege on [$db1/$view1/id]")
+
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(sum_id) FROM $db1.$view2 WHERE
sum_id > 10").show()))(
+ s"does not have [select] privilege on [$db1/$view2/sum_id]")
+
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(scope) FROM $db1.$table1 WHERE id >
10").show()))(
+ s"does not have [select] privilege on
[$db1/$table1/scope,$db1/$table1/id]")
+
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(scope) FROM $db1.$view1 WHERE id >
10").show()))(
+ s"does not have [select] privilege on
[$db1/$view1/scope,$db1/$view1/id]")
+
+ interceptContains[AccessControlException](
+ doAs(someone, sql(s"SELECT count(cnt) FROM $db1.$view2 WHERE sum_id
> 10").show()))(
+ s"does not have [select] privilege on
[$db1/$view2/cnt,$db1/$view2/sum_id]")
}
}
}
