Details

    • Type: New Feature
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha1
    • Component/s: fs/s3
    • Labels:
      None
    • Hadoop Flags:
      Reviewed
    • Release Note:
      The S3A Hadoop-compatible file system now support reading its S3 credentials from the Hadoop Credential Provider API in addition to XML configuration files.

      Description

      It would be good if we could read s3 creds from a source other than via a java property/Hadoop configuration option

      1. HADOOP-12548-branch-2.07.patch
        20 kB
        Chris Nauroth
      2. HADOOP-12548-07.patch
        19 kB
        Larry McCay
      3. HADOOP-12548-06.patch
        18 kB
        Larry McCay
      4. HADOOP-12548-05.patch
        13 kB
        Larry McCay
      5. HADOOP-12548-04.patch
        13 kB
        Larry McCay
      6. CredentialProviderAPIforS3FS-002.pdf
        143 kB
        Larry McCay
      7. HADOOP-12548-03.patch
        13 kB
        Larry McCay
      8. HADOOP-12548-02.patch
        10 kB
        Larry McCay
      9. HADOOP-12548-01.patch
        10 kB
        Larry McCay

        Issue Links

          Activity

          Hide
          stevel@apache.org Steve Loughran added a comment -

          not very generic. what about making it possible/easy for tools to read in an XML property file at the end of a URL; file:// and hdfs:// would then be examples

          Show
          stevel@apache.org Steve Loughran added a comment - not very generic. what about making it possible/easy for tools to read in an XML property file at the end of a URL; file:// and hdfs:// would then be examples
          Hide
          aw Allen Wittenauer added a comment -

          Makes sense.

          Show
          aw Allen Wittenauer added a comment - Makes sense.
          Hide
          lmccay Larry McCay added a comment -

          Maybe we should consider the CredentialProvider API?
          This could buy you multiple implementation types.

          Show
          lmccay Larry McCay added a comment - Maybe we should consider the CredentialProvider API? This could buy you multiple implementation types.
          Hide
          aw Allen Wittenauer added a comment -

          Maybe we should consider the CredentialProvider API? This could buy you multiple implementation types.

          I agree on a fundamental level, but .. it comes with a pretty big complexity cost for the average user. Editing a file is going to be significantly easier than learning a whole set of commands that they (and, frankly, the vast majority of committers!) barely know exist.

          That said, nothing is preventing this particular issue being solved multiple ways.

          Show
          aw Allen Wittenauer added a comment - Maybe we should consider the CredentialProvider API? This could buy you multiple implementation types. I agree on a fundamental level, but .. it comes with a pretty big complexity cost for the average user. Editing a file is going to be significantly easier than learning a whole set of commands that they (and, frankly, the vast majority of committers!) barely know exist. That said, nothing is preventing this particular issue being solved multiple ways.
          Hide
          lmccay Larry McCay added a comment -

          Understood.

          The problem comes down to putting it in a file in clear text.
          Even when it is protected with file permissions it is often flagged as clear text and therefore an issue.
          A keystore isn't clear text though the real security still requires file permissions but does usually pass the test.

          A credential server that authenticated users with kerberos would be secure though.
          The CredentialProvider API is a path to get there.

          I can lend a hand there if you'd like to go in that direction.

          Show
          lmccay Larry McCay added a comment - Understood. The problem comes down to putting it in a file in clear text. Even when it is protected with file permissions it is often flagged as clear text and therefore an issue. A keystore isn't clear text though the real security still requires file permissions but does usually pass the test. A credential server that authenticated users with kerberos would be secure though. The CredentialProvider API is a path to get there. I can lend a hand there if you'd like to go in that direction.
          Hide
          aw Allen Wittenauer added a comment -

          Help is always appreciated, esp since I'm pretty swamped with non-build (ha ha) issues right now.

          Show
          aw Allen Wittenauer added a comment - Help is always appreciated, esp since I'm pretty swamped with non-build (ha ha) issues right now.
          Hide
          lmccay Larry McCay added a comment -


          If you can point me to the code that is consuming the java property now then I can look at what is involved.
          I'll through together a one pager for adding the credential provider API and tying it into relevant config, etc.
          We may also want to provide a env variable over java property - at least they won't show up in ps output.

          Show
          lmccay Larry McCay added a comment - If you can point me to the code that is consuming the java property now then I can look at what is involved. I'll through together a one pager for adding the credential provider API and tying it into relevant config, etc. We may also want to provide a env variable over java property - at least they won't show up in ps output.
          Hide
          cnauroth Chris Nauroth added a comment -

          Hi Larry McCay. Thanks for jumping in!

          The current codebase uses this class to encapsulate retrieving the S3 credentials:

          https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3/S3Credentials.java

          Here is where you can see the s3n file system using that class:

          https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3native/Jets3tNativeFileSystemStore.java#L79-L80

          The s3a file system does it a little differently, without going through the S3Credentials class:

          https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java#L160-L161

          IMO, we don't need to worry about looking at the s3 file system, only s3n and s3a. Maybe Allen can confirm or deny that though as the requester.

          Show
          cnauroth Chris Nauroth added a comment - Hi Larry McCay . Thanks for jumping in! The current codebase uses this class to encapsulate retrieving the S3 credentials: https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3/S3Credentials.java Here is where you can see the s3n file system using that class: https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3native/Jets3tNativeFileSystemStore.java#L79-L80 The s3a file system does it a little differently, without going through the S3Credentials class: https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java#L160-L161 IMO, we don't need to worry about looking at the s3 file system, only s3n and s3a. Maybe Allen can confirm or deny that though as the requester.
          Hide
          cnauroth Chris Nauroth added a comment -

          Also, this documentation page describes the current configuration setup, in case you're looking for something more human readable.

          http://hadoop.apache.org/docs/r2.7.1/hadoop-aws/tools/hadoop-aws/index.html

          Show
          cnauroth Chris Nauroth added a comment - Also, this documentation page describes the current configuration setup, in case you're looking for something more human readable. http://hadoop.apache.org/docs/r2.7.1/hadoop-aws/tools/hadoop-aws/index.html
          Hide
          aw Allen Wittenauer added a comment -

          Awesome, thanks! Looks like Chris beat me to the background bits. haha.

          We may also want to provide a env variable over java property - at least they won't show up in ps output.

          They do, however, show up in /proc on Linux....

          Show
          aw Allen Wittenauer added a comment - Awesome, thanks! Looks like Chris beat me to the background bits. haha. We may also want to provide a env variable over java property - at least they won't show up in ps output. They do, however, show up in /proc on Linux....
          Hide
          lmccay Larry McCay added a comment -

          Looks like secretAccessKey is already using CredentialProvider API in fs S3Credentials.

              if (secretAccessKey == null) {
                final char[] pass = conf.getPassword(secretAccessKeyProperty);
                if (pass != null) {
                  secretAccessKey = (new String(pass)).trim();
                }
              }
          
          Show
          lmccay Larry McCay added a comment - Looks like secretAccessKey is already using CredentialProvider API in fs S3Credentials. if (secretAccessKey == null ) { final char [] pass = conf.getPassword(secretAccessKeyProperty); if (pass != null ) { secretAccessKey = ( new String (pass)).trim(); } }
          Hide
          cnauroth Chris Nauroth added a comment -

          Looks like secretAccessKey is already using CredentialProvider API in fs S3Credentials.

          Oh, you're right! I missed that.

          There is still a gap for s3a, which is using plain Configuration#get to retrieve its key.

          Show
          cnauroth Chris Nauroth added a comment - Looks like secretAccessKey is already using CredentialProvider API in fs S3Credentials. Oh, you're right! I missed that. There is still a gap for s3a, which is using plain Configuration#get to retrieve its key.
          Hide
          aw Allen Wittenauer added a comment -

          This also sounds like a perfect example to use for a "real world" example of credentials. Because even though I know the credential commands exist, I know I have no clue as to how to actually use them, never mind to connect to S3.

          Show
          aw Allen Wittenauer added a comment - This also sounds like a perfect example to use for a "real world" example of credentials. Because even though I know the credential commands exist, I know I have no clue as to how to actually use them, never mind to connect to S3.
          Hide
          lmccay Larry McCay added a comment -

          Chris Nauroth - I can add it to s3a. Are we okay with leaving accessKey the way it is or do we want to get that from credentials as well?

          Allen Wittenauer - what would you like to see for the example?

          Show
          lmccay Larry McCay added a comment - Chris Nauroth - I can add it to s3a. Are we okay with leaving accessKey the way it is or do we want to get that from credentials as well? Allen Wittenauer - what would you like to see for the example?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I don't want anyone going near s3n unless they volunteer to take on all testing and support calls related to regressions. It's been brittle in the past and as it is stable now I'm not going to +1 anything that isn't a critical patch

          Show
          stevel@apache.org Steve Loughran added a comment - I don't want anyone going near s3n unless they volunteer to take on all testing and support calls related to regressions. It's been brittle in the past and as it is stable now I'm not going to +1 anything that isn't a critical patch
          Hide
          stevel@apache.org Steve Loughran added a comment -

          S3a can also talk to Amazon's EC2 metadata service to fetch them off EC2 infrastructure ... this is probably only useful on EC2 nodes. You may be able to use other AWSCredentialsProvider subclasses, such as amazon's own PropertiesFileCredentialsProvider and ClasspathPropertiesFileCredentialsProvider.

          1. s3a is the one where this stuff should take place
          2. & yes, it should be the CredentialsProvider to ask for a password. And an account
          3. allowing clients to select specific AWSCredentialsProvider implementations by classname could be very extensible.
          4. if you are going to do this, why not add it to sf/swift too?

          One recurrent "how do I" question is "how do I support multiple s3 buckets with different accounts? The swift code does it by swift endpoint; there's nothing in the s3 code. Would it be possible to somehow use the bucket name when looking up a username/password? Maybe do a first attempt to lookup {{fs.s3a.$

          {BUCKET}

          .access.key}} & same for password, falling back to the base one if unset?

          Show
          stevel@apache.org Steve Loughran added a comment - S3a can also talk to Amazon's EC2 metadata service to fetch them off EC2 infrastructure ... this is probably only useful on EC2 nodes. You may be able to use other AWSCredentialsProvider subclasses, such as amazon's own PropertiesFileCredentialsProvider and ClasspathPropertiesFileCredentialsProvider . s3a is the one where this stuff should take place & yes, it should be the CredentialsProvider to ask for a password. And an account allowing clients to select specific AWSCredentialsProvider implementations by classname could be very extensible. if you are going to do this, why not add it to sf/swift too? One recurrent "how do I" question is "how do I support multiple s3 buckets with different accounts? The swift code does it by swift endpoint; there's nothing in the s3 code. Would it be possible to somehow use the bucket name when looking up a username/password? Maybe do a first attempt to lookup {{fs.s3a.$ {BUCKET} .access.key}} & same for password, falling back to the base one if unset?
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          I would like to point you to HADOOP-11670, where an earlier patch to let s3a use S3Credentials was reverted as that broke IAM support.

          Show
          Thomas Demoor Thomas Demoor added a comment - I would like to point you to HADOOP-11670 , where an earlier patch to let s3a use S3Credentials was reverted as that broke IAM support.
          Hide
          aw Allen Wittenauer added a comment -

          what would you like to see for the example?

          I literally want to see step-by-step instructions. "Given this access key and secret, this is how you do a distcp with s3 and hdfs using the hadoop commands and a credential file." I think the only end user documentation we have on this is just in the hadoop command reference. It only talks about manipulating a credential file. If this was a spelling bee, that'd be the definition. Now I'm asking the judge to use it in sentence.

          FWIW: based upon my inbox just in the past 12 hours or so, I'm clearly not the only support/ops-type person who has no clue how to actually use this stuff.

          Show
          aw Allen Wittenauer added a comment - what would you like to see for the example? I literally want to see step-by-step instructions. "Given this access key and secret, this is how you do a distcp with s3 and hdfs using the hadoop commands and a credential file." I think the only end user documentation we have on this is just in the hadoop command reference. It only talks about manipulating a credential file. If this was a spelling bee, that'd be the definition. Now I'm asking the judge to use it in sentence. FWIW: based upon my inbox just in the past 12 hours or so, I'm clearly not the only support/ops-type person who has no clue how to actually use this stuff.
          Hide
          lmccay Larry McCay added a comment -

          Allen Wittenauer - sure that sounds useful. Since I am certainly not an expert on s3 fs or distcp for that matter - I will try and locate general instructions for that usecase and interleave the use of the credentials commands, providers and path config. If you have a pointer to good instructions to start from that would be helpful.

          Steve Loughran - I'll post a one pager starting from simplified a credentials provider integration POV and we can iterate over what new providers may be needed in order to integrate AWS classes. Unless you think that there is more value in going directly to the AWSCredentialsProvider or other classes rather than through the hadoop credential providers.

          Show
          lmccay Larry McCay added a comment - Allen Wittenauer - sure that sounds useful. Since I am certainly not an expert on s3 fs or distcp for that matter - I will try and locate general instructions for that usecase and interleave the use of the credentials commands, providers and path config. If you have a pointer to good instructions to start from that would be helpful. Steve Loughran - I'll post a one pager starting from simplified a credentials provider integration POV and we can iterate over what new providers may be needed in order to integrate AWS classes. Unless you think that there is more value in going directly to the AWSCredentialsProvider or other classes rather than through the hadoop credential providers.
          Hide
          aw Allen Wittenauer added a comment - - edited

          If you have a pointer to good instructions to start from that would be helpful.

          distcp has its own document:

          (source) hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm
          (rendered) https://hadoop.apache.org/docs/current/hadoop-distcp/DistCp.html

          (... and this is probably where the directions should go as well.)

          S3 docs:

          (source) hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md
          (rendered) https://hadoop.apache.org/docs/current/hadoop-aws/tools/hadoop-aws/index.html

          Thanks! This will be awesome!

          Show
          aw Allen Wittenauer added a comment - - edited If you have a pointer to good instructions to start from that would be helpful. distcp has its own document: (source) hadoop-tools/hadoop-distcp/src/site/markdown/DistCp.md.vm (rendered) https://hadoop.apache.org/docs/current/hadoop-distcp/DistCp.html (... and this is probably where the directions should go as well.) S3 docs: (source) hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md (rendered) https://hadoop.apache.org/docs/current/hadoop-aws/tools/hadoop-aws/index.html Thanks! This will be awesome!
          Hide
          cnauroth Chris Nauroth added a comment -

          On the structure of the documentation, I suggest:

          1. CredentialProvider gets its own dedicated page, hyperlinked directly from the left nav in the top-level site documentation.
          2. The new CredentialProvider page includes a catalog of the features that can make use of it. For each such feature, hyperlink to the documentation page of that feature. For example, put a hyperlink in the HDFS permissions guide where it talks about the LDAP group mapper.
          3. We also do the reverse: hyperlink from features back to the CredentialProvider page.

          if you are going to do this, why not add it to sf/swift too?

          Add hadoop-azure to the list of things that don't use CredentialProvider, but could.

          The scope is growing here. I wouldn't mind seeing the effort split into multiple JIRAs if others agree.

          Show
          cnauroth Chris Nauroth added a comment - On the structure of the documentation, I suggest: CredentialProvider gets its own dedicated page, hyperlinked directly from the left nav in the top-level site documentation. The new CredentialProvider page includes a catalog of the features that can make use of it. For each such feature, hyperlink to the documentation page of that feature. For example, put a hyperlink in the HDFS permissions guide where it talks about the LDAP group mapper. We also do the reverse: hyperlink from features back to the CredentialProvider page. if you are going to do this, why not add it to sf/swift too? Add hadoop-azure to the list of things that don't use CredentialProvider, but could. The scope is growing here. I wouldn't mind seeing the effort split into multiple JIRAs if others agree.
          Hide
          cnauroth Chris Nauroth added a comment -

          I don't want anyone going near s3n unless they volunteer to take on all testing and support calls related to regressions. It's been brittle in the past and as it is stable now I'm not going to +1 anything that isn't a critical patch

          Understood. s3n is no longer relevant to the scope of this issue, because Larry has pointed out that it already uses S3Credentials, which has a call to Configuration#getPassword.

          Show
          cnauroth Chris Nauroth added a comment - I don't want anyone going near s3n unless they volunteer to take on all testing and support calls related to regressions. It's been brittle in the past and as it is stable now I'm not going to +1 anything that isn't a critical patch Understood. s3n is no longer relevant to the scope of this issue, because Larry has pointed out that it already uses S3Credentials , which has a call to Configuration#getPassword .
          Hide
          lmccay Larry McCay added a comment -

          One (4) pager to describe proposed support for credential provider API and a description of the distcp usecase. The usecase description to be used to inform actual documentation for the credential provider API in follow up work.

          Show
          lmccay Larry McCay added a comment - One (4) pager to describe proposed support for credential provider API and a description of the distcp usecase. The usecase description to be used to inform actual documentation for the credential provider API in follow up work.
          Hide
          lmccay Larry McCay added a comment -

          Allen Wittenauer, Steve Loughran and Chris Nauroth - please see the attached document.

          Show
          lmccay Larry McCay added a comment - Allen Wittenauer , Steve Loughran and Chris Nauroth - please see the attached document.
          Hide
          lmccay Larry McCay added a comment -

          It seems the doc needs to be updated to use "s3a://" scheme instead of "s3://" - is that correct?

          Show
          lmccay Larry McCay added a comment - It seems the doc needs to be updated to use "s3a://" scheme instead of "s3://" - is that correct?
          Hide
          cnauroth Chris Nauroth added a comment -

          Larry McCay, this proposal looks great to me. Yes, we'd want to target the proposal to the "s3a" scheme instead of "s3".

          Show
          cnauroth Chris Nauroth added a comment - Larry McCay , this proposal looks great to me. Yes, we'd want to target the proposal to the "s3a" scheme instead of "s3".
          Hide
          lmccay Larry McCay added a comment -

          Cool.

          Open Issues from the doc:

          1. Do we want to protect the accessKey or just leave it as is? The change described above enables you to protect it if you like and it shortens the target URI without requiring any userInfo. You can still not provision it and provide it alone in the userInfo too - which reduces the provisioning steps above to only the accessSecret step.
          2. Are the current Hadoop providers sufficient for the usecase or do we want to add some support for AWS specific providers?

          #1 is interesting considering the implementation in S3Credentials doesn't protect accessKey. This could end up with a couple possibilities:
          a. only the secret will be protected for s3:// and s3n:// and both would be supported for s3a://
          b. we change S3Credentials to also protect the accessKey and they will all be the same but it would affect s3n:// as well
          c. we only protect secret in s3a:// and all three are the same but s3n:// isn't affected

          Show
          lmccay Larry McCay added a comment - Cool. Open Issues from the doc: 1. Do we want to protect the accessKey or just leave it as is? The change described above enables you to protect it if you like and it shortens the target URI without requiring any userInfo. You can still not provision it and provide it alone in the userInfo too - which reduces the provisioning steps above to only the accessSecret step. 2. Are the current Hadoop providers sufficient for the usecase or do we want to add some support for AWS specific providers? #1 is interesting considering the implementation in S3Credentials doesn't protect accessKey. This could end up with a couple possibilities: a. only the secret will be protected for s3:// and s3n:// and both would be supported for s3a:// b. we change S3Credentials to also protect the accessKey and they will all be the same but it would affect s3n:// as well c. we only protect secret in s3a:// and all three are the same but s3n:// isn't affected
          Hide
          lmccay Larry McCay added a comment -

          Hmmm - I notice that the override that semantics in S3Credentials is not aligned with what I proposed here.
          The current implementation overrides the credentials provided in the userInfo of the URI with those found through the credential provider API.

          We should decide which behavior we want in S3A.

          It seems to me that if you were to indicate credentials in the URI from the launch of distcp that you are being more specific and that those should override what may have been previously provisioned in a credentialStore.

          Thoughts?

          Show
          lmccay Larry McCay added a comment - Hmmm - I notice that the override that semantics in S3Credentials is not aligned with what I proposed here. The current implementation overrides the credentials provided in the userInfo of the URI with those found through the credential provider API. We should decide which behavior we want in S3A. It seems to me that if you were to indicate credentials in the URI from the launch of distcp that you are being more specific and that those should override what may have been previously provisioned in a credentialStore. Thoughts?
          Hide
          aw Allen Wittenauer added a comment -

          It seems to me that if you were to indicate credentials in the URI from the launch of distcp that you are being more specific and that those should override what may have been previously provisioned in a credentialStore.

          Agree.

          Show
          aw Allen Wittenauer added a comment - It seems to me that if you were to indicate credentials in the URI from the launch of distcp that you are being more specific and that those should override what may have been previously provisioned in a credentialStore. Agree.
          Hide
          aw Allen Wittenauer added a comment -

          Also, on the document: specifying this in core-site.xml isn't really usable when the goal is to keep core-site.xml as generic for all users as possible. The example should really be using -D on the command line to specify the property.

          Show
          aw Allen Wittenauer added a comment - Also, on the document: specifying this in core-site.xml isn't really usable when the goal is to keep core-site.xml as generic for all users as possible. The example should really be using -D on the command line to specify the property.
          Hide
          lmccay Larry McCay added a comment -

          Now that you bring that up, Allen Wittenauer - I don't see where the system property is supported in S3A.
          I am probably just missing it - can you point it out?

          Show
          lmccay Larry McCay added a comment - Now that you bring that up, Allen Wittenauer - I don't see where the system property is supported in S3A. I am probably just missing it - can you point it out?
          Hide
          aw Allen Wittenauer added a comment - - edited

          Usually importing command line options that set properties are handled by whatever calls the file system. In this case, distcp.

          Show
          aw Allen Wittenauer added a comment - - edited Usually importing command line options that set properties are handled by whatever calls the file system. In this case, distcp.
          Hide
          lmccay Larry McCay added a comment -

          I see.
          So, are you actually saying that the provider path property should be put into the in-memory Configuration by distcp through a system property?
          Does this automatically happen or do we have to add support for this property?

          Show
          lmccay Larry McCay added a comment - I see. So, are you actually saying that the provider path property should be put into the in-memory Configuration by distcp through a system property? Does this automatically happen or do we have to add support for this property?
          Hide
          lmccay Larry McCay added a comment -

          Looks like DistCpOptions and DistCpOptionSwitch would have to be modified to do this.
          I'm thinking that this is more of a filesystem patch and we should follow up with a distCp one - make sense?
          Can certainly update the usecase in the doc for distcp to include that.

          Show
          lmccay Larry McCay added a comment - Looks like DistCpOptions and DistCpOptionSwitch would have to be modified to do this. I'm thinking that this is more of a filesystem patch and we should follow up with a distCp one - make sense? Can certainly update the usecase in the doc for distcp to include that.
          Hide
          aw Allen Wittenauer added a comment - - edited

          Sorry I should have been more explicit. Running 'java blah blah blah -Dfoo=bar blah blah blah' will inject the 'foo' property with the 'bar' value. No special support is required. In the case of hadoop executables, 'hadoop distcp -Dfoo=bar blah blah' should work.

          Show
          aw Allen Wittenauer added a comment - - edited Sorry I should have been more explicit. Running 'java blah blah blah -Dfoo=bar blah blah blah' will inject the 'foo' property with the 'bar' value. No special support is required. In the case of hadoop executables, 'hadoop distcp -Dfoo=bar blah blah' should work.
          Hide
          lmccay Larry McCay added a comment -

          Nice!

          Show
          lmccay Larry McCay added a comment - Nice!
          Hide
          stevel@apache.org Steve Loughran added a comment -

          There's confusion here.

          1. Java sysprops. Set with -Dkey=value, must be on the CLI before the actual main classname to load. Can usually be set in whatever ENV var lets you set options for that exe.
          1. Hadoop toolrunner definitions. Set with -D key=value. Note the space? Must come after the classname. Any entry point built on ToolRunner get this —these are the ones automatically set into the Configuration instance loaded by that tool (and not to others in the JVM, incidentally). —that is, provided the proption perty can be overridden
          Show
          stevel@apache.org Steve Loughran added a comment - There's confusion here. Java sysprops. Set with -Dkey=value , must be on the CLI before the actual main classname to load. Can usually be set in whatever ENV var lets you set options for that exe. Hadoop toolrunner definitions. Set with -D key=value . Note the space? Must come after the classname. Any entry point built on ToolRunner get this —these are the ones automatically set into the Configuration instance loaded by that tool (and not to others in the JVM, incidentally). —that is, provided the proption perty can be overridden
          Hide
          lmccay Larry McCay added a comment -

          Thanks - that is clear!

          Show
          lmccay Larry McCay added a comment - Thanks - that is clear!
          Hide
          lmccay Larry McCay added a comment -

          Initial patch to implement proposal as described in document. Including protection for both access key and secret key. Override semantics that allow userInfo to override credential providers.

          Show
          lmccay Larry McCay added a comment - Initial patch to implement proposal as described in document. Including protection for both access key and secret key. Override semantics that allow userInfo to override credential providers.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          note that jenkins doesn't test s3; reviewers will have to be set up for an s3 test run already...

          Show
          stevel@apache.org Steve Loughran added a comment - note that jenkins doesn't test s3; reviewers will have to be set up for an s3 test run already...
          Hide
          lmccay Larry McCay added a comment -

          Bummer - but that makes sense.

          Show
          lmccay Larry McCay added a comment - Bummer - but that makes sense.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 5s docker + precommit patch detected.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 3m 16s trunk passed
          +1 compile 0m 11s trunk passed with JDK v1.8.0_60
          +1 compile 0m 13s trunk passed with JDK v1.7.0_79
          +1 checkstyle 0m 8s trunk passed
          +1 mvneclipse 0m 15s trunk passed
          +1 findbugs 0m 31s trunk passed
          -1 javadoc 0m 12s hadoop-aws in trunk failed with JDK v1.8.0_60.
          +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79
          +1 mvninstall 0m 14s the patch passed
          +1 compile 0m 10s the patch passed with JDK v1.8.0_60
          +1 javac 0m 10s the patch passed
          +1 compile 0m 12s the patch passed with JDK v1.7.0_79
          +1 javac 0m 12s the patch passed
          -1 checkstyle 0m 8s Patch generated 2 new checkstyle issues in hadoop-tools/hadoop-aws (total was 45, now 46).
          +1 mvneclipse 0m 12s the patch passed
          -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix.
          -1 findbugs 0m 39s hadoop-tools/hadoop-aws introduced 1 new FindBugs issues.
          -1 javadoc 0m 12s hadoop-aws in the patch failed with JDK v1.8.0_60.
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_79
          +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60.
          +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.7.0_79.
          +1 asflicense 0m 25s Patch does not generate ASF License warnings.
          8m 53s



          Reason Tests
          FindBugs module:hadoop-tools/hadoop-aws
            Should org.apache.hadoop.fs.s3a.S3AFileSystem$AWSAccessKeys be a static inner class? At S3AFileSystem.java:inner class? At S3AFileSystem.java:[lines 1197-1210]



          Subsystem Report/Notes
          Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-06
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771122/HADOOP-12548-01.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile
          uname Linux 8cd96fb092f5 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh
          git revision trunk / bf6aa30
          Default Java 1.7.0_79
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/whitespace-eol.txt
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/new-findbugs-hadoop-tools_hadoop-aws.html
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Max memory used 227MB
          Powered by Apache Yetus http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 5s docker + precommit patch detected. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 3m 16s trunk passed +1 compile 0m 11s trunk passed with JDK v1.8.0_60 +1 compile 0m 13s trunk passed with JDK v1.7.0_79 +1 checkstyle 0m 8s trunk passed +1 mvneclipse 0m 15s trunk passed +1 findbugs 0m 31s trunk passed -1 javadoc 0m 12s hadoop-aws in trunk failed with JDK v1.8.0_60. +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79 +1 mvninstall 0m 14s the patch passed +1 compile 0m 10s the patch passed with JDK v1.8.0_60 +1 javac 0m 10s the patch passed +1 compile 0m 12s the patch passed with JDK v1.7.0_79 +1 javac 0m 12s the patch passed -1 checkstyle 0m 8s Patch generated 2 new checkstyle issues in hadoop-tools/hadoop-aws (total was 45, now 46). +1 mvneclipse 0m 12s the patch passed -1 whitespace 0m 0s The patch has 1 line(s) that end in whitespace. Use git apply --whitespace=fix. -1 findbugs 0m 39s hadoop-tools/hadoop-aws introduced 1 new FindBugs issues. -1 javadoc 0m 12s hadoop-aws in the patch failed with JDK v1.8.0_60. +1 javadoc 0m 13s the patch passed with JDK v1.7.0_79 +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60. +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.7.0_79. +1 asflicense 0m 25s Patch does not generate ASF License warnings. 8m 53s Reason Tests FindBugs module:hadoop-tools/hadoop-aws   Should org.apache.hadoop.fs.s3a.S3AFileSystem$AWSAccessKeys be a static inner class? At S3AFileSystem.java:inner class? At S3AFileSystem.java: [lines 1197-1210] Subsystem Report/Notes Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-06 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771122/HADOOP-12548-01.patch JIRA Issue HADOOP-12548 Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile uname Linux 8cd96fb092f5 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh git revision trunk / bf6aa30 Default Java 1.7.0_79 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt whitespace https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/whitespace-eol.txt findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/new-findbugs-hadoop-tools_hadoop-aws.html javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Max memory used 227MB Powered by Apache Yetus http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8042/console This message was automatically generated.
          Hide
          lmccay Larry McCay added a comment -

          Addressing issues with previous patch.

          Show
          lmccay Larry McCay added a comment - Addressing issues with previous patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 5s docker + precommit patch detected.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 2m 59s trunk passed
          +1 compile 0m 11s trunk passed with JDK v1.8.0_60
          +1 compile 0m 13s trunk passed with JDK v1.7.0_79
          +1 checkstyle 0m 7s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 0m 30s trunk passed
          -1 javadoc 0m 12s hadoop-aws in trunk failed with JDK v1.8.0_60.
          +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79
          +1 mvninstall 0m 14s the patch passed
          +1 compile 0m 12s the patch passed with JDK v1.8.0_60
          +1 javac 0m 12s the patch passed
          +1 compile 0m 12s the patch passed with JDK v1.7.0_79
          +1 javac 0m 12s the patch passed
          -1 checkstyle 0m 8s Patch generated 3 new checkstyle issues in hadoop-tools/hadoop-aws (total was 45, now 47).
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 38s the patch passed
          -1 javadoc 0m 12s hadoop-aws in the patch failed with JDK v1.8.0_60.
          +1 javadoc 0m 14s the patch passed with JDK v1.7.0_79
          +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60.
          +1 unit 0m 12s hadoop-aws in the patch passed with JDK v1.7.0_79.
          +1 asflicense 0m 22s Patch does not generate ASF License warnings.
          8m 26s



          Subsystem Report/Notes
          Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-07
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771150/HADOOP-12548-02.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile
          uname Linux 17a6a75760d1 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh
          git revision trunk / bf6aa30
          Default Java 1.7.0_79
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Max memory used 225MB
          Powered by Apache Yetus http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 5s docker + precommit patch detected. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 2m 59s trunk passed +1 compile 0m 11s trunk passed with JDK v1.8.0_60 +1 compile 0m 13s trunk passed with JDK v1.7.0_79 +1 checkstyle 0m 7s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 30s trunk passed -1 javadoc 0m 12s hadoop-aws in trunk failed with JDK v1.8.0_60. +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79 +1 mvninstall 0m 14s the patch passed +1 compile 0m 12s the patch passed with JDK v1.8.0_60 +1 javac 0m 12s the patch passed +1 compile 0m 12s the patch passed with JDK v1.7.0_79 +1 javac 0m 12s the patch passed -1 checkstyle 0m 8s Patch generated 3 new checkstyle issues in hadoop-tools/hadoop-aws (total was 45, now 47). +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 38s the patch passed -1 javadoc 0m 12s hadoop-aws in the patch failed with JDK v1.8.0_60. +1 javadoc 0m 14s the patch passed with JDK v1.7.0_79 +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60. +1 unit 0m 12s hadoop-aws in the patch passed with JDK v1.7.0_79. +1 asflicense 0m 22s Patch does not generate ASF License warnings. 8m 26s Subsystem Report/Notes Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-07 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771150/HADOOP-12548-02.patch JIRA Issue HADOOP-12548 Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile uname Linux 17a6a75760d1 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh git revision trunk / bf6aa30 Default Java 1.7.0_79 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Max memory used 225MB Powered by Apache Yetus http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8044/console This message was automatically generated.
          Hide
          lmccay Larry McCay added a comment -

          Resolved all of the javadoc issues in S3AFileSystem and the checkstyle issues.

          Show
          lmccay Larry McCay added a comment - Resolved all of the javadoc issues in S3AFileSystem and the checkstyle issues.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s docker + precommit patch detected.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 5m 29s trunk passed
          +1 compile 0m 35s trunk passed with JDK v1.8.0_60
          +1 compile 0m 23s trunk passed with JDK v1.7.0_79
          +1 checkstyle 0m 15s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 0m 58s trunk passed
          -1 javadoc 0m 37s hadoop-aws in trunk failed with JDK v1.8.0_60.
          +1 javadoc 0m 29s trunk passed with JDK v1.7.0_79
          +1 mvninstall 0m 28s the patch passed
          +1 compile 0m 31s the patch passed with JDK v1.8.0_60
          +1 javac 0m 31s the patch passed
          +1 compile 0m 24s the patch passed with JDK v1.7.0_79
          +1 javac 0m 24s the patch passed
          -1 checkstyle 0m 14s Patch generated 1 new checkstyle issues in hadoop-tools/hadoop-aws (total was 45, now 45).
          +1 mvneclipse 0m 25s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 1m 8s the patch passed
          -1 javadoc 0m 35s hadoop-aws in the patch failed with JDK v1.8.0_60.
          +1 javadoc 0m 28s the patch passed with JDK v1.7.0_79
          +1 unit 0m 31s hadoop-aws in the patch passed with JDK v1.8.0_60.
          +1 unit 0m 26s hadoop-aws in the patch passed with JDK v1.7.0_79.
          +1 asflicense 0m 44s Patch does not generate ASF License warnings.
          17m 24s



          Subsystem Report/Notes
          Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-07
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771169/HADOOP-12548-03.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile
          uname Linux 9a3e5550f8c1 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh
          git revision trunk / 2801b42
          Default Java 1.7.0_79
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Max memory used 228MB
          Powered by Apache Yetus http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s docker + precommit patch detected. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 5m 29s trunk passed +1 compile 0m 35s trunk passed with JDK v1.8.0_60 +1 compile 0m 23s trunk passed with JDK v1.7.0_79 +1 checkstyle 0m 15s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 0m 58s trunk passed -1 javadoc 0m 37s hadoop-aws in trunk failed with JDK v1.8.0_60. +1 javadoc 0m 29s trunk passed with JDK v1.7.0_79 +1 mvninstall 0m 28s the patch passed +1 compile 0m 31s the patch passed with JDK v1.8.0_60 +1 javac 0m 31s the patch passed +1 compile 0m 24s the patch passed with JDK v1.7.0_79 +1 javac 0m 24s the patch passed -1 checkstyle 0m 14s Patch generated 1 new checkstyle issues in hadoop-tools/hadoop-aws (total was 45, now 45). +1 mvneclipse 0m 25s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 1m 8s the patch passed -1 javadoc 0m 35s hadoop-aws in the patch failed with JDK v1.8.0_60. +1 javadoc 0m 28s the patch passed with JDK v1.7.0_79 +1 unit 0m 31s hadoop-aws in the patch passed with JDK v1.8.0_60. +1 unit 0m 26s hadoop-aws in the patch passed with JDK v1.7.0_79. +1 asflicense 0m 44s Patch does not generate ASF License warnings. 17m 24s Subsystem Report/Notes Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-07 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771169/HADOOP-12548-03.patch JIRA Issue HADOOP-12548 Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile uname Linux 9a3e5550f8c1 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh git revision trunk / 2801b42 Default Java 1.7.0_79 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Max memory used 228MB Powered by Apache Yetus http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8046/console This message was automatically generated.
          Hide
          stevel@apache.org Steve Loughran added a comment -
          1. Can you pick up the test constants from org.apache.hadoop.fs.s3a.Constants so there's 0 risk of typos
          2. when you catch & log IOEs: what's going to cause those, and how much noise will in create on a multi-threaded run, such as an HDFS backup?
          Show
          stevel@apache.org Steve Loughran added a comment - Can you pick up the test constants from org.apache.hadoop.fs.s3a.Constants so there's 0 risk of typos when you catch & log IOEs: what's going to cause those, and how much noise will in create on a multi-threaded run, such as an HDFS backup?
          Hide
          lmccay Larry McCay added a comment -

          Steve Loughran - thanks for the review!

          1. sure
          2. good question: it depends on the credential provider being used but in general the inability to resolve a credential due to some unexpected error - like not having permission to read a keystore or file, etc. So, if initialize were to be called in every thread in such a circumstance there may be a lot of noise. If it is called in every thread then it may make sense to change that from a warn to a debug.

          Show
          lmccay Larry McCay added a comment - Steve Loughran - thanks for the review! 1. sure 2. good question: it depends on the credential provider being used but in general the inability to resolve a credential due to some unexpected error - like not having permission to read a keystore or file, etc. So, if initialize were to be called in every thread in such a circumstance there may be a lot of noise. If it is called in every thread then it may make sense to change that from a warn to a debug.
          Hide
          lmccay Larry McCay added a comment -

          Addressed review comments and checkstyle issue.

          Show
          lmccay Larry McCay added a comment - Addressed review comments and checkstyle issue.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 6s docker + precommit patch detected.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 2m 55s trunk passed
          +1 compile 0m 11s trunk passed with JDK v1.8.0_60
          +1 compile 0m 13s trunk passed with JDK v1.7.0_79
          +1 checkstyle 0m 7s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 0m 29s trunk passed
          -1 javadoc 0m 12s hadoop-aws in trunk failed with JDK v1.8.0_60.
          +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79
          +1 mvninstall 0m 14s the patch passed
          +1 compile 0m 11s the patch passed with JDK v1.8.0_60
          +1 javac 0m 11s the patch passed
          +1 compile 0m 13s the patch passed with JDK v1.7.0_79
          +1 javac 0m 13s the patch passed
          +1 checkstyle 0m 8s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 38s the patch passed
          -1 javadoc 0m 12s hadoop-aws in the patch failed with JDK v1.8.0_60.
          +1 javadoc 0m 14s the patch passed with JDK v1.7.0_79
          +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60.
          +1 unit 0m 13s hadoop-aws in the patch passed with JDK v1.7.0_79.
          +1 asflicense 0m 22s Patch does not generate ASF License warnings.
          8m 26s



          Subsystem Report/Notes
          Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-07
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771179/HADOOP-12548-04.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile
          uname Linux 1b807e4f4c27 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh
          git revision trunk / 2801b42
          Default Java 1.7.0_79
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79
          findbugs v3.0.0
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt
          JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Max memory used 226MB
          Powered by Apache Yetus http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 6s docker + precommit patch detected. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 2m 55s trunk passed +1 compile 0m 11s trunk passed with JDK v1.8.0_60 +1 compile 0m 13s trunk passed with JDK v1.7.0_79 +1 checkstyle 0m 7s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 29s trunk passed -1 javadoc 0m 12s hadoop-aws in trunk failed with JDK v1.8.0_60. +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79 +1 mvninstall 0m 14s the patch passed +1 compile 0m 11s the patch passed with JDK v1.8.0_60 +1 javac 0m 11s the patch passed +1 compile 0m 13s the patch passed with JDK v1.7.0_79 +1 javac 0m 13s the patch passed +1 checkstyle 0m 8s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 38s the patch passed -1 javadoc 0m 12s hadoop-aws in the patch failed with JDK v1.8.0_60. +1 javadoc 0m 14s the patch passed with JDK v1.7.0_79 +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60. +1 unit 0m 13s hadoop-aws in the patch passed with JDK v1.7.0_79. +1 asflicense 0m 22s Patch does not generate ASF License warnings. 8m 26s Subsystem Report/Notes Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-07 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771179/HADOOP-12548-04.patch JIRA Issue HADOOP-12548 Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile uname Linux 1b807e4f4c27 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh git revision trunk / 2801b42 Default Java 1.7.0_79 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79 findbugs v3.0.0 javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/artifact/patchprocess/branch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt javadoc https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/artifact/patchprocess/patch-javadoc-hadoop-tools_hadoop-aws-jdk1.8.0_60.txt JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Max memory used 226MB Powered by Apache Yetus http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8048/console This message was automatically generated.
          Hide
          lmccay Larry McCay added a comment -

          New version of document with scheme references fixed to s3a and additional description of config through hadoop system properties on the distcp command line.

          Show
          lmccay Larry McCay added a comment - New version of document with scheme references fixed to s3a and additional description of config through hadoop system properties on the distcp command line.
          Hide
          lmccay Larry McCay added a comment -

          Remaining javadoc issues are in files that I haven't touched.
          I don't know that we want to fix all of these do we?
          I certainly can but it seems to overreach and could make merges into different branches more difficult than necessary.

          Show
          lmccay Larry McCay added a comment - Remaining javadoc issues are in files that I haven't touched. I don't know that we want to fix all of these do we? I certainly can but it seems to overreach and could make merges into different branches more difficult than necessary.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          I'm going to revert the patch that broke javadocs on 8; people have to not +1 things that break the expanded build.

          Show
          stevel@apache.org Steve Loughran added a comment - I'm going to revert the patch that broke javadocs on 8; people have to not +1 things that break the expanded build.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          no- its' been reverted already inb commit #4794185...new test run should be your code only

          Show
          stevel@apache.org Steve Loughran added a comment - no- its' been reverted already inb commit #4794185...new test run should be your code only
          Hide
          lmccay Larry McCay added a comment -

          How could I have broken javadocs on files that I didn't touch?
          Is there some policy for fixing all existing javadoc issues in a module when its touched for the first time since the rules were changed or something like that?

          Show
          lmccay Larry McCay added a comment - How could I have broken javadocs on files that I didn't touch? Is there some policy for fixing all existing javadoc issues in a module when its touched for the first time since the rules were changed or something like that?
          Hide
          stevel@apache.org Steve Loughran added a comment -

          it wasn't you larry, don't worry. A different patch went in where the failure of the javadoc wasn't treated as a veto on the commit. Fixed now.

          Show
          stevel@apache.org Steve Loughran added a comment - it wasn't you larry, don't worry. A different patch went in where the failure of the javadoc wasn't treated as a veto on the commit. Fixed now.
          Hide
          aw Allen Wittenauer added a comment -

          Larry McCay, test-patch uses the last attached thing for tests... so it's trying to use your pdf doc now....

          Show
          aw Allen Wittenauer added a comment - Larry McCay , test-patch uses the last attached thing for tests... so it's trying to use your pdf doc now....
          Hide
          lmccay Larry McCay added a comment -

          Ugh. Sorry about that.

          Show
          lmccay Larry McCay added a comment - Ugh. Sorry about that.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 6s docker + precommit patch detected.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 3m 12s trunk passed
          +1 compile 0m 12s trunk passed with JDK v1.8.0_60
          +1 compile 0m 13s trunk passed with JDK v1.7.0_79
          +1 checkstyle 0m 7s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 0m 31s trunk passed
          +1 javadoc 0m 13s trunk passed with JDK v1.8.0_60
          +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79
          +1 mvninstall 0m 14s the patch passed
          +1 compile 0m 11s the patch passed with JDK v1.8.0_60
          +1 javac 0m 11s the patch passed
          +1 compile 0m 13s the patch passed with JDK v1.7.0_79
          +1 javac 0m 13s the patch passed
          +1 checkstyle 0m 7s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 38s the patch passed
          +1 javadoc 0m 12s the patch passed with JDK v1.8.0_60
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_79
          +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60.
          +1 unit 0m 14s hadoop-aws in the patch passed with JDK v1.7.0_79.
          +1 asflicense 0m 23s Patch does not generate ASF License warnings.
          8m 45s



          Subsystem Report/Notes
          Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-09
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771368/HADOOP-12548-04.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile
          uname Linux 9d6db3cfc302 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh
          git revision trunk / ef926b2
          Default Java 1.7.0_79
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79
          findbugs v3.0.0
          JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8055/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Max memory used 226MB
          Powered by Apache Yetus http://yetus.apache.org
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8055/console

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 6s docker + precommit patch detected. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 3m 12s trunk passed +1 compile 0m 12s trunk passed with JDK v1.8.0_60 +1 compile 0m 13s trunk passed with JDK v1.7.0_79 +1 checkstyle 0m 7s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 31s trunk passed +1 javadoc 0m 13s trunk passed with JDK v1.8.0_60 +1 javadoc 0m 14s trunk passed with JDK v1.7.0_79 +1 mvninstall 0m 14s the patch passed +1 compile 0m 11s the patch passed with JDK v1.8.0_60 +1 javac 0m 11s the patch passed +1 compile 0m 13s the patch passed with JDK v1.7.0_79 +1 javac 0m 13s the patch passed +1 checkstyle 0m 7s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 38s the patch passed +1 javadoc 0m 12s the patch passed with JDK v1.8.0_60 +1 javadoc 0m 13s the patch passed with JDK v1.7.0_79 +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_60. +1 unit 0m 14s hadoop-aws in the patch passed with JDK v1.7.0_79. +1 asflicense 0m 23s Patch does not generate ASF License warnings. 8m 45s Subsystem Report/Notes Docker Client=1.7.1 Server=1.7.1 Image:test-patch-base-hadoop-date2015-11-09 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12771368/HADOOP-12548-04.patch JIRA Issue HADOOP-12548 Optional Tests asflicense javac javadoc mvninstall unit findbugs checkstyle compile uname Linux 9d6db3cfc302 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HADOOP-Build/patchprocess/apache-yetus-ee5baeb/precommit/personality/hadoop.sh git revision trunk / ef926b2 Default Java 1.7.0_79 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_60 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_79 findbugs v3.0.0 JDK v1.7.0_79 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8055/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Max memory used 226MB Powered by Apache Yetus http://yetus.apache.org Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8055/console This message was automatically generated.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          Relating to HADOOP-11694: s3a phase II improvements and bugfixes. Do we want to make this a subtask?

          Show
          Thomas Demoor Thomas Demoor added a comment - Relating to HADOOP-11694 : s3a phase II improvements and bugfixes. Do we want to make this a subtask?
          Hide
          lmccay Larry McCay added a comment -

          Hi Allen Wittenauer, Steve Loughran, Chris Nauroth - can one or more of you review the available patch and the initial patch for credential provider docs in HADOOP-11031?

          I don't know that we need anything beyond a good starting point for the docs in order to get this change in.
          Considering that the credential provider API is already used in s3n, this is just getting s3a up to par.

          I really need some opinion on the initial patch for the docs too and in particular whether the references to ecosystem projects external to hadoop-common. Once we get a good starting point on the docs then we can iterate on HADOOP-11031 and get that in. Then adding the azure and openstack support will be easier.

          thanks!

          Show
          lmccay Larry McCay added a comment - Hi Allen Wittenauer , Steve Loughran , Chris Nauroth - can one or more of you review the available patch and the initial patch for credential provider docs in HADOOP-11031 ? I don't know that we need anything beyond a good starting point for the docs in order to get this change in. Considering that the credential provider API is already used in s3n, this is just getting s3a up to par. I really need some opinion on the initial patch for the docs too and in particular whether the references to ecosystem projects external to hadoop-common. Once we get a good starting point on the docs then we can iterate on HADOOP-11031 and get that in. Then adding the azure and openstack support will be easier. thanks!
          Hide
          lmccay Larry McCay added a comment -

          Related documentation for overall credential provider API.

          Show
          lmccay Larry McCay added a comment - Related documentation for overall credential provider API.
          Hide
          mattpaduano Matthew Paduano added a comment -

          I applied the patch 04 to trunk and attempted the end to end steps described in the PDF.

          Placing the credential file in HDFS and using the correct URI succeeded with distcp.
          Great work! thanks!

          Other notes:

          • in getAWSAccessKeys(), exceptions from accessing the config are simply eaten. This includes stuff like typo's in filenames and mistyped URI's and so on. All the user gets is a cryptic message with com.amazonaws.AmazonClientException. I recommend letting exceptions through as the eaten exceptions all had nice messages that would have helped with debugging.
          • in the PDF there is a sample distcp command line using the -D flag. The -D flag is not parsed by distcp in the way that is expected and this fails. Recommend removing that -D (which is already in brackets) from the example. Also, this means that to run distcp one has to edit core-site.xml to pass the provider path. For unit tests, one can add the property to contract-test-options.xml and auth-keys.xml. Is there anything like that for hadoop command line? e.g. hadoop distcp --property_overrides=props.xml ...
          • the HDFS creds cannot be tested using the unit tests. I am not sure what can be done about it, but it is disappointing.
          • are there plans to take this functionality to S3FileSystem or NativeS3FileSystem?
          Show
          mattpaduano Matthew Paduano added a comment - I applied the patch 04 to trunk and attempted the end to end steps described in the PDF. Placing the credential file in HDFS and using the correct URI succeeded with distcp . Great work! thanks! Other notes: in getAWSAccessKeys() , exceptions from accessing the config are simply eaten. This includes stuff like typo's in filenames and mistyped URI's and so on. All the user gets is a cryptic message with com.amazonaws.AmazonClientException . I recommend letting exceptions through as the eaten exceptions all had nice messages that would have helped with debugging. in the PDF there is a sample distcp command line using the -D flag. The -D flag is not parsed by distcp in the way that is expected and this fails. Recommend removing that -D (which is already in brackets) from the example. Also, this means that to run distcp one has to edit core-site.xml to pass the provider path. For unit tests, one can add the property to contract-test-options.xml and auth-keys.xml . Is there anything like that for hadoop command line? e.g. hadoop distcp --property_overrides=props.xml ... the HDFS creds cannot be tested using the unit tests. I am not sure what can be done about it, but it is disappointing. are there plans to take this functionality to S3FileSystem or NativeS3FileSystem ?
          Hide
          lmccay Larry McCay added a comment -

          Hi Matthew Paduano - thanks for the review. I'm curious - since the wrapping in the PDF makes it hard to see - whether you included the space after the -D and before the provider path param? I have seen this work on for the hadoop CLI - as long as the space is provided.

          % hadoop distcp hdfs://hostname:9001/user/lmccay/007020615 s3a://lmccay/ [-­D hadoop.security.credential.provider.path=localjceks://file/home/lmccay/aws.jcek
          s]

          Show
          lmccay Larry McCay added a comment - Hi Matthew Paduano - thanks for the review. I'm curious - since the wrapping in the PDF makes it hard to see - whether you included the space after the -D and before the provider path param? I have seen this work on for the hadoop CLI - as long as the space is provided. % hadoop distcp hdfs://hostname:9001/user/lmccay/007020615 s3a://lmccay/ [-­D hadoop.security.credential.provider.path=localjceks://file/home/lmccay/aws.jcek s]
          Hide
          mattpaduano Matthew Paduano added a comment -

          I see the comment above about the details of using the -D flag on a command line
          and how it must be carefully placed etc. I still recommend changing the example
          so that the flag is properly placed

          I also see the comment above re S3FileSystem and S3Credentials.

          Show
          mattpaduano Matthew Paduano added a comment - I see the comment above about the details of using the -D flag on a command line and how it must be carefully placed etc. I still recommend changing the example so that the flag is properly placed I also see the comment above re S3FileSystem and S3Credentials.
          Hide
          lmccay Larry McCay added a comment -

          Better formatting for clarity:

          % hadoop distcp hdfs://hostname:9001/user/lmccay/007020615 s3a://lmccay/
          [-­D hadoop.security.credential.provider.path=localjceks://file/home/lmccay/aws.jceks]

          Show
          lmccay Larry McCay added a comment - Better formatting for clarity: % hadoop distcp hdfs://hostname:9001/user/lmccay/007020615 s3a://lmccay/ [-­D hadoop.security.credential.provider.path=localjceks://file/home/lmccay/aws.jceks]
          Hide
          lmccay Larry McCay added a comment -

          Will do.

          I am looking at the swallowed exceptions issue that you raised as well....
          Can you elaborate a bit more?

          I'm only catching the IOException what others are being thrown and how are they being swallowed?

          Show
          lmccay Larry McCay added a comment - Will do. I am looking at the swallowed exceptions issue that you raised as well.... Can you elaborate a bit more? I'm only catching the IOException what others are being thrown and how are they being swallowed?
          Hide
          mattpaduano Matthew Paduano added a comment -

          OK. I played with this a little. This is the command that worked for me, with and without the space.
          But the -D flag had to immediately follow "distcp". That was the trick...

          hadoop distcp -Dhadoop.security.credential.provider.path=jceks://hdfs@localhost:9000/user/mattp/aws.jceks -libjars /home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/jackson-core-2.2.3.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/jackson-annotations-2.2.3.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/jackson-databind-2.2.3.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/hadoop-aws-3.0.0-SNAPSHOT.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/aws-java-sdk-core-1.10.6.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/aws-java-sdk-kms-1.10.6.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/aws-java-sdk-s3-1.10.6.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/joda-time-2.8.1.jar hdfs://localhost:9000/user/mattp/testmore s3a://test-s3-out/

          Show
          mattpaduano Matthew Paduano added a comment - OK. I played with this a little. This is the command that worked for me, with and without the space. But the -D flag had to immediately follow "distcp" . That was the trick... hadoop distcp -Dhadoop.security.credential.provider.path=jceks://hdfs@localhost:9000/user/mattp/aws.jceks -libjars /home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/jackson-core-2.2.3.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/jackson-annotations-2.2.3.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/jackson-databind-2.2.3.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/hadoop-aws-3.0.0-SNAPSHOT.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/aws-java-sdk-core-1.10.6.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/aws-java-sdk-kms-1.10.6.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/aws-java-sdk-s3-1.10.6.jar,/home/mattp/hadoop/hadoop-dist/target/hadoop-3.0.0-SNAPSHOT/share/hadoop/tools/lib/joda-time-2.8.1.jar hdfs://localhost:9000/user/mattp/testmore s3a://test-s3-out/
          Hide
          mattpaduano Matthew Paduano added a comment -

          + } catch (IOException ioe)

          { + // log as appropriate but continue to check userInfo for creds + LOG.debug("Error encountered while retrieving AWS secret key.", ioe); + }

          if one mistypes the filename or some part of the URI etc., the IOE is eaten (maybe logged)
          and then return an invalid AWSAccessKeys which blows up in initialize() later on:

          "com.amazonaws.AmazonClientException: Unable to load AWS credentials from..."

          The object is invalid if the key/secret are both null. And while all the various layers
          are careful to throw a nice exception with message, one has to enable debug logging
          and find the logs to find a typo. Sort of a pain, assuming one can even find/configure
          the logs. Why proceed quietly from getAWSAccessKeys() if the object is not valid?

          Show
          mattpaduano Matthew Paduano added a comment - + } catch (IOException ioe) { + // log as appropriate but continue to check userInfo for creds + LOG.debug("Error encountered while retrieving AWS secret key.", ioe); + } if one mistypes the filename or some part of the URI etc., the IOE is eaten (maybe logged) and then return an invalid AWSAccessKeys which blows up in initialize() later on: "com.amazonaws.AmazonClientException: Unable to load AWS credentials from..." The object is invalid if the key/secret are both null. And while all the various layers are careful to throw a nice exception with message, one has to enable debug logging and find the logs to find a typo. Sort of a pain, assuming one can even find/configure the logs. Why proceed quietly from getAWSAccessKeys() if the object is not valid?
          Hide
          lmccay Larry McCay added a comment -

          Ahhhh - I see.

          That was changed to debug due to questions from Steve Loughran earlier in this discussion.
          We can certainly change it back to warn but there was a question wrt to the amount of noise in a multithreaded app - I assume like distcp.

          Show
          lmccay Larry McCay added a comment - Ahhhh - I see. That was changed to debug due to questions from Steve Loughran earlier in this discussion. We can certainly change it back to warn but there was a question wrt to the amount of noise in a multithreaded app - I assume like distcp.
          Hide
          stevel@apache.org Steve Loughran added a comment -

          Matthew,

          1. I'm reluctant go near s3n for anything; every time someone touches it, something else breaks.
          2. We should add support for credential providers in the s3 tests, either as an alternative, or for some specific credential-provider tests
          Show
          stevel@apache.org Steve Loughran added a comment - Matthew, I'm reluctant go near s3n for anything ; every time someone touches it, something else breaks. We should add support for credential providers in the s3 tests, either as an alternative, or for some specific credential-provider tests
          Hide
          mattpaduano Matthew Paduano added a comment -

          I was thinking of something more like this:
          {{
          AWSAccessKeys getAWSAccessKeys(URI name, Configuration conf) {
          String accessKey = null;
          String secretKey = null;
          String userInfo = name.getUserInfo();
          if (userInfo != null) {
          int index = userInfo.indexOf(':');
          if (index != -1)

          { accessKey = userInfo.substring(0, index); secretKey = userInfo.substring(index + 1); }

          else

          { accessKey = userInfo; }

          }
          if (accessKey == null) {
          final char[] key = conf.getPassword(ACCESS_KEY);
          if (key != null)

          { accessKey = (new String(key)).trim() }

          }
          if (secretKey == null) {
          final char[] pass = conf.getPassword(SECRET_KEY);
          if (pass != null)

          { secretKey = (new String(pass)).trim(); }

          }
          if (accessKey == null || secretKey == null)

          { throw new IOException("cannot find AWS access or secret key. required!"); }

          return new AWSAccessKeys(accessKey, secretKey);
          }
          }}

          Show
          mattpaduano Matthew Paduano added a comment - I was thinking of something more like this: {{ AWSAccessKeys getAWSAccessKeys(URI name, Configuration conf) { String accessKey = null; String secretKey = null; String userInfo = name.getUserInfo(); if (userInfo != null) { int index = userInfo.indexOf(':'); if (index != -1) { accessKey = userInfo.substring(0, index); secretKey = userInfo.substring(index + 1); } else { accessKey = userInfo; } } if (accessKey == null) { final char[] key = conf.getPassword(ACCESS_KEY); if (key != null) { accessKey = (new String(key)).trim() } } if (secretKey == null) { final char[] pass = conf.getPassword(SECRET_KEY); if (pass != null) { secretKey = (new String(pass)).trim(); } } if (accessKey == null || secretKey == null) { throw new IOException("cannot find AWS access or secret key. required!"); } return new AWSAccessKeys(accessKey, secretKey); } }}
          Hide
          lmccay@apache.org larry mccay added a comment -

          I see. Good thoughts. I will create a new patch.

          Show
          lmccay@apache.org larry mccay added a comment - I see. Good thoughts. I will create a new patch.
          Hide
          lmccay Larry McCay added a comment -

          Attaching latest patch revision addressing review comments.

          Show
          lmccay Larry McCay added a comment - Attaching latest patch revision addressing review comments.
          Hide
          lmccay Larry McCay added a comment -

          Apologies on getting this revision done - been swamped.

          Show
          lmccay Larry McCay added a comment - Apologies on getting this revision done - been swamped.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 2s trunk passed
          +1 compile 0m 12s trunk passed with JDK v1.8.0_66
          +1 compile 0m 13s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 14s trunk passed
          +1 mvnsite 0m 20s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 30s trunk passed
          +1 javadoc 0m 13s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 14s the patch passed
          +1 compile 0m 9s the patch passed with JDK v1.8.0_66
          +1 javac 0m 9s the patch passed
          +1 compile 0m 11s the patch passed with JDK v1.7.0_91
          +1 javac 0m 11s the patch passed
          -1 checkstyle 0m 12s hadoop-tools/hadoop-aws: patch generated 1 new + 40 unchanged - 1 fixed = 41 total (was 41)
          +1 mvnsite 0m 17s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 41s the patch passed
          +1 javadoc 1m 7s hadoop-tools_hadoop-aws-jdk1.8.0_66 with JDK v1.8.0_66 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15)
          +1 javadoc 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_66.
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_91
          +1 unit 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 12s hadoop-aws in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 17s Patch does not generate ASF License warnings.
          13m 17s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787281/HADOOP-12548-05.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ff657a98dac7 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 802a7ed
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8585/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8585/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Max memory used 77MB
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8585/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 2s trunk passed +1 compile 0m 12s trunk passed with JDK v1.8.0_66 +1 compile 0m 13s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 20s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 30s trunk passed +1 javadoc 0m 13s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 14s the patch passed +1 compile 0m 9s the patch passed with JDK v1.8.0_66 +1 javac 0m 9s the patch passed +1 compile 0m 11s the patch passed with JDK v1.7.0_91 +1 javac 0m 11s the patch passed -1 checkstyle 0m 12s hadoop-tools/hadoop-aws: patch generated 1 new + 40 unchanged - 1 fixed = 41 total (was 41) +1 mvnsite 0m 17s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 41s the patch passed +1 javadoc 1m 7s hadoop-tools_hadoop-aws-jdk1.8.0_66 with JDK v1.8.0_66 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15) +1 javadoc 0m 11s hadoop-aws in the patch passed with JDK v1.8.0_66. +1 javadoc 0m 13s the patch passed with JDK v1.7.0_91 +1 unit 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66. +1 unit 0m 12s hadoop-aws in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 17s Patch does not generate ASF License warnings. 13m 17s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787281/HADOOP-12548-05.patch JIRA Issue HADOOP-12548 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ff657a98dac7 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 802a7ed Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8585/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8585/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Max memory used 77MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8585/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          lmccay Larry McCay added a comment -

          Seriously? I removed more lines from initialize() than I added and it has too many lines in it - by one?
          I'll create another patch...

          Show
          lmccay Larry McCay added a comment - Seriously? I removed more lines from initialize() than I added and it has too many lines in it - by one? I'll create another patch...
          Hide
          aw Allen Wittenauer added a comment -

          Someone suggested that the Yetus logo should really be a middle finger.

          Show
          aw Allen Wittenauer added a comment - Someone suggested that the Yetus logo should really be a middle finger.
          Hide
          lmccay Larry McCay added a comment -

          Maybe with the developer's hand giving it right back!

          Show
          lmccay Larry McCay added a comment - Maybe with the developer's hand giving it right back!
          Hide
          lmccay Larry McCay added a comment -

          Extracted a handful of methods out of the large initialize method in order to get it down to a more reasonable size.

          Show
          lmccay Larry McCay added a comment - Extracted a handful of methods out of the large initialize method in order to get it down to a more reasonable size.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 18s trunk passed
          +1 compile 0m 12s trunk passed with JDK v1.8.0_66
          +1 compile 0m 14s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 14s trunk passed
          +1 mvnsite 0m 20s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 0m 31s trunk passed
          +1 javadoc 0m 12s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 13s the patch passed
          +1 compile 0m 9s the patch passed with JDK v1.8.0_66
          +1 javac 0m 9s the patch passed
          +1 compile 0m 11s the patch passed with JDK v1.7.0_91
          +1 javac 0m 11s the patch passed
          +1 checkstyle 0m 12s hadoop-tools/hadoop-aws: patch generated 0 new + 40 unchanged - 1 fixed = 40 total (was 41)
          +1 mvnsite 0m 17s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 42s the patch passed
          +1 javadoc 1m 8s hadoop-tools_hadoop-aws-jdk1.8.0_66 with JDK v1.8.0_66 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15)
          +1 javadoc 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66.
          +1 javadoc 0m 14s the patch passed with JDK v1.7.0_91
          +1 unit 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 12s hadoop-aws in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 18s Patch does not generate ASF License warnings.
          13m 38s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787286/HADOOP-12548-06.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 9f927f692ac9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / a429f85
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8586/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Max memory used 77MB
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8586/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 10s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 18s trunk passed +1 compile 0m 12s trunk passed with JDK v1.8.0_66 +1 compile 0m 14s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 20s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 0m 31s trunk passed +1 javadoc 0m 12s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 13s the patch passed +1 compile 0m 9s the patch passed with JDK v1.8.0_66 +1 javac 0m 9s the patch passed +1 compile 0m 11s the patch passed with JDK v1.7.0_91 +1 javac 0m 11s the patch passed +1 checkstyle 0m 12s hadoop-tools/hadoop-aws: patch generated 0 new + 40 unchanged - 1 fixed = 40 total (was 41) +1 mvnsite 0m 17s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 42s the patch passed +1 javadoc 1m 8s hadoop-tools_hadoop-aws-jdk1.8.0_66 with JDK v1.8.0_66 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15) +1 javadoc 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66. +1 javadoc 0m 14s the patch passed with JDK v1.7.0_91 +1 unit 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66. +1 unit 0m 12s hadoop-aws in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 13m 38s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787286/HADOOP-12548-06.patch JIRA Issue HADOOP-12548 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9f927f692ac9 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / a429f85 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8586/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Max memory used 77MB Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8586/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          Larry McCay, thanks for your diligence! FWIW, I think this is a case where you would have been justified flipping the bird right back at pre-commit and choosing not to change the patch. The cleanup is appreciated though.

          Show
          cnauroth Chris Nauroth added a comment - Larry McCay , thanks for your diligence! FWIW, I think this is a case where you would have been justified flipping the bird right back at pre-commit and choosing not to change the patch. The cleanup is appreciated though.
          Hide
          lmccay Larry McCay added a comment -

          HA - I figured as much but I would just be moving it to the next poor sap that has to change this file.
          Which might end up being me anyway.

          Show
          lmccay Larry McCay added a comment - HA - I figured as much but I would just be moving it to the next poor sap that has to change this file. Which might end up being me anyway.
          Hide
          cnauroth Chris Nauroth added a comment -
          1. Instead of catching and logging IOException from the Configuration#getPassword calls, I'm wondering if it makes more sense to just let the exceptions propagate up through S3AFileSystem#initialize and let them abort initialization. Catching and proceeding might put the process into unusual states that are difficult for an operator to reason about. For example, suppose the credential provider is a keystore file saved to an HDFS URL, and HDFS goes down after successful retrieval of the access key but before retrieval of the secret key. That would leave the process running in a state where initialization succeeded, but it doesn't really have complete credentials, and access to S3 will fail.
          2. In S3AFileSystem#getAWSAccessKeys:
                if (accessKey == null || secretKey == null) {
                  throw new IOException("Cannot find AWS access or secret key. required!");
                }
            

            I don't think we can throw an exception here if there is no access key/secret key in configuration. This would break environments that don't configure credentials in Hadoop configuration and instead rely on one of the other providers in the chain, like InstanceProfileCredentialsProvider. It's OK to construct an instance of BasicAWSCredentialsProvider using null values. It will throw an AmazonClientException later when anything tries to get credentials from it. The logic of AWSCredentialsProviderChain#getCredentials is to iterate through each provider in the chain, try to get credentials from it, and ignore exceptions. The first provider that doesn't throw an exception and returns non-null credentials will be used.

          3. Just a minor nit-pick: please use lower-case "test" for the test method names.
          Show
          cnauroth Chris Nauroth added a comment - Instead of catching and logging IOException from the Configuration#getPassword calls, I'm wondering if it makes more sense to just let the exceptions propagate up through S3AFileSystem#initialize and let them abort initialization. Catching and proceeding might put the process into unusual states that are difficult for an operator to reason about. For example, suppose the credential provider is a keystore file saved to an HDFS URL, and HDFS goes down after successful retrieval of the access key but before retrieval of the secret key. That would leave the process running in a state where initialization succeeded, but it doesn't really have complete credentials, and access to S3 will fail. In S3AFileSystem#getAWSAccessKeys : if (accessKey == null || secretKey == null ) { throw new IOException( "Cannot find AWS access or secret key. required!" ); } I don't think we can throw an exception here if there is no access key/secret key in configuration. This would break environments that don't configure credentials in Hadoop configuration and instead rely on one of the other providers in the chain, like InstanceProfileCredentialsProvider . It's OK to construct an instance of BasicAWSCredentialsProvider using null values. It will throw an AmazonClientException later when anything tries to get credentials from it. The logic of AWSCredentialsProviderChain#getCredentials is to iterate through each provider in the chain, try to get credentials from it, and ignore exceptions. The first provider that doesn't throw an exception and returns non-null credentials will be used. Just a minor nit-pick: please use lower-case "test" for the test method names.
          Hide
          lmccay Larry McCay added a comment -

          Chris Nauroth - thanks for catching that logic issue. I am going to change the catch and logging to a catch and throw a new IOException with the previous logging message for additional context and remove the later test for null credentials and exception.

          The test method names... I didn't add that test class and continued the upperclass style as odd as it felt. I can certainly change all the methods to proper case if that makes. If I don't hear otherwise in a few hours, I will just go ahead and do that. Test method names shouldn't be referenced anywhere directly but I will try and ensure that they aren't.

          Show
          lmccay Larry McCay added a comment - Chris Nauroth - thanks for catching that logic issue. I am going to change the catch and logging to a catch and throw a new IOException with the previous logging message for additional context and remove the later test for null credentials and exception. The test method names... I didn't add that test class and continued the upperclass style as odd as it felt. I can certainly change all the methods to proper case if that makes. If I don't hear otherwise in a few hours, I will just go ahead and do that. Test method names shouldn't be referenced anywhere directly but I will try and ensure that they aren't.
          Hide
          lmccay Larry McCay added a comment -

          Addressed Chris' review comments and cleaned up the Test method names in TestS3AConfiguration to be proper camelCase.

          Show
          lmccay Larry McCay added a comment - Addressed Chris' review comments and cleaned up the Test method names in TestS3AConfiguration to be proper camelCase.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 11s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 7m 20s trunk passed
          +1 compile 0m 13s trunk passed with JDK v1.8.0_66
          +1 compile 0m 14s trunk passed with JDK v1.7.0_91
          +1 checkstyle 0m 15s trunk passed
          +1 mvnsite 0m 19s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 30s trunk passed
          +1 javadoc 0m 13s trunk passed with JDK v1.8.0_66
          +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91
          +1 mvninstall 0m 14s the patch passed
          +1 compile 0m 9s the patch passed with JDK v1.8.0_66
          +1 javac 0m 9s the patch passed
          +1 compile 0m 11s the patch passed with JDK v1.7.0_91
          +1 javac 0m 11s the patch passed
          +1 checkstyle 0m 12s hadoop-tools/hadoop-aws: patch generated 0 new + 40 unchanged - 1 fixed = 40 total (was 41)
          +1 mvnsite 0m 16s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s Patch has no whitespace issues.
          +1 findbugs 0m 40s the patch passed
          +1 javadoc 1m 5s hadoop-tools_hadoop-aws-jdk1.8.0_66 with JDK v1.8.0_66 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15)
          +1 javadoc 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66.
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_91
          +1 unit 0m 9s hadoop-aws in the patch passed with JDK v1.8.0_66.
          +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.7.0_91.
          +1 asflicense 0m 19s Patch does not generate ASF License warnings.
          13m 42s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:0ca8df7
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787680/HADOOP-12548-07.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0b9ea14a1a08 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 51fc7f5
          Default Java 1.7.0_91
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91
          findbugs v3.0.0
          JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8610/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8610/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 11s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 20s trunk passed +1 compile 0m 13s trunk passed with JDK v1.8.0_66 +1 compile 0m 14s trunk passed with JDK v1.7.0_91 +1 checkstyle 0m 15s trunk passed +1 mvnsite 0m 19s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 30s trunk passed +1 javadoc 0m 13s trunk passed with JDK v1.8.0_66 +1 javadoc 0m 15s trunk passed with JDK v1.7.0_91 +1 mvninstall 0m 14s the patch passed +1 compile 0m 9s the patch passed with JDK v1.8.0_66 +1 javac 0m 9s the patch passed +1 compile 0m 11s the patch passed with JDK v1.7.0_91 +1 javac 0m 11s the patch passed +1 checkstyle 0m 12s hadoop-tools/hadoop-aws: patch generated 0 new + 40 unchanged - 1 fixed = 40 total (was 41) +1 mvnsite 0m 16s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s Patch has no whitespace issues. +1 findbugs 0m 40s the patch passed +1 javadoc 1m 5s hadoop-tools_hadoop-aws-jdk1.8.0_66 with JDK v1.8.0_66 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15) +1 javadoc 0m 10s hadoop-aws in the patch passed with JDK v1.8.0_66. +1 javadoc 0m 13s the patch passed with JDK v1.7.0_91 +1 unit 0m 9s hadoop-aws in the patch passed with JDK v1.8.0_66. +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.7.0_91. +1 asflicense 0m 19s Patch does not generate ASF License warnings. 13m 42s Subsystem Report/Notes Docker Image:yetus/hadoop:0ca8df7 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12787680/HADOOP-12548-07.patch JIRA Issue HADOOP-12548 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0b9ea14a1a08 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 51fc7f5 Default Java 1.7.0_91 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_66 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_91 findbugs v3.0.0 JDK v1.7.0_91 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8610/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8610/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          mattpaduano Matthew Paduano added a comment -

          I grabbed patch 07 and tested it. When the user does everything correctly, it works.
          Thanks!

          It still gives that awful and misleading message "Unable to load AWS credentials from any provider in the chain" when one, say, mistypes a long crazy filename b/c it is eating the nice informative IOE that is saying "that filename you typed does not exist". Too bad we can't fix that woeful messaging. All you need to do is not eat those IOE's... let 'em rise. I think that is what Chris was recommending in section (1) of his comment?

          Show
          mattpaduano Matthew Paduano added a comment - I grabbed patch 07 and tested it. When the user does everything correctly, it works. Thanks! It still gives that awful and misleading message "Unable to load AWS credentials from any provider in the chain" when one, say, mistypes a long crazy filename b/c it is eating the nice informative IOE that is saying "that filename you typed does not exist". Too bad we can't fix that woeful messaging. All you need to do is not eat those IOE's... let 'em rise. I think that is what Chris was recommending in section (1) of his comment?
          Hide
          lmccay Larry McCay added a comment -

          Hmmm...

          I'm having trouble interpreting things here for some reason.
          I thought that if I caught the IOEs and added more context and threw the new IOE and the fact that initialize doesn't catch the IOE that it would rise as Chris suggested. What am I missing there.

          Also, where am I eating IOE's? Is adding the additional context and throwing a new one losing previous context?

          I can't even find the error that you are seeing.

          What am I missing?

          Show
          lmccay Larry McCay added a comment - Hmmm... I'm having trouble interpreting things here for some reason. I thought that if I caught the IOEs and added more context and threw the new IOE and the fact that initialize doesn't catch the IOE that it would rise as Chris suggested. What am I missing there. Also, where am I eating IOE's? Is adding the additional context and throwing a new one losing previous context? I can't even find the error that you are seeing. What am I missing?
          Hide
          cnauroth Chris Nauroth added a comment -

          Matthew Paduano, thank you for testing. Patch v007 does look like the kind of thing I had in mind for letting the exceptions propagate during initialization, so I'm also curious to see more details on any shortcomings you noticed in the error messages.

          Are you referring to this error from within the Amazon code?

          http://grepcode.com/file/repo1.maven.org/maven2/com.amazonaws/aws-java-sdk/1.8.1/com/amazonaws/auth/AWSCredentialsProviderChain.java#112

          I would have expected that with patch v007, a misconfiguration would be an initialization error, so execution never would even reach the point of trying to walk through the credential chain. Maybe I am missing something.

          Show
          cnauroth Chris Nauroth added a comment - Matthew Paduano , thank you for testing. Patch v007 does look like the kind of thing I had in mind for letting the exceptions propagate during initialization, so I'm also curious to see more details on any shortcomings you noticed in the error messages. Are you referring to this error from within the Amazon code? http://grepcode.com/file/repo1.maven.org/maven2/com.amazonaws/aws-java-sdk/1.8.1/com/amazonaws/auth/AWSCredentialsProviderChain.java#112 I would have expected that with patch v007, a misconfiguration would be an initialization error, so execution never would even reach the point of trying to walk through the credential chain. Maybe I am missing something.
          Hide
          mattpaduano Matthew Paduano added a comment -

          I want to clarify my comment. Upon more careful debugging, certain typos are caught and messaged appropriately. The initial typo I tried did not. It depends on where in the URL one happens to munge the string. The issue does not seem to be related to the code in this patch.

          I am using a test command with an option:

          -Dhadoop.security.credential.provider.path=jceks://hdfs@localhost:9000*/user/mattp/aws.jceks*

          there are (at least) three possible places one might mistype. An error in the protocol or the filesystem scheme will be caught/notified appropriately. But an error in the HDFS filename itself (the last portion of the above string) does not. I did not track it down, but it may be a feature/bug in the conf processing code.

          Show
          mattpaduano Matthew Paduano added a comment - I want to clarify my comment. Upon more careful debugging, certain typos are caught and messaged appropriately. The initial typo I tried did not. It depends on where in the URL one happens to munge the string. The issue does not seem to be related to the code in this patch. I am using a test command with an option: -Dhadoop.security.credential.provider.path= jceks :// hdfs @localhost:9000*/user/mattp/aws.jceks* there are (at least) three possible places one might mistype. An error in the protocol or the filesystem scheme will be caught/notified appropriately. But an error in the HDFS filename itself (the last portion of the above string) does not. I did not track it down, but it may be a feature/bug in the conf processing code .
          Hide
          mattpaduano Matthew Paduano added a comment -

          I am not sure this is directly related to my original comment. But you might be changing the type of the exception. Things catching FileNotFoundException won't catch the IOE you re-throw...

          Show
          mattpaduano Matthew Paduano added a comment - I am not sure this is directly related to my original comment. But you might be changing the type of the exception. Things catching FileNotFoundException won't catch the IOE you re-throw...
          Hide
          mattpaduano Matthew Paduano added a comment -

          I did a little more debugging.

          When the first string ("jceks") is munged, an exception is thrown at:

          Caused by: java.io.IOException: Configuration problem with provider path.
          at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2059)
          at org.apache.hadoop.conf.Configuration.getPassword(Configuration.java:1992)
          at org.apache.hadoop.fs.s3a.S3AFileSystem.getAWSAccessKeys(S3AFileSystem.java:308)
          ... 11 more
          Caused by: java.io.IOException: No CredentialProviderFactory for ceks://hdfs@localhost:9000/user/mattp/aws.jceks in hadoop.security.credential.provider.path
          at org.apache.hadoop.security.alias.CredentialProviderFactory.getProviders(CredentialProviderFactory.java:66)
          at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2024)

          When the second string ("hdfs") is munged, an exception is thrown at:

          Caused by: java.io.IOException: Configuration problem with provider path.
          at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2059)
          at org.apache.hadoop.conf.Configuration.getPassword(Configuration.java:1992)
          at org.apache.hadoop.fs.s3a.S3AFileSystem.getAWSAccessKeys(S3AFileSystem.java:308)
          ... 11 more
          Caused by: java.io.IOException: No FileSystem for scheme: dfs
          at org.apache.hadoop.fs.FileSystem.getFileSystemClass(FileSystem.java:2774)
          at org.apache.hadoop.fs.FileSystem.createFileSystem(FileSystem.java:2785)

          But when the third string ("aws.jceks") is munged, the code in Configuration.getPassword* does not
          bother to throw an exception. In particular, a line in getPasswordFromCredentialProviders() that might
          throw an exc returns null instead:

          CredentialEntry entry = provider.getCredentialEntry(name);

          I did not find the specific impl of this code and did not trace it through the HDFS access. I am
          guessing something in that layer threw some sort of FileNotFoundExc and it got eaten in the impl code?

          Show
          mattpaduano Matthew Paduano added a comment - I did a little more debugging. When the first string ("jceks") is munged, an exception is thrown at: Caused by: java.io.IOException: Configuration problem with provider path. at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2059) at org.apache.hadoop.conf.Configuration.getPassword(Configuration.java:1992) at org.apache.hadoop.fs.s3a.S3AFileSystem.getAWSAccessKeys(S3AFileSystem.java:308) ... 11 more Caused by: java.io.IOException: No CredentialProviderFactory for ceks://hdfs@localhost:9000/user/mattp/aws.jceks in hadoop.security.credential.provider.path at org.apache.hadoop.security.alias.CredentialProviderFactory.getProviders(CredentialProviderFactory.java:66) at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2024) When the second string ("hdfs") is munged, an exception is thrown at: Caused by: java.io.IOException: Configuration problem with provider path. at org.apache.hadoop.conf.Configuration.getPasswordFromCredentialProviders(Configuration.java:2059) at org.apache.hadoop.conf.Configuration.getPassword(Configuration.java:1992) at org.apache.hadoop.fs.s3a.S3AFileSystem.getAWSAccessKeys(S3AFileSystem.java:308) ... 11 more Caused by: java.io.IOException: No FileSystem for scheme: dfs at org.apache.hadoop.fs.FileSystem.getFileSystemClass(FileSystem.java:2774) at org.apache.hadoop.fs.FileSystem.createFileSystem(FileSystem.java:2785) But when the third string ("aws.jceks") is munged, the code in Configuration.getPassword* does not bother to throw an exception. In particular, a line in getPasswordFromCredentialProviders() that might throw an exc returns null instead: CredentialEntry entry = provider.getCredentialEntry(name); I did not find the specific impl of this code and did not trace it through the HDFS access. I am guessing something in that layer threw some sort of FileNotFoundExc and it got eaten in the impl code?
          Hide
          cnauroth Chris Nauroth added a comment -

          Hi Matthew Paduano. I think this is because the jceks provider is implemented as "create keystore on first use". The relevant code for this is in org.apache.hadoop.security.alias.AbstractJavaKeyStoreProvider. I don't know for sure if it's an intentional design choice or a bug that this behavior kicks in at read time through Configuration#getPassword. Larry McCay, maybe you know better?

          If there is a problem in there, then let's address it outside the scope of this patch and instead file a new JIRA.

          Show
          cnauroth Chris Nauroth added a comment - Hi Matthew Paduano . I think this is because the jceks provider is implemented as "create keystore on first use". The relevant code for this is in org.apache.hadoop.security.alias.AbstractJavaKeyStoreProvider . I don't know for sure if it's an intentional design choice or a bug that this behavior kicks in at read time through Configuration#getPassword . Larry McCay , maybe you know better? If there is a problem in there, then let's address it outside the scope of this patch and instead file a new JIRA.
          Hide
          lmccay Larry McCay added a comment -

          Interesting...

          I'll write some tests for this using the provider path that Matthew Paduano is using there and track down the issue.
          I agree that this should be done outside of this patch so that it can easily be added to other branches if a fix is needed.

          Show
          lmccay Larry McCay added a comment - Interesting... I'll write some tests for this using the provider path that Matthew Paduano is using there and track down the issue. I agree that this should be done outside of this patch so that it can easily be added to other branches if a fix is needed.
          Hide
          lmccay Larry McCay added a comment -

          I see...
          So, when you say munged, you mean mistyped.

          The mistyping of a filename isn't really as easily determined. It could be your intention to check for an optionally available keystore at that location and name. The fact that it returns null from getPassword is indicating that no password was found by the valid jceks provider in the HDFS filesystem location that you specified. That is the intended behavior.

          I could be convinced that this is misleading. Matthew Paduano - can you tell me whether there was any logging that indicated the password couldn't be found in the specified location? Perhaps a bit of better diagnostic logging would help if there wasn't.

          Show
          lmccay Larry McCay added a comment - I see... So, when you say munged, you mean mistyped. The mistyping of a filename isn't really as easily determined. It could be your intention to check for an optionally available keystore at that location and name. The fact that it returns null from getPassword is indicating that no password was found by the valid jceks provider in the HDFS filesystem location that you specified. That is the intended behavior. I could be convinced that this is misleading. Matthew Paduano - can you tell me whether there was any logging that indicated the password couldn't be found in the specified location? Perhaps a bit of better diagnostic logging would help if there wasn't.
          Hide
          lmccay Larry McCay added a comment -

          I should also point out that when a config value like the same credentials are supposed to be stored in config but are missing it doesn't throw an exception - it returns null or the provided default.

          Show
          lmccay Larry McCay added a comment - I should also point out that when a config value like the same credentials are supposed to be stored in config but are missing it doesn't throw an exception - it returns null or the provided default.
          Hide
          mattpaduano Matthew Paduano added a comment -

          I don't see anything in the log.

          I think this may be happening in AbstractJavaKeyStoreProvider

                if (keystoreExists()) {
                  stashOriginalFilePermissions();
                  try (InputStream in = getInputStreamForFile()) {
                    keyStore.load(in, password);
                  }
                } else {
                  createPermissions("700");
                  // required to create an empty keystore. *sigh*
                  keyStore.load(null, password);
                }
          

          and from JavaKeyStoreProvider

            protected boolean keystoreExists() throws IOException {
              return fs.exists(getPath());
            }
          
            protected InputStream getInputStreamForFile() throws IOException {
              return fs.open(getPath());
            }
          

          If the fs.open() were simply attempted without the fs.exists() a FileNotFoundException should be thrown.
          But if other code relies on that alternate behavior, then I suppose all that might be done is to add a log message?

          Show
          mattpaduano Matthew Paduano added a comment - I don't see anything in the log. I think this may be happening in AbstractJavaKeyStoreProvider if (keystoreExists()) { stashOriginalFilePermissions(); try (InputStream in = getInputStreamForFile()) { keyStore.load(in, password); } } else { createPermissions( "700" ); // required to create an empty keystore. *sigh* keyStore.load( null , password); } and from JavaKeyStoreProvider protected boolean keystoreExists() throws IOException { return fs.exists(getPath()); } protected InputStream getInputStreamForFile() throws IOException { return fs.open(getPath()); } If the fs.open() were simply attempted without the fs.exists() a FileNotFoundException should be thrown. But if other code relies on that alternate behavior, then I suppose all that might be done is to add a log message?
          Hide
          lmccay Larry McCay added a comment -

          You may be right.
          Based on that code, it seems that you should have ended up with a new keystore in that location but only if flush() were to be called.

          Yes - it seems that the intent is that when you load a provider with a potentially valid path that the keystore will be loaded there and available to be written to. If you were to do an operation that required a write - such as: add or delete a credential then flush would be called to write it to disk.

          Configuration.getPassword() does not require a write so it probably never got realized on disk.

          So in essence, we are seeing the creation of a previously non-existent keystore through the JCEKS provider in a valid location within HDFS but it is only in memory. We then ask for an alias that does not exist in it and it returns null which is the expected behavior. I don't believe that the keystore is being written to disk.

          If this is a concern then we can take it up in a new JIRA and would have to consider the other consumers of the credential provider API as you said - such as the credential CLI command. I think throwing an exception at this point would affect a bunch of code around protecting SSL related credentials and the like and across different projects.

          Show
          lmccay Larry McCay added a comment - You may be right. Based on that code, it seems that you should have ended up with a new keystore in that location but only if flush() were to be called. Yes - it seems that the intent is that when you load a provider with a potentially valid path that the keystore will be loaded there and available to be written to. If you were to do an operation that required a write - such as: add or delete a credential then flush would be called to write it to disk. Configuration.getPassword() does not require a write so it probably never got realized on disk. So in essence, we are seeing the creation of a previously non-existent keystore through the JCEKS provider in a valid location within HDFS but it is only in memory. We then ask for an alias that does not exist in it and it returns null which is the expected behavior. I don't believe that the keystore is being written to disk. If this is a concern then we can take it up in a new JIRA and would have to consider the other consumers of the credential provider API as you said - such as the credential CLI command. I think throwing an exception at this point would affect a bunch of code around protecting SSL related credentials and the like and across different projects.
          Hide
          cnauroth Chris Nauroth added a comment -

          +1 for patch v07. I did a successful full test run using my S3 credentials. I did find a test failure that is unrelated to this patch. I filed HADOOP-12801 to track that.

          Thanks very much, Larry! I'm going to hold off committing for a while in case any of the other reviewers want to take another look.

          I didn't realize the existing tests in this suite had the wrong capitalization. Thanks for the diligence on code cleanup too.

          Things catching FileNotFoundException won't catch the IOE you re-throw...

          I think this is OK. The user-facing API where these exceptions would propagate is FileSystem#initialize. The documented contract of that method is to throw IOException, with no extra significance attached to FileNotFoundException or any other subclass.

          Show
          cnauroth Chris Nauroth added a comment - +1 for patch v07. I did a successful full test run using my S3 credentials. I did find a test failure that is unrelated to this patch. I filed HADOOP-12801 to track that. Thanks very much, Larry! I'm going to hold off committing for a while in case any of the other reviewers want to take another look. I didn't realize the existing tests in this suite had the wrong capitalization. Thanks for the diligence on code cleanup too. Things catching FileNotFoundException won't catch the IOE you re-throw... I think this is OK. The user-facing API where these exceptions would propagate is FileSystem#initialize . The documented contract of that method is to throw IOException , with no extra significance attached to FileNotFoundException or any other subclass.
          Hide
          lmccay Larry McCay added a comment -

          While looking at the docs for S3A, I notice:

          1. There is nothing in the existing docs for the added use of the credential providers in S3 and S3N that already existed. I could add that as well. Though there are semantic differences in the processing order of precedence between what I just added in S3A and the existing functionality there. If those filesystems are being deprecated or used less maybe we shouldn't add anything there?
          2. There is a proxyPassword parameter as well that we could also protect use credential providers. I don't think that I would want to hold up this patch for it but would be willing to file a new JIRA to add that support as well - if we want that. Currently it seems that the proxyPassword is in the config in clear text.

          String proxyUsername = conf.getTrimmed(PROXY_USERNAME);
          String proxyPassword = conf.getTrimmed(PROXY_PASSWORD);
          if ((proxyUsername == null) != (proxyPassword == null))

          Unknown macro: { String msg = "Proxy error}

          I will add the docs for S3A use of credential providers in HADOOP-11031 and get a patch ready for review there next.

          Just let me know about #2 above.

          Show
          lmccay Larry McCay added a comment - While looking at the docs for S3A, I notice: 1. There is nothing in the existing docs for the added use of the credential providers in S3 and S3N that already existed. I could add that as well. Though there are semantic differences in the processing order of precedence between what I just added in S3A and the existing functionality there. If those filesystems are being deprecated or used less maybe we shouldn't add anything there? 2. There is a proxyPassword parameter as well that we could also protect use credential providers. I don't think that I would want to hold up this patch for it but would be willing to file a new JIRA to add that support as well - if we want that. Currently it seems that the proxyPassword is in the config in clear text. String proxyUsername = conf.getTrimmed(PROXY_USERNAME); String proxyPassword = conf.getTrimmed(PROXY_PASSWORD); if ((proxyUsername == null) != (proxyPassword == null)) Unknown macro: { String msg = "Proxy error} I will add the docs for S3A use of credential providers in HADOOP-11031 and get a patch ready for review there next. Just let me know about #2 above.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          Hi Larry McCay,

          thanks, finally someone cleaned up our initialize() mess Possibly, some outstanding patches will no longer merge but that's a minor inconvenience, it needed to be done.

          1. My take:
            • Don't think s3 is still used, the 'inode-tricks' it tries to pull-off are guaranteed data loss
            • s3n is still used but is planned to be deprecated once s3a is considered fully stable: Steve Loughran has been rejecting non-critical patches to s3n and pointing those people to s3a for a while now (recent example: HADOOP-12353). FYI Most work on stabilizing s3a is grouped in HADOOP-11694. HADOOP-9565 is also relevant (2x write performance).
          2. Good idea. Now that your patch has secured the access credentials it makes sense to do the same for the proxy password.
          Show
          Thomas Demoor Thomas Demoor added a comment - Hi Larry McCay , thanks, finally someone cleaned up our initialize() mess Possibly, some outstanding patches will no longer merge but that's a minor inconvenience, it needed to be done. My take: Don't think s3 is still used, the 'inode-tricks' it tries to pull-off are guaranteed data loss s3n is still used but is planned to be deprecated once s3a is considered fully stable: Steve Loughran has been rejecting non-critical patches to s3n and pointing those people to s3a for a while now (recent example: HADOOP-12353 ). FYI Most work on stabilizing s3a is grouped in HADOOP-11694 . HADOOP-9565 is also relevant (2x write performance). Good idea. Now that your patch has secured the access credentials it makes sense to do the same for the proxy password.
          Hide
          lmccay Larry McCay added a comment -

          Thanks for the insights, Thomas Demoor!

          You can thank Yetus for both the cleanup and the broken patches.

          I think that I will leave the credential provider docs out of the S3 and S3n sections then.
          I will file a new JIRA for the proxy password.

          Show
          lmccay Larry McCay added a comment - Thanks for the insights, Thomas Demoor ! You can thank Yetus for both the cleanup and the broken patches. I think that I will leave the credential provider docs out of the S3 and S3n sections then. I will file a new JIRA for the proxy password.
          Hide
          Thomas Demoor Thomas Demoor added a comment -

          Right, I forgot we started using Allen Wittenauer's magic. Thanks!

          Show
          Thomas Demoor Thomas Demoor added a comment - Right, I forgot we started using Allen Wittenauer 's magic. Thanks!
          Hide
          cnauroth Chris Nauroth added a comment -

          There is a slight merge conflict for this patch on branch-2, because HADOOP-11684 made changes to S3AFileSystem that are only in trunk. Just to be safe, I'm attaching a branch-2 patch for a pre-commit run. I'm also repeating a live test run against the S3 service from the branch-2 code.

          Show
          cnauroth Chris Nauroth added a comment - There is a slight merge conflict for this patch on branch-2, because HADOOP-11684 made changes to S3AFileSystem that are only in trunk. Just to be safe, I'm attaching a branch-2 patch for a pre-commit run. I'm also repeating a live test run against the S3 service from the branch-2 code.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 9m 5s branch-2 passed
          +1 compile 0m 11s branch-2 passed with JDK v1.8.0_72
          +1 compile 0m 13s branch-2 passed with JDK v1.7.0_95
          +1 checkstyle 0m 15s branch-2 passed
          +1 mvnsite 0m 20s branch-2 passed
          +1 mvneclipse 1m 16s branch-2 passed
          +1 findbugs 0m 37s branch-2 passed
          +1 javadoc 0m 13s branch-2 passed with JDK v1.8.0_72
          +1 javadoc 0m 14s branch-2 passed with JDK v1.7.0_95
          +1 mvninstall 0m 14s the patch passed
          +1 compile 0m 8s the patch passed with JDK v1.8.0_72
          +1 javac 0m 8s the patch passed
          +1 compile 0m 11s the patch passed with JDK v1.7.0_95
          +1 javac 0m 11s the patch passed
          -1 checkstyle 0m 11s hadoop-tools/hadoop-aws: patch generated 1 new + 46 unchanged - 2 fixed = 47 total (was 48)
          +1 mvnsite 0m 16s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 1s Patch has no whitespace issues.
          +1 findbugs 0m 39s the patch passed
          +1 javadoc 1m 1s hadoop-tools_hadoop-aws-jdk1.8.0_72 with JDK v1.8.0_72 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15)
          +1 javadoc 0m 9s hadoop-aws in the patch passed with JDK v1.8.0_72.
          +1 javadoc 0m 13s the patch passed with JDK v1.7.0_95
          +1 unit 0m 9s hadoop-aws in the patch passed with JDK v1.8.0_72.
          +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.7.0_95.
          +1 asflicense 0m 18s Patch does not generate ASF License warnings.
          16m 29s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:babe025
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788279/HADOOP-12548-branch-2.07.patch
          JIRA Issue HADOOP-12548
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 0efbd0fa89ac 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 91c4a3a
          Default Java 1.7.0_95
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8646/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt
          JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8646/testReport/
          modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8646/console
          Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 9m 5s branch-2 passed +1 compile 0m 11s branch-2 passed with JDK v1.8.0_72 +1 compile 0m 13s branch-2 passed with JDK v1.7.0_95 +1 checkstyle 0m 15s branch-2 passed +1 mvnsite 0m 20s branch-2 passed +1 mvneclipse 1m 16s branch-2 passed +1 findbugs 0m 37s branch-2 passed +1 javadoc 0m 13s branch-2 passed with JDK v1.8.0_72 +1 javadoc 0m 14s branch-2 passed with JDK v1.7.0_95 +1 mvninstall 0m 14s the patch passed +1 compile 0m 8s the patch passed with JDK v1.8.0_72 +1 javac 0m 8s the patch passed +1 compile 0m 11s the patch passed with JDK v1.7.0_95 +1 javac 0m 11s the patch passed -1 checkstyle 0m 11s hadoop-tools/hadoop-aws: patch generated 1 new + 46 unchanged - 2 fixed = 47 total (was 48) +1 mvnsite 0m 16s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 1s Patch has no whitespace issues. +1 findbugs 0m 39s the patch passed +1 javadoc 1m 1s hadoop-tools_hadoop-aws-jdk1.8.0_72 with JDK v1.8.0_72 generated 0 new + 8 unchanged - 7 fixed = 8 total (was 15) +1 javadoc 0m 9s hadoop-aws in the patch passed with JDK v1.8.0_72. +1 javadoc 0m 13s the patch passed with JDK v1.7.0_95 +1 unit 0m 9s hadoop-aws in the patch passed with JDK v1.8.0_72. +1 unit 0m 11s hadoop-aws in the patch passed with JDK v1.7.0_95. +1 asflicense 0m 18s Patch does not generate ASF License warnings. 16m 29s Subsystem Report/Notes Docker Image:yetus/hadoop:babe025 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12788279/HADOOP-12548-branch-2.07.patch JIRA Issue HADOOP-12548 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 0efbd0fa89ac 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 91c4a3a Default Java 1.7.0_95 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_72 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/8646/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-aws.txt JDK v1.7.0_95 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/8646/testReport/ modules C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/8646/console Powered by Apache Yetus 0.2.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          cnauroth Chris Nauroth added a comment -

          The Checkstyle warning is on code that this patch doesn't touch. My branch-2 test run against the S3 service passed, except for an unrelated problem that I found in TestS3ContractRootDir, which is unrelated to this patch. I filed HADOOP-12815 for follow-up.

          I'm going to proceed with commits here.

          Show
          cnauroth Chris Nauroth added a comment - The Checkstyle warning is on code that this patch doesn't touch. My branch-2 test run against the S3 service passed, except for an unrelated problem that I found in TestS3ContractRootDir , which is unrelated to this patch. I filed HADOOP-12815 for follow-up. I'm going to proceed with commits here.
          Hide
          cnauroth Chris Nauroth added a comment -

          I have committed this to trunk, branch-2 and branch-2.8. Larry, thank you for contributing this patch. Everyone watching, thank you for your help with code reviews and testing.

          Show
          cnauroth Chris Nauroth added a comment - I have committed this to trunk, branch-2 and branch-2.8. Larry, thank you for contributing this patch. Everyone watching, thank you for your help with code reviews and testing.
          Hide
          lmccay Larry McCay added a comment -

          Thanks, Chris Nauroth - and everyone for your reviews and insight!

          Show
          lmccay Larry McCay added a comment - Thanks, Chris Nauroth - and everyone for your reviews and insight!
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Hadoop-trunk-Commit #9315 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9315/)
          HADOOP-12548. Read s3a creds from a Credential Provider. Contributed by (cnauroth: rev 76fab26c5c02cef38924d04136407489fd9457d9)

          • hadoop-common-project/hadoop-common/CHANGES.txt
          • hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AConfiguration.java
          • hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Hadoop-trunk-Commit #9315 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9315/ ) HADOOP-12548 . Read s3a creds from a Credential Provider. Contributed by (cnauroth: rev 76fab26c5c02cef38924d04136407489fd9457d9) hadoop-common-project/hadoop-common/CHANGES.txt hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AConfiguration.java hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java

            People

            • Assignee:
              lmccay Larry McCay
              Reporter:
              aw Allen Wittenauer
            • Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development