You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "Mingliang Liu (JIRA)" <ji...@apache.org> on 2017/03/04 00:28:45 UTC
[jira] [Comment Edited] (HADOOP-13945) Azure: Add Kerberos and
Delegation token support to WASB client.
[ https://issues.apache.org/jira/browse/HADOOP-13945?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15895301#comment-15895301 ]
Mingliang Liu edited comment on HADOOP-13945 at 3/4/17 12:28 AM:
-----------------------------------------------------------------
Patch looks good to me overall. For a few places using UGI, I need 2nd opinion. [~stevel@apache.org] I'll hold on commit 3 days in case you'd like to review.
# {{fs.azure.authorization.remote.service.url}} should be a separate constant final variable as well as a config key (e.g. in core-default.xml)?
# In code to log a message, let's keep the exception in log message. e.g.
{code:title=RemoteSASKeyGeneratorImpl#initialize()}
LOG.error("Error in fetching the WASB delegation token");
{code}
# I see at least two places using the logic of finding the expected token,
{code}
114 Iterator<Token<? extends TokenIdentifier>> tokenIterator = null;
115 try {
116 tokenIterator = UserGroupInformation.getCurrentUser().getCredentials()
117 .getAllTokens().iterator();
118 while (tokenIterator.hasNext()) {
119 Token<? extends TokenIdentifier> iteratedToken = tokenIterator.next();
120 if (iteratedToken.getKind().equals(WasbDelegationTokenIdentifier.TOKEN_KIND)) {
121 delegationToken = iteratedToken.encodeToUrlString();
122 }
123 }
{code}
Can we use {{UserGroupInformation.getCurrentUser().getCredentials().getToken(WasbDelegationTokenIdentifier.TOKEN_KIND)}}? We have to test this.
# {{if (isSecurityEnabled && (delegationToken != null && !delegationToken.isEmpty()))}}. This is a nit. We don't need {{()}} for && right?
# {{package-info.java}} is preferred to {{package.html}} since Java 5.
# I see a few duplicate code, can we create a helper method for that? e.g.
{code}
94 final UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
95 UserGroupInformation connectUgi = ugi.getRealUser();
96 final UserGroupInformation proxyUser = connectUgi;
97 if (connectUgi == null) {
98 connectUgi = ugi;
99 }
100 if(!connectUgi.hasKerberosCredentials()){
101 connectUgi = UserGroupInformation.getLoginUser();
102 }
{code}
and
{code}
72 final String credServiceUrl = conf.get(Constants.KEY_CRED_SERVICE_URL,
73 String.format("http://%s:%s",
74 InetAddress.getLocalHost().getCanonicalHostName(),
75 Constants.DEFAULT_CRED_SERVICE_PORT));
{code}
# For unit tests, we should add more assertion with clear error message if the test fails.
# {{ContractTestUtils}} has a few helper methods that can be used. Please consider that.
# For unit tests, we need to use paths which are unique for the test suite; using the working path doesn't do that. We need this for parallel test execution. See Steve's comment in [HADOOP-13930].
# test path must be robustly deleted in teardown, e.g. write a @After tearDown() method to clean up the parent directory. This method will be executed if the test fails with exception.
# try-catch and rethrow exception is not ideal. Fail with clearer error message will be better.
was (Author: liuml07):
Patch looks good to me overall. For a few places using UGI, I need 2nd opinion. [~stevel@apache.org] I'll hold on commit 3 days in case you'd like to review.
# {{fs.azure.authorization.remote.service.url}} should be a separate constant final variable as well as a config key (e.g. in core-default.xml)?
# In code to log a message, let's keep the exception in log message. e.g.
{code:title=RemoteSASKeyGeneratorImpl#initialize()}
LOG.error("Error in fetching the WASB delegation token");
{code}
# I see at least two places using the logic of finding the expected token,
{code}
114 Iterator<Token<? extends TokenIdentifier>> tokenIterator = null;
115 try {
116 tokenIterator = UserGroupInformation.getCurrentUser().getCredentials()
117 .getAllTokens().iterator();
118 while (tokenIterator.hasNext()) {
119 Token<? extends TokenIdentifier> iteratedToken = tokenIterator.next();
120 if (iteratedToken.getKind().equals(WasbDelegationTokenIdentifier.TOKEN_KIND)) {
121 delegationToken = iteratedToken.encodeToUrlString();
122 }
123 }
{code}
Can we use {{UserGroupInformation.getCurrentUser().getCredentials().getToken(WasbDelegationTokenIdentifier.TOKEN_KIND)}}? We have to test this.
# {{if (isSecurityEnabled && (delegationToken != null && !delegationToken.isEmpty()))}}. This is a nit. We don't need {{()}} for && right?
# {{package-info.java}} is preferred to {{package.html}} since Java 5.
# I see a few duplicate code, can we create a helper method for that? e.g.
{code}
94 final UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
95 UserGroupInformation connectUgi = ugi.getRealUser();
96 final UserGroupInformation proxyUser = connectUgi;
97 if (connectUgi == null) {
98 connectUgi = ugi;
99 }
100 if(!connectUgi.hasKerberosCredentials()){
101 connectUgi = UserGroupInformation.getLoginUser();
102 }
{code}
and
{code}
72 final String credServiceUrl = conf.get(Constants.KEY_CRED_SERVICE_URL,
73 String.format("http://%s:%s",
74 InetAddress.getLocalHost().getCanonicalHostName(),
75 Constants.DEFAULT_CRED_SERVICE_PORT));
{code}
> Azure: Add Kerberos and Delegation token support to WASB client.
> ----------------------------------------------------------------
>
> Key: HADOOP-13945
> URL: https://issues.apache.org/jira/browse/HADOOP-13945
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs/azure
> Affects Versions: 2.8.0
> Reporter: Santhosh G Nayak
> Assignee: Santhosh G Nayak
> Attachments: HADOOP-13945.1.patch, HADOOP-13945.2.patch, HADOOP-13945.3.patch, HADOOP-13945.4.patch, HADOOP-13945.5.patch
>
>
> Current implementation of Azure storage client for Hadoop ({{WASB}}) does not support Kerberos Authentication and FileSystem authorization, which makes it unusable in secure environments with multi user setup.
> To make {{WASB}} client more suitable to run in Secure environments, there are 2 initiatives under way for providing the authorization (HADOOP-13930) and fine grained access control (HADOOP-13863) support.
> This JIRA is created to add Kerberos and delegation token support to {{WASB}} client to fetch Azure Storage SAS keys (from Remote service as discussed in HADOOP-13863), which provides fine grained timed access to containers and blobs.
> For delegation token management, the proposal is it use the same REST service which being used to generate the SAS Keys.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org