fix(apigateway): cannot remove first api key from usage plan (aws#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 - 142bd0e - 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 aws#11876 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd · Aug 26, 2021 · c92c8c4 · c92c8c4
1 parent 239b3e5
commit c92c8c4
Show file tree

Hide file tree

Showing 6 changed files with 187 additions and 82 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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',
+        ]);
+      });
+    });
   });
 });