Kai Zheng commented on HADOOP-13609:


I thought we missed to consolidate the codes in the following two places, as I 
said at the beginning. 

In {{CredentialsProvider getCredentialsProvider}}:
    if (StringUtils.isEmpty(className)) {
      Configuration newConf =
      String accessKeyId =
          AliyunOSSUtils.getValueWithKey(newConf, ACCESS_KEY_ID);
      String accessKeySecret =
          AliyunOSSUtils.getValueWithKey(newConf, ACCESS_KEY_SECRET);
      credentials = new DefaultCredentialProvider(
          new DefaultCredentials(accessKeyId, accessKeySecret));

And in {{AliyunCredentialsProvider(URI uri, Configuration conf)}
  public AliyunCredentialsProvider(URI uri, Configuration conf)
      throws IOException {
    String accessKeyId =
        AliyunOSSUtils.getValueWithKey(conf, ACCESS_KEY_ID);
    String accessKeySecret =
        AliyunOSSUtils.getValueWithKey(conf, ACCESS_KEY_SECRET);
    String securityToken =
        AliyunOSSUtils.getValueWithKey(conf, SECURITY_TOKEN);

    if (!StringUtils.isEmpty(accessKeyId)
        && !StringUtils.isEmpty(accessKeySecret)) {
      credentials = new DefaultCredentials(accessKeyId, accessKeySecret,
    } else {
      throw new InvalidCredentialsException(
          "AccessKeyId and AccessKeySecret should not be null or empty.");

We might have two AliyunCredentialsProvider constructors, one going with a 
security token, the other without token. In the current constructor, the 
{{uri}} parameter isn't used and needed. {{AliyunCredentialsProvider#NAME}} is 
also not needed.

Would you help clean up and refine further? Thanks!

> Refine credential provider related codes for AliyunOss integration
> ------------------------------------------------------------------
>                 Key: HADOOP-13609
>                 URL: https://issues.apache.org/jira/browse/HADOOP-13609
>             Project: Hadoop Common
>          Issue Type: Sub-task
>          Components: fs
>    Affects Versions: HADOOP-12756
>            Reporter: Kai Zheng
>            Assignee: Genmao Yu
>             Fix For: HADOOP-12756
>         Attachments: HADOOP-13609-HADOOP-12756.001.patch, 
> HADOOP-13609-HADOOP-12756.002.patch, HADOOP-13609-HADOOP-12756.003.patch
> looking at the AliyunOss integration codes, some findings:
> 1. {{TemporaryAliyunCredentialsProvider}} could be better named;
> 2. TemporaryAliyunCredentialsProvider shared many codes with 
> {{AliyunOSSUtils#getCredentialsProvider}}, and the dup can be resolved;
> 3. {{AliyunOSSUtils#getPassword}} is rather confusing, as used to get other 
> things than password.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to