Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Invalid
    • Affects Version/s: 3.0.0
    • Fix Version/s: None
    • Component/s: security
    • Labels:
      None
    • Target Version/s:
    1. HADOOP-10799.patch
      117 kB
      Alejandro Abdelnur
    2. HADOOP-10799.patch
      113 kB
      Alejandro Abdelnur
    3. HADOOP-10799.patch
      113 kB
      Alejandro Abdelnur
    4. HADOOP-10799.patch
      112 kB
      Alejandro Abdelnur
    5. HADOOP-10799.patch
      112 kB
      Alejandro Abdelnur
    6. HADOOP-10799.patch
      81 kB
      Alejandro Abdelnur
    7. HADOOP-10799.patch
      80 kB
      Alejandro Abdelnur
    8. HADOOP-10799.patch
      80 kB
      Alejandro Abdelnur

      Issue Links

        Activity

        Hide
        Alejandro Abdelnur added a comment -

        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.

        Show
        Alejandro Abdelnur added a comment - 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.
        Hide
        Varun Vasudev added a comment -

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

        Show
        Varun Vasudev added a comment - Alejandro Abdelnur just curious - any particular reason for this or simply refactoring?
        Hide
        Alejandro Abdelnur added a comment -

        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)

        Show
        Alejandro Abdelnur added a comment - 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)
        Hide
        Varun Vasudev added a comment -

        Sorry I missed the parent ticket. My apologies.

        Show
        Varun Vasudev added a comment - Sorry I missed the parent ticket. My apologies.
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Alejandro Abdelnur added a comment -

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

        Show
        Alejandro Abdelnur added a comment - original patch was using InetAddress.getLoopbackAddress() method which is a JDK7 method, changed to use InetAddress.getLocalHost() .
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Alejandro Abdelnur added a comment -

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

        Show
        Alejandro Abdelnur added a comment - fixing javadoc and findbugs -1s. failing tests are unrelated.
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Alejandro Abdelnur added a comment -

        test failures are unrelated

        Show
        Alejandro Abdelnur added a comment - test failures are unrelated
        Hide
        Alejandro Abdelnur added a comment -

        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)
        Show
        Alejandro Abdelnur added a comment - 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)
        Hide
        Varun Vasudev added a comment -

        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.

        Show
        Varun Vasudev added a comment - 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.
        Hide
        Varun Vasudev added a comment -

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

        Show
        Varun Vasudev added a comment - I forgot to mention, there's a lot of similar code between YARN-2247 and YARN-2100 .
        Hide
        Varun Vasudev added a comment -

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

        Show
        Varun Vasudev added a comment - Sorry I hit send too soon, I meant similar code between YARN-2247 and the current TimelineServer delegation token auth.
        Hide
        Alejandro Abdelnur added a comment -

        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.

        Show
        Alejandro Abdelnur added a comment - 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.
        Hide
        Alejandro Abdelnur added a comment -

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

        Show
        Alejandro Abdelnur added a comment - new patch addressing my previous comments and adding several more testcases including kerberos ones using minikdc
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Alejandro Abdelnur added a comment -

        patch fixing the javac warnings

        Show
        Alejandro Abdelnur added a comment - patch fixing the javac warnings
        Hide
        Alejandro Abdelnur added a comment -

        test cases unrelated

        Show
        Alejandro Abdelnur added a comment - test cases unrelated
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Zhijie Shen added a comment -

        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.

        Show
        Zhijie Shen added a comment - 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.
        Hide
        Alejandro Abdelnur added a comment -

        adding a missing constructor to DelegationTokenAuthenticatedURL.

        Show
        Alejandro Abdelnur added a comment - adding a missing constructor to DelegationTokenAuthenticatedURL .
        Hide
        Alejandro Abdelnur added a comment -

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

        Show
        Alejandro Abdelnur added a comment - Zhijie Shen , thanks for pointing out HADOOP-10453 , I was not aware of it, I've just commented there.
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Alejandro Abdelnur added a comment -

        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

        Show
        Alejandro Abdelnur added a comment - 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
        Hide
        Alejandro Abdelnur added a comment -

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

        Show
        Alejandro Abdelnur added a comment - ok, found the issue, a commit from yesterday YARN-2233 , was overlapping POM changes with the patch, patch rebased.
        Hide
        Hadoop QA added a comment -

        -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.

        Show
        Hadoop QA added a comment - -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.
        Hide
        Alejandro Abdelnur added a comment -

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

        Show
        Alejandro Abdelnur added a comment - canceling patch as some minor tweaks seems to be required in the DT client API to make DT wiring transparent.
        Hide
        Daryn Sharp added a comment -

        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?

        Show
        Daryn Sharp added a comment - 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?
        Hide
        Alejandro Abdelnur added a comment -

        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:

        Show
        Alejandro Abdelnur added a comment - 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
        Hide
        Alejandro Abdelnur added a comment -

        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.

        Show
        Alejandro Abdelnur added a comment - Daryn Sharp , Varun Vasudev , on moving the token to a header. I think we can do it in a backwards compatible way: The new client side logic doing DT would use a header. The new client side logic doing DT would support a CONFIG switch to use querystring. 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.
        Hide
        Varun Vasudev added a comment -

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

        Show
        Varun Vasudev added a comment - Alejandro Abdelnur I'm fine with that. I've filed YARN-2290 for 3.
        Hide
        Aaron T. Myers added a comment -

        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.

        Show
        Aaron T. Myers added a comment - 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.
        Hide
        Alejandro Abdelnur added a comment -

        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.

        Show
        Alejandro Abdelnur added a comment - 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.
        Hide
        Daryn Sharp added a comment -

        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.

        Show
        Daryn Sharp added a comment - 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.
        Hide
        Alejandro Abdelnur added a comment -

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

        Show
        Alejandro Abdelnur added a comment - Daryn Sharp , created HADOOP-10880 for that, made it blocker.

          People

          • Assignee:
            Alejandro Abdelnur
            Reporter:
            Alejandro Abdelnur
          • Votes:
            0 Vote for this issue
            Watchers:
            9 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development