Skip to content

Commit 1096b96

Browse files
srinathshankarhvanhovell
authored andcommitted
[SC-4988] Match modified universe SparkSqlAclClient interface
## What changes were proposed in this pull request? There was a minor change in SparkSqlAclClient in universe during refactoring in this PR: https://github.com/databricks/universe/pull/14004 This makes the corresponding change to the client end in spark ## How was this patch tested? Ran all the *Acl* unit tests. Had to modify stub methods in ReflectionBackedAclSuite. Author: Srinath Shankar <[email protected]> Closes apache#139 from srinathshankar/fixup.
1 parent 9739b65 commit 1096b96

File tree

3 files changed

+60
-21
lines changed

3 files changed

+60
-21
lines changed

sql/core/src/main/scala/com/databricks/sql/acl/ReflectionBackedAclClient.scala

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,32 +20,36 @@ class ReflectionBackedAclClient(session: SparkSession) extends AclClient {
2020

2121
private[this] lazy val backend = backendClazz.newInstance()
2222

23+
private[this] lazy val getTokenFromLocalContextMethod = {
24+
backendClazz.getMethod("getTokenFromLocalContext")
25+
}
26+
2327
private[this] lazy val getValidPermissionsMethod = {
2428
backendClazz.getMethod(
2529
"getValidPermissions",
26-
classOf[Option[String]],
30+
classOf[String],
2731
classOf[Traversable[(String, String)]])
2832
}
2933

3034
private[this] lazy val getOwnersMethod = {
3135
backendClazz.getMethod(
3236
"getOwners",
33-
classOf[Option[String]],
37+
classOf[String],
3438
classOf[Traversable[String]])
3539
}
3640

3741
private[this] lazy val listPermissionsMethod = {
3842
backendClazz.getMethod(
3943
"listPermissions",
40-
classOf[Option[String]],
44+
classOf[String],
4145
classOf[Option[String]],
4246
classOf[String])
4347
}
4448

4549
private[this] lazy val modifyMethod = {
4650
backendClazz.getMethod(
4751
"modify",
48-
classOf[Option[String]],
52+
classOf[String],
4953
classOf[Seq[(String, String, String, String)]])
5054
}
5155

@@ -100,8 +104,10 @@ class ReflectionBackedAclClient(session: SparkSession) extends AclClient {
100104
modifyMethod.invoke(backend, token, argument)
101105
}
102106

103-
private def token: Option[String] = {
107+
private def token: String = {
104108
Option(session.sparkContext.getLocalProperty(TokenConf.TOKEN_KEY))
109+
.orElse(getTokenFromLocalContextMethod.invoke(backend).asInstanceOf[Option[String]])
110+
.getOrElse(throw new SecurityException("No token to authorize principal"))
105111
}
106112
}
107113

sql/core/src/test/scala/com/databricks/sql/acl/AclExtensionsSuite.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class AclExtensionsSuite extends SparkFunSuite with BeforeAndAfterEach {
3939

4040
try {
4141
session.udf.register("plusOne", (value: Int) => value + 1)
42+
session.sparkContext.setLocalProperty(TokenConf.TOKEN_KEY, "token")
4243
session.sql("select plusOne(id) from values 1,2,3,4,5 t(id)")
4344
assert(AclClientBackend.lastCommandArguments.nonEmpty)
4445
} finally {

sql/core/src/test/scala/com/databricks/sql/acl/ReflectionBackedAclClientSuite.scala

Lines changed: 48 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,56 +27,59 @@ class ReflectionBackedAclClientSuite extends SparkFunSuite with SharedSQLContext
2727
super.beforeAll()
2828
}
2929

30-
private def token(token: String): Unit = {
30+
private def sparkContextToken(token: String): Unit = {
3131
spark.sparkContext.setLocalProperty(TokenConf.TOKEN_KEY, token)
3232
}
3333

34+
private def driverToken(token: String): Unit = {
35+
AclClientBackend.localContextToken = Option(token)
36+
}
37+
3438
test("getValidPermissions") {
35-
token("usr1")
39+
sparkContextToken("usr1")
40+
driverToken(null)
3641
val input = Set((tbl1, Select), (tbl1, Modify))
3742
assert(input === client.getValidPermissions(input))
38-
assert(AclClientBackend.token === Some("usr1"))
43+
assert(AclClientBackend.token === "usr1")
3944
assert(AclClientBackend.lastCommandArguments === Seq(
4045
("/CATALOG/`default`/DATABASE/`db1`/TABLE/`tbl_x`", "SELECT"),
4146
("/CATALOG/`default`/DATABASE/`db1`/TABLE/`tbl_x`", "MODIFY")))
4247
}
4348

4449
test("getOwners") {
45-
token(null)
50+
sparkContextToken("usr3")
4651
val input = Seq(tbl1, fn1)
4752
assert(client.getOwners(input) === Map((tbl1, principal1), (fn1, principal1)))
48-
assert(AclClientBackend.token === None)
53+
assert(AclClientBackend.token === "usr3")
4954
assert(AclClientBackend.lastCommandArguments === Seq(
5055
"/CATALOG/`default`/DATABASE/`db1`/TABLE/`tbl_x`",
5156
"/CATALOG/`default`/DATABASE/`db2`/FUNCTION/`fn1`"))
5257
}
5358

5459
test("listPermissions") {
55-
token("usr4")
60+
sparkContextToken("usr4")
5661
assert(client.listPermissions(None, tbl1) ===
5762
Seq(Permission(principal2, Select, tbl1)))
58-
assert(AclClientBackend.token === Some("usr4"))
63+
assert(AclClientBackend.token === "usr4")
5964
assert(AclClientBackend.lastCommandArguments ===
6065
Seq("/CATALOG/`default`/DATABASE/`db1`/TABLE/`tbl_x`"))
61-
token(null)
6266
assert(client.listPermissions(Some(principal1), fn1) ===
6367
Seq(Permission(principal1, Select, fn1)))
64-
assert(AclClientBackend.token === None)
6568
assert(AclClientBackend.lastCommandArguments === Seq(
6669
"USER_1",
6770
"/CATALOG/`default`/DATABASE/`db2`/FUNCTION/`fn1`"))
6871
}
6972

7073
test("modify") {
71-
token("usr8")
74+
sparkContextToken("usr8")
7275
client.modify(Seq(
7376
GrantPermission(Permission(principal1, Select, tbl1)),
7477
RevokePermission(Permission(principal2, Select, fn1)),
7578
DropSecurable(fn1),
7679
Rename(fn1, fn1),
7780
CreateSecurable(tbl1),
7881
ChangeOwner(tbl1, principal2)))
79-
assert(AclClientBackend.token === Some("usr8"))
82+
assert(AclClientBackend.token === "usr8")
8083
assert(AclClientBackend.lastCommandArguments === Seq(
8184
("GRANT", "USER_1", "SELECT",
8285
"/CATALOG/`default`/DATABASE/`db1`/TABLE/`tbl_x`"),
@@ -92,32 +95,61 @@ class ReflectionBackedAclClientSuite extends SparkFunSuite with SharedSQLContext
9295
("CHANGE_OWNER",
9396
"/CATALOG/`default`/DATABASE/`db1`/TABLE/`tbl_x`", "USER_2", "")))
9497
}
98+
99+
test("Context token before driver") {
100+
sparkContextToken("sc")
101+
driverToken("driver")
102+
client.getValidPermissions(Set.empty[(Securable, Action)])
103+
assert(AclClientBackend.token == "sc")
104+
105+
driverToken(null)
106+
client.getValidPermissions(Set.empty[(Securable, Action)])
107+
assert(AclClientBackend.token == "sc")
108+
}
109+
110+
test("Driver token") {
111+
sparkContextToken(null)
112+
driverToken("driver")
113+
client.getValidPermissions(Set.empty[(Securable, Action)])
114+
assert(AclClientBackend.token == "driver")
115+
}
116+
117+
test("No token") {
118+
sparkContextToken(null)
119+
driverToken(null)
120+
intercept[SecurityException] {
121+
client.getValidPermissions(Set.empty[(Securable, Action)])
122+
}
123+
}
95124
}
96125

97126
object AclClientBackend {
98-
var token: Option[String] = None
127+
var token: String = _
99128
var lastCommandArguments: Seq[AnyRef] = Seq.empty
129+
var localContextToken: Option[String] = None
100130
}
101131

102132
class AclClientBackend {
133+
def getTokenFromLocalContext: Option[String] = AclClientBackend.localContextToken
134+
103135
def getValidPermissions(
104-
token: Option[String],
136+
token: String,
105137
requests: Traversable[(String, String)]): Set[(String, String)] = {
106138
AclClientBackend.token = token
107139
AclClientBackend.lastCommandArguments = requests.toSeq
108140
requests.toSet
109141
}
110142

111143
def getOwners(
112-
token: Option[String],
144+
token: String,
113145
securables: Traversable[String]): Map[String, String] = {
114146
AclClientBackend.token = token
115147
AclClientBackend.lastCommandArguments = securables.toSeq
116148
securables.map(_ -> "USER_1").toMap
117149
}
118150

119151
def listPermissions(
120-
token: Option[String],
152+
token: String,
121153
principal: Option[String],
122154
securable: String): Seq[(String, String, String)] = {
123155
AclClientBackend.token = token
@@ -126,7 +158,7 @@ class AclClientBackend {
126158
}
127159

128160
def modify(
129-
token: Option[String],
161+
token: String,
130162
modifications: Seq[(String, String, String, String)]): Unit = {
131163
AclClientBackend.token = token
132164
AclClientBackend.lastCommandArguments = modifications

0 commit comments

Comments
 (0)