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

(aws-apigatewayv2-alpha): Set route throttling with HttpStage #19626

Closed
TkinterinShanghai opened this issue Mar 30, 2022 · 8 comments · Fixed by #19776
Closed

(aws-apigatewayv2-alpha): Set route throttling with HttpStage #19626

TkinterinShanghai opened this issue Mar 30, 2022 · 8 comments · Fixed by #19776
Assignees
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@TkinterinShanghai
Copy link

What is the problem?

Unlike the module aws_apigateway where I would just do

const api = new apigateway.RestApi(this, 'hello-api');
const plan = api.addUsagePlan('UsagePlan', {
  name: 'Easy',
  throttle: {
    rateLimit: 10,
    burstLimit: 2
  }
});

there doesnt seem to be an equivalent way of doing this in aws-apigatewayv2-alpha.

Reproduction Steps

I created an HTTP Api

export class APIConstruct extends Construct {
  public api: HttpApi;
  constructor(scope: Construct, id: idType, authorizer: HttpUserPoolAuthorizer) {
    super(scope, id);
    const throtting = new UsagePlan(this, "Throttling", {
      throttle: { burstLimit: 999, rateLimit: 99 },
    });
    this.api = new HttpApi(this, id, {
      apiName: id,
      defaultAuthorizer: authorizer,
    });
  }
}

and tried to just add throttling in the same scope, hoping that it would automatically apply to the only api gateway specified

const throtting = new UsagePlan(this, "TypiRateLimiting", { throttle: { burstLimit: 999, rateLimit: 99 } });

What did you expect to happen?

Have a method .addUsagePlan on the Http Api class or having a field in props that allows me to link the UsagePlan to the Api.

What actually happened?

Throttling was not configured for Api

CDK CLI Version

2.17.0

Framework Version

No response

Node.js Version

16.12

OS

Windows

Language

Typescript

Language Version

No response

Other information

No response

@TkinterinShanghai TkinterinShanghai added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 30, 2022
@github-actions github-actions bot added the @aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 label Mar 30, 2022
@peterwoodworth peterwoodworth added feature-request A feature should be added or improved. needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 31, 2022
@peterwoodworth
Copy link
Contributor

Hey @TkinterinShanghai,

The UsagePlan option is available in the apigateway package and not v2 because CloudFormation makes the distinction between v1 and v2 here. CloudFormation has a UsagePlan resource for v1, and no such resource for V2. So there's not much we can do here

@peterwoodworth
Copy link
Contributor

I'm not too familiar with these services, but I believe there is no usage plan construct because the functionality lies somewhere else in v2, see here.

If you still believe there should be a UsagePlan construct for CloudFormation, then I recommend opening an issue with the CloudFormation coverage roadmap

@peterwoodworth peterwoodworth added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 31, 2022
@peterwoodworth peterwoodworth self-assigned this Mar 31, 2022
@TkinterinShanghai
Copy link
Author

I just don't see the Option to add a Usage Plan/Throttling at all. I'd be happy with any way of doing this as long as it's feasible. I think adding a method to the Api, the Stage or having a Construct by itself would all be intuitive ways of implementing this. If adding throttling to the Http Api Gateway is possible via the website, I would expect Cdk v2 to also be able to handle that. I still haven't found that option. Maybe it's just well hidden and I'm not aware it exists

@peterwoodworth
Copy link
Contributor

The CDK is limited by CloudFormation - so we can only work with what they offer us.

Looking at their docs, we set throttling in ApiGatewayV2::Stage.RouteSettings. The CDK doesn't directly configure these properties yet with our HttpStage construct as far as I can tell, so we can get a feature in for that 🙂

In the meantime, you can access the underlying CloudFormation Stage resource with escape hatches

You'll want to do something like this before we directly support throttling:

const stage = new apigwv2-alpha.HttpStage(...);
const cfnStage = stage.node.defaultChild as apigwv2.CfnStage;
cfnStage.addPropertyOverride('RouteSettings', ...);

But, like I said before, no UsagePlan resource exists in Cloudformation's ApiGatewayV2 module, so there's nothing the CDK can do about that.

@peterwoodworth peterwoodworth changed the title (aws-apigatewayv2-alpha): Missing Usage Plan option for Http Api Gateway (aws-apigatewayv2-alpha): Set route throttling with HttpStage Mar 31, 2022
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-cfn This issue is waiting on changes to CloudFormation before it can be addressed. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Mar 31, 2022
@TkinterinShanghai
Copy link
Author

Thanks, you put me on the right track. This worked:

const cfnStage = api.defaultStage?.node.defaultChild as CfnStage; // api is of type HttpApi
cfnStage.addPropertyOverride("DefaultRouteSettings", {ThrottlingBurstLimit: 1500, ThrottlingRateLimit: 1000});

But it would be nice if this could be incorporated into ApiGatewayV2, as I think it is a somewhat important missing feature and this way of doing it is pretty hacky

@peterwoodworth
Copy link
Contributor

I hear you @TkinterinShanghai, I've created a PR for this feature 🙂

@mergify mergify bot closed this as completed in #19776 Apr 8, 2022
mergify bot pushed a commit that referenced this issue Apr 8, 2022
fixes #19626

added integ tests
let me know if change to readme is necessary

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [x] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Apr 8, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

otaviomacedo pushed a commit that referenced this issue Apr 11, 2022
fixes #19626

added integ tests
let me know if change to readme is necessary

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [x] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
StevePotter pushed a commit to StevePotter/aws-cdk that referenced this issue Apr 27, 2022
fixes aws#19626

added integ tests
let me know if change to readme is necessary

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [x] Did you use `cdk-integ` to deploy the infrastructure and generate the snapshot (i.e. `cdk-integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@TkinterinShanghai
Copy link
Author

This seems to be a bug elsewhere but creating a stage doesnt work because "Stage already exists", even when setting createDefaultStage to false in the HttpApi class. But this bug affects this fix, because the fix relies on the Stage working. So I think additionally having a method such as createDefaultThrottling on the HttpApi or a setThrottling method on the HttpStage would work even with the current bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-apigatewayv2 Related to Amazon API Gateway v2 effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants