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

[ZEPPELIN-1968] Added property to disable hive user impersonation #2051

Closed
wants to merge 4 commits into from

Conversation

@gfalcone
Copy link

gfalcone commented Feb 21, 2017

What is this PR for?

Added new property "hive.proxy.user" to disable hive impersonation (on some clusters, this option is disabled) in order to make Hive Interpreter even without this

What type of PR is it?

Feature

Todos

What is the Jira issue?

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

How should this be tested?

Set "hive.proxy.user" to true in the jdbc interpreter setttings, and you should see "Using hive proxy user" in the jdbc logs.

If "hive.proxy.user" has another value, this is not mentionned in the logs

You can also test with the appropriate hive configuration, but this could take longer :)

Screenshots (if appropriate)

Questions:

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

This comment has been minimized.

Copy link
Author

gfalcone commented Feb 22, 2017

CI passed all the tests : https://travis-ci.org/gfalcone/zeppelin

@prabhjyotsingh

This comment has been minimized.

Copy link
Contributor

prabhjyotsingh commented Feb 22, 2017

This looks OK to me, can you add a line note on https://github.com/apache/zeppelin/blob/master/docs/interpreter/jdbc.md#apache-hive to update the documentation.

Paolo Genissel
@gfalcone

This comment has been minimized.

Copy link
Author

gfalcone commented Feb 22, 2017

@prabhjyotsingh thank you for reviewing, just pushed the documentation :)

@@ -374,16 +374,22 @@ public Connection getConnection(String propertyKey, InterpreterContext interpret
if (lastIndexOfUrl == -1) {
lastIndexOfUrl = connectionUrl.length();
}
connectionUrl.insert(lastIndexOfUrl, ";hive.server2.proxy.user=" + user + ";");
if (!property.getProperty("hive.proxy.user").equals("false")){

This comment has been minimized.

Copy link
@prabhjyotsingh

prabhjyotsingh Feb 22, 2017

Contributor

Just realised, can you do a null check for this, if the property key doesn't exists, this will throw NPE.

@gfalcone

This comment has been minimized.

Copy link
Author

gfalcone commented Feb 22, 2017

@prabhjyotsingh right, didn't see that

@gfalcone

This comment has been minimized.

Copy link
Author

gfalcone commented Feb 27, 2017

@prabhjyotsingh is everything good for you :) ?

@prabhjyotsingh

This comment has been minimized.

Copy link
Contributor

prabhjyotsingh commented Feb 27, 2017

Yes, LGTM!

@prabhjyotsingh

This comment has been minimized.

Copy link
Contributor

prabhjyotsingh commented Mar 2, 2017

Merging this if no more discussion.

try {
ugi = UserGroupInformation.createProxyUser(user,
UserGroupInformation.getCurrentUser());
ugi = property.getProperty("hive.proxy.user").equals("false") ?

This comment has been minimized.

Copy link
@prabhjyotsingh

prabhjyotsingh Mar 3, 2017

Contributor

NPE check need to be applied here as well. Missed this last time.

Paolo Genissel
@gfalcone

This comment has been minimized.

Copy link
Author

gfalcone commented Mar 3, 2017

@prabhjyotsingh OK, so I deleted that line because I realized that we were in the condition where it's not the hive jdbc, so we don't have to worry about that.

@prabhjyotsingh

This comment has been minimized.

Copy link
Contributor

prabhjyotsingh commented Mar 6, 2017

Sure thank you @gfalcone, will merge this soon.

@gfalcone

This comment has been minimized.

Copy link
Author

gfalcone commented Mar 6, 2017

@prabhjyotsingh the CI passed !

@asfgit asfgit closed this in 79ace93 Mar 7, 2017
IceKhan13 added a commit to IceKhan13/zeppelin that referenced this pull request Mar 16, 2017
### What is this PR for?

Added new property "hive.proxy.user"  to disable hive impersonation (on some clusters, this option is disabled) in order to make Hive Interpreter even without this

### What type of PR is it?
Feature

### Todos

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

### How should this be tested?
Set "hive.proxy.user" to true in the jdbc interpreter setttings, and you should see "Using hive proxy user" in the jdbc logs.

If "hive.proxy.user" has another value, this is not mentionned in the logs

You can also test with the appropriate hive configuration, but this could take longer :)

### Screenshots (if appropriate)

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

Author: Paolo Genissel <paolo.genissel-monsallier@1000mercis.com>

Closes apache#2051 from gfalcone/hive_impersonation and squashes the following commits:

a39d11c [Paolo Genissel] Fixed last NPE
1f7f685 [Paolo Genissel] Fixed NPE when getting hive.proxy.user property
433eefb [Paolo Genissel] Added documentation for feature
d6f0c62 [Paolo Genissel] Added property to disable hive user impersonation
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Sep 1, 2017
### What is this PR for?

Added new property "hive.proxy.user"  to disable hive impersonation (on some clusters, this option is disabled) in order to make Hive Interpreter even without this

### What type of PR is it?
Feature

### Todos

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

### How should this be tested?
Set "hive.proxy.user" to true in the jdbc interpreter setttings, and you should see "Using hive proxy user" in the jdbc logs.

If "hive.proxy.user" has another value, this is not mentionned in the logs

You can also test with the appropriate hive configuration, but this could take longer :)

### Screenshots (if appropriate)

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

Author: Paolo Genissel <paolo.genissel-monsallier@1000mercis.com>

Closes apache#2051 from gfalcone/hive_impersonation and squashes the following commits:

a39d11c [Paolo Genissel] Fixed last NPE
1f7f685 [Paolo Genissel] Fixed NPE when getting hive.proxy.user property
433eefb [Paolo Genissel] Added documentation for feature
d6f0c62 [Paolo Genissel] Added property to disable hive user impersonation
prabhjyotsingh added a commit to prabhjyotsingh/zeppelin that referenced this pull request Oct 23, 2017
### What is this PR for?

Added new property "hive.proxy.user"  to disable hive impersonation (on some clusters, this option is disabled) in order to make Hive Interpreter even without this

### What type of PR is it?
Feature

### Todos

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

### How should this be tested?
Set "hive.proxy.user" to true in the jdbc interpreter setttings, and you should see "Using hive proxy user" in the jdbc logs.

If "hive.proxy.user" has another value, this is not mentionned in the logs

You can also test with the appropriate hive configuration, but this could take longer :)

### Screenshots (if appropriate)

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

Author: Paolo Genissel <paolo.genissel-monsallier@1000mercis.com>

Closes apache#2051 from gfalcone/hive_impersonation and squashes the following commits:

a39d11c [Paolo Genissel] Fixed last NPE
1f7f685 [Paolo Genissel] Fixed NPE when getting hive.proxy.user property
433eefb [Paolo Genissel] Added documentation for feature
d6f0c62 [Paolo Genissel] Added property to disable hive user impersonation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.