Skip to content

Commit

Permalink
fix(apigateway): cannot remove first api key from usage plan (#12505)
Browse files Browse the repository at this point in the history
This is caused by a [commit introduced] back in Nov 2019.
The original change tried to be overly ambitious in not triggering a
resource replacement on customers who already had a single key usage
plan configured.

This results in stack update failures for a usage plan with multiple
keys, when the **first** key is removed. Removal of the first key
re-adjusts the logical ids of all the keys in a way that makes the
update look like the first key is changing and a subsequent key is being
removed.

The fix is to simply not special case the first key.

Resource replacement of Usage Plan Key resource should not create any
impact on running applications.

fixes #11876

[commit introduced]: 142bd0e

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Niranjan Jayakar committed Jan 21, 2021
1 parent 638b995 commit 96cbe32
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 5 deletions.
4 changes: 1 addition & 3 deletions packages/@aws-cdk/aws-apigateway/lib/usage-plan.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,8 @@ export class UsagePlan extends Resource {
* @param apiKey
*/
public addApiKey(apiKey: IApiKey): void {
const prefix = 'UsagePlanKeyResource';

// 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}:${Names.nodeUniqueId(apiKey.node)}` : prefix;
const id = `UsagePlanKeyResource:${Names.nodeUniqueId(apiKey.node)}`;

new CfnUsagePlanKey(this, id, {
keyId: apiKey.keyId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,7 @@
"UsagePlanName": "Basic"
}
},
"myapiUsagePlanUsagePlanKeyResource050D133F": {
"myapiUsagePlanUsagePlanKeyResourcetestapigatewayrestapimyapiApiKeyC43601CB600D112D": {
"Type": "AWS::ApiGateway::UsagePlanKey",
"Properties": {
"KeyId": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"myusageplan4B391740": {
"Type": "AWS::ApiGateway::UsagePlan"
},
"myusageplanUsagePlanKeyResource095B4EA9": {
"myusageplanUsagePlanKeyResourcetestapigatewayusageplanmultikeymyapikey1DDABC389A2809A73": {
"Type": "AWS::ApiGateway::UsagePlanKey",
"Properties": {
"KeyId": {
Expand Down
28 changes: 28 additions & 0 deletions packages/@aws-cdk/aws-apigateway/test/usage-plan.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,4 +205,32 @@ describe('usage plan', () => {
},
}, ResourcePart.Properties);
});

test('UsagePlanKeys have unique logical ids', () => {
// GIVEN
const app = new cdk.App();
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',
]);
});
});

0 comments on commit 96cbe32

Please sign in to comment.