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

(apigateway): support deployment salting for imported RestApi #26668

Open
1 of 2 tasks
Dzhuneyt opened this issue Aug 8, 2023 · 2 comments
Open
1 of 2 tasks

(apigateway): support deployment salting for imported RestApi #26668

Dzhuneyt opened this issue Aug 8, 2023 · 2 comments
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1

Comments

@Dzhuneyt
Copy link
Contributor

Dzhuneyt commented Aug 8, 2023

Describe the feature

This is a continuation of the discussion in the (already closed) ticket: #12417

Having an API Gateway and its routes spread out across CDK/CloudFormation stacks is a good idea for a variety of reasons:

  • Avoiding the 500 resources limit of CloudFormation
  • Allowing different teams to maintain different parts of the API (Microservices pattern)

Use Case

A moderately sized API Gateway with a few dozens of Resources and Methods inside, quickly exhausts the 500 resources limit of CloudFormation.

This is mostly because of the fact that every Resource and Method is composed of multiple sub-resources like, the Lambda, some IAM roles, CloudWatch LogGroup, Retention Policies to that LogGroup, etc, etc. So reaching the 500 count is very easy.

Proposed Solution

The main reason why the previous ticket was closed was because the new Deployment() resource has no way of knowing when and if it should be redeployed, considering that routes (Resources and Methods) were possibly added by a different stack.

The reason why the new Deployment() is conditionally recreated properly when everything is in the same stack, is because the Resources and Methods can be attached as "dependencies" to the Deployment construct, so it is invalidated as soon as a Resource or Method is added or removed.

My proposal here is simple - develop a Custom Resource (Lambda) that internally queries the state of the API Gateway (collect all current Resources and Methods) and hashes them to a stable string. That string can then be used as a singular dependency to the new Deployment() construct, causing it to be recreated conditionally, only when the hash changes.

Here's a typical structure based on current recommendations:

Stack A:

  • API GW
  • Routes A
  • Routes B
  • Routes N
  • Deployment
  • Stage

What people want (me included):

Stack A:

  • API GW

Stack B:

  • Routes A
  • Routes B
  • Routes N

Stack C:

  • Deployment
  • Stage

The solution I'm proposing would change Stack C, to something like this (pseudo code):

const hasher = new ApiGatewayDeploymentSalter(this, 'Salter', {apiId: '123456'});
const deployment = new Deployment(this, 'Deployment')
const stage = new Stage(this, 'prod', {deployment});

// The `hasher` is a Lambda backed custom resource that hashes the current state of the API Gateway, based on the resources and method "tree"
deployment.addToLogicalID(hasher.ref);

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.90.0

Environment details (OS name and version, etc.)

MacOS

@Dzhuneyt Dzhuneyt added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2023
@github-actions github-actions bot added the @aws-cdk/aws-apigateway Related to Amazon API Gateway label Aug 8, 2023
@peterwoodworth peterwoodworth added p1 effort/large Large work item – several weeks of effort and removed needs-triage This issue or PR still needs to be triaged. labels Aug 8, 2023
@peterwoodworth
Copy link
Contributor

Thanks for proposing a solution for this!

I'm not sure if this proposed solution will work - but I can tell you that we probably wouldn't be able to work on this solution any time soon ourselves, which would mean we'll need a contributor for this feature 🙂 I do fear though that this kind of solution is what Otavio was referring to in the original thread as a "hack" - this kind of API you propose (if I'm understanding it right) wouldn't follow the use pattern all our other constructs have

@Dzhuneyt
Copy link
Contributor Author

I see. So you are saying even if there is a contributor jumping to implement this (e.g. me), you are suggesting the workaround is a bit too "grey area" to be worthy to be inside the CDK core? I can understand the reasoning.

However, I also consider a multi-stack setup a fairly common practice. People wanting to split their API Gateway and routes between Stack A and Stack B for a variety of reasons (the CloudFormation 500 resources limit being one of them). Currently, such splitting, just DOES NOT WORK, because of a conceptual bug in how API Gateway works (or maybe it's a "feature", I don't know). People are required to resort to hacks anyway to get this to work (create a new Deployment() and new Stage()` in Stack C, after Stack A and Stack B; where Stack C is forced to be recreated every time by passing a unique ID like the current timestamp to salt its logical ID).

The thing that bothers me is that people have to dig through dozens of GitHub issues and code snippets to, first of all understand about this limitation, and second of all apply the solution to their use case.

All of this trouble can be saved, by, for example, making the L2 construct new Deployment() "smarter" so it detects if the API Gateway being provided to it is actually created or imported in the current stack, and the deployment conditionally queries for the routes of that API and hashes them (through an AWS Lambda backed custom resource). This keeps the implementation code in real life project cleaner.
Alternatively, this logic can be extracted in another construct to not pollute the new Deployment() construct, which is the approach I suggest in my first comment of this thread.

In any case, I think it's worth implementing this bugfix within the CDK framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigateway Related to Amazon API Gateway effort/large Large work item – several weeks of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

2 participants