[1/6] Migrate AWS variable sources to SDK v3#193
Draft
GrahamCampbell wants to merge 1 commit intomainfrom
Draft
Conversation
d2651f1 to
74b7641
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates AWS-backed variable sources used during configuration/variable resolution from the legacy AWS SDK v2 provider.request(...) flow to direct AWS SDK v3 clients, while introducing a shared cached-command sender to preserve caching and limit concurrency.
Changes:
- Switch
aws,cf,s3, andssmvariable sources to use AWS SDK v3 clients/commands via a shared cached sender utility. - Add a generic, concurrency-limited cached command sender for AWS variable sources.
- Add an
s3BodyToStringhelper (with tests) to safely convert various S3 GetObject body shapes into strings.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| lib/configuration/variables/sources/instance-dependent/get-ssm.js | Use @aws-sdk/client-ssm + cached sender; update not-found detection logic for v3-style errors. |
| lib/configuration/variables/sources/instance-dependent/get-s3.js | Use @aws-sdk/client-s3 + cached sender; convert GetObject bodies via s3BodyToString. |
| lib/configuration/variables/sources/instance-dependent/get-cf.js | Use @aws-sdk/client-cloudformation + cached sender; treat missing-stack as null while surfacing other ValidationErrors. |
| lib/configuration/variables/sources/instance-dependent/get-aws.js | Use @aws-sdk/client-sts + cached sender for ${aws:accountId}. |
| lib/configuration/variables/sources/instance-dependent/create-cached-aws-variable-source-command-sender.js | New shared cached sender (per-region client cache, per-command+input cache, concurrency limit). |
| lib/aws/s3-body-to-string.js | New helper to convert SDK v3 S3 bodies (string/buffer/streams/blob/etc.) to string safely. |
| test/unit/lib/configuration/variables/sources/instance-dependent/get-ssm.test.js | Rewrite tests to stub AWS SDK v3 SSM client/command; add caching/credential-provider assertions. |
| test/unit/lib/configuration/variables/sources/instance-dependent/get-s3.test.js | Rewrite tests to stub AWS SDK v3 S3 client/command; add body-shape + caching assertions. |
| test/unit/lib/configuration/variables/sources/instance-dependent/get-cf.test.js | Rewrite tests to stub AWS SDK v3 CF client/command; add caching + ValidationError behavior assertions. |
| test/unit/lib/configuration/variables/sources/instance-dependent/get-aws.test.js | Rewrite tests to stub AWS SDK v3 STS client/command; add caching/credential-provider assertions. |
| test/unit/lib/configuration/variables/sources/instance-dependent/create-cached-aws-variable-source-command-sender.test.js | New unit tests covering caching, eviction on failure, and concurrency limiting behavior. |
| test/unit/lib/aws/s3-body-to-string.test.js | New unit tests covering supported/unsupported S3 body shapes and error propagation. |
| package.json | Add AWS SDK v3 clients for CloudFormation, SSM, and STS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrate all AWS-backed variable sources to SDK v3 in one PR.
Target files:
lib/configuration/variables/sources/instance-dependent/get-ssm.jslib/configuration/variables/sources/instance-dependent/get-cf.jslib/configuration/variables/sources/instance-dependent/get-s3.jslib/configuration/variables/sources/instance-dependent/get-aws.jslib/configuration/variables/sources/instance-dependent/create-cached-aws-variable-source-command-sender.jslib/aws/s3-body-to-string.jstest/unit/lib/configuration/variables/sources/instance-dependent/test/unit/lib/configuration/variables/sources/instance-dependent/test/unit/lib/aws/s3-body-to-string.test.jsAdd dependencies:
@aws-sdk/client-ssm:^3.975.0@aws-sdk/client-sts:^3.975.0@aws-sdk/client-cloudformation:^3.975.0Keep existing dependency:
@aws-sdk/client-s3:^3.975.0PR 1 Helper Design
Add a variable-source-local cached SDK v3 command sender. Suggested API:
Required behavior:
getProvider()until a command is sent.variable-sourcein the helper filename, exported function name, and test filename.effectiveRegionasregion === undefined ? provider.getRegion() : region.await getProvider().getAwsSdkV3Config({ region: effectiveRegion }).new Client(config).new Command(input)throughclient.send(command).transformResult({ result, commandName, input, region, effectiveRegion }), defaulting to identity, so callers can convert non-reusable SDK results before caching.GetObjectCommand, cache the converted string value, not a raw response with a consumableBodystream.Non-goals for this helper:
provider.request().PR 1 SSM Variable Migration
Use
SSMClientandGetParameterCommand.Preserve:
${ssm:/path}syntax.${ssm(region):/path}syntax.rawoption.noDecryptoption.Stringresult handling.StringListsplitting unlessrawis set.SecureStringJSON parsing behavior.null.{ useCache: true }.V3 missing-parameter detection:
error.name === 'ParameterNotFound'as missing.error.Code === 'ParameterNotFound'as missing if present.Credential-provider error behavior:
PR 1 CloudFormation Variable Migration
Use
CloudFormationClientandDescribeStacksCommand.Preserve:
${cf:stack.output}syntax.${cf(region):stack.output}syntax.null.null.{ useCache: true }.V3 missing-stack detection:
error.name === 'ValidationError'plus a message containingdoes not existas missing.PR 1 S3 Variable Migration
Use
S3ClientandGetObjectCommand.Preserve:
${s3:bucket/key}syntax.null.{ useCache: true }.GetObjectCommandoutput with a consumableBodyas the final reusable value.aws-s3-accelerateto S3 variableGetObjectreads.Add a reusable S3 body helper for
GetObjectCommandoutput bodies. It should live outside the variable source so it can be reused later by rollback and artifact-read migrations.Recommended helper file:
lib/aws/s3-body-to-string.jsRecommended helper behavior:
utf8.''fornullorundefinedbodies after the caller has already determined that the object exists.body.transformToString(encoding)when present, because this is the AWS SDK v3SdkStreamMixinpath documented for S3GetObjectCommandoutput.Buffer,Uint8Array, andArrayBufferwithBuffer.BlobviaarrayBuffer()whenBlobis available.ReadableStreamviagetReader()when available.ServerlessErrorwith a stable code, such asUNSUPPORTED_S3_GET_OBJECT_BODY, for unsupported body shapes.String(body)for unsupported objects, because real SDK streams would become"[object Object]".ContentEncodingis response metadata and should be handled later by a response-level helper if needed.Supported body inputs:
stringBufferUint8ArrayArrayBufferReadableStreamBlobtransformToString()V3 missing-key detection:
error.name === 'NoSuchKey'as missing.error.Code === 'NoSuchKey'as missing if present.PR 1 AWS Variable Migration
Use
STSClientandGetCallerIdentityCommandfor${aws:accountId}.Preserve:
${aws:region}behavior without AWS calls.${aws:accountId}value extraction fromAccount.{ useCache: true }.PR 1 Test Plan
Helper tests:
provider.getAwsSdkV3Config({ region: effectiveRegion }).transformResultpromise is evicted.transformResultreceives{ result, commandName, input, region, effectiveRegion }.S3 body helper tests:
Buffer.Uint8Array.ArrayBuffer.ReadableStream.Blobwhen available.transformToString()when present.transformToString().UNSUPPORTED_S3_GET_OBJECT_BODYfor unsupported objects without stringifying contents.SSM tests:
StringListresolves to array by default.StringListwithrawresolves to raw comma-delimited string.SecureStringJSON resolves to object by default.SecureStringnon-JSON resolves to string.SecureStringwithrawresolves to raw string.noDecryptsetsWithDecryption: false.null.WithDecryptionis not treated as same cache entry.CloudFormation tests:
null.null.ValidationErrorsurfaces as a variable resolution error.S3 tests:
null.Bodystream twice.AWS source tests:
${aws:accountId}usesSTSClientandGetCallerIdentityCommand.Accountis returned unchanged.${aws:region}remains independent of SDK clients.Documentation tests are not needed, but docs must be reviewed for overclaiming.
PR 1 Documentation
Do not update user-facing variable docs in PR 1.
Do not claim framework-wide AWS SSO/IAM Identity Center support.
If release notes are written separately, they must say only that internal AWS-backed variable sources moved to SDK v3 credential resolution, and they must explicitly avoid saying the framework supports SSO.
Deploy, remove, logs, metrics, packaging, and runtime AWS operations continue to migrate separately.
PR 1 Validation Commands
Run targeted helper and variable tests:
Run AWS/provider safety slice:
Run formatting and lint:
npm run prettify && npm run lintOptional full suite before merge:
npm test -- -bManual test config:
PR 1 definition of done: