feat(aws-apigateway): add support for UsagePlan, ApiKey, UsagePlanKey… #1221
Conversation
| */ | ||
| export interface ThrottleSettings { | ||
| /** | ||
| * Represents the steady-state rate for the API stage or method. |
There was a problem hiding this comment.
I like numeric fields to carry some form of a unit hint. At the very least, the documentation should mention it. Similar to previously, this probably should be a copy from the documentation in the CloudFormation documentation for the AWS::ApiGateway::UsagePlan resource.
There was a problem hiding this comment.
I was hoping that a reviewer would point this out tbh with you. I was of the opinion that there may (must) be a way we can enforce certain validation of the inputs as part of these number. For example, burstLimit is required to be an Integer value. If not, the App deployment would fail. Perhaps throwing an error during runtime before cfn is deployed, CDK emit an error with appropriate messaging. Think of it like cdk.ValidateNumber but for Integers.
There was a problem hiding this comment.
Diligent code could validate types beyond what the TypeScript (really, JavaScript) types can represent (Javascript has no concept of an int type, but you can use Number.isInteger to check & do the right thing).
| /** | ||
| * A name for the API key. | ||
| * @link http://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-apigateway-apikey.html#cfn-apigateway-apikey-name | ||
| * @default none |
There was a problem hiding this comment.
I think you mean to say "automically generated name" instead of "no name", right?
| constructor(parent: cdk.Construct, id: string, props: ApiKeyProps = {}) { | ||
| super(parent, id); | ||
|
|
||
| const customerId = props && props.customerId; |
There was a problem hiding this comment.
All these don't need props && since props always has a value.
| const stageKeys = this.renderStageKeys(props && props.resources); | ||
|
|
||
| const resource = new cloudformation.ApiKeyResource(this, 'Resource', { | ||
| customerId, |
There was a problem hiding this comment.
Don't know why this couldn't just be
customerId: props.customerId,
description: props.description,
// etc
?
| /** | ||
| * The API request steady-state rate limit (average requests per second over an extended period of time) | ||
| */ | ||
| rateLimit?: number; |
There was a problem hiding this comment.
averageRequestsPerSecond?
avgRequestsPerSec?
| constructor(parent: cdk.Construct, name: string, props?: UsagePlanProps) { | ||
| super(parent, name); | ||
| let resource: cloudformation.UsagePlanResource; | ||
| if (props !== undefined) { |
There was a problem hiding this comment.
Interesting way of modeling defaults. Should this path also be taken if the user passes {}?
To be honest, this makes me a little uncomfortable. I'd rather you default props to {} and do processing that makes sense in both cases.
| this.usagePlanId = resource.ref; | ||
| } | ||
|
|
||
| public addApiKey(apiKey: ApiKey): void { |
|
@akkaash feel free to reopen if/when you wish to continue working on this. |
|
@akkaash Any plans on continuing this PR? |
|
I did some work on this one and added a new PR: #2564. Thanks, |
Adds support for UsagePlans, ApiKey and UsagePlanKey
Allows for adding usage (quota and throttling) limits for the API Gateway API. ApiKey can be used to distribute to clients to authorize access to API resources and methods. UsagePlanKey can be used to associate a Usage Plan with an API Key to identify the customers the plan applies to.
(#723)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.