You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "AngersZhuuuu (via GitHub)" <gi...@apache.org> on 2023/11/15 06:48:11 UTC
[PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
AngersZhuuuu opened a new pull request, #5699:
URL: https://github.com/apache/kyuubi/pull/5699
### _Why are the changes needed?_
To close #5698
Hudi privilege table should ignore meta column,
For simple sql
```
SELECT * from hudi_ns.table1_hoodie
```
will throw such excepton, we should ignore meta columns
```
Permission denied: user [someone] does not have [select] privilege on [hudi_ns/table1_hoodie/_hoodie_commit_time,hudi_ns/table1_hoodie/_hoodie_commit_seqno,hudi_ns/table1_hoodie/_hoodie_record_key,hudi_ns/table1_hoodie/_hoodie_partition_path,hudi_ns/table1_hoodie/_hoodie_file_name,hudi_ns/table1_hoodie/id,hudi_ns/table1_hoodie/name,hudi_ns/table1_hoodie/city]
```
### _How was this patch tested?_
- [ ] 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?_
No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#issuecomment-1812052130
## [Codecov](https://app.codecov.io/gh/apache/kyuubi/pull/5699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`905170d`)](https://app.codecov.io/gh/apache/kyuubi/commit/905170d7a59660d136ed36a587686e1ab97a3885?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.43% compared to head [(`7fcfc6e`)](https://app.codecov.io/gh/apache/kyuubi/pull/5699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 61.45%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #5699 +/- ##
============================================
+ Coverage 61.43% 61.45% +0.01%
Complexity 23 23
============================================
Files 607 608 +1
Lines 35727 35741 +14
Branches 4891 4892 +1
============================================
+ Hits 21950 21964 +14
- Misses 11393 11399 +6
+ Partials 2384 2378 -6
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/kyuubi/pull/5699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1393772292
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegeObject.scala:
##########
@@ -71,7 +72,7 @@ object PrivilegeObject {
actionType,
table.database.orNull,
table.table,
- columns,
+ columns.filter(!HudiUtils.isHudiMetaField(_)),
Review Comment:
So if the table fields happens to be the same as meta field names, are the columns getting away from privilege checks?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1393780776
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegeObject.scala:
##########
@@ -71,7 +72,7 @@ object PrivilegeObject {
actionType,
table.database.orNull,
table.table,
- columns,
+ columns.filter(!HudiUtils.isHudiMetaField(_)),
Review Comment:
> So if the table fields happens to be the same as meta field names, are the columns getting away from privilege checks?
good question....,maybe we need to check if it's a hudi table.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#issuecomment-1820307208
Close this since it's not a issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu closed pull request #5699: [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column
URL: https://github.com/apache/kyuubi/pull/5699
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1393769867
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/ranger/HudiCatalogRangerSparkExtensionSuite.scala:
##########
@@ -616,4 +616,31 @@ class HudiCatalogRangerSparkExtensionSuite extends RangerSparkExtensionSuite {
doAs(admin, sql(dropIndex))
}
}
+
+ test("[KYUUBI #5698][Authz] Hudi privilege table should ignore meta column") {
Review Comment:
```suggestion
test("[KYUUBI #5698][Authz] Ignore meta columns for Hudi table") {
```
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/util/HudiUtils.scala:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.kyuubi.plugin.spark.authz.util
+
+object HudiUtils {
+ private val COMMIT_TIME_METADATA_FIELD = "_hoodie_commit_time"
+ private val COMMIT_SEQNO_METADATA_FIELD = "_hoodie_commit_seqno"
+ private val RECORD_KEY_METADATA_FIELD = "_hoodie_record_key"
+ private val PARTITION_PATH_METADATA_FIELD = "_hoodie_partition_path"
+ private val FILENAME_METADATA_FIELD = "_hoodie_file_name"
+
+ private val metaFields = Seq(
Review Comment:
```suggestion
private val metaFields = Set(
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1395120119
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegeObject.scala:
##########
@@ -71,7 +72,7 @@ object PrivilegeObject {
actionType,
table.database.orNull,
table.table,
- columns,
+ columns.filter(!HudiUtils.isHudiMetaField(_)),
Review Comment:
@yaooqinn What do you think about this? Iceberg also have similar problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1395120119
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegeObject.scala:
##########
@@ -71,7 +72,7 @@ object PrivilegeObject {
actionType,
table.database.orNull,
table.table,
- columns,
+ columns.filter(!HudiUtils.isHudiMetaField(_)),
Review Comment:
@yaooqinn What do you think about this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#issuecomment-1811894919
ping @yaooqinn
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1393772292
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegeObject.scala:
##########
@@ -71,7 +72,7 @@ object PrivilegeObject {
actionType,
table.database.orNull,
table.table,
- columns,
+ columns.filter(!HudiUtils.isHudiMetaField(_)),
Review Comment:
If the table fields happen to be the same as meta field names, are the columns escaping from privilege checks?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#issuecomment-1820215328
The case in PR desc looks reasonable to me. Anything belonging to the table shall have proper privileges. If you use * to retrieve them, then authorize them. Otherwise, * shall not point to the meta columns instead of skipping them in authz.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1393782559
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegeObject.scala:
##########
@@ -71,7 +72,7 @@ object PrivilegeObject {
actionType,
table.database.orNull,
table.table,
- columns,
+ columns.filter(!HudiUtils.isHudiMetaField(_)),
Review Comment:
Yes, if we have to implement it this time. Make sure the clarification are mentioned in PR description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on code in PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#discussion_r1395123396
##########
extensions/spark/kyuubi-spark-authz/src/main/scala/org/apache/kyuubi/plugin/spark/authz/PrivilegeObject.scala:
##########
@@ -71,7 +72,7 @@ object PrivilegeObject {
actionType,
table.database.orNull,
table.table,
- columns,
+ columns.filter(!HudiUtils.isHudiMetaField(_)),
Review Comment:
Checked iceberg won't have such problem...
```
"Permission denied: user [someone] does not have [select] privilege on [iceberg_ns/table1/id,iceberg_ns/table1/name,iceberg_ns/table1/city]" did not contain "xxxx"
ScalaTestFailureLocation: org.apache.kyuubi.plugin.spark.authz.ranger.IcebergCatalogRangerSparkExtensionSuite at (IcebergCatalogRangerSparkExtensionSuite.scala:373)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
Re: [PR] [KYUUBI #5698][Authz] Hudi privilege object for table should ignore meta column [kyuubi]
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on PR #5699:
URL: https://github.com/apache/kyuubi/pull/5699#issuecomment-1820307068
> The case in PR desc looks reasonable to me. Anything belonging to the table shall have proper privileges. If you use * to retrieve them, then authorize them. Otherwise, * shall not point to the meta columns instead of skipping them in authz.
Oh...I make a mistake, just checked with the hudi dev that in our prod, `SELECT * FROM HUDI` will ignore the HUDI meta column, but for real case, it will show hudi's meta columns too. Since we write unit test with `collect()` not `show()`, didn't find this.
<img width="1170" alt="截屏2023-11-21 下午2 18 38" src="https://github.com/apache/kyuubi/assets/46485123/51239ee6-dd5f-45ee-9841-89b13af0f57f">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org