[
https://issues.apache.org/jira/browse/HIVE-2616?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13170726#comment-13170726
]
jirapos...@reviews.apache.org commented on HIVE-2616:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2975/#review3939
-----------------------------------------------------------
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/2975/#comment8885>
All properties that appear in HiveConf also need to appear in
conf/hive-default.xml.template along with a description.
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/2975/#comment8893>
I think it would make sense to change the name to
'hive.metastore.client.enable.setugi'. Also, I think this feature should be
disabled by default.
trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/2975/#comment8894>
Please add a new property hive.metastore.server.enable.setugi that allows
this RPC to be disabled on the server side, and set the default value to false.
trunk/metastore/if/hive_metastore.thrift
<https://reviews.apache.org/r/2975/#comment8895>
Please add a comment explaining what this call does.
trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java
<https://reviews.apache.org/r/2975/#comment8886>
When I apply your changes and run the thriftif ant target I see a small
diff in this file. Did you use Thrift 0.7.0 to generate these files?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment8887>
Indentation.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment8888>
So instead of checking the hive.metastore.sasl.enabled property we now just
check to see if we're using a security enabled shim, and if so assume that the
user wants to enable security? I don't think this is correct behavior since the
fact that we're using a secure version of Hadoop does not necessarily imply
that we actually have security enabled.
Also, it looks like this change deprecates the hive.metastore.sasl.enabled
configuration property. In line with my comment above I think it makes sense to
leave this property in, but if you do remove it then you need to release note
the change and remove this property from HiveConf and
conf/hive-default.xml.template.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment8896>
Indentation.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment8897>
We're initializing SASL even if isSecure=false?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
<https://reviews.apache.org/r/2975/#comment8889>
Formatting.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
<https://reviews.apache.org/r/2975/#comment8891>
Formatting: please add spaces.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
<https://reviews.apache.org/r/2975/#comment8898>
"new client talking to old metastore" seems to imply that we're able to
determine whether or not we're talking to an old server, which isn't true. In
reality, the onus is on the admin to ensure that both sides support this
feature. What happens if the client calls set_ugi(), but the server doesn't
support it? Is the error message helpful?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
<https://reviews.apache.org/r/2975/#comment8892>
Should this be "Failed to login to the MetaStore Server..."?
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
<https://reviews.apache.org/r/2975/#comment8901>
I think it's more accurate to say that the processor "*checks* to see if
the first call is to set_ugi()..." instead of saying that it *expects* the
first call to be to set_ugi().
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
<https://reviews.apache.org/r/2975/#comment8902>
+1 to referencing the THRIFT JIRA. I think the class comment should call
out that this is a temporary workaround cite a TODO.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
<https://reviews.apache.org/r/2975/#comment8899>
Formatting: '} else {'
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
<https://reviews.apache.org/r/2975/#comment8900>
There's a TAB here. Please remove.
trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java
<https://reviews.apache.org/r/2975/#comment8903>
Spacing.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java
<https://reviews.apache.org/r/2975/#comment8904>
Please change the name of this method to isSecurityEnabled(), or create a
new method with that name instead of modifying this method. The current name is
misleading and will cause confusion (e.g. my comments in HiveMetaStore).
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java
<https://reviews.apache.org/r/2975/#comment8905>
Spacing.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java
<https://reviews.apache.org/r/2975/#comment8906>
Spacing.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TUGIAssumingTransport.java
<https://reviews.apache.org/r/2975/#comment8907>
Missing ASF header.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TUGIAssumingTransport.java
<https://reviews.apache.org/r/2975/#comment8909>
If this is client-specific code then maybe the package should be
o.a.h.hive.thrift.client.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TUGIAssumingTransport.java
<https://reviews.apache.org/r/2975/#comment8908>
What's the point of these assert statements? Remove?
trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java
<https://reviews.apache.org/r/2975/#comment8910>
Please add javadoc for doAs() and createRemoteUser().
trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge.java
<https://reviews.apache.org/r/2975/#comment8911>
Spacing.
trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge.java
<https://reviews.apache.org/r/2975/#comment8912>
Spacing.
trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TFilterTransport.java
<https://reviews.apache.org/r/2975/#comment8913>
ASF header.
- Carl
On 2011-12-03 00:07:25, Ashutosh Chauhan wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2975/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-12-03 00:07:25)
bq.
bq.
bq. Review request for hive.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Pass user identity in metastore connection in unsecure mode
bq.
bq.
bq. This addresses bug HIVE-2616.
bq. https://issues.apache.org/jira/browse/HIVE-2616
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1209772
bq. trunk/metastore/if/hive_metastore.thrift 1209772
bq. trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.h 1209772
bq. trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore.cpp 1209772
bq.
trunk/metastore/src/gen/thrift/gen-cpp/ThriftHiveMetastore_server.skeleton.cpp
1209772
bq.
trunk/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java
1209772
bq.
trunk/metastore/src/gen/thrift/gen-php/hive_metastore/ThriftHiveMetastore.php
1209772
bq.
trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote
1209772
bq.
trunk/metastore/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py
1209772
bq. trunk/metastore/src/gen/thrift/gen-rb/thrift_hive_metastore.rb 1209772
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
1209772
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
1209772
bq.
trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
PRE-CREATION
bq. trunk/shims/ivy.xml 1209772
bq.
trunk/shims/src/0.20/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java
1209772
bq.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java
1209772
bq.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java
1209772
bq.
trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/TUGIAssumingTransport.java
PRE-CREATION
bq.
trunk/shims/src/common/java/org/apache/hadoop/hive/shims/HadoopShims.java
1209772
bq.
trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge.java
1209772
bq.
trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TFilterTransport.java
PRE-CREATION
bq.
trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java
PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/2975/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. All the tests in metastore dir passes. Manually tested that file on hdfs
is owned by user running the client and not by user running metastore server.
bq.
bq.
bq. Thanks,
bq.
bq. Ashutosh
bq.
bq.
> Passing user identity from metastore client to server in non-secure mode
> ------------------------------------------------------------------------
>
> Key: HIVE-2616
> URL: https://issues.apache.org/jira/browse/HIVE-2616
> Project: Hive
> Issue Type: Bug
> Components: Metastore
> Reporter: Ashutosh Chauhan
> Assignee: Ashutosh Chauhan
> Attachments: hive-2616.patch, hive-2616_1.patch, hive-2616_3.patch
>
>
> Currently in unsecure mode client don't pass on user identity. As a result
> hdfs and other operations done by server gets executed by user running
> metastore process instead of being done in context of client. This results in
> problem as reported here:
> http://mail-archives.apache.org/mod_mbox/hive-user/201111.mbox/%3CCAK0mCrRC3aPqtRHDe2J25Rm0JX6TS1KXxd7KPjqJjoqBjg=a...@mail.gmail.com%3E
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
