Support STS for OSS ufs through RAMRole #16481

StephenRi · 2022-11-07T04:39:16Z

What changes are proposed in this pull request?

Support STS for OSS ufs

Why are the changes needed?

Plaintext AccessKey/AccessSecret is not safe and not Recommended for Aliyun
STS(Security Token Service) for OSS is more safe. For details, see https://help.aliyun.com/document_detail/32016.html

Does this PR introduce any user facing changes?

addition property keys

UNDERFS_OSS_STS_ENABLED
UNDERFS_OSS_RETRY_MAX
UNDERFS_OSS_ECS_RAM_ROLE

#16510

StephenRi · 2022-11-10T01:43:03Z

@fuzhengjia How can I apply a reviewer

underfs/oss/src/main/java/alluxio/underfs/oss/StsOssClientProvider.java

underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java

Jackson-Wang-7 · 2022-11-14T07:58:55Z

So the PR is to support using RAM role to get STS temporary credentials to access OSS ufs. I think it's mostly good.
And I know there are several other Aliyun STS endpoint to get the temporary credentials, like AssumeRole. I'm not sure the title of the PR is clear enough.

Jackson-Wang-7

LGTM

StephenRi · 2022-11-16T06:37:48Z

@Jackson-Wang-7 has finished the review. @dbw9580 Does this need another reviewer?

core/common/src/main/java/alluxio/conf/PropertyKey.java

underfs/oss/src/main/java/alluxio/underfs/oss/StsOssClientProvider.java

StephenRi · 2022-11-16T12:35:38Z

@dbw9580 All advices have been resolved, please check again.

dbw9580

thanks for the improvements!

underfs/oss/src/main/java/alluxio/underfs/oss/OSSUnderFileSystem.java

underfs/oss/src/main/java/alluxio/underfs/oss/StsOssClientProvider.java

dbw9580 · 2022-11-21T04:34:40Z

After digging into the OSS apis, I found there is a com.aliyun.oss.OSSClientBuilder#build(java.lang.String, com.aliyun.oss.common.auth.CredentialsProvider, com.aliyun.oss.ClientBuilderConfiguration) method which takes a CredentialsProvider as argument. You can implement the CredentialsProvider interface and put the token refreshing logic there. Actually I think CustomSessionCredentialsProvider already did the same thing for you.

For credentials, InstanceProfileCredentials has a method willSoonExpire which tells you whether the token needs to be refreshed.

This way, we can decouple the refreshing of credentials from the construction of the OSS client, in case in the future we want to support AssumeRole as another way to do authentication, since there is already a STSAssumeRoleSessionCredentialsProvider.

StephenRi · 2022-11-21T06:21:05Z

After digging into the OSS apis, I found there is a com.aliyun.oss.OSSClientBuilder#build(java.lang.String, com.aliyun.oss.common.auth.CredentialsProvider, com.aliyun.oss.ClientBuilderConfiguration) method which takes a CredentialsProvider as argument. You can implement the CredentialsProvider interface and put the token refreshing logic there. Actually I think CustomSessionCredentialsProvider already did the same thing for you.

For credentials, InstanceProfileCredentials has a method willSoonExpire which tells you whether the token needs to be refreshed.

This way, we can decouple the refreshing of credentials from the construction of the OSS client, in case in the future we want to support AssumeRole as another way to do authentication, since there is already a STSAssumeRoleSessionCredentialsProvider.

@dbw9580 I'm glad to hear about CustomSessionCredentialsProvider.

There is another project in out group which implement STS like this PR did, and it works well. So I just use STS in Alluxio as they do.

Maybe I will consider implementing it using CustomSessionCredentialsProvider and testing it later.

LuQQiu · 2023-01-03T19:57:03Z

@dbw9580 @StephenRi Is the current code good to be merged and @StephenRi can help improve the credential provider logic and add related OSS doc in another PR?

underfs/oss/src/test/java/alluxio/underfs/oss/StsOssClientProviderTest.java

dbw9580

LGTM. The proposed improvement can happen in another PR.

LuQQiu · 2023-01-04T18:58:19Z

@StephenRi please help resolve the final minor comments, and this PR is good to be merged
Please help improve the credential provider logic and add related OSS doc in following PRs.

Co-authored-by: Bowen Ding <6999708+dbw9580@users.noreply.github.com>

StephenRi · 2023-01-05T06:19:07Z

@StephenRi please help resolve the final minor comments, and this PR is good to be merged Please help improve the credential provider logic and add related OSS doc in following PRs.

@LuQQiu @dbw9580 All the comments have been resolved. Please check.

StephenRi · 2023-01-05T06:40:09Z

I will use CustomSessionCredentialsProvider to improve the credential provider later. It is in my TODO list

LuQQiu · 2023-01-05T20:11:13Z

alluxio-bot, merge this please

### What changes are proposed in this pull request? Support STS for OSS ufs ### Why are the changes needed? 1. Plaintext AccessKey/AccessSecret is not safe and not Recommended for Aliyun 2. STS(Security Token Service) for OSS is more safe. For details, see https://help.aliyun.com/document_detail/32016.html ### Does this PR introduce any user facing changes? addition property keys 1. UNDERFS_OSS_STS_ENABLED 2. UNDERFS_OSS_RETRY_MAX 3. UNDERFS_OSS_ECS_RAM_ROLE Alluxio#16510 pr-link: Alluxio#16481 change-id: cid-7d486e84f82e5ab5211238b1f180b4a8fd8e5742

JiamingMai · 2023-04-25T05:15:09Z

alluxio-bot, cherry-pick this to dora please

alluxio-bot · 2023-04-25T05:15:11Z

Auto cherry-pick unsuccessful:
Insufficient permissions to trigger a cherry-pick. user: JiamingMai

This is a PR to cherry-pick committed PR #16481 and #16122 into target branch dora. pr-link: #17320 change-id: cid-2056ac34c59ae3eb9678be69c8c24d55c72603e3

Support STS for OSS ufs 1. Plaintext AccessKey/AccessSecret is not safe and not Recommended for Aliyun 2. STS(Security Token Service) for OSS is more safe. For details, see https://help.aliyun.com/document_detail/32016.html addition property keys 1. UNDERFS_OSS_STS_ENABLED 2. UNDERFS_OSS_RETRY_MAX 3. UNDERFS_OSS_ECS_RAM_ROLE pr-link: Alluxio#16481 change-id: cid-7d486e84f82e5ab5211238b1f180b4a8fd8e5742

### What changes are proposed in this pull request? Support STS for OSS ufs ### Why are the changes needed? 1. Plaintext AccessKey/AccessSecret is not safe and not Recommended for Aliyun 2. STS(Security Token Service) for OSS is more safe. For details, see https://help.aliyun.com/document_detail/32016.html ### Does this PR introduce any user facing changes? addition property keys 1. UNDERFS_OSS_STS_ENABLED 2. UNDERFS_OSS_RETRY_MAX 3. UNDERFS_OSS_ECS_RAM_ROLE Alluxio#16510 pr-link: Alluxio#16481 change-id: cid-7d486e84f82e5ab5211238b1f180b4a8fd8e5742

support sts for oss ufs

d61ac1a

alluxio-bot added the API Change Changes covering public API label Nov 7, 2022

alluxio-bot assigned fuzhengjia Nov 7, 2022

fix checkstyle for OSS STS client

1cf833b

dbw9580 requested a review from Jackson-Wang-7 November 14, 2022 06:21

Jackson-Wang-7 reviewed Nov 14, 2022

View reviewed changes

StephenRi changed the title ~~Support STS for OSS ufs~~ Support STS for OSS ufs through RAMRole Nov 14, 2022

Jackson-Wang-7 approved these changes Nov 14, 2022

View reviewed changes

StephenRi force-pushed the feature/support-sts-for-oss branch from 59e9b09 to bef0ffd Compare November 14, 2022 11:11

extract PropertyKey for STS-OSS

b3715d6

StephenRi force-pushed the feature/support-sts-for-oss branch from bef0ffd to b3715d6 Compare November 14, 2022 11:37

dbw9580 suggested changes Nov 16, 2022

View reviewed changes

dbw9580 reviewed Nov 16, 2022

View reviewed changes

underfs/oss/src/main/java/alluxio/underfs/oss/StsOssClientProvider.java Outdated Show resolved Hide resolved

underfs/oss/src/main/java/alluxio/underfs/oss/StsOssClientProvider.java Outdated Show resolved Hide resolved

StephenRi added 2 commits November 16, 2022 20:12

refine some code for OSS STS

7d31144

refine some order for PropertyKey

0b82ffa

StephenRi force-pushed the feature/support-sts-for-oss branch from 72c330e to 25ca781 Compare November 17, 2022 02:14

simplify logic for OSS STS

873637b

StephenRi force-pushed the feature/support-sts-for-oss branch from 25ca781 to 873637b Compare November 17, 2022 03:10

StephenRi requested a review from dbw9580 November 17, 2022 03:45

dbw9580 suggested changes Nov 17, 2022

View reviewed changes

refine some code for OSS STS

5725a87

StephenRi force-pushed the feature/support-sts-for-oss branch 2 times, most recently from a4c0cf3 to 2db909e Compare November 17, 2022 10:22

fix checkstyle for OSS STS

f5401a0

StephenRi force-pushed the feature/support-sts-for-oss branch from 2db909e to f5401a0 Compare November 17, 2022 10:28

StephenRi requested a review from dbw9580 November 18, 2022 01:51

alluxio-bot added the POM Change label Nov 18, 2022

StephenRi added 2 commits November 18, 2022 18:02

fix checkstyle

557fd41

fix checkstyle for final field

00a1a0c

StephenRi requested a review from dbw9580 November 18, 2022 14:07

LuQQiu added the needs-response waiting on alluxio response label Jan 3, 2023

dbw9580 reviewed Jan 4, 2023

View reviewed changes

underfs/oss/src/test/java/alluxio/underfs/oss/StsOssClientProviderTest.java Outdated Show resolved Hide resolved

underfs/oss/src/test/java/alluxio/underfs/oss/StsOssClientProviderTest.java Outdated Show resolved Hide resolved

dbw9580 approved these changes Jan 4, 2023

View reviewed changes

StephenRi and others added 2 commits January 5, 2023 10:19

Apply suggestions from code review

49da721

Co-authored-by: Bowen Ding <6999708+dbw9580@users.noreply.github.com>

remove unused code for ut StsOssClientProviderTest

3e4a890

StephenRi force-pushed the feature/support-sts-for-oss branch from eb42127 to 3e4a890 Compare January 5, 2023 03:02

dbw9580 approved these changes Jan 5, 2023

View reviewed changes

alluxio-bot merged commit bbe99b3 into Alluxio:master Jan 5, 2023

LuQQiu mentioned this pull request Jan 5, 2023

Refactor s3 low level output stream and support OSS and OBS #16122

Merged

LuQQiu added area-ufs Under File Storage area-security Alluxio security relate labels Feb 4, 2023

JiamingMai mentioned this pull request Apr 25, 2023

Support STS for OSS UFS and refactor Low Level Output Stream #17320

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support STS for OSS ufs through RAMRole #16481

Support STS for OSS ufs through RAMRole #16481

StephenRi commented Nov 7, 2022 •

edited

Loading

StephenRi commented Nov 10, 2022

Jackson-Wang-7 commented Nov 14, 2022

Jackson-Wang-7 left a comment

StephenRi commented Nov 16, 2022

StephenRi commented Nov 16, 2022

dbw9580 left a comment

dbw9580 commented Nov 21, 2022

StephenRi commented Nov 21, 2022 •

edited

Loading

LuQQiu commented Jan 3, 2023 •

edited

Loading

dbw9580 left a comment

LuQQiu commented Jan 4, 2023

StephenRi commented Jan 5, 2023

StephenRi commented Jan 5, 2023

LuQQiu commented Jan 5, 2023

JiamingMai commented Apr 25, 2023

alluxio-bot commented Apr 25, 2023

Support STS for OSS ufs through RAMRole #16481

Support STS for OSS ufs through RAMRole #16481

Conversation

StephenRi commented Nov 7, 2022 • edited Loading

What changes are proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user facing changes?

StephenRi commented Nov 10, 2022

Jackson-Wang-7 commented Nov 14, 2022

Jackson-Wang-7 left a comment

Choose a reason for hiding this comment

StephenRi commented Nov 16, 2022

StephenRi commented Nov 16, 2022

dbw9580 left a comment

Choose a reason for hiding this comment

dbw9580 commented Nov 21, 2022

StephenRi commented Nov 21, 2022 • edited Loading

LuQQiu commented Jan 3, 2023 • edited Loading

dbw9580 left a comment

Choose a reason for hiding this comment

LuQQiu commented Jan 4, 2023

StephenRi commented Jan 5, 2023

StephenRi commented Jan 5, 2023

LuQQiu commented Jan 5, 2023

JiamingMai commented Apr 25, 2023

alluxio-bot commented Apr 25, 2023

StephenRi commented Nov 7, 2022 •

edited

Loading

StephenRi commented Nov 21, 2022 •

edited

Loading

LuQQiu commented Jan 3, 2023 •

edited

Loading