Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3113

httpfs does not support delegation tokens

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.23.3
    • Fix Version/s: 2.0.2-alpha
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      httpfs does not support calls to get/renew tokens nor delegation token authentication.

      1. HDFS-3113.patch
        129 kB
        Alejandro Abdelnur
      2. HDFS-3113.patch
        139 kB
        Alejandro Abdelnur
      3. HDFS-3113.patch
        139 kB
        Alejandro Abdelnur
      4. HDFS-3113.patch
        141 kB
        Alejandro Abdelnur
      5. HDFS-3113.patch
        175 kB
        Alejandro Abdelnur
      6. HDFS-3113.patch
        176 kB
        Alejandro Abdelnur
      7. HDFS-3113.patch
        176 kB
        Alejandro Abdelnur

        Issue Links

          Activity

          Hide
          Alejandro Abdelnur added a comment -

          patch introduces a testKerberos profile in hadoop0-httpfs POM to test delegation tokens with Kerberos ON.

          Show
          Alejandro Abdelnur added a comment - patch introduces a testKerberos profile in hadoop0-httpfs POM to test delegation tokens with Kerberos ON.
          Hide
          Daryn Sharp added a comment -

          Please give me a little time to review to ensure this doesn't affect host-based tokens.

          Show
          Daryn Sharp added a comment - Please give me a little time to review to ensure this doesn't affect host-based tokens.
          Hide
          Alejandro Abdelnur added a comment -

          This patch adds tests to verify things works in doAs calls and in proxyuser doAs calls with Kerberos ON.

          Show
          Alejandro Abdelnur added a comment - This patch adds tests to verify things works in doAs calls and in proxyuser doAs calls with Kerberos ON.
          Hide
          Hadoop QA added a comment -

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

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

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

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2597//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/12531002/HDFS-3113.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 new or modified test files. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2597//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          A lot of the patch doesn't apply, so could you please update to make reviewing easier?

          Show
          Daryn Sharp added a comment - A lot of the patch doesn't apply, so could you please update to make reviewing easier?
          Hide
          Alejandro Abdelnur added a comment -

          @Daryn, please se the requires linked JIRAs, you'll have to apply those patches first. thx

          Show
          Alejandro Abdelnur added a comment - @Daryn, please se the requires linked JIRAs, you'll have to apply those patches first. thx
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk-Commit #2463 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2463/)
          Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602)

          Result = SUCCESS
          tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2463 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2463/ ) Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602) Result = SUCCESS tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #2394 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2394/)
          Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602)

          Result = SUCCESS
          tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2394 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2394/ ) Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602) Result = SUCCESS tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #2413 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2413/)
          Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602)

          Result = FAILURE
          tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2413 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2413/ ) Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602) Result = FAILURE tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Hdfs-trunk #1090 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1090/)
          Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602)

          Result = FAILURE
          tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1090 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1090/ ) Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602) Result = FAILURE tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #1123 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1123/)
          Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602)

          Result = FAILURE
          tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602
          Files :

          • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1123 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1123/ ) Amendment to previous commit, correct JIRA is HDFS-3113 (Revision 1354602) Result = FAILURE tucu : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1354602 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
          Hide
          Alejandro Abdelnur added a comment -

          reuploading same patch for our friend jenkins to take notice.

          Show
          Alejandro Abdelnur added a comment - reuploading same patch for our friend jenkins to take notice.
          Hide
          Hadoop QA added a comment -

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

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

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs-httpfs.html
          Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//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/12533915/HDFS-3113.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 new or modified test files. -1 javac. The applied patch generated 2083 javac compiler warnings (more than the trunk's current 2070 warnings). -1 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs-httpfs.html Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2722//console This message is automatically generated.
          Hide
          Alejandro Abdelnur added a comment -

          the findbugs warning of lack of synchronization is in a method used for test initialization, synchronizing is not necessary and it does not make sense as it would no work due to how the getAuthority() method works. I'll add the findbug exclusion at commit time.

          Show
          Alejandro Abdelnur added a comment - the findbugs warning of lack of synchronization is in a method used for test initialization, synchronizing is not necessary and it does not make sense as it would no work due to how the getAuthority() method works. I'll add the findbug exclusion at commit time.
          Hide
          Daryn Sharp added a comment -

          This is a big patch, so here's what I've immediately noticed:

          The uri should be constructed as URI.create(uri.getScheme()+"://"+uri.getAuthority()) in case it doesn't include the default port. Otherwise the current patch I think will cause -1 to be in the uri.

          The httpFSAddr (used for the token service) needs to be based on getCanonicalUri which adds the default port if necessary. Similarly, getDefaultPort should be implemented.

          A TokenRenewer is needed that is registered for the service loader.

          Show
          Daryn Sharp added a comment - This is a big patch, so here's what I've immediately noticed: The uri should be constructed as URI.create(uri.getScheme()+"://"+uri.getAuthority()) in case it doesn't include the default port. Otherwise the current patch I think will cause -1 to be in the uri. The httpFSAddr (used for the token service) needs to be based on getCanonicalUri which adds the default port if necessary. Similarly, getDefaultPort should be implemented. A TokenRenewer is needed that is registered for the service loader.
          Hide
          Alejandro Abdelnur added a comment -

          Thanks Daryn. The new patch takes care of your first 2 comments.

          Regarding the token renewal, yes I know that is missing from the HttpFSFileSystem implementation. However, keep in mind this FileSystem implementation is not used by clients, clients use the WebHdfsFileSystem implementation against HttpFS. The HttpFSFileSystem implementation is used by the HttpFSServer as well as for the testcases. This could change when we unify the codebase of webhdfs and httpfs.

          Show
          Alejandro Abdelnur added a comment - Thanks Daryn. The new patch takes care of your first 2 comments. Regarding the token renewal, yes I know that is missing from the HttpFSFileSystem implementation. However, keep in mind this FileSystem implementation is not used by clients, clients use the WebHdfsFileSystem implementation against HttpFS. The HttpFSFileSystem implementation is used by the HttpFSServer as well as for the testcases. This could change when we unify the codebase of webhdfs and httpfs.
          Hide
          Hadoop QA added a comment -

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

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

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

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

          -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) 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-common hadoop-hdfs-project/hadoop-hdfs-httpfs:

          org.apache.hadoop.fs.viewfs.TestViewFsTrash

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs-httpfs.html
          Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//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/12534076/HDFS-3113.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 11 new or modified test files. -1 javac. The applied patch generated 2083 javac compiler warnings (more than the trunk's current 2070 warnings). -1 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) 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-common hadoop-hdfs-project/hadoop-hdfs-httpfs: org.apache.hadoop.fs.viewfs.TestViewFsTrash +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs-httpfs.html Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//artifact/trunk/trunk/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2725//console This message is automatically generated.
          Hide
          Daryn Sharp added a comment -

          I think the new patch addressed the few token service issues I cited. I don't feel comfortable though reviewing the entire patch, so I'd like someone else to help out.

          keep in mind this FileSystem implementation is not used by clients, clients use the WebHdfsFileSystem implementation against HttpFS.

          Would you please elaborate further on why a token renewer isn't needed? If a job is using web hdfs, and tries to renew the web hdfs token, doesn't that mean web hdfs needs to in turn renew the httpfs token? Or how do web and http fs and their tokens interact?

          BTW, Tom added a really nifty Token#decodeIdentifier method that you can use.

          Show
          Daryn Sharp added a comment - I think the new patch addressed the few token service issues I cited. I don't feel comfortable though reviewing the entire patch, so I'd like someone else to help out. keep in mind this FileSystem implementation is not used by clients, clients use the WebHdfsFileSystem implementation against HttpFS. Would you please elaborate further on why a token renewer isn't needed? If a job is using web hdfs, and tries to renew the web hdfs token, doesn't that mean web hdfs needs to in turn renew the httpfs token? Or how do web and http fs and their tokens interact? BTW, Tom added a really nifty Token#decodeIdentifier method that you can use.
          Hide
          Alejandro Abdelnur added a comment -

          Would you please elaborate ...

          If a client uses webhdfs:// they'll be using WebHdfsFileSystem class, the HttpFSFileSystem class is not avail for clients. WebHDFSFileSystem usas the token renewer, and that one will kick in. The WebHdfsFileSystem client will get a token from HttpFS server and will renew it against HttpFS. It is not the intention for a token from WebHdfs server to be used against HttpFS server or viceversa. Does this address your concern?

          I'll look at the Token#decoderIndentifier, thx

          Show
          Alejandro Abdelnur added a comment - Would you please elaborate ... If a client uses webhdfs:// they'll be using WebHdfsFileSystem class, the HttpFSFileSystem class is not avail for clients. WebHDFSFileSystem usas the token renewer, and that one will kick in. The WebHdfsFileSystem client will get a token from HttpFS server and will renew it against HttpFS. It is not the intention for a token from WebHdfs server to be used against HttpFS server or viceversa. Does this address your concern? I'll look at the Token#decoderIndentifier, thx
          Hide
          Alejandro Abdelnur added a comment -

          @Daryn, I'm looking at the Token#decodeIdentifier, but I'm sure where I can use that method instead of what the patch is currently doing. Would you please elaborate? thx

          Show
          Alejandro Abdelnur added a comment - @Daryn, I'm looking at the Token#decodeIdentifier, but I'm sure where I can use that method instead of what the patch is currently doing. Would you please elaborate? thx
          Hide
          Eli Collins added a comment -

          Hey Tucu,

          Overall looks good, only minor comments follow. What testing have you done with these patches, confirmed distcp against httpfs with kerberos enabled now works?

          pom.xml

          • Intend to comment out <forkMode> always?

          ServerWebApp.java

          • Annotate setAuthority VisibleForTesting?

          HttpFSKerberosAuthenticator

          • Why wrap AuthenticationException in IOEs for the token methods?

          DelegationTokenManagerException

          • This class should be public / stable right?

          HttpFSUtils

          • Use HttpFSFileSystem.SCHEME

          HttpFSKerberosAuthenticationHandler

          • The last part of the authenticate javadoc is not filled out

          NetUtils

          • The diff is a nop
          Show
          Eli Collins added a comment - Hey Tucu, Overall looks good, only minor comments follow. What testing have you done with these patches, confirmed distcp against httpfs with kerberos enabled now works? pom.xml Intend to comment out <forkMode> always? ServerWebApp.java Annotate setAuthority VisibleForTesting? HttpFSKerberosAuthenticator Why wrap AuthenticationException in IOEs for the token methods? DelegationTokenManagerException This class should be public / stable right? HttpFSUtils Use HttpFSFileSystem.SCHEME HttpFSKerberosAuthenticationHandler The last part of the authenticate javadoc is not filled out NetUtils The diff is a nop
          Hide
          Daryn Sharp added a comment -

          Just to make sure I understand: WebHDFS client -> HttpFS -> SomeFS. Is HttpFS acting like a pure proxy and relaying the real token for SomeFS back to the client? If so, what's the purpose of the secret manager in httpfs?

          Regarding token decoding, this:

          ByteArrayInputStream buf = new ByteArrayInputStream(token.getIdentifier());
          DataInputStream dis = new DataInputStream(buf);
          DelegationTokenIdentifier id = new DelegationTokenIdentifier();
          id.readFields(dis);
          dis.close();

          can be replaced with DelegationTokenIdentifier id = token.decodeIdentifier()

          BTW, have you tested this with oozie to ensure proxy tokens work correctly?

          Show
          Daryn Sharp added a comment - Just to make sure I understand: WebHDFS client -> HttpFS -> SomeFS. Is HttpFS acting like a pure proxy and relaying the real token for SomeFS back to the client? If so, what's the purpose of the secret manager in httpfs? Regarding token decoding, this: ByteArrayInputStream buf = new ByteArrayInputStream(token.getIdentifier()); DataInputStream dis = new DataInputStream(buf); DelegationTokenIdentifier id = new DelegationTokenIdentifier(); id.readFields(dis); dis.close(); can be replaced with DelegationTokenIdentifier id = token.decodeIdentifier() BTW, have you tested this with oozie to ensure proxy tokens work correctly?
          Hide
          Alejandro Abdelnur added a comment -

          @eli, thanks for the review. I'll address your comments today or tomorrow.
          @daryn, HttpFS issues it own delegation tokens, and they can be used only against the HttpFs service. Hope this clarifies.

          Show
          Alejandro Abdelnur added a comment - @eli, thanks for the review. I'll address your comments today or tomorrow. @daryn, HttpFS issues it own delegation tokens, and they can be used only against the HttpFs service. Hope this clarifies.
          Hide
          Alejandro Abdelnur added a comment -

          New patch addressing Eli's feedback, except for:

          HttpFSKerberosAuthenticator. Why wrap AuthenticationException in IOEs for the token methods?

          This ends up used by FileSystem methods which throw only IOExceptions.

          DelegationTokenManagerException This class should be public / stable right?

          Not really, all this is HttpFS internal.

          Show
          Alejandro Abdelnur added a comment - New patch addressing Eli's feedback, except for: HttpFSKerberosAuthenticator. Why wrap AuthenticationException in IOEs for the token methods? This ends up used by FileSystem methods which throw only IOExceptions. DelegationTokenManagerException This class should be public / stable right? Not really, all this is HttpFS internal.
          Hide
          Hadoop QA added a comment -

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

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs-httpfs.html
          Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//artifact/trunk/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//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/12537853/HDFS-3113.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 14 new or modified test files. -1 javac. The applied patch generated 2020 javac compiler warnings (more than the trunk's current 2007 warnings). +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. -1 findbugs. The patch appears to introduce 1 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//artifact/trunk/patchprocess/newPatchFindbugsWarningshadoop-hdfs-httpfs.html Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//artifact/trunk/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2903//console This message is automatically generated.
          Hide
          Alejandro Abdelnur added a comment -

          findbugs warning is a false positive, I'll exclude it at commit time.

          Show
          Alejandro Abdelnur added a comment - findbugs warning is a false positive, I'll exclude it at commit time.
          Hide
          Alejandro Abdelnur added a comment -

          new patch includes findbugs exclusion.

          Show
          Alejandro Abdelnur added a comment - new patch includes findbugs exclusion.
          Hide
          Hadoop QA added a comment -

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

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

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2904//testReport/
          Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2904//artifact/trunk/patchprocess/diffJavacWarnings.txt
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2904//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/12537901/HDFS-3113.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 14 new or modified test files. -1 javac. The applied patch generated 2020 javac compiler warnings (more than the trunk's current 2007 warnings). +1 javadoc. The javadoc tool did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2904//testReport/ Javac warnings: https://builds.apache.org/job/PreCommit-HDFS-Build/2904//artifact/trunk/patchprocess/diffJavacWarnings.txt Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2904//console This message is automatically generated.
          Hide
          Alejandro Abdelnur added a comment -

          taking care of javac warnings as well as unhanded condition when canceling the delegation token without a token parameter (was setting both BAD_REQUEST and SC_OK as response, now doing BAD_REQUEST only).

          Created a follow up JIRA to annotate all classes in HTTPFS as private HDFS-3724

          Show
          Alejandro Abdelnur added a comment - taking care of javac warnings as well as unhanded condition when canceling the delegation token without a token parameter (was setting both BAD_REQUEST and SC_OK as response, now doing BAD_REQUEST only). Created a follow up JIRA to annotate all classes in HTTPFS as private HDFS-3724
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12537931/HDFS-3113.patch
          against trunk revision .

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

          +1 tests included. The patch appears to include 14 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 did not generate any warning messages.

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

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

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

          +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs.

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

          Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2908//testReport/
          Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2908//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/12537931/HDFS-3113.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 14 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 did not generate any warning messages. +1 eclipse:eclipse. The patch built with eclipse:eclipse. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed unit tests in hadoop-hdfs-project/hadoop-hdfs-httpfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2908//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2908//console This message is automatically generated.
          Hide
          Aaron T. Myers added a comment -

          +1, the latest patch looks good to me.

          Show
          Aaron T. Myers added a comment - +1, the latest patch looks good to me.
          Hide
          Alejandro Abdelnur added a comment -

          committed to trunk and branch-2.

          Show
          Alejandro Abdelnur added a comment - committed to trunk and branch-2.
          Hide
          Daryn Sharp added a comment -

          I see this is handling the real user for proxy tokens the same way as HDFS-3509 for webhdfs. After my work on HDFS-3553, I suggested on HDFS-3509 that perhaps the doAs required by SPNEGO should be pushed lower in the stack instead of all the callers implementing the same logic at a high level. If so, I think that will provide a solution for httpfs, webhdfs, fetchdt, and future callers so the new logic can be removed in this jira.

          Other than that, I'm not confident in my knowledge to fully review the patch, but I'm +1 based on my knowledge and Aaron's +1.

          Show
          Daryn Sharp added a comment - I see this is handling the real user for proxy tokens the same way as HDFS-3509 for webhdfs. After my work on HDFS-3553 , I suggested on HDFS-3509 that perhaps the doAs required by SPNEGO should be pushed lower in the stack instead of all the callers implementing the same logic at a high level. If so, I think that will provide a solution for httpfs, webhdfs, fetchdt, and future callers so the new logic can be removed in this jira. Other than that, I'm not confident in my knowledge to fully review the patch, but I'm +1 based on my knowledge and Aaron's +1.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development