-
Notifications
You must be signed in to change notification settings - Fork 61
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
revert: Update @aws-sdk
client libraries (#16839)"
#16884
Conversation
This reverts commit 94f3f89.
WalkthroughThe pull request involves modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
infra/package.json (2)
28-28
: Document the reason for dependency downgradeThis is a significant version downgrade that's part of reverting PR #16839. Please document:
- What issues were encountered with version 3.687.0?
- Are there any breaking changes that necessitated this revert?
- Is this a temporary fix or a long-term solution?
28-31
: Consider AWS SDK version consistencyThe project is using a mix of AWS SDK v2 (
aws-sdk: ^2.1003.0
) and v3 clients. This could lead to:
- Inconsistent behavior between different AWS service clients
- Increased bundle size from including both SDK versions
- Maintenance overhead from managing two SDK versions
Consider:
- Standardizing on either v2 or v3 across the project
- If v3 is preferred, migrate remaining v2 usage to v3
- If v2 is preferred, consider reverting other v3 clients
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
infra/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (2)
infra/package.json
(1 hunks)package.json
(2 hunks)
🔇 Additional comments (3)
infra/package.json (1)
28-28
: Verify security implications of downgrading @aws-sdk/client-ssm
The downgrade from 3.687.0 to 3.145.0 is significant. Let's verify there are no known security vulnerabilities in this version.
✅ Verification successful
Let me gather more information about the AWS SDK usage in the codebase to understand the impact better.
Let me check the actual SSM client usage in the identified files to understand the implementation details.
Downgrading @aws-sdk/client-ssm is safe for this implementation
The codebase uses basic SSM client functionality (getParameters) that has remained stable across versions. Based on the verification:
- No security vulnerabilities found for version 3.145.0
- Implementation uses only core features (initialization and getParameters)
- No breaking changes that would affect the current usage
- Test mocks confirm simple parameter fetching patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for security advisories for @aws-sdk/client-ssm
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "@aws-sdk/client-ssm") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
# Check the changelog between versions for breaking changes
curl -s "https://raw.githubusercontent.com/aws/aws-sdk-js-v3/main/clients/client-ssm/CHANGELOG.md" | grep -A 5 "3.145.0"
Length of output: 625
Script:
#!/bin/bash
# Check for AWS SDK v3 SSM client usage
rg -l "client-ssm|SSMClient" -g "!package*.json"
# Check for other AWS SDK v3 client versions in package.json
rg "@aws-sdk/client-" infra/package.json
# Check for AWS SDK v2 SSM usage
rg "require.*aws-sdk.*SSM|import.*aws-sdk.*SSM" -g "!package*.json"
Length of output: 292
Script:
#!/bin/bash
# Check SSM client implementation in the identified files
cat infra/src/dsl/adapters/get-ssm-params.ts
cat infra/src/cli/cli.spec.ts
# Check for any AWS SDK version-specific features being used
rg -A 5 "new SSMClient|SSMClient\." infra/src/
Length of output: 5633
package.json (2)
72-77
: Verify AWS SDK v2 and v3 compatibility.
The project uses both AWS SDK v2 ([email protected]) and v3 packages. While this is supported, the significant version differences between v3 packages might cause issues.
Also applies to: 322-323
✅ Verification successful
Based on the search results, I can now generate the final response as we have sufficient information about AWS SDK usage patterns in the codebase.
AWS SDK v2 to v3 migration in progress - No immediate compatibility issues
The codebase shows a controlled transition from AWS SDK v2 to v3, with clear separation of concerns:
- V2 SDK is used primarily in older services for SES, CloudFront, and Elasticsearch connectivity
- V3 SDK is consistently used in newer implementations, particularly for S3 and SQS operations
- No instances found where both SDK versions are used for the same AWS service within a single module
The version differences between v3 packages (all at 3.662.0) are not a concern as they are synchronized.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for AWS SDK v2 and v3 usage patterns to identify potential compatibility issues
rg -A 5 "require\('aws-sdk'\)" || rg -A 5 "from 'aws-sdk'"
rg -A 5 "from '@aws-sdk/"
Length of output: 16778
322-323
: Security concern: Significantly older versions detected.
The AWS SDK packages client-ses
and client-ssm
are being downgraded to a much older version (3.145.0). This could potentially expose security vulnerabilities.
"@aws-sdk/client-s3": "3.662.0", | ||
"@aws-sdk/client-sqs": "3.662.0", | ||
"@aws-sdk/credential-provider-node": "3.662.0", | ||
"@aws-sdk/lib-storage": "3.662.0", | ||
"@aws-sdk/s3-presigned-post": "3.662.0", | ||
"@aws-sdk/s3-request-presigner": "3.662.0", |
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.
Version mismatch detected in AWS SDK packages.
Most AWS SDK packages are set to version 3.662.0, but some packages in devDependencies are set to 3.145.0. This inconsistency might lead to compatibility issues.
Consider aligning all AWS SDK package versions to 3.662.0:
"@aws-sdk/client-s3": "3.662.0",
"@aws-sdk/client-sqs": "3.662.0",
"@aws-sdk/credential-provider-node": "3.662.0",
"@aws-sdk/lib-storage": "3.662.0",
"@aws-sdk/s3-presigned-post": "3.662.0",
"@aws-sdk/s3-request-presigner": "3.662.0",
Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16884 +/- ##
=======================================
Coverage 36.44% 36.44%
=======================================
Files 6851 6851
Lines 143569 143569
Branches 40980 40980
=======================================
Hits 52330 52330
Misses 91239 91239
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
|
@aws-sdk
client libraries (#16839)"@aws-sdk
client libraries (#16839)"
This reverts commit 94f3f89.
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
@aws-sdk/client-s3
,@aws-sdk/client-sqs
,@aws-sdk/credential-provider-node
,@aws-sdk/lib-storage
,@aws-sdk/s3-presigned-post
, and@aws-sdk/s3-request-presigner
from3.687.0
to3.662.0
.@aws-sdk/client-ses
and@aws-sdk/client-ssm
from3.687.0
to3.145.0
.