From 036d869dc1382d3fb2d8541f5adf534ea3424667 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Tue, 6 Apr 2021 18:07:37 +0100 Subject: [PATCH] fix(apigateway): cannot remove first api key from usage plan (#13817) The UsagePlanKey resource connects an ApiKey with a UsagePlan. The API Gateway service does not allow more than one UsagePlanKey for any given UsagePlan and ApiKey combination. For this reason, CloudFormation cannot replace this resource without either the UsagePlan or ApiKey changing. A feature was added back in Nov 2019 - 142bd0e2 - that allows multiple UsagePlanKey resources. The above limitation was recognized and logical id of the existing UsagePlanKey was retained. However, this unintentionally caused the logical id of the UsagePlanKey to be sensitive to order. That is, when the 'first' UsagePlanKey resource is removed, the logical id of the what was the 'second' UsagePlanKey is changed to be the logical id of what was the 'first'. This change to the logical id is, again, disallowed. To get out of this mess, we do two things - 1. introduce a feature flag that changes the default behaviour for all new CDK apps. 2. for customers with existing CDK apps who are would want to remove UsagePlanKey resource, introduce a 'overrideLogicalId' option that they can manually configure with the existing logical id. fixes #11876 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/@aws-cdk/aws-apigateway/README.md | 65 ++++---- .../@aws-cdk/aws-apigateway/lib/usage-plan.ts | 34 +++- .../test/integ.restapi.expected.json | 2 +- .../integ.usage-plan.multikey.expected.json | 2 +- .../aws-apigateway/test/usage-plan.test.ts | 148 ++++++++++++------ packages/@aws-cdk/cx-api/lib/features.ts | 18 +++ 6 files changed, 187 insertions(+), 82 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/README.md b/packages/@aws-cdk/aws-apigateway/README.md index 926f45daf1436..4607442308138 100644 --- a/packages/@aws-cdk/aws-apigateway/README.md +++ b/packages/@aws-cdk/aws-apigateway/README.md @@ -24,7 +24,7 @@ running on AWS Lambda, or any web application. - [Breaking up Methods and Resources across Stacks](#breaking-up-methods-and-resources-across-stacks) - [AWS Lambda-backed APIs](#aws-lambda-backed-apis) - [Integration Targets](#integration-targets) -- [API Keys](#api-keys) +- [Usage Plan & API Keys](#usage-plan--api-keys) - [Working with models](#working-with-models) - [Default Integration and Method Options](#default-integration-and-method-options) - [Proxy Routes](#proxy-routes) @@ -168,34 +168,36 @@ const getMessageIntegration = new apigateway.AwsIntegration({ }); ``` -## API Keys +## Usage Plan & API Keys -The following example shows how to use an API Key with a usage plan: +A usage plan specifies who can access one or more deployed API stages and methods, and the rate at which they can be +accessed. The plan uses API keys to identify API clients and meters access to the associated API stages for each key. +Usage plans also allow configuring throttling limits and quota limits that are enforced on individual client API keys. -```ts -const hello = new lambda.Function(this, 'hello', { - runtime: lambda.Runtime.NODEJS_12_X, - handler: 'hello.handler', - code: lambda.Code.fromAsset('lambda') -}); +The following example shows how to create and asscociate a usage plan and an API key: -const api = new apigateway.RestApi(this, 'hello-api', { }); -const integration = new apigateway.LambdaIntegration(hello); +```ts +const api = new apigateway.RestApi(this, 'hello-api'); const v1 = api.root.addResource('v1'); const echo = v1.addResource('echo'); const echoMethod = echo.addMethod('GET', integration, { apiKeyRequired: true }); -const key = api.addApiKey('ApiKey'); const plan = api.addUsagePlan('UsagePlan', { name: 'Easy', - apiKey: key, throttle: { rateLimit: 10, burstLimit: 2 } }); +const key = api.addApiKey('ApiKey'); +plan.addApiKey(key); +``` + +To associate a plan to a given RestAPI stage: + +```ts plan.addApiStage({ stage: api.deploymentStage, throttle: [ @@ -233,26 +235,36 @@ following code provides read permission to an API key. importedKey.grantRead(lambda); ``` -In scenarios where you need to create a single api key and configure rate limiting for it, you can use `RateLimitedApiKey`. -This construct lets you specify rate limiting properties which should be applied only to the api key being created. -The API key created has the specified rate limits, such as quota and throttles, applied. +### ⚠️ Multiple API Keys -The following example shows how to use a rate limited api key : +It is possible to specify multiple API keys for a given Usage Plan, by calling `usagePlan.addApiKey()`. + +When using multiple API keys, a past bug of the CDK prevents API key associations to a Usage Plan to be deleted. +If the CDK app had the [feature flag] - `@aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId` - enabled when the API +keys were created, then the app will not be affected by this bug. + +If this is not the case, you will need to ensure that the CloudFormation [logical ids] of the API keys that are not +being deleted remain unchanged. +Make note of the logical ids of these API keys before removing any, and set it as part of the `addApiKey()` method: ```ts -const hello = new lambda.Function(this, 'hello', { - runtime: lambda.Runtime.NODEJS_12_X, - handler: 'hello.handler', - code: lambda.Code.fromAsset('lambda') +usageplan.addApiKey(apiKey, { + overrideLogicalId: '...', }); +``` -const api = new apigateway.RestApi(this, 'hello-api', { }); -const integration = new apigateway.LambdaIntegration(hello); +[feature flag]: https://docs.aws.amazon.com/cdk/latest/guide/featureflags.html +[logical ids]: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/resources-section-structure.html -const v1 = api.root.addResource('v1'); -const echo = v1.addResource('echo'); -const echoMethod = echo.addMethod('GET', integration, { apiKeyRequired: true }); +### Rate Limited API Key + +In scenarios where you need to create a single api key and configure rate limiting for it, you can use `RateLimitedApiKey`. +This construct lets you specify rate limiting properties which should be applied only to the api key being created. +The API key created has the specified rate limits, such as quota and throttles, applied. +The following example shows how to use a rate limited api key : + +```ts const key = new apigateway.RateLimitedApiKey(this, 'rate-limited-api-key', { customerId: 'hello-customer', resources: [api], @@ -261,7 +273,6 @@ const key = new apigateway.RateLimitedApiKey(this, 'rate-limited-api-key', { period: apigateway.Period.MONTH } }); - ``` ## Working with models diff --git a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts index 49b3ee19cd017..82f45bc538db6 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts @@ -1,4 +1,5 @@ -import { Lazy, Names, Resource, Token } from '@aws-cdk/core'; +import { FeatureFlags, Lazy, Names, Resource, Token } from '@aws-cdk/core'; +import { APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID } from '@aws-cdk/cx-api'; import { Construct } from 'constructs'; import { IApiKey } from './api-key'; import { CfnUsagePlan, CfnUsagePlanKey } from './apigateway.generated'; @@ -139,10 +140,22 @@ export interface UsagePlanProps { /** * ApiKey to be associated with the usage plan. * @default none + * @deprecated use `addApiKey()` */ readonly apiKey?: IApiKey; } +/** + * Options to the UsagePlan.addApiKey() method + */ +export interface AddApiKeyOptions { + /** + * Override the CloudFormation logical id of the AWS::ApiGateway::UsagePlanKey resource + * @default - autogenerated by the CDK + */ + readonly overrideLogicalId?: string; +} + export class UsagePlan extends Resource { /** * @attribute @@ -176,19 +189,28 @@ export class UsagePlan extends Resource { /** * Adds an ApiKey. * - * @param apiKey + * @param apiKey the api key to associate with this usage plan + * @param options options that control the behaviour of this method */ - public addApiKey(apiKey: IApiKey): void { + public addApiKey(apiKey: IApiKey, options?: AddApiKeyOptions): void { + let id: string; const prefix = 'UsagePlanKeyResource'; - // Postfixing apikey id only from the 2nd child, to keep physicalIds of UsagePlanKey for existing CDK apps unmodified. - const id = this.node.tryFindChild(prefix) ? `${prefix}:${Names.nodeUniqueId(apiKey.node)}` : prefix; + if (FeatureFlags.of(this).isEnabled(APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID)) { + id = `${prefix}:${Names.nodeUniqueId(apiKey.node)}`; + } else { + // Postfixing apikey id only from the 2nd child, to keep physicalIds of UsagePlanKey for existing CDK apps unmodified. + id = this.node.tryFindChild(prefix) ? `${prefix}:${Names.nodeUniqueId(apiKey.node)}` : prefix; + } - new CfnUsagePlanKey(this, id, { + const resource = new CfnUsagePlanKey(this, id, { keyId: apiKey.keyId, keyType: UsagePlanKeyType.API_KEY, usagePlanId: this.usagePlanId, }); + if (options?.overrideLogicalId) { + resource.overrideLogicalId(options?.overrideLogicalId); + } } /** diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json index 91af3471593eb..a0fb6357db3c7 100644 --- a/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json +++ b/packages/@aws-cdk/aws-apigateway/test/integ.restapi.expected.json @@ -602,7 +602,7 @@ "UsagePlanName": "Basic" } }, - "myapiUsagePlanUsagePlanKeyResource050D133F": { + "myapiUsagePlanUsagePlanKeyResourcetestapigatewayrestapimyapiApiKeyC43601CB600D112D": { "Type": "AWS::ApiGateway::UsagePlanKey", "Properties": { "KeyId": { diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.expected.json b/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.expected.json index 8e761f40e2a26..9dee2e7aa07b0 100644 --- a/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.expected.json +++ b/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.expected.json @@ -3,7 +3,7 @@ "myusageplan4B391740": { "Type": "AWS::ApiGateway::UsagePlan" }, - "myusageplanUsagePlanKeyResource095B4EA9": { + "myusageplanUsagePlanKeyResourcetestapigatewayusageplanmultikeymyapikey1DDABC389A2809A73": { "Type": "AWS::ApiGateway::UsagePlanKey", "Properties": { "KeyId": { diff --git a/packages/@aws-cdk/aws-apigateway/test/usage-plan.test.ts b/packages/@aws-cdk/aws-apigateway/test/usage-plan.test.ts index f183d08796388..cef19db1e9789 100644 --- a/packages/@aws-cdk/aws-apigateway/test/usage-plan.test.ts +++ b/packages/@aws-cdk/aws-apigateway/test/usage-plan.test.ts @@ -1,6 +1,8 @@ import '@aws-cdk/assert/jest'; import { ResourcePart } from '@aws-cdk/assert'; import * as cdk from '@aws-cdk/core'; +import * as cxapi from '@aws-cdk/cx-api'; +import { testFutureBehavior } from 'cdk-build-tools/lib/feature-flag'; import * as apigateway from '../lib'; const RESOURCE_TYPE = 'AWS::ApiGateway::UsagePlan'; @@ -149,60 +151,112 @@ describe('usage plan', () => { }, ResourcePart.Properties); }); - test('UsagePlanKey', () => { - // GIVEN - const stack = new cdk.Stack(); - const usagePlan: apigateway.UsagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan', { - name: 'Basic', + describe('UsagePlanKey', () => { + + test('default', () => { + // GIVEN + const stack = new cdk.Stack(); + const usagePlan: apigateway.UsagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan', { + name: 'Basic', + }); + const apiKey: apigateway.ApiKey = new apigateway.ApiKey(stack, 'my-api-key'); + + // WHEN + usagePlan.addApiKey(apiKey); + + // THEN + expect(stack).toHaveResource('AWS::ApiGateway::UsagePlanKey', { + KeyId: { + Ref: 'myapikey1B052F70', + }, + KeyType: 'API_KEY', + UsagePlanId: { + Ref: 'myusageplan23AA1E32', + }, + }, ResourcePart.Properties); }); - const apiKey: apigateway.ApiKey = new apigateway.ApiKey(stack, 'my-api-key'); - // WHEN - usagePlan.addApiKey(apiKey); + test('multiple keys', () => { + // GIVEN + const stack = new cdk.Stack(); + const usagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan'); + const apiKey1 = new apigateway.ApiKey(stack, 'my-api-key-1', { + apiKeyName: 'my-api-key-1', + }); + const apiKey2 = new apigateway.ApiKey(stack, 'my-api-key-2', { + apiKeyName: 'my-api-key-2', + }); - // THEN - expect(stack).toHaveResource('AWS::ApiGateway::UsagePlanKey', { - KeyId: { - Ref: 'myapikey1B052F70', - }, - KeyType: 'API_KEY', - UsagePlanId: { - Ref: 'myusageplan23AA1E32', - }, - }, ResourcePart.Properties); - }); + // WHEN + usagePlan.addApiKey(apiKey1); + usagePlan.addApiKey(apiKey2); - test('UsagePlan can have multiple keys', () => { - // GIVEN - const stack = new cdk.Stack(); - const usagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan'); - const apiKey1 = new apigateway.ApiKey(stack, 'my-api-key-1', { - apiKeyName: 'my-api-key-1', + // THEN + expect(stack).toHaveResource('AWS::ApiGateway::ApiKey', { + Name: 'my-api-key-1', + }, ResourcePart.Properties); + expect(stack).toHaveResource('AWS::ApiGateway::ApiKey', { + Name: 'my-api-key-2', + }, ResourcePart.Properties); + expect(stack).toHaveResource('AWS::ApiGateway::UsagePlanKey', { + KeyId: { + Ref: 'myapikey11F723FC7', + }, + }, ResourcePart.Properties); + expect(stack).toHaveResource('AWS::ApiGateway::UsagePlanKey', { + KeyId: { + Ref: 'myapikey2ABDEF012', + }, + }, ResourcePart.Properties); }); - const apiKey2 = new apigateway.ApiKey(stack, 'my-api-key-2', { - apiKeyName: 'my-api-key-2', + + test('overrideLogicalId', () => { + // GIVEN + const app = new cdk.App(); + const stack = new cdk.Stack(app); + const usagePlan: apigateway.UsagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan', { name: 'Basic' }); + const apiKey: apigateway.ApiKey = new apigateway.ApiKey(stack, 'my-api-key'); + + // WHEN + usagePlan.addApiKey(apiKey, { overrideLogicalId: 'mylogicalid' }); + + // THEN + const template = app.synth().getStackByName(stack.stackName).template; + const logicalIds = Object.entries(template.Resources) + .filter(([_, v]) => (v as any).Type === 'AWS::ApiGateway::UsagePlanKey') + .map(([k, _]) => k); + expect(logicalIds).toEqual(['mylogicalid']); }); - // WHEN - usagePlan.addApiKey(apiKey1); - usagePlan.addApiKey(apiKey2); + describe('future flag: @aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId', () => { + const flags = { [cxapi.APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID]: true }; - // THEN - expect(stack).toHaveResource('AWS::ApiGateway::ApiKey', { - Name: 'my-api-key-1', - }, ResourcePart.Properties); - expect(stack).toHaveResource('AWS::ApiGateway::ApiKey', { - Name: 'my-api-key-2', - }, ResourcePart.Properties); - expect(stack).toHaveResource('AWS::ApiGateway::UsagePlanKey', { - KeyId: { - Ref: 'myapikey11F723FC7', - }, - }, ResourcePart.Properties); - expect(stack).toHaveResource('AWS::ApiGateway::UsagePlanKey', { - KeyId: { - Ref: 'myapikey2ABDEF012', - }, - }, ResourcePart.Properties); + testFutureBehavior('UsagePlanKeys have unique logical ids', flags, cdk.App, (app) => { + // GIVEN + const stack = new cdk.Stack(app, 'my-stack'); + const usagePlan = new apigateway.UsagePlan(stack, 'my-usage-plan'); + const apiKey1 = new apigateway.ApiKey(stack, 'my-api-key-1', { + apiKeyName: 'my-api-key-1', + }); + const apiKey2 = new apigateway.ApiKey(stack, 'my-api-key-2', { + apiKeyName: 'my-api-key-2', + }); + + // WHEN + usagePlan.addApiKey(apiKey1); + usagePlan.addApiKey(apiKey2); + + // THEN + const template = app.synth().getStackByName(stack.stackName).template; + const logicalIds = Object.entries(template.Resources) + .filter(([_, v]) => (v as any).Type === 'AWS::ApiGateway::UsagePlanKey') + .map(([k, _]) => k); + + expect(logicalIds).toEqual([ + 'myusageplanUsagePlanKeyResourcemystackmyapikey1EE9AA1B359121274', + 'myusageplanUsagePlanKeyResourcemystackmyapikey2B4E8EB1456DC88E9', + ]); + }); + }); }); }); diff --git a/packages/@aws-cdk/cx-api/lib/features.ts b/packages/@aws-cdk/cx-api/lib/features.ts index 91f6039625a1b..cdb2217a16b4e 100644 --- a/packages/@aws-cdk/cx-api/lib/features.ts +++ b/packages/@aws-cdk/cx-api/lib/features.ts @@ -119,6 +119,22 @@ export const ECS_REMOVE_DEFAULT_DESIRED_COUNT = '@aws-cdk/aws-ecs-patterns:remov */ export const RDS_LOWERCASE_DB_IDENTIFIER = '@aws-cdk/aws-rds:lowercaseDbIdentifier'; +/** + * The UsagePlanKey resource connects an ApiKey with a UsagePlan. API Gateway does not allow more than one UsagePlanKey + * for any given UsagePlan and ApiKey combination. For this reason, CloudFormation cannot replace this resource without + * either the UsagePlan or ApiKey changing. + * + * The feature addition to support multiple UsagePlanKey resources - 142bd0e2 - recognized this and attempted to keep + * existing UsagePlanKey logical ids unchanged. + * However, this intentionally caused the logical id of the UsagePlanKey to be sensitive to order. That is, when + * the 'first' UsagePlanKey resource is removed, the logical id of the 'second' assumes what was originally the 'first', + * which again is disallowed. + * + * In effect, there is no way to get out of this mess in a backwards compatible way, while supporting existing stacks. + * This flag changes the logical id layout of UsagePlanKey to not be sensitive to order. + */ +export const APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID = '@aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId'; + /** * This map includes context keys and values for feature flags that enable * capabilities "from the future", which we could not introduce as the default @@ -133,6 +149,7 @@ export const RDS_LOWERCASE_DB_IDENTIFIER = '@aws-cdk/aws-rds:lowercaseDbIdentifi * Tests must cover the default (disabled) case and the future (enabled) case. */ export const FUTURE_FLAGS: { [key: string]: any } = { + [APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID]: true, [ENABLE_STACK_NAME_DUPLICATES_CONTEXT]: 'true', [ENABLE_DIFF_NO_FAIL_CONTEXT]: 'true', [STACK_RELATIVE_EXPORTS_CONTEXT]: 'true', @@ -159,6 +176,7 @@ export const FUTURE_FLAGS_EXPIRED: string[] = [ * explicitly configured. */ const FUTURE_FLAGS_DEFAULTS: { [key: string]: boolean } = { + [APIGATEWAY_USAGEPLANKEY_ORDERINSENSITIVE_ID]: false, [ENABLE_STACK_NAME_DUPLICATES_CONTEXT]: false, [ENABLE_DIFF_NO_FAIL_CONTEXT]: false, [STACK_RELATIVE_EXPORTS_CONTEXT]: false,