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

feat(stepfunctions-tasks): step functions task for cross-region AWS API call #30061

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

Conversation

tmokmss
Copy link
Contributor

@tmokmss tmokmss commented May 4, 2024

Issue # (if applicable)

Closes #29918.

Reason for this change

It would be useful if we could call AWS API across regions from a Step Functions state machine. Currently it is not officially supported even with AWS SDK integration tasks.

Our usecase is to automate a cross-region failover scenario in a multi-region application. This requires you to orchestrate multiple API calls for both active and standby regions (e.g. failover Aurora DB cluster, rewrite AppConfig parameter, etc), and it would be great if we can manage these operations in a single state machine.

Description of changes

This PR adds a new construct CallAwsServiceCrossRegion that deploys 1. a Lambda function to call AWS API in different regions 2. SFn task to call the function.

Because most properties are compatible with the existing CallAwsService construct, you can use the new construct by just adding the region property.

Additionally, it also allows to set endpoint to override AWS API endpoint, because some AWS APIs requires you to override it. (e.g. Route53 ARC)

Description of how you validated changes

Added unit tests and integ tests.

Checklist


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

@github-actions github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels May 4, 2024
@aws-cdk-automation aws-cdk-automation requested a review from a team May 4, 2024 14:35
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

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.

@tmokmss tmokmss changed the title feat(aws-stepfunctions-tasks): Step Functions task for cross-region AWS API call feat(stepfunctions-tasks): step functions task for cross-region AWS API call May 4, 2024
@aws-cdk-automation aws-cdk-automation dismissed their stale review May 4, 2024 14:38

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

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 4, 2024
Copy link
Contributor

@nmussy nmussy left a comment

Choose a reason for hiding this comment

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

Looks good overall, just some concerns and minor comments. I'm also wondering if it's better for maintainability not to create a new class, and to reuse the existing CallAwsService, and just to add an optional crossRegionTarget prop


try {
// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
const pkg = require(`@aws-sdk/client-${event.service}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct me if I'm wrong, but normalizeServiceName is not being ran for your handler? It would allow us to keep the same pattern as other services props, see AwsSdkCall:

this.service = normalizeServiceName(service);

export function normalizeServiceName(service: string) {
service = service.toLowerCase(); // Lowercase
service = service.replace(/^@aws-sdk\/client-/, ''); // Strip the start of a V3 package name
service = v2ToV3Mapping()?.[service] ?? service; // Optionally map v2 name -> v3 name
return service;
}

Copy link
Contributor Author

@tmokmss tmokmss May 7, 2024

Choose a reason for hiding this comment

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

I'd like to keep this feature and API as simple as possible to lower the maintenance cost. AwsSdkCall code has become such complicated mostly for backward compatibility reason. Since this feature is new, we don't have to introduce the complexity at all.

Also the API is aimed to be compatible with the existing CallAwsService construct (and it should), which also does not support normalizing service names.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll let the maintainer weigh in, I think it's a nice piece of QoL, but I understand your maintainability concern

*/
export interface CallAwsServiceCrossRegionProps extends sfn.TaskStateBaseProps {
/**
* The AWS service to call in AWS SDK for JavaScript v3 style.
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment about normalizeServiceName

// esbuild-disable unsupported-require-call -- not esbuildable but that's fine
const pkg = require(`@aws-sdk/client-${event.service}`);
const Client = findV3ClientConstructor(pkg);
const Command = findCommandClass(pkg, event.action);
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, you should be able to use normalizeActionName, see AwsSdkCall:

export function normalizeServiceName(service: string) {
service = service.toLowerCase(); // Lowercase
service = service.replace(/^@aws-sdk\/client-/, ''); // Strip the start of a V3 package name
service = v2ToV3Mapping()?.[service] ?? service; // Optionally map v2 name -> v3 name
return service;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as service name, I'd like to intentionally only support action names in camelCase.

/**
* The API action to call.
*
* Use camelCase.
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment about normalizeActionName

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label May 6, 2024
@tmokmss
Copy link
Contributor Author

tmokmss commented May 7, 2024

Hi @nmussy, thanks for the comments.

reuse the existing CallAwsService, and just to add an optional crossRegionTarget prop

I considered this approach as well but chose the current approach, because with the current approach we can easily avoid from future breaking changes when SFn will officially start to support cross-region API call; we can just deprecate CallAwsServiceCrossRegion construct and add the official functionality to CallAwsService construct.

Copy link
Contributor

@nmussy nmussy left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of pending comments

@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label May 7, 2024
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 pr/needs-maintainer-review This PR needs a review from a Core Team Member star-contributor [Pilot] contributed between 25-49 PRs to the CDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stepfunctions-tasks: Step Functions task for cross-region AWS API call
3 participants