Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AUTHZ] Shade spark authz plugin #5427

Closed
wants to merge 6 commits into from
Closed

[AUTHZ] Shade spark authz plugin #5427

wants to merge 6 commits into from

Conversation

yikf
Copy link
Contributor

@yikf yikf commented Oct 16, 2023

Why are the changes needed?

This PR aims to shade the kyuubi spark authz plugin to simplify the user's use.

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 locally before make a pull request

Was this patch authored or co-authored using generative AI tooling?

No

@yikf
Copy link
Contributor Author

yikf commented Oct 16, 2023

Since kyuubi supports different spark versions and ranger versions, and the cost of integration testing is too high, we initially use the default ranger version for functional testing.

In this PR, I test manually with ranger 2.4.0 and spark 3.4.0;

case 1: user root does not have select privilege on default/t1/id
image
image

case 2: user root have select privilege on default/t1/id
image

image

@yikf
Copy link
Contributor Author

yikf commented Oct 16, 2023

cc @pan3793

@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2023

Codecov Report

Merging #5427 (695133d) into master (ffebc64) will not change coverage.
Report is 49 commits behind head on master.
The diff coverage is n/a.

❗ Current head 695133d differs from pull request most recent head d2f7ea8. Consider uploading reports for the commit d2f7ea8 to get more accurate results

@@          Coverage Diff           @@
##           master   #5427   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         590     588    -2     
  Lines       33436   33466   +30     
  Branches     4422    4401   -21     
======================================
- Misses      33436   33466   +30     

see 29 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@yikf
Copy link
Contributor Author

yikf commented Oct 17, 2023

In addition, I want to change the jar package name of shade so as not to affect the use of existing users, so two jars will be produced:

  1. kyuubi-spark-authz_${scala.binary.version}-${project.version}.jar is original jar
  2. shaded-kyuubi-spark-authz_${scala.binary.version}-${project.version} is a fat jar

Any thought? @yaooqinn @pan3793

@yaooqinn
Copy link
Member

I suggest using a separate module for bundling, naming it kyuubi-spark-authz_xx-bundle.jar is fair enough?

Also the doc needs to be updated if we provide a new installation approach

@yikf
Copy link
Contributor Author

yikf commented Oct 17, 2023

Also the doc needs to be updated if we provide a new installation approach

Thank you for mentioning it. I will.

I suggest using a separate module for bundling, naming it kyuubi-spark-authz_xx-bundle.jar is fair enough?

Just a personal question, what are the benefits of this?

@github-actions github-actions bot added kind:documentation Documentation is a feature! and removed module:authz labels Oct 17, 2023
pom.xml Outdated Show resolved Hide resolved
@bowenliang123 bowenliang123 changed the title [KYUUBI][AUTHZ] Shade spark authz plugin [AUTHZ] Shade spark authz plugin Oct 19, 2023
<properties>
<!-- the following components' version may need to tune to align w/ the ranger.version-->
<gethostname4j.version>1.0.0</gethostname4j.version>
<jersey.client.version>1.19.4</jersey.client.version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the jersey-client is a transitive dependency from the ranger client, shall we need to specify the jersey-client version when shading the authz plugin?

Copy link
Contributor Author

@yikf yikf Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emm, i can't find this transitive dependency
image

@yikf
Copy link
Contributor Author

yikf commented Oct 20, 2023

Hi all, Any other comments? : )

@yikf yikf added this to the v1.9.0 milestone Oct 20, 2023
@yikf yikf self-assigned this Oct 20, 2023
@yikf yikf closed this in 59c25b9 Oct 20, 2023
@yikf
Copy link
Contributor Author

yikf commented Oct 20, 2023

Thanks all , merged to master.

<include>org.apache.kyuubi:kyuubi-util</include>
<include>org.apache.ranger:ranger-plugins-common</include>
<include>org.apache.ranger:ranger-plugins-audit</include>
<include>org.codehaus.jackson:jackson-jaxrs</include>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's may not sufficient, please include the transitive jackson libs (spark is going to remove the jackson 1.x deps from the release tarball)

davidyuan1223 pushed a commit to davidyuan1223/kyuubi that referenced this pull request Oct 26, 2023
### _Why are the changes needed?_

This PR aims to shade the kyuubi spark authz plugin to simplify the user's use.

### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible

- [x] 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

Closes apache#5427 from Yikf/shade-authz.

Closes apache#5427

d2f7ea8 [yikaifei] fix
695133d [Kent Yao] Update docs/security/authorization/spark/install.md
f3a6531 [Kent Yao] Update docs/security/authorization/spark/build.md
963cab3 [yikaifei] bundle
2068c98 [yikaifei] relocation
6c6e50e [yikaifei] Shade spark authz plugin

Lead-authored-by: yikaifei <[email protected]>
Co-authored-by: Kent Yao <[email protected]>
Signed-off-by: yikaifei <[email protected]>
pan3793 pushed a commit that referenced this pull request Oct 27, 2023
### _Why are the changes needed?_

As description #5427 (comment). This PR aims to:

- Authz shaded include the transitive jackson libs
- Add LICENSE and NOTICE file to exactly match the content of jar
- Add scala-2.13 profile to make authz shade module support scala 2.13

### _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

- [x] [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

Closes #5496 from Yikf/jackson.

Closes #5496

6f44249 [yikaifei] trigger
8eeec2b [yikaifei] trigger
6a3cc0c [yikaifei] license
758d834 [yikaifei] shade jackson

Authored-by: yikaifei <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
pan3793 added a commit that referenced this pull request Nov 8, 2023
### _Why are the changes needed?_

Current now, in spark-engine module, some session-level configurations are ignored due to the complexity of get session-level configurations in kyuubi spark engine, so As discussed in #5410 (comment). If we need unit test use withSessionConf method, we need make the code get configuration from the right session

The PR is unfinished, it need wait the pr #5410 success so that i can use the new change in unit test

closes #5438
### _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

- [x] [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

Closes #5487 from davidyuan1223/5438_add_common_method_to_support_session_config.

Closes #5438

e1ded36 [davidyuan] add more optional session level to get conf
84c4568 [davidyuan] add more optional session level to get conf
4d70902 [davidyuan] add more optional session level to get conf
96d7cde [davidyuan] Revert "add more optional session level to get conf"
940f8f8 [davidyuan] add more optional session level to get conf
15641e8 [davidyuan] add more optional session level to get conf
d838931 [davidyuan] Merge branch '5438_add_common_method_to_support_session_config' of https://github.com/davidyuan1223/kyuubi into 5438_add_common_method_to_support_session_config
2de96b5 [davidyuan] add common method to get session level config
3ec73ad [liangbowen] [KYUUBI #5522] [BATCH] Ignore main class for PySpark batch job submission
d8b808d [Cheng Pan] [KYUUBI #5523] [DOC] Update the Kyuubi supported components version
c7d15ae [Cheng Pan] [KYUUBI #5483] Release Spark TPC-H/DS Connectors with Scala 2.13
4a1db42 [zwangsheng] [KYUUBI #5513][BATCH] Always redirect delete batch request to Kyuubi instance that owns batch session
b06e044 [labbomb] [KYUUBI #5517] [UI] Initial implement the SQL Lab page
88bb6b4 [liangbowen] [KYUUBI #5486] Bump Kafka client version from 3.4.0 to 3.5.1
538a648 [davidyuan] [KYUUBI #4186] Spark showProgress with JobInfo
682e5b5 [Xianxun Ye] [KYUUBI #5405] [FLINK] Support Flink 1.18
c71528e [Cheng Pan] [KYUUBI #5484] Remove legacy Web UI
ee52b2a [Angerszhuuuu] [KYUUBI #5446][AUTHZ] Support Create/Drop/Show/Reresh index command for Hudi
6a5bb10 [weixi] [KYUUBI #5380][UT] Create PySpark batch jobs tests for RESTful API
86f692d [Kent Yao] [KYUUBI #5512] [AuthZ] Remove the non-existent query specs in Deletes and Updates
dfdd7a3 [fwang12] [KYUUBI #5499][KYUUBI #2503] Catch any exception when closing idle session
b7b3544 [伟程] [KYUUBI #5212] Fix configuration errors causing by helm charts of prometheus services
d123a5a [liupeiyue] [KYUUBI #5282] Support configure Trino session conf in `kyuubi-default.conf`
0750437 [yangming] [KYUUBI #5294] [DOC] Update supported dialects for JDBC engine
9c75d82 [zwangsheng] [KYUUBI #5435][INFRA][TEST] Improve Kyuubi On Kubernetes IT
1dc264a [Angerszhuuuu] [KYUUBI #5479][AUTHZ] Support Hudi CallProcedureHoodieCommand for stored procedures
bc3fcbb [Angerszhuuuu] [KYUUBI #5472] Permanent View should pass column when child plan no output
a67b824 [Fantasy-Jay] [KYUUBI #5382][JDBC] Duplication cleanup improvement in JdbcDialect and schema helpers
c039e1b [Kent Yao] [KYUUBI #5497] [AuthZ] Simplify debug message for missing field/method in ReflectUtils
0c8be79 [Angerszhuuuu] [KYUUBI #5475][FOLLOWUP] Authz check permanent view's subquery should check view's correct privilege
1293cf2 [Kent Yao] [KYUUBI #5500] Add Kyuubi Code Program to Doc
e2754fe [Angerszhuuuu] [KYUUBI #5492][AUTHZ] saveAsTable create DataSource table miss db info
0c53d00 [Angerszhuuuu] [KYUUBI #5447][FOLLOWUP] Remove unrelated debug prints in TableIdentifierTableExtractor
119c393 [Angerszhuuuu] [KYUUBI #5447][AUTHZ] Support Hudi DeleteHoodieTableCommand/UpdateHoodieTableCommand/MergeIntoHoodieTableCommand
3af5ed1 [yikaifei] [KYUUBI #5427] [AUTHZ] Shade spark authz plugin
503c3f7 [davidyuan] Merge remote-tracking branch 'origin/5438_add_common_method_to_support_session_config' into 5438_add_common_method_to_support_session_config
7a67ace [davidyuan] add common method to get session level config
3f42317 [davidyuan] add common method to get session level config
bb5d5ce [davidyuan] add common method to get session level config
623200f [davidyuan] Merge remote-tracking branch 'origin/5438_add_common_method_to_support_session_config' into 5438_add_common_method_to_support_session_config
8011959 [davidyuan] add common method to get session level config
605ef16 [davidyuan] Merge remote-tracking branch 'origin/5438_add_common_method_to_support_session_config' into 5438_add_common_method_to_support_session_config
bb63ed8 [davidyuan] add common method to get session level config
d9cf248 [davidyuan] add common method to get session level config
c8647ef [davidyuan] add common method to get session level config
618c0f6 [david yuan] Merge branch 'apache:master' into 5438_add_common_method_to_support_session_config
c1024bd [david yuan] Merge branch 'apache:master' into 5438_add_common_method_to_support_session_config
32028f9 [davidyuan] add common method to get session level config
03e2887 [davidyuan] add common method to get session level config

Lead-authored-by: David Yuan <[email protected]>
Co-authored-by: davidyuan <[email protected]>
Co-authored-by: Angerszhuuuu <[email protected]>
Co-authored-by: Cheng Pan <[email protected]>
Co-authored-by: Kent Yao <[email protected]>
Co-authored-by: liangbowen <[email protected]>
Co-authored-by: david yuan <[email protected]>
Co-authored-by: zwangsheng <[email protected]>
Co-authored-by: yangming <[email protected]>
Co-authored-by: 伟程 <[email protected]>
Co-authored-by: weixi <[email protected]>
Co-authored-by: fwang12 <[email protected]>
Co-authored-by: Xianxun Ye <[email protected]>
Co-authored-by: liupeiyue <[email protected]>
Co-authored-by: Fantasy-Jay <[email protected]>
Co-authored-by: yikaifei <[email protected]>
Co-authored-by: labbomb <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
pan3793 added a commit that referenced this pull request Nov 8, 2023
### _Why are the changes needed?_

Current now, in spark-engine module, some session-level configurations are ignored due to the complexity of get session-level configurations in kyuubi spark engine, so As discussed in #5410 (comment). If we need unit test use withSessionConf method, we need make the code get configuration from the right session

The PR is unfinished, it need wait the pr #5410 success so that i can use the new change in unit test

closes #5438
### _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

- [x] [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

Closes #5487 from davidyuan1223/5438_add_common_method_to_support_session_config.

Closes #5438

e1ded36 [davidyuan] add more optional session level to get conf
84c4568 [davidyuan] add more optional session level to get conf
4d70902 [davidyuan] add more optional session level to get conf
96d7cde [davidyuan] Revert "add more optional session level to get conf"
940f8f8 [davidyuan] add more optional session level to get conf
15641e8 [davidyuan] add more optional session level to get conf
d838931 [davidyuan] Merge branch '5438_add_common_method_to_support_session_config' of https://github.com/davidyuan1223/kyuubi into 5438_add_common_method_to_support_session_config
2de96b5 [davidyuan] add common method to get session level config
3ec73ad [liangbowen] [KYUUBI #5522] [BATCH] Ignore main class for PySpark batch job submission
d8b808d [Cheng Pan] [KYUUBI #5523] [DOC] Update the Kyuubi supported components version
c7d15ae [Cheng Pan] [KYUUBI #5483] Release Spark TPC-H/DS Connectors with Scala 2.13
4a1db42 [zwangsheng] [KYUUBI #5513][BATCH] Always redirect delete batch request to Kyuubi instance that owns batch session
b06e044 [labbomb] [KYUUBI #5517] [UI] Initial implement the SQL Lab page
88bb6b4 [liangbowen] [KYUUBI #5486] Bump Kafka client version from 3.4.0 to 3.5.1
538a648 [davidyuan] [KYUUBI #4186] Spark showProgress with JobInfo
682e5b5 [Xianxun Ye] [KYUUBI #5405] [FLINK] Support Flink 1.18
c71528e [Cheng Pan] [KYUUBI #5484] Remove legacy Web UI
ee52b2a [Angerszhuuuu] [KYUUBI #5446][AUTHZ] Support Create/Drop/Show/Reresh index command for Hudi
6a5bb10 [weixi] [KYUUBI #5380][UT] Create PySpark batch jobs tests for RESTful API
86f692d [Kent Yao] [KYUUBI #5512] [AuthZ] Remove the non-existent query specs in Deletes and Updates
dfdd7a3 [fwang12] [KYUUBI #5499][KYUUBI #2503] Catch any exception when closing idle session
b7b3544 [伟程] [KYUUBI #5212] Fix configuration errors causing by helm charts of prometheus services
d123a5a [liupeiyue] [KYUUBI #5282] Support configure Trino session conf in `kyuubi-default.conf`
0750437 [yangming] [KYUUBI #5294] [DOC] Update supported dialects for JDBC engine
9c75d82 [zwangsheng] [KYUUBI #5435][INFRA][TEST] Improve Kyuubi On Kubernetes IT
1dc264a [Angerszhuuuu] [KYUUBI #5479][AUTHZ] Support Hudi CallProcedureHoodieCommand for stored procedures
bc3fcbb [Angerszhuuuu] [KYUUBI #5472] Permanent View should pass column when child plan no output
a67b824 [Fantasy-Jay] [KYUUBI #5382][JDBC] Duplication cleanup improvement in JdbcDialect and schema helpers
c039e1b [Kent Yao] [KYUUBI #5497] [AuthZ] Simplify debug message for missing field/method in ReflectUtils
0c8be79 [Angerszhuuuu] [KYUUBI #5475][FOLLOWUP] Authz check permanent view's subquery should check view's correct privilege
1293cf2 [Kent Yao] [KYUUBI #5500] Add Kyuubi Code Program to Doc
e2754fe [Angerszhuuuu] [KYUUBI #5492][AUTHZ] saveAsTable create DataSource table miss db info
0c53d00 [Angerszhuuuu] [KYUUBI #5447][FOLLOWUP] Remove unrelated debug prints in TableIdentifierTableExtractor
119c393 [Angerszhuuuu] [KYUUBI #5447][AUTHZ] Support Hudi DeleteHoodieTableCommand/UpdateHoodieTableCommand/MergeIntoHoodieTableCommand
3af5ed1 [yikaifei] [KYUUBI #5427] [AUTHZ] Shade spark authz plugin
503c3f7 [davidyuan] Merge remote-tracking branch 'origin/5438_add_common_method_to_support_session_config' into 5438_add_common_method_to_support_session_config
7a67ace [davidyuan] add common method to get session level config
3f42317 [davidyuan] add common method to get session level config
bb5d5ce [davidyuan] add common method to get session level config
623200f [davidyuan] Merge remote-tracking branch 'origin/5438_add_common_method_to_support_session_config' into 5438_add_common_method_to_support_session_config
8011959 [davidyuan] add common method to get session level config
605ef16 [davidyuan] Merge remote-tracking branch 'origin/5438_add_common_method_to_support_session_config' into 5438_add_common_method_to_support_session_config
bb63ed8 [davidyuan] add common method to get session level config
d9cf248 [davidyuan] add common method to get session level config
c8647ef [davidyuan] add common method to get session level config
618c0f6 [david yuan] Merge branch 'apache:master' into 5438_add_common_method_to_support_session_config
c1024bd [david yuan] Merge branch 'apache:master' into 5438_add_common_method_to_support_session_config
32028f9 [davidyuan] add common method to get session level config
03e2887 [davidyuan] add common method to get session level config

Lead-authored-by: David Yuan <[email protected]>
Co-authored-by: davidyuan <[email protected]>
Co-authored-by: Angerszhuuuu <[email protected]>
Co-authored-by: Cheng Pan <[email protected]>
Co-authored-by: Kent Yao <[email protected]>
Co-authored-by: liangbowen <[email protected]>
Co-authored-by: david yuan <[email protected]>
Co-authored-by: zwangsheng <[email protected]>
Co-authored-by: yangming <[email protected]>
Co-authored-by: 伟程 <[email protected]>
Co-authored-by: weixi <[email protected]>
Co-authored-by: fwang12 <[email protected]>
Co-authored-by: Xianxun Ye <[email protected]>
Co-authored-by: liupeiyue <[email protected]>
Co-authored-by: Fantasy-Jay <[email protected]>
Co-authored-by: yikaifei <[email protected]>
Co-authored-by: labbomb <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit 9615db5)
Signed-off-by: Cheng Pan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants