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

[KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0 #3771

Closed
wants to merge 7 commits into from

Conversation

zhouyifan279
Copy link
Contributor

Why are the changes needed?

Fix #3744

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

@github-actions github-actions bot added kind:documentation Documentation is a feature! module:ha labels Nov 7, 2022
@zhouyifan279
Copy link
Contributor Author

cc @cxzl25 @turboFei @pan3793

* Since Kyuubi 1.7, Kyuubi returns engine's information for `GetInfo` request instead of server. To restore the previous behavior, set `kyuubi.server.info.provider` to `SERVER`.
* Since Kyuubi 1.7, Kyuubi returns engine's information for `GetInfo` request instead of server.
To restore the previous behavior, set `kyuubi.server.info.provider` to `SERVER`.
* Since Kyuubi 1.7, `kyuubi.kinit.principal` & `kyuubi.kinit.keytab` are filtered out from Kyuubi engine's conf for better security.
Copy link
Member

Choose a reason for hiding this comment

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

it happened since 1.6.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected

When Kyuubi engine does kerberos authentication with zookeeper, user needs to explicitly set `kyuubi.ha.zookeeper.engine.auth.type` to `KERBEROS`.

## Upgrading from Kyuubi 1.5 to 1.6
* Kyuubi engine get zookeeper principal & keytab from `kyuubi.ha.zookeeper.auth.principal` & `kyuubi.ha.zookeeper.auth.keytab`.
Copy link
Member

Choose a reason for hiding this comment

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

get => gets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@codecov-commenter
Copy link

Codecov Report

Merging #3771 (554e889) into master (84297ea) will increase coverage by 0.69%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             master    #3771      +/-   ##
============================================
+ Coverage     51.93%   52.62%   +0.69%     
  Complexity       13       13              
============================================
  Files           484      494      +10     
  Lines         27122    27799     +677     
  Branches       3784     3835      +51     
============================================
+ Hits          14086    14630     +544     
- Misses        11669    11773     +104     
- Partials       1367     1396      +29     
Impacted Files Coverage Δ
...la/org/apache/kyuubi/ha/HighAvailabilityConf.scala 98.47% <100.00%> (+0.01%) ⬆️
...apache/kyuubi/service/TBinaryFrontendService.scala 47.77% <0.00%> (-39.98%) ⬇️
...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala 63.93% <0.00%> (-16.07%) ⬇️
.../org/apache/kyuubi/session/KyuubiSessionImpl.scala 75.22% <0.00%> (-5.84%) ⬇️
...che/kyuubi/server/KyuubiTHttpFrontendService.scala 60.00% <0.00%> (-4.97%) ⬇️
...he/kyuubi/plugin/spark/authz/util/AuthZUtils.scala 54.71% <0.00%> (-4.38%) ⬇️
.../apache/kyuubi/plugin/spark/authz/v2Commands.scala 89.51% <0.00%> (-1.11%) ⬇️
...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala 80.74% <0.00%> (-0.63%) ⬇️
.../apache/kyuubi/server/api/v1/BatchesResource.scala 69.19% <0.00%> (-0.36%) ⬇️
...a/org/apache/kyuubi/service/TFrontendService.scala 90.98% <0.00%> (-0.19%) ⬇️
... and 72 more

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

.fallbackConf(HA_ZK_AUTH_TYPE)
.stringConf
.checkValues(AuthTypes.values.map(_.toString))
.createWithDefault(AuthTypes.NONE.toString)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Engine defaults to access Zookeeper without authentication

@pan3793 pan3793 added this to the v1.6.1 milestone Nov 7, 2022
@pan3793
Copy link
Member

pan3793 commented Nov 7, 2022

Thanks, merigng to master/1.6

@pan3793 pan3793 closed this in 78e80b8 Nov 7, 2022
pan3793 pushed a commit that referenced this pull request Nov 7, 2022
…ubi 1.6.0

Fix #3744

- [ ] 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.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #3771 from zhouyifan279/3744.

Closes #3744

5876429 [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
554e889 [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
a6bfa3d [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
c90470f [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
fe55f4a [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
e262872 [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
ed5e8bd [zhouyifan279] [KYUUBI #3744][Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0

Authored-by: zhouyifan279 <zhouyifan279@gmail.com>
Signed-off-by: Cheng Pan <chengpan@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] InvalidACL appears in engine after upgrade to Kyuubi 1.6.0
4 participants