-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HADOOP-19648. [hotfix] Cos use token credential will lose token field #7867
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
base: trunk
Are you sure you want to change the base?
Conversation
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@slfan1989 @cnauroth hi guys,could you help me review this pr?thx : ) |
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.
@leosanqing , thank you for the patch. I commented with a few suggestions.
Can you or someone else make sure the integration tests are passing? I don't have the means to run those myself.
...d-storage-project/hadoop-cos/src/test/java/org/apache/hadoop/fs/cosn/TestCosCredentials.java
Outdated
Show resolved
Hide resolved
...d-storage-project/hadoop-cos/src/test/java/org/apache/hadoop/fs/cosn/TestCosCredentials.java
Outdated
Show resolved
Hide resolved
...d-storage-project/hadoop-cos/src/test/java/org/apache/hadoop/fs/cosn/TestCosCredentials.java
Outdated
Show resolved
Hide resolved
Thx bro. Sure, I'v added a new ITest, and pass the test on my local env.(the failed tests are Junit method not found, I've looked up each failed tests) When I'm not change code. This ITest will throw exception like this. |
For newest, I add the test dependence, all tests are passed. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Sorry for committing so many times, I'm not used for hadoop code style. Pls forgive me. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Description of PR
In the org.apache.hadoop.fs.cosn.CosNativeFileSystemStore#initCOSClient method, when the client is initialized, it only passes the access key and secret key, completely ignoring the session token. This causes all subsequent operations that rely on these temporary credentials to fail.
Furthermore, this re-initialization step seems unnecessary. Instead of creating a new client with incomplete credentials, the existing credential provider (which already contains the AK, SK, and token) should be passed down directly.
How was this patch tested?
For code changes: