Skip to content

Migrate to AWS SDK V3 - Part 1 #65

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Migrate to AWS SDK V3 - Part 1 #65

wants to merge 19 commits into from

Conversation

GrahamCampbell
Copy link
Contributor

@GrahamCampbell GrahamCampbell commented Jun 8, 2025

This PR adds AWS SDK v3 support for everything. Opt in to v3 with SLS_AWS_SDK_V3=1. I am running the CI on both versions in the Node 22 job... however mostly things are mocked, so this is not testing much. A notable difference is that the v2 SDK has memoization that we expose with useCache. We are not doing that for v3. Do we want that?

In a follow-up PR we could flip the default (Part 2), and in another follow-up, remove v2 support (Part 3).


Smashing through this with Claude Code. 🤖

@GrahamCampbell GrahamCampbell changed the title Migrate to AWS SDK V3 Migrate to AWS SDK V3 - Part 1 Jun 8, 2025
@GrahamCampbell GrahamCampbell marked this pull request as ready for review June 8, 2025 23:25
@GrahamCampbell GrahamCampbell requested a review from mnapoli June 8, 2025 23:26
@GrahamCampbell GrahamCampbell marked this pull request as draft June 8, 2025 23:57
@GrahamCampbell GrahamCampbell marked this pull request as ready for review June 9, 2025 00:44
return { value: Account };
const provider = serverlessInstance.getProvider('aws');
const result = await provider.request('STS', 'getCallerIdentity', {}, { useCache: true });
return { value: result.Account };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we revert formatting changes like these? This will make the diff much smaller, and also will minimize changes with upstream. It seems like there are 4-5 files impacted.

ListAttachedRolePoliciesCommand,
CreateRoleCommand,
AttachRolePolicyCommand,
} = require('@aws-sdk/client-iam');
const AWSClientFactory = require('../../../../../aws/client-factory');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't these files the custom resource handlers?

If yes, they shouldn't be impacted by the changes in the CLI. And they wouldn't be able to require files from the CLI package (because they would run in Lambda).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this was a fix to make sure they are re-using the same client.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants