SDK | Upgrade AWS SDK to v3 - Block Store S3 #9067
Conversation
|
""" WalkthroughThe changes migrate all AWS S3 and STS interactions from AWS SDK v2 to the modular AWS SDK v3. This includes updating client instantiations, configuration parameters, credential handling, and method calls across implementation, utility, and test files. The Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant cloud_utils.js
participant STSClient
participant S3
Caller->>cloud_utils.js: createSTSS3Client(params, additionalParams)
cloud_utils.js->>STSClient: new STSClient(config)
cloud_utils.js->>STSClient: send(AssumeRoleWithWebIdentityCommand)
STSClient-->>cloud_utils.js: { Credentials }
cloud_utils.js->>S3: new S3({ credentials, ... })
cloud_utils.js-->>Caller: S3 client instance
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
naveenpaul1
force-pushed
the
block_store_ser_sdkv3
branch
5 times, most recently
from
e811dd7 to
c9c3fde
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.json(1 hunks)src/agent/block_store_services/block_store_s3.js(14 hunks)src/test/unit_tests/test_cloud_utils.js(3 hunks)src/util/cloud_utils.js(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`src/test/**/*.*`: Ensure that the PR includes tests for the changes.
src/test/**/*.*: Ensure that the PR includes tests for the changes.
src/test/unit_tests/test_cloud_utils.js
🔇 Additional comments (13)
package.json (1)
85-85: Verify if aws-sdk v2 is still needed.The old AWS SDK v2 dependency is still present. If this upgrade is intended to remove v2 usage from the Block Store S3 component, verify if this dependency is still needed elsewhere in the codebase.
#!/bin/bash # Description: Check if aws-sdk v2 is still used in the codebase # Expect: If only used in files not related to this PR, it's safe to keep echo "Checking aws-sdk v2 usage in the codebase:" rg -A 3 "require\('aws-sdk'\)" --type js echo -e "\nChecking AWS.S3 usage:" rg -A 3 "AWS\.S3" --type js echo -e "\nChecking new AWS SDK v3 usage:" rg -A 3 "@aws-sdk/" --type jssrc/test/unit_tests/test_cloud_utils.js (4)
7-7: LGTM: Import correctly updated for SDK v3.The import has been properly updated to use the modular AWS SDK v3 client.
25-32: LGTM: Stubbing correctly updated for SDK v3.The stubbing approach is correctly updated to mock the
sendmethod onSTSClient.prototype, which is the proper way to stub SDK v3 client calls.
49-49: Verify the timeout extension rationale.The timeout was extended from the default to 50 seconds. Ensure this is necessary for STS operations or if there are specific performance considerations.
59-63: LGTM: Correctly handles asynchronous credential access.The test properly uses
awaitto access credentials and region from the S3 client configuration, which is required for SDK v3.src/agent/block_store_services/block_store_s3.js (5)
5-5: LGTM: Import correctly updated for SDK v3.The import has been properly updated to use the modular AWS SDK v3 S3 client.
15-15: LGTM: Added proper HTTP handler import.The NodeHttpHandler import is correctly added for SDK v3 HTTP configuration.
43-54: LGTM: S3 client instantiation correctly updated for SDK v3.The S3 client configuration is properly updated with:
- Credentials passed as an object
forcePathStyleinstead ofs3ForcePathStyle- Proper
requestHandlerconfiguration withNodeHttpHandler
61-72: LGTM: S3 compatible endpoint configuration updated correctly.The configuration for S3-compatible endpoints is properly updated with:
applyChecksumreplacing the deprecateds3DisableBodySigning- Proper HTTP agent configuration through
requestHandler
82-85: LGTM: S3 method calls correctly updated for SDK v3.All S3 method calls have been properly updated by removing the
.promise()chaining, which is correct since SDK v3 methods return promises natively.Also applies to: 109-112, 167-170, 194-199, 241-248, 265-268, 309-315, 325-328, 345-350, 353-358, 389-396, 424-427
src/util/cloud_utils.js (3)
13-14: LGTM: Imports correctly updated for SDK v3.The imports have been properly updated to include the modular AWS SDK v3 STS client and commands, along with the NodeHttpHandler.
36-44: LGTM: S3 client creation correctly updated for SDK v3.The
createSTSS3Clientfunction is properly updated with:
- New S3 client from SDK v3
- Proper
requestHandlerconfigurationforcePathStyleinstead of the deprecateds3ForcePathStyle
49-67: LGTM: STS credential generation correctly updated for SDK v3.The
generate_aws_sts_credsfunction is properly updated with:
- STSClient instantiation with region configuration
- AssumeRoleWithWebIdentityCommand usage
- Proper credential extraction from the response
- Maintained error handling and validation
naveenpaul1
force-pushed
the
block_store_ser_sdkv3
branch
2 times, most recently
from
f256437 to
139e101
Compare
Signed-off-by: naveenpaul1 <napaul@redhat.com>
naveenpaul1
force-pushed
the
block_store_ser_sdkv3
branch
from
139e101 to
80d4fc2
Compare
|
after this PR, STS request in namespace_s3 is failing, Will update the Namespce_s3 SDK version aslo. |
Describe the Problem
Upgrade AWS SDK to v3 - Block Store S3
Explain the Changes
Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
Summary by CodeRabbit