The code in this patch is 99% from httpfs, with minor changes to make it reusable.

we cannot have it in hadoop-auth because all the dependencies from common (configuration, delegationtokens, etc).

HADOOP-10800 will remove the DT code from httpfs and use this one.

Alejandro Abdelnur just curious - any particular reason for this or simply refactoring?

Varun Vasudev, as the parent JIRA states it is to be able to use the same code from multiple places in Hadoop (HttpFS, KMS, etc)

Sorry I missed the parent ticket. My apologies.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12654614/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 5 new or modified test files.

-1 javac. The patch appears to cause the build to fail.

Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4241//console

This message is automatically generated.

original patch was using InetAddress.getLoopbackAddress() method which is a JDK7 method, changed to use InetAddress.getLocalHost().

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12654926/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 5 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

-1 javadoc. The javadoc tool appears to have generated 1 warning messages.
See https://builds.apache.org/job/PreCommit-HADOOP-Build/4243//artifact/trunk/patchprocess/diffJavadocWarnings.txt for details.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

-1 findbugs. The patch appears to introduce 3 new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common:

org.apache.hadoop.fs.TestSymlinkLocalFSFileContext
org.apache.hadoop.ipc.TestIPC
org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4243//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4243//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4243//console

This message is automatically generated.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12654926/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 5 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

-1 javadoc. The javadoc tool appears to have generated 1 warning messages.
See https://builds.apache.org/job/PreCommit-HADOOP-Build/4244//artifact/trunk/patchprocess/diffJavadocWarnings.txt for details.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

-1 findbugs. The patch appears to introduce 3 new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common:

org.apache.hadoop.ipc.TestIPC
org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem
org.apache.hadoop.fs.TestSymlinkLocalFSFileContext

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4244//testReport/
Findbugs warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4244//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-common.html
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4244//console

This message is automatically generated.

fixing javadoc and findbugs -1s. failing tests are unrelated.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12654942/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 5 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common:

org.apache.hadoop.fs.TestSymlinkLocalFSFileContext
org.apache.hadoop.ipc.TestIPC
org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4245//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4245//console

This message is automatically generated.

test failures are unrelated

canceling patch, need to make the following changes:

• support DTs with pseudo authentication (per YARN-2228 TS seems to need that)
• use UGI current user on pseudo DT auth client side.
• There should be a way of providing a DelegationTokenSecretManager (ie to support a DTSM that uses a secret shared with other process)

Alejandro Abdelnur I'm not sure if this is the right ticket but it would be preferable to pass tokens as headers instead of URL parameters. URLs can get logged and passed on as part of the referrer header which exposes the delegation token. In addition, users can pass around links with delegation tokens by mistake. YARN-2247(waiting to be reviewed) also implements auth using delegation tokens for the RM web services but passes the tokens as a header. My plan was to file a ticket to shift the TimelineServer auth to the header model once YARN-2247 got committed. I'd be happy to hear your thoughts.

I forgot to mention, there's a lot of similar code between YARN-2247 and YARN-2100.

Sorry I hit send too soon, I meant similar code between YARN-2247 and the current TimelineServer delegation token auth.

Varun Vasudev, i would like to, but we cannot break backwards compatibility in webhdfs. And given that using a query string param for delegation token is part of the public REST API (WebHDFS) we should stick to that.

new patch addressing my previous comments and adding several more testcases including kerberos ones using minikdc

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12655305/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 4 new or modified test files.

-1 javac. The applied patch generated 1264 javac compiler warnings (more than the trunk's current 1260 warnings).

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common:

org.apache.hadoop.ipc.TestIPC
org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem
org.apache.hadoop.fs.TestSymlinkLocalFSFileContext

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4252//testReport/
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4252//artifact/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4252//console

This message is automatically generated.

patch fixing the javac warnings

test cases unrelated

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12655325/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 4 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common:

org.apache.hadoop.ipc.TestIPC
org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem
org.apache.hadoop.fs.TestSymlinkLocalFSFileContext

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4255//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4255//console

This message is automatically generated.

Hi Alejandro Abdelnur, I'm not sure whether you have noticed HADOOP-10453, which aims to default AuthenticatedURL. In this jira, on the contrary, you'd like to improve feature of http authentication, including AuthenticatedURL and it sub-classes. In HADOOP-10453 description, folks have mentioned the drawbacks of AuthenticatedURL. Would you mind share your insights about it?

Currently, YARN RM (YARN-1695) and Timeline web modules (YARN-1530) have adopted AuthenticatedURL, we may have to do the adjustment accordingly.

adding a missing constructor to DelegationTokenAuthenticatedURL.

Zhijie Shen, thanks for pointing out HADOOP-10453, I was not aware of it, I've just commented there.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12656034/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 4 new or modified test files.

-1 javac. The applied patch generated 1268 javac compiler warnings (more than the trunk's current 1260 warnings).

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common:

org.apache.hadoop.fs.TestSymlinkLocalFSFileContext
org.apache.hadoop.ipc.TestIPC
org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4291//testReport/
Javac warnings: https://builds.apache.org/job/PreCommit-HADOOP-Build/4291//artifact/trunk/patchprocess/diffJavacWarnings.txt
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4291//console

This message is automatically generated.

the javac warnings are warnings from Maven regarding the maven-jar-plugin defined twice (it does not seem to be the case). I'm not able to reproduce the warnings locally with Maven 3.0.4

ok, found the issue, a commit from yesterday YARN-2233, was overlapping POM changes with the patch, patch rebased.

-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12656090/HADOOP-10799.patch
against trunk revision .

+1 @author. The patch does not contain any @author tags.

+1 tests included. The patch appears to include 4 new or modified test files.

+1 javac. The applied patch does not increase the total number of javac compiler warnings.

+1 javadoc. There were no new javadoc warning messages.

+1 eclipse:eclipse. The patch built with eclipse:eclipse.

+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

+1 release audit. The applied patch does not increase the total number of release audit warnings.

-1 core tests. The patch failed these unit tests in hadoop-common-project/hadoop-auth hadoop-common-project/hadoop-common:

org.apache.hadoop.fs.TestSymlinkLocalFSFileSystem
org.apache.hadoop.fs.TestSymlinkLocalFSFileContext
org.apache.hadoop.ipc.TestIPC

+1 contrib tests. The patch passed contrib unit tests.

Test results: https://builds.apache.org/job/PreCommit-HADOOP-Build/4294//testReport/
Console output: https://builds.apache.org/job/PreCommit-HADOOP-Build/4294//console

This message is automatically generated.

canceling patch as some minor tweaks seems to be required in the DT client API to make DT wiring transparent.

I agree with Varun Vasudev that the token needs to be passed via headers, probably using digest auth just like sasl. Alejandro Abdelnur and I once talked offline about the insecurity of passing it in the url. With that said, do we really want to propagate the "bad behavior" of webhdfs into common for multiple components to use? We'll never be able to change w/o multiple incompatibilities, right?

Minor changes in the public API, removing some methods that don't need to be public. Additional testcases and some other minor improvements/clean up.

This is belongs to a string of patches built on top of each other:

• HADOOP-10817 proxyuser logic supporting custom prefix properties
• HADOOP-10799 HTTP delegation token built in client/server authentication classes
• HADOOP-10800 HttpFS using HADOOP-10799 and removing custom code
• HADOOP-10835 HTTP proxyuser logic built in client/server authentication classes
• HADOOP-10836 HttpFS using HADOOP-10835 and removing custom code
• HADOOP-10770 KMS delegation support using HADOOP-10799
• HADOOP-10698 KMS proxyuser support using HADOOP-10835

Daryn Sharp, Varun Vasudev, on moving the token to a header. I think we can do it in a backwards compatible way:

1. The new client side logic doing DT would use a header.
2. The new client side logic doing DT would support a CONFIG switch to use querystring.
3. The server logic doing DT will do header and fallback to querystring.

2 will allow a new client to interact with an old server.
3 will ensure the new server side DT handling works with older clients.

If you agree, I'll open a follow up JIRA to that change.

Alejandro Abdelnur I'm fine with that. I've filed YARN-2290 for 3.

This is pretty tough to review as it is, since most of this is renames. Tucu - would you mind please commenting which parts of this patch are mostly or entirely just a class rename, and which parts are net new and so warrant more careful review? Thanks a lot.

Aaron T. Myers, thanks for looking at it. I've refactored HADOOP-10799 & HADOOP-10800 patches into a script (moving/deleting files) and patch, I hope that makes the review easier. I'm attaching the script and patch in the parent JIRA, HADOOP-10771 as it combines the 2 tasks.

Haven't reviewed, but if you haven't already, can you please ensure that the ability to use a query string param is confined to webhdfs. If we allow it to spread to new services then we will never be able to remove it.

Daryn Sharp, created HADOOP-10880 for that, made it blocker.