chore(cdk): add sdkv3 support to sdk.ts #31501

HBobertz · 2024-09-19T20:15:56Z

Reason for this change

Adding support for the SDKv2 to v3 migration in the cdk

Description of changes

Created copies of the SDK and ISDK classes with v3 support. Future migration efforts involving this class should use SDKv3. Once everything is migrated we will delete the original implementation and rename v3 to just SDK

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

otaviomacedo · 2024-09-20T13:47:05Z

packages/aws-cdk/lib/api/aws-auth/sdk.ts

+  lambda(): Lambda;
+  cloudFormation(): CloudFormation;
+  ec2(): EC2;
+  iam(): IAM;
+  ssm(): SSM;
+  s3(): S3;
+  route53(): Route53;
+  ecr(): ECR;
+  ecs(): ECS;
+  elbv2(): ElasticLoadBalancingV2;
+  secretsManager(): SecretsManager;
+  kms(): KMS;
+  stepFunctions(): SFN;
+  codeBuild(): CodeBuild;
+  cloudWatchLogs(): CloudWatchLogs;
+  appsync(): AppSync;


Let's not change any of these methods for now. Create new ones, that return the corresponding service client (e.g., S3Client) and incrementally switch to these new methods. Once no one is using the old methods anymore, we delete them.

actually I realized I took a different approach to the one you described. I just created a completely updated SDKv3 class with all of the services upgraded to v3 and then left the original SDK class unchanged.

My thought was that when migrating something which depends on SDK we'd swap it to SDKv3 and then when everything is swapped over we delete the original and rename SDKv3 to SDK

Let me know if you think this approach isn't as effective and if you'd just rather see me keep the same class and then we swap out services out one at a time every time we update a file.

Yes, a brand new class and interface also works. My only suggestion now is to include in this PR a change in some other file that uses the ISDK interface, just to smoke test the change. There's a lot of fiddling about with error handling and whatnot here. Let's make sure they still work in v3.

otaviomacedo · 2024-09-23T07:37:26Z

packages/@aws-cdk/cx-api/FEATURE_FLAGS.md

@@ -73,6 +73,7 @@ Flags come in three types:
 | [@aws-cdk/custom-resources:logApiResponseDataPropertyTrueDefault](#aws-cdkcustom-resourceslogapiresponsedatapropertytruedefault) | When enabled, the custom resource used for `AwsCustomResource` will configure the `logApiResponseData` property as true by default | 2.145.0 | (fix) |
 | [@aws-cdk/aws-s3:keepNotificationInImportedBucket](#aws-cdkaws-s3keepnotificationinimportedbucket) | When enabled, Adding notifications to a bucket in the current stack will not remove notification from imported stack. | 2.155.0 | (fix) |
 | [@aws-cdk/aws-stepfunctions-tasks:useNewS3UriParametersForBedrockInvokeModelTask](#aws-cdkaws-stepfunctions-tasksusenews3uriparametersforbedrockinvokemodeltask) | When enabled, use new props for S3 URI field in task definition of state machine for bedrock invoke model. | 2.156.0 | (fix) |
+| [@aws-cdk/aws-ecs:reduceEc2FargateCloudWatchPermissions](#aws-cdkaws-ecsreduceec2fargatecloudwatchpermissions) | When enabled, we will only grant the necessary permissions when users specify cloudwatch log group through logConfiguration | 2.159.0 | (fix) |


Is this supposed to be here?

yeah I didn't manually update this, this ame along with some build or something. Merged from main and got rid of this

otaviomacedo · 2024-09-23T08:19:13Z

packages/aws-cdk/lib/api/aws-auth/sdk.ts

+  lambda(): Lambda;
+  cloudFormation(): CloudFormation;
+  ec2(): EC2;
+  iam(): IAM;
+  ssm(): SSM;
+  s3(): S3;
+  route53(): Route53;
+  ecr(): ECR;
+  ecs(): ECS;
+  elbv2(): ElasticLoadBalancingV2;
+  secretsManager(): SecretsManager;
+  kms(): KMS;
+  stepFunctions(): SFN;
+  codeBuild(): CodeBuild;
+  cloudWatchLogs(): CloudWatchLogs;
+  appsync(): AppSync;


Yes, a brand new class and interface also works. My only suggestion now is to include in this PR a change in some other file that uses the ISDK interface, just to smoke test the change. There's a lot of fiddling about with error handling and whatnot here. Let's make sure they still work in v3.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

aws-cdk-automation · 2024-09-28T16:06:01Z

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

otaviomacedo · 2024-10-01T14:17:25Z

packages/aws-cdk/lib/api/util/placeholders.ts

+/**
+ * Replace the {ACCOUNT} and {REGION} placeholders in all strings found in a complex object.
+ */
+export async function replaceEnvPlaceholdersv3<A extends { }>(object: A, env: cxapi.Environment, sdkProvider: SdkProviderv3): Promise<A> {


Instead of duplicating this function, another way of doing this is:

Create a new interface that only has the baseCredentialsPartition method.

Both SdkProvider and SdkProviderv3 implement it.

Change the last parameter of this method to be of the new interface.

This doesn't change any behavior and we get to write less code. No need for isSdkProviderV3 and its usage, for example.

Like the suggestion, but I think I need to change this code anyway, I'll figure that out today, TBD on this suggestion until I figure out what I'm doing with provideContextValues

otaviomacedo · 2024-10-01T14:29:26Z

packages/aws-cdk/lib/context-providers/index.ts

+/**
+ * Iterate over the list of missing context values and invoke the appropriate providers from the map to retrieve them
+ */
+export async function provideContextValuesv3(


Where is this used?

it was used previously, I don't know if I still need it yet lol. I'll be sure to get rid of it before I pull it out of draft. I'm just keeping it around for safe keeping for now

otaviomacedo · 2024-10-01T14:46:50Z

packages/aws-cdk/test/context-providers/asymmetric-vpcs.test.ts

@@ -118,9 +121,9 @@ test('looks up the requested (symmetric) VPC', async () => {
 });

 test('throws when no such VPC is found', async () => {
-  AWS.mock('EC2', 'describeVpcs', (params: aws.EC2.DescribeVpcsRequest, cb: AwsCallback<aws.EC2.DescribeVpcsResult>) => {
+  ec2Mock.on(DescribeVpcsCommand).callsFake(params => {


We can leave it like this or we can use the slightly more readable pattern:

ec2Mock .on(DescribeVpcsCommand, { Filters: [{ Name: 'foo', Values: ['bar'] }] }) .resolves({});

aws-cdk-automation · 2024-10-01T15:08:18Z

The pull request linter fails with the following errors:

❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/31501/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

✅ A exemption request has been requested. Please wait for a maintainer's review.

aws-cdk-automation · 2024-10-02T13:40:23Z

The pull request linter fails with the following errors:

❌ CLI code has changed. A maintainer must run the code through the testing pipeline (git fetch origin pull/31501/head && git push -f origin FETCH_HEAD:test-main-pipeline), then add the 'pr-linter/cli-integ-tested' label when the pipeline succeeds.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

✅ A exemption request has been requested. Please wait for a maintainer's review.

HBobertz · 2024-10-02T13:50:09Z

we're not pushing this last commit I made dd9aff8

This is purely for a discussion I'm having with Otavio.

aws-cdk-automation · 2024-10-02T13:53:10Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: dd9aff8
Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

initial sdkv3 migration

81dbe7c

aws-cdk-automation requested a review from a team September 19, 2024 20:16

github-actions bot added the p2 label Sep 19, 2024

mergify bot added the contribution/core This is a PR that came from AWS. label Sep 19, 2024

HBobertz had a problem deploying to test-pipeline September 19, 2024 20:16 — with GitHub Actions Failure

aws-cdk-automation previously requested changes Sep 19, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Sep 19, 2024

otaviomacedo requested changes Sep 20, 2024

View reviewed changes

add backwards compatability

a3e23e7

HBobertz had a problem deploying to test-pipeline September 23, 2024 01:12 — with GitHub Actions Failure

HBobertz marked this pull request as ready for review September 23, 2024 01:15

Merge branch 'main' into bobertzh/v3Migration

380912c

HBobertz had a problem deploying to test-pipeline September 23, 2024 01:16 — with GitHub Actions Failure

HBobertz changed the title ~~chore(aws-cdk): update sdk.ts and dependents from v2 to v3~~ chore(cdk): add sdkv3 support to sdk.ts Sep 23, 2024

reorder imports

64a2c73

HBobertz had a problem deploying to test-pipeline September 23, 2024 01:37 — with GitHub Actions Failure

fix import order

0061d2f

HBobertz had a problem deploying to test-pipeline September 23, 2024 02:23 — with GitHub Actions Failure

update third party licenses

b13f33f

HBobertz had a problem deploying to test-pipeline September 23, 2024 02:39 — with GitHub Actions Failure

otaviomacedo reviewed Sep 23, 2024

View reviewed changes

update third party licensces

410c591

HBobertz had a problem deploying to test-pipeline September 23, 2024 19:34 — with GitHub Actions Failure

fix lock file and third party licenses

0a39896

HBobertz had a problem deploying to test-pipeline September 23, 2024 19:44 — with GitHub Actions Failure

update third party licenses

81b68a5

HBobertz had a problem deploying to test-pipeline September 24, 2024 01:03 — with GitHub Actions Failure

remove waywardly added packagelock

20ceec8

HBobertz had a problem deploying to test-pipeline September 24, 2024 01:46 — with GitHub Actions Failure

HBobertz had a problem deploying to test-pipeline September 24, 2024 02:57 — with GitHub Actions Failure

HBobertz force-pushed the bobertzh/v3Migration branch from f239220 to 47bb682 Compare September 25, 2024 18:51

HBobertz temporarily deployed to test-pipeline September 25, 2024 18:51 — with GitHub Actions Inactive

HBobertz added pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested and removed pr/needs-cli-test-run This PR needs CLI tests run against it. labels Sep 26, 2024

broken tests, gotta fix

099b45c

HBobertz had a problem deploying to test-pipeline September 27, 2024 01:35 — with GitHub Actions Failure

HBobertz marked this pull request as draft September 27, 2024 13:58

update vpc tests

248b2c1

HBobertz had a problem deploying to test-pipeline September 28, 2024 15:33 — with GitHub Actions Failure

make tests better

4adb0e5

HBobertz marked this pull request as ready for review September 28, 2024 16:03

Merge branch 'main' into bobertzh/v3Migration

1d94db2

HBobertz temporarily deployed to test-pipeline September 28, 2024 16:04 — with GitHub Actions Inactive

HBobertz added pr/needs-cli-test-run This PR needs CLI tests run against it. pr/do-not-merge This PR should not be merged at this time. and removed pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested labels Sep 28, 2024

HBobertz marked this pull request as draft September 28, 2024 20:09

aws-cdk-automation requested changes Sep 28, 2024

View reviewed changes

add generic context provider

b40ab78

HBobertz had a problem deploying to test-pipeline September 30, 2024 20:26 — with GitHub Actions Failure

otaviomacedo reviewed Oct 1, 2024

View reviewed changes

initial credential work

dd9aff8

HBobertz had a problem deploying to test-pipeline October 2, 2024 13:39 — with GitHub Actions Failure

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(cdk): add sdkv3 support to sdk.ts #31501

chore(cdk): add sdkv3 support to sdk.ts #31501

HBobertz commented Sep 19, 2024 •

edited

Loading

aws-cdk-automation left a comment

otaviomacedo Sep 20, 2024

HBobertz Sep 23, 2024

HBobertz Sep 23, 2024 •

edited

Loading

otaviomacedo Sep 23, 2024

otaviomacedo Sep 23, 2024

HBobertz Sep 24, 2024

otaviomacedo Sep 23, 2024

aws-cdk-automation commented Sep 28, 2024

aws-cdk-automation left a comment

aws-cdk-automation left a comment

otaviomacedo Oct 1, 2024

HBobertz Oct 1, 2024 •

edited

Loading

otaviomacedo Oct 1, 2024

HBobertz Oct 1, 2024

otaviomacedo Oct 1, 2024

aws-cdk-automation commented Oct 1, 2024

aws-cdk-automation commented Oct 2, 2024

HBobertz commented Oct 2, 2024

aws-cdk-automation commented Oct 2, 2024

chore(cdk): add sdkv3 support to sdk.ts #31501

Are you sure you want to change the base?

chore(cdk): add sdkv3 support to sdk.ts #31501

Conversation

HBobertz commented Sep 19, 2024 • edited Loading

Reason for this change

Description of changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HBobertz Sep 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Sep 28, 2024

aws-cdk-automation left a comment

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HBobertz Oct 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Oct 1, 2024

aws-cdk-automation commented Oct 2, 2024

HBobertz commented Oct 2, 2024

aws-cdk-automation commented Oct 2, 2024

AWS CodeBuild CI Report

HBobertz commented Sep 19, 2024 •

edited

Loading

HBobertz Sep 23, 2024 •

edited

Loading

HBobertz Oct 1, 2024 •

edited

Loading