From c3e175ca808d729afb2542f5abb96367a47ab523 Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Thu, 7 Nov 2019 16:26:56 +0000 Subject: [PATCH 1/3] fix(apigateway): allow multiple api keys to the same usage plan fixes #4860 --- .../@aws-cdk/aws-apigateway/lib/restapi.ts | 2 +- .../@aws-cdk/aws-apigateway/lib/usage-plan.ts | 2 +- .../test/integ.restapi.expected.json | 2 +- .../aws-apigateway/test/test.usage-plan.ts | 38 ++++++++++++++++++- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts index cb3706d433056..b91b81c8746f8 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/restapi.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/restapi.ts @@ -294,7 +294,7 @@ export class RestApi extends Resource implements IRestApi { /** * Adds a usage plan. */ - public addUsagePlan(id: string, props: UsagePlanProps): UsagePlan { + public addUsagePlan(id: string, props: UsagePlanProps = {}): UsagePlan { return new UsagePlan(this, id, props); } diff --git a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts index 51b44840f5daa..5397f1cc3cca3 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts @@ -178,7 +178,7 @@ export class UsagePlan extends Resource { * @param apiKey */ public addApiKey(apiKey: IApiKey): void { - new CfnUsagePlanKey(this, 'UsagePlanKeyResource', { + new CfnUsagePlanKey(this, `UsagePlanKey${apiKey.node.id}`, { keyId: apiKey.keyId, keyType: UsagePlanKeyType.API_KEY, usagePlanId: this.usagePlanId 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 7832f919726b3..42eca0bcfabf3 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": { + "myapiUsagePlanUsagePlanKeyApiKeyF40DC38C": { "Type": "AWS::ApiGateway::UsagePlanKey", "Properties": { "KeyId": { diff --git a/packages/@aws-cdk/aws-apigateway/test/test.usage-plan.ts b/packages/@aws-cdk/aws-apigateway/test/test.usage-plan.ts index 7ece505a1bb11..1a8b7112a2d0a 100644 --- a/packages/@aws-cdk/aws-apigateway/test/test.usage-plan.ts +++ b/packages/@aws-cdk/aws-apigateway/test/test.usage-plan.ts @@ -129,5 +129,41 @@ export = { }, ResourcePart.Properties)); test.done(); - } + }, + + 'UsagePlan can have multiple keys'(test: Test) { + // 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' + }); + + // WHEN + usagePlan.addApiKey(apiKey1); + usagePlan.addApiKey(apiKey2); + + // THEN + expect(stack).to(haveResource('AWS::ApiGateway::ApiKey', { + Name: 'my-api-key-1' + }, ResourcePart.Properties)); + expect(stack).to(haveResource('AWS::ApiGateway::ApiKey', { + Name: 'my-api-key-2' + }, ResourcePart.Properties)); + expect(stack).to(haveResource('AWS::ApiGateway::UsagePlanKey', { + KeyId: { + Ref: 'myapikey11F723FC7' + } + }, ResourcePart.Properties)); + expect(stack).to(haveResource('AWS::ApiGateway::UsagePlanKey', { + KeyId: { + Ref: 'myapikey2ABDEF012' + } + }, ResourcePart.Properties)); + + test.done(); + }, }; From b7a0d3ff190c10948b89a9b7585ebf71a8b677de Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Fri, 8 Nov 2019 11:47:15 +0000 Subject: [PATCH 2/3] Updated to not change physical ids on existing UsagePlanKey --- .../@aws-cdk/aws-apigateway/lib/usage-plan.ts | 4 +- .../test/integ.restapi.expected.json | 2 +- .../integ.usage-plan.multikey.expected.json | 44 +++++++++++++++++++ .../test/integ.usage-plan.multikey.ts | 20 +++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.expected.json create mode 100644 packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.ts diff --git a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts index 5397f1cc3cca3..f57a87c15653e 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts @@ -178,7 +178,9 @@ export class UsagePlan extends Resource { * @param apiKey */ public addApiKey(apiKey: IApiKey): void { - new CfnUsagePlanKey(this, `UsagePlanKey${apiKey.node.id}`, { + const prefix = 'UsagePlanKeyResource'; + const id = this.node.tryFindChild(prefix) ? `${prefix}:${apiKey.node.id}` : prefix; + new CfnUsagePlanKey(this, id, { keyId: apiKey.keyId, keyType: UsagePlanKeyType.API_KEY, usagePlanId: this.usagePlanId 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 42eca0bcfabf3..7832f919726b3 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" } }, - "myapiUsagePlanUsagePlanKeyApiKeyF40DC38C": { + "myapiUsagePlanUsagePlanKeyResource050D133F": { "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 new file mode 100644 index 0000000000000..09341521f915b --- /dev/null +++ b/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.expected.json @@ -0,0 +1,44 @@ +{ + "Resources": { + "myusageplan4B391740": { + "Type": "AWS::ApiGateway::UsagePlan", + "Properties": {} + }, + "myusageplanUsagePlanKeyResource095B4EA9": { + "Type": "AWS::ApiGateway::UsagePlanKey", + "Properties": { + "KeyId": { + "Ref": "myapikey18B056ACE" + }, + "KeyType": "API_KEY", + "UsagePlanId": { + "Ref": "myusageplan4B391740" + } + } + }, + "myusageplanUsagePlanKeyResourcemyapikey25756C43A": { + "Type": "AWS::ApiGateway::UsagePlanKey", + "Properties": { + "KeyId": { + "Ref": "myapikey250C8F11B" + }, + "KeyType": "API_KEY", + "UsagePlanId": { + "Ref": "myusageplan4B391740" + } + } + }, + "myapikey18B056ACE": { + "Type": "AWS::ApiGateway::ApiKey", + "Properties": { + "Enabled": true + } + }, + "myapikey250C8F11B": { + "Type": "AWS::ApiGateway::ApiKey", + "Properties": { + "Enabled": true + } + } + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.ts b/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.ts new file mode 100644 index 0000000000000..da83c33a8e224 --- /dev/null +++ b/packages/@aws-cdk/aws-apigateway/test/integ.usage-plan.multikey.ts @@ -0,0 +1,20 @@ +import cdk = require('@aws-cdk/core'); +import apigateway = require('../lib'); + +class Test extends cdk.Stack { + constructor(scope: cdk.App, id: string) { + super(scope, id); + + const usageplan = new apigateway.UsagePlan(this, 'myusageplan'); + const apikey1 = new apigateway.ApiKey(this, 'myapikey1'); + const apikey2 = new apigateway.ApiKey(this, 'myapikey2'); + usageplan.addApiKey(apikey1); + usageplan.addApiKey(apikey2); + } +} + +const app = new cdk.App(); + +new Test(app, 'test-apigateway-usageplan-multikey'); + +app.synth(); From eab5f59ca2e629e5b2797d53606c3c617584dc7f Mon Sep 17 00:00:00 2001 From: Niranjan Jayakar Date: Fri, 8 Nov 2019 14:32:34 +0000 Subject: [PATCH 3/3] Changed to use uniqueId --- packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts | 5 ++++- .../test/integ.usage-plan.multikey.expected.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts index f57a87c15653e..c0d9d9d18be65 100644 --- a/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts +++ b/packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts @@ -179,7 +179,10 @@ export class UsagePlan extends Resource { */ public addApiKey(apiKey: IApiKey): void { const prefix = 'UsagePlanKeyResource'; - const id = this.node.tryFindChild(prefix) ? `${prefix}:${apiKey.node.id}` : prefix; + + // Postfixing apikey id only from the 2nd child, to keep physicalIds of UsagePlanKey for existing CDK apps unmodifed. + const id = this.node.tryFindChild(prefix) ? `${prefix}:${apiKey.node.uniqueId}` : prefix; + new CfnUsagePlanKey(this, id, { keyId: apiKey.keyId, keyType: UsagePlanKeyType.API_KEY, 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 09341521f915b..669c3c14f0c20 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 @@ -16,7 +16,7 @@ } } }, - "myusageplanUsagePlanKeyResourcemyapikey25756C43A": { + "myusageplanUsagePlanKeyResourcetestapigatewayusageplanmultikeymyapikey29D6460C6AE8DE59D": { "Type": "AWS::ApiGateway::UsagePlanKey", "Properties": { "KeyId": {