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

docs: add migration guide for API Gateway & Lambda #1308

Merged
merged 1 commit into from
May 26, 2022
Merged

Conversation

jacobwinch
Copy link
Contributor

What does this change?

Add a migration guide for the GuApiLambda and GuApiGatewayWithLambdaByPath patterns.

How to test

This is the process that I followed for the mobile-save-for-later migration:

The next time we follow these docs we should make any necessary corrections (I've probably forgotten something!).

How can we measure success?

Users can attempt self-service migrations to two more GuCDK patterns!

Have we considered potential risks?

Yes, I have tested this migration path for the GuApiGatewayWithLambdaByPath pattern but not the GuApiLambda pattern. I'm pretty confident that the process will be the same, but I can't be 100% sure as I haven't tested it.

Checklist

  • I have listed any breaking changes, along with a migration path 1
  • I have updated the documentation as required for the described changes 2

Footnotes

  1. Consider whether this is something that will mean changes to projects that have already been migrated, or to the CDK CLI tool. If changes are required, consider adding a checklist here and/or linking to related PRs.

  2. If you are adding a new construct or pattern, has new documentation been added? If you are amending defaults or changing behaviour, are the existing docs still valid?

Comment on lines +87 to +92
2. Create an instance of the `CfnDomainName` and `CfnBasePathMapping` classes in your template.

In order to avoid downtime, pass the logical id of your `AWS::ApiGateway::DomainName` into the `CfnDomainName` class and
the logical id of the `AWS::ApiGateway::BasePathMapping` into your `CfnBasePathMapping` class.

Ensure that the properties passed into these classes line up with those specified in your CloudFormation template.
Copy link
Member

Choose a reason for hiding this comment

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

This is a lot! Is it worth us creating a L2 construct that does this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's a bit awkward, but it doesn't end up being that much code:

const cfnDomainName = new CfnDomainName(this, "ApiDomainName", {
  domainName: props.domainName,
  certificateArn,
});

new CfnBasePathMapping(this, "ApiMapping", {
  domainName: cfnDomainName.ref,
  restApiId: yamlDefinedResources.getResource("SaveForLaterApi").ref,
  stage: props.stage,
});

Were you thinking the L2 construct would just be used for removing boilerplate or were you intending to use it to help in some other way?

Copy link
Member

Choose a reason for hiding this comment

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

Were you thinking the L2 construct would just be used for removing boilerplate or were you intending to use it to help in some other way?

Just boilerplate. Agree though, it's not too bad as is.

Comment on lines +70 to +71
Note that you **must** use the `functionNames` parameter for this to work.
The alternative strategy (which uses `lookupByTags`), will not work with the dual-stack approach.
Copy link
Member

Choose a reason for hiding this comment

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

Could we do something similar to the dual-ASG trick here? As in, add an identifier tag to the new resources to help Riff-Raff distinguish between them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think this would be possible.

However using the names actually seems a lot more straightforward than following the slightly awkward process that we use to cope with having two ASGs with the same tags. See here & here.

So I'm unsure if we really want to introduce another way of doing things here (especially as lookupByTags seems to be false by default anyway).

@jacobwinch jacobwinch merged commit 2b65959 into main May 26, 2022
@jacobwinch jacobwinch deleted the jw-api-docs branch May 26, 2022 12:45
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