Skip to content
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

chore(custom-resources): update aws sdk v2 to v3 mapping #27270

Closed

Conversation

WinterYukky
Copy link
Contributor

update aws sdk v2 to v3 mapping from https://github.com/awslabs/aws-sdk-js-codemod/tree/main/src/transforms/v2-to-v3/config.


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

@aws-cdk-automation aws-cdk-automation requested a review from a team September 25, 2023 06:36
@github-actions github-actions bot added admired-contributor [Pilot] contributed between 13-24 PRs to the CDK p2 labels Sep 25, 2023
rix0rrr
rix0rrr previously approved these changes Sep 25, 2023
@mergify mergify bot dismissed rix0rrr’s stale review September 25, 2023 13:25

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

rix0rrr added a commit that referenced this pull request Sep 27, 2023
In the `AwsCustomResource` and the `Assertions` libraries, we advertise
accepting all of the following service name formats:

* The SDKv3 service name: `api-gateway`
* The full SDKv3 package name: `@aws-sdk/client-api-gateway`
* The SDKv2 constructor name: `APIGateway`
* The SDKv2 constructor name in all lower case: `apigateway`

And the following action name formats:

* The API call name: `GetRestApi`
* The API call name with a lowercase starting letter method name: `getRestApi`
* The SDKv3 command class name: `GetRestApiCommand`

However, the code that was taking care of mapping service names into an
IAM name was not handling all cases correctly. There was also an issue
with some commands that end in the word `"Command"`, like ECS's
`ExecuteCommand`, which according to the rules above should work both
written as `ExecuteCommand` as well as `ExecuteCommandCommand`: we did
not have enough information to know if we saw the string
`ExecuteCommand`, whether we should interpret it as `Execute` or
`ExecuteCommand`.

Also, we were recommending to use the full SDKv3 package name and class
name formats:

```
{
  service: '@aws-sdk/client-api-gateway',
  action: 'GetRestApiCommand',
}
```

Which looks ugly (imo) and leaks too many of the underlying
implementation details.

This PR changes the following:

- Deprecate the `sdk-api-metadata.json` we extracted from SDKv2.
- From SDKv3 models, extract a new `sdk-v3-metadata.json` which contains
  the following information:
  - IAM prefix for every service
  - A list of APIs that end in the word `Command`, so we can
    disambiguate around these.
- From `aws-sdk-codemod`, extract a mapping from SDKv2 service names to
  SDKv3 service names (replacing the copy/pasted code we used to have
  with a build-time extraction).
- Unfortunately, both of these mappings are duplicated: once for the
  construct library, and once for the handlers. I did not want to go
  into deduplicating between these for now.
- At runtime, we now map a potential V2 service name to a V3 service
  name, then look up the V3 metadata to determine the IAM prefix and
  the normalized action name.
- There was a lot of duplication between the `assertions` handler and
  the `AwsCustomResource` handler. Introduce a new `ApiCall` class that
  unifies the behavior between these two call sites.
- Change the recommendation in the README from using SDKv3 names to
  using shorter form names (`api-gateway` and `GetRestApi`).

Fixes #27255, closes #27268, closes #27270.
@mergify mergify bot closed this in #27313 Oct 5, 2023
mergify bot pushed a commit that referenced this pull request Oct 5, 2023
In the `AwsCustomResource` and the `Assertions` libraries, we advertise accepting all of the following service name formats:

* The SDKv3 service name: `api-gateway`
* The full SDKv3 package name: `@aws-sdk/client-api-gateway`
* The SDKv2 constructor name: `APIGateway`
* The SDKv2 constructor name in all lower case: `apigateway`

And the following action name formats:

* The API call name: `GetRestApi`
* The API call name with a lowercase starting letter method name: `getRestApi`
* The SDKv3 command class name: `GetRestApiCommand`

However, the code that was taking care of mapping service names into an IAM name was not handling all cases correctly. There was also an issue with some commands that end in the word `"Command"`, like ECS's `ExecuteCommand`, which according to the rules above should work both written as `ExecuteCommand` as well as `ExecuteCommandCommand`: we did not have enough information to know if we saw the string `ExecuteCommand`, whether we should interpret it as `Execute` or `ExecuteCommand`.

Also, we were recommending to use the full SDKv3 package name and class name formats:

```
{
  service: '@aws-sdk/client-api-gateway',
  action: 'GetRestApiCommand',
}
```

Which looks ugly (imo) and leaks too many of the underlying implementation details.

This PR changes the following:

- Deprecate the `sdk-api-metadata.json` we extracted from SDKv2.
- From SDKv3 models, extract a new `sdk-v3-metadata.json` which contains the following information:
  - IAM prefix for every service
  - A list of APIs that end in the word `Command`, so we can disambiguate around these.
- From `aws-sdk-codemod`, extract a mapping from SDKv2 service names to SDKv3 service names (replacing the copy/pasted code we used to have with a build-time extraction).
- Unfortunately, both of these mappings are duplicated: once for the construct library, and once for the handlers. I did not want to go into deduplicating between these for now.
- At runtime, we now map a potential V2 service name to a V3 service name, then look up the V3 metadata to determine the IAM prefix and the normalized action name.
- There was a lot of duplication between the `assertions` handler and the `AwsCustomResource` handler (and to a lesser degree, the `events.ApiCall` handler), around loading SDKs and coercing values. Introduce a new `ApiCall` class that unifies the behavior between these call sites.
- Change the recommendation in the README from using SDKv3 names to using shorter form names (`api-gateway` and `GetRestApi`).
- Add "dynamic reuqire" protection to the `esbuild` commands for custom resources. 

Fixes #27255, closes #27268, closes #27270, closes #27395.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admired-contributor [Pilot] contributed between 13-24 PRs to the CDK p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants