-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19535: S3A: Support WebIdentityTokenFileCredentialsProvider #7802
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…fault S3 credential provider chain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 to adding to the default chain, it changes behaviour in a way which isn't obvious to anyone looking at a system.
if you deploy on an older release, auth will fail. asking for an explicit declaration of the provider makes it work everywhere.
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/AWSCredentialProviderList.java
Outdated
Show resolved
Hide resolved
| EC2_IAM_CREDENTIALS_V2); | ||
| v1v2CredentialProviderMap.put(ENVIRONMENT_CREDENTIALS_V1, | ||
| ENVIRONMENT_CREDENTIALS_V2); | ||
| v1v2CredentialProviderMap.put(WEB_IDENTITY_CREDENTIALS_V1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to care about v1 to v2 migration; this wasn't something in use. if it was, we'd have had complaints about
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ack
|
@steveloughran - I have omitted the default credential provider chain changes and kept the rest to make the current AWSCredentialProviderList.java compatible with WebIdentityTokenFileCredentialsProvider or anyother credentials provider which can throw exception other than SDKException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, but see comment about logging
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/AWSCredentialProviderList.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
@steveloughran - Are we good to merge this ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
+1
|
@shameersss1 merged. for a 3.4.x backport, I see we don't have that authentication.md file. Options
IMO option #2 is the best combination of ease and utility. |
|
+1 for option 2 |
|
ok, do that and I'll merge without any review, though of course I expect a test run... |
Description of PR
Support For Adding S3 Credential providers like WebIdentityTokenFileCredentialsProvider which can throw other than SDKException when failing to resolve credentials
How was this patch tested?
mvn -Dparallel-tests -DtestsThreadCount=8 clean verifyFor code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?