Skip to content

Conversation

@jongyoul
Copy link
Member

@jongyoul jongyoul commented Aug 2, 2016

What is this PR for?

Enabling each user to run same interpreter.

What type of PR is it?

[Improvement]

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1210

How should this be tested?

  1. Enable shiro to use authentication mode
  2. Check per user in your interpreter tab
  3. Run different paragraphs with different users
    1. Run %spark sc.version, you will see the two res0: ... in your paragraphs

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@echarles
Copy link
Member

echarles commented Aug 2, 2016

Quickly went through the changes, but not sure to get it.
In which way is this related to the various intepreter modes we already have?
In which way is this related to ZEPPELIN-1000 (Multiple simultaneous users on a single WEB server)?

# To use a different strategy (LDAP / Database / ...) check the shiro doc at http://shiro.apache.org/configuration.html#Configuration-INISections
admin = password
admin = password, admin
user1 = user1, role1
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should revert this file.

Copy link
Member Author

Choose a reason for hiding this comment

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

@prabhjyotsingh It's not affected actually because the default option for security is none. It would be affected if /** = authc is activated but there's no test for it.

@jongyoul
Copy link
Member Author

jongyoul commented Aug 3, 2016

@echarles For now, Zeppelin supports per note option but doesn't do per user. If two different users log in Zeppelin and want to use same spark interpreter with same note, they cannot run a paragraph simultaneously. This PR enable that feature. Concerning ZEPPELIN-1000, I don't know the meaning of the management of single user exactly. AFAIK, Zeppelin manages multiple users at the same time. Could you please describe in details? I think you'd better answer the moon's statement.

@jongyoul
Copy link
Member Author

jongyoul commented Aug 8, 2016

@jongyoul jongyoul closed this Aug 8, 2016
@jongyoul jongyoul reopened this Aug 8, 2016
@bzz
Copy link
Member

bzz commented Aug 11, 2016

@jongyoul this looks very interesting! Could you plz help me to understand - does this changes mean for Zeepelin to run a new separate interpreter process for every user and schedulle only his jobs to be executed there?

@zjffdu
Copy link
Contributor

zjffdu commented Aug 11, 2016

@jongyoul Does that mean the login user is the process owner ? Because it matters for security reason.
e.g.

  • For spark interpreter, it would be better to launch the yarn app as the user who login in zeppelin.
  • For shell interpreter, it would be better to launch the shell as the user who login in zeppelin. Otherwise it is pretty dangerous. The user can delete all the files owner by others.

BTW, could you write a simple design doc, as the PR is pretty large, not easy to review without a design doc.

@jongyoul
Copy link
Member Author

@bzz If different users run a same interpreter with 'isolated', Zeppelin runs multiple process for that interpreters, and with 'scoped', Zeppelin runs a single process for it but the users feel like a separate process because Zeppelin interpreter use different class loader per user. For instance, if some users use SparkInterpreter with 'isolated' and 'scoped', all users look like running each SparkInterpreter.

@jongyoul
Copy link
Member Author

@zjffdu This is a first step for resolving security issues. For the next step, I'll pass some properties for identifying user. BTW, InterpreterContext includes user information, actually, thus it might not be a big change. :-)

@sourav-mazumder
Copy link

@jongyoul
I suggest while we enable this feature we also enable that user can access only the data what he/she is authorized to.

For example if there is a folder/table in HDFS/FileServer/Hive which user is trying to access he/she should not be able to access that unless he/she is authorized for the same.

To enable this, while starting the spark-submit, Zepplein needs to ensure that it is started with the current user mode (the Shiro user).

Regards,
Sourav

@zjffdu
Copy link
Contributor

zjffdu commented Aug 16, 2016

@sourav-mazumder 's suggestion is about another point of multiple user support, this is might not be a trivial task to implement in this PR. I think we have a discussion about the multiple-user support for zeppelin in the mail list recently. There's lots of works to do, I will create a umbrella ticket for that so that we can have a more clear whole picture of that.

@zjffdu
Copy link
Contributor

zjffdu commented Aug 17, 2016

Create umbrella ticket for multiple user support.
https://issues.apache.org/jira/browse/ZEPPELIN-1337

@jongyoul
Copy link
Member Author

Merging if there's no more discussion

@Leemoonsoo
Copy link
Member

Leemoonsoo commented Aug 21, 2016

@jongyoul Thanks for the great work. I tested this PR a bit and it works as expected. However, when i remove a Note, (with "perNote" checked), i think removal of Interpreter Process (isolated mode) or Interpreter Instance (scoped mode) related to that Note is expected. However this PR does not remove them.

The same interpreter process/instance removal should happen when user unbind interpreter setting from Note.

Could you check these cases?

@jongyoul
Copy link
Member Author

@Leemoonsoo Thanks for the review. I'll check that.

@jongyoul jongyoul closed this Aug 23, 2016
@jongyoul jongyoul reopened this Aug 23, 2016
@jongyoul jongyoul force-pushed the ZEPPELIN-1210 branch 3 times, most recently from 2c0d69e to 2f1f6cb Compare August 29, 2016 04:25
@jongyoul
Copy link
Member Author

@Leemoonsoo I've fixed it. Check it please. This is because remoteProcess doesn't destroy correctly.

@zjffdu
Copy link
Contributor

zjffdu commented Sep 2, 2016

@jongyoul I don't see the per user in the interpreter page. Do I miss anything ?
image

@jongyoul
Copy link
Member Author

jongyoul commented Sep 2, 2016

It looks looks like a browser cache issue. Can you clean cache and try it
again?

On Friday, 2 September 2016, Jeff Zhang [email protected] wrote:

@jongyoul https://github.com/jongyoul I don't see the per user in the
interpreter page. Do I miss anything ?
[image: image]
https://cloud.githubusercontent.com/assets/164491/18191192/03daf4ba-70fb-11e6-9753-03a81a4bbd75.png


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1265 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ADcfltXnJyMjMfmsoNqDPfBlE5pOO4Z3ks5ql470gaJpZM4Jauiv
.

이종열, Jongyoul Lee, 李宗烈
http://madeng.net

@zjffdu
Copy link
Contributor

zjffdu commented Sep 2, 2016

Thanks @jongyoul after clean cache, I can see the per user. But unfortunately, two users still share the same interpreter. Here's what I see. I use yarn-client mode and only see one yarn app is launched.

image
image
image
image

@cloverhearts
Copy link
Member

cloverhearts commented Oct 19, 2016

Tested.
I was tested for spark, jdbc, markdown, file and other interpreters.
It's working well. (per user/ per note/ globally)

@zjffdu
Copy link
Contributor

zjffdu commented Oct 19, 2016

@jongyoul my mistake, it works now.

@jongyoul
Copy link
Member Author

@cloverhearts Thank you!!

@jongyoul
Copy link
Member Author

jongyoul commented Oct 19, 2016

CI become green and @cloverhearts tested almost cases. Merging if there's no more discussion.

@asfgit asfgit closed this in 908b2a7 Oct 20, 2016
darionyaphet pushed a commit to darionyaphet/zeppelin that referenced this pull request Oct 27, 2016
### What is this PR for?
Enabling each user to run same interpreter.

### What type of PR is it?
[Improvement]

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1210

### How should this be tested?
1. Enable shiro to use authentication mode
1. Check `per user` in your interpreter tab
1. Run different paragraphs with different users
  1. Run `%spark sc.version`, you will see the two `res0: ...` in your paragraphs

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: CloverHearts <[email protected]>
Author: Jongyoul Lee <[email protected]>

Closes apache#1265 from jongyoul/ZEPPELIN-1210 and squashes the following commits:

48a0d8e [Jongyoul Lee] Fixed ZEPPELIN-1542 Fixed flaky test
e84703d [Jongyoul Lee] Fixed ZEPPELIN-1542
ad80951 [Jongyoul Lee] Fixed some wrong logic of getInterpreterInstanceKey
cee39f4 [Jongyoul Lee] Fixed to pass shiro information to InterpreterFactory from ZeppelinServer
5e7da34 [Jongyoul Lee] Changed instanceKey and processKey for dealing with new UI
d201950 [CloverHearts] fix eqeqeq issue for frontweb
b18bff4 [CloverHearts] implement frontend for interpreter per user mode and misc mode
1f64e52 [CloverHearts] change default value for pernote and peruser
787a366 [CloverHearts] change Back-end test cases and member type (perNote, perUser)
8586e1f [CloverHearts] change ui for interpreter running Per x mode
0b5d671 [Jongyoul Lee] Fixed the style
960bde1 [Jongyoul Lee] Removed SecurityUtils.getPrincipal Added fromMessage.principal to get right user
01c7cf1 [Jongyoul Lee] Fixed NPE while testing ZeppelinSparkClusterTest
1fb50ab [Jongyoul Lee] Fixed NPE while testing ZeppelinSparkClusterTest
d1c4344 [Jongyoul Lee] Fixed getEditorSetting for having users' info
12a27db [Jongyoul Lee] Fixed test after rebase
510942b [Jongyoul Lee] Fixed test after rebase
cb66946 [Jongyoul Lee] Fixed test after rebase
18b39bd [Jongyoul Lee] Fixed test after rebase
daa634f [Jongyoul Lee] Fixed some tests after rebase
ed558be [Jongyoul Lee] Fixed some tests after rebase
fa7fccb [Jongyoul Lee] Fixed destroying process of remoteInterpreterProcess
0a73241 [Jongyoul Lee] Fixed conflict while rebasing.
df423d3 [Jongyoul Lee] Fixed NotebookRestApiTest
b151366 [Jongyoul Lee] Fixed some codes after rebase
a32afd7 [Jongyoul Lee] Fixed some tests
7b7eb78 [Jongyoul Lee] Fixed some tests
47cc668 [Jongyoul Lee] Fixed tests to use AuthenticationInfo
012cf99 [Jongyoul Lee] Fixed some mismatch after rebase
9a03d40 [Jongyoul Lee] Reverted some value to default ones
8589545 [Jongyoul Lee] Added option in UI
ccbedc1 [Jongyoul Lee] WIP
94dfed2 [Jongyoul Lee] WIP
6480d1d [Jongyoul Lee] resolved conflicts
pedrozatta pushed a commit to pedrozatta/zeppelin that referenced this pull request Oct 27, 2016
### What is this PR for?
Enabling each user to run same interpreter.

### What type of PR is it?
[Improvement]

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1210

### How should this be tested?
1. Enable shiro to use authentication mode
1. Check `per user` in your interpreter tab
1. Run different paragraphs with different users
  1. Run `%spark sc.version`, you will see the two `res0: ...` in your paragraphs

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No

Author: CloverHearts <[email protected]>
Author: Jongyoul Lee <[email protected]>

Closes apache#1265 from jongyoul/ZEPPELIN-1210 and squashes the following commits:

48a0d8e [Jongyoul Lee] Fixed ZEPPELIN-1542 Fixed flaky test
e84703d [Jongyoul Lee] Fixed ZEPPELIN-1542
ad80951 [Jongyoul Lee] Fixed some wrong logic of getInterpreterInstanceKey
cee39f4 [Jongyoul Lee] Fixed to pass shiro information to InterpreterFactory from ZeppelinServer
5e7da34 [Jongyoul Lee] Changed instanceKey and processKey for dealing with new UI
d201950 [CloverHearts] fix eqeqeq issue for frontweb
b18bff4 [CloverHearts] implement frontend for interpreter per user mode and misc mode
1f64e52 [CloverHearts] change default value for pernote and peruser
787a366 [CloverHearts] change Back-end test cases and member type (perNote, perUser)
8586e1f [CloverHearts] change ui for interpreter running Per x mode
0b5d671 [Jongyoul Lee] Fixed the style
960bde1 [Jongyoul Lee] Removed SecurityUtils.getPrincipal Added fromMessage.principal to get right user
01c7cf1 [Jongyoul Lee] Fixed NPE while testing ZeppelinSparkClusterTest
1fb50ab [Jongyoul Lee] Fixed NPE while testing ZeppelinSparkClusterTest
d1c4344 [Jongyoul Lee] Fixed getEditorSetting for having users' info
12a27db [Jongyoul Lee] Fixed test after rebase
510942b [Jongyoul Lee] Fixed test after rebase
cb66946 [Jongyoul Lee] Fixed test after rebase
18b39bd [Jongyoul Lee] Fixed test after rebase
daa634f [Jongyoul Lee] Fixed some tests after rebase
ed558be [Jongyoul Lee] Fixed some tests after rebase
fa7fccb [Jongyoul Lee] Fixed destroying process of remoteInterpreterProcess
0a73241 [Jongyoul Lee] Fixed conflict while rebasing.
df423d3 [Jongyoul Lee] Fixed NotebookRestApiTest
b151366 [Jongyoul Lee] Fixed some codes after rebase
a32afd7 [Jongyoul Lee] Fixed some tests
7b7eb78 [Jongyoul Lee] Fixed some tests
47cc668 [Jongyoul Lee] Fixed tests to use AuthenticationInfo
012cf99 [Jongyoul Lee] Fixed some mismatch after rebase
9a03d40 [Jongyoul Lee] Reverted some value to default ones
8589545 [Jongyoul Lee] Added option in UI
ccbedc1 [Jongyoul Lee] WIP
94dfed2 [Jongyoul Lee] WIP
6480d1d [Jongyoul Lee] resolved conflicts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants