-
Notifications
You must be signed in to change notification settings - Fork 4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(@aws-cdk/aws-apigateway): Wrong Logical ID for UsagePlanKeyResource #11876
Comments
Hi @makruege - I'm looking to reproduce this issue. I started with a simple CDK app - import { App, Stack } from '@aws-cdk/core';
import { RestApi } from '@aws-cdk/aws-apigateway';
const app = new App();
const stack = new Stack(app, 'MyStack');
const api = new RestApi(stack, 'MyRestApi');
api.root.addMethod('ANY');
const users = [ 'user1', 'user2', 'user3'];
const usagePlan = api.addUsagePlan('MyUsagePlan');
users.forEach(u => {
api.addApiKey(`${u}-apikey`, {
apiKeyName: `${u}`
});
}); I'm unable to see any of the symptoms you're pointing at. The logical ids look normal -
I was able to deploy the stack and follow it up with updates to remove these users. They all seem to work fine. I suspect there may be something else in your app that's causing this? |
Hi @nija-at! I've created a blank new cdk project and used your code, so we get the same results. Yes, the API Key IDs are fine. You have to add the API Keys to the Usage Plan to get the "error":
And this is the result of a cdk diff:
You can see three UsagePlanKeys and the first one (MyRestApiMyUsagePlanUsagePlanKeyResource19FC21A7) differs. After a initial deployment with all three users I remove user2 and user3 (const users = ["user1"];):
Seems fine! Now I remove user1 (const users = ["user2", "user3"];):
It is the same result as in my original comment (replacement) and it is not dependent on the project. |
Thanks for responding. This is indeed a bug in the CDK, that was introduced more than a year ago - 142bd0e#diff-8ec617398ff0629bdc92654be9d4a063d7e8d64fedaf33263604579c2f4e9ea4 |
Thank you a lot @nija-at! Hopefully it will be fixed soon :-) |
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
I've prepared a fix here - #12505 |
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*
|
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 aws#11876 [commit introduced]: aws@142bd0e ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Unfortunately, the fix has caused unintended regression during stack updates. This change is going to be reverted - #12745 Re-opening this issue. |
Hi @nija-at! I had to make the following changes to apply the fix to our already deployed stacks:
After the deployment with these changes and the upgrade to 1.87.0 I have reverted these changes and deployed the stacks again, so the UsagePlan IDs and Stages were as before. This is our original code without the changes:
And now with the mentioned changes:
Maybe these information are useful to you. |
Thanks @makruege. I'll use these to see if there's a better fix for this. Versions 1.87.1 and later will not carry this fix above. |
@nija-at I have a stack managed by java cdk. I see that logical id for apikeyusageplan is changing sometimes when cdk synth command runs. This causes the stack to complain that resource already exists. Do we have a fix for this? |
Hi @nija-at! Because this is a really old bug: What are the chances that there will be a fix in the near future? When we want to be able to remove certain entries in our project we have to stay on version 1.87.0 and cannot upgrade our project to the latest versions. |
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, two things are done - 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
@makruege - They key to this issue is that the logical id of the const usagePlanKeys = usagePlan.node.children.filter(c => c instanceof CfnUsagePlanKey) as CfnUsagePlanKey[]; and then setting the logical id to the value that it already has by using the I've taken another stab at preparing a fix for this - #13817. I hope for better luck with this change. |
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 #11876 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
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*
@nija-at @makru21 I'm running into this problem now when trying to upgrade to CDK v2 (albeit in Python) and as someone newer to CDK I'm having trouble figuring out how to patch this code.
api_key = api.add_api_key(f"{id}APIKey", value=value)
usage_plan = api.add_usage_plan(f"{id}APIUsagePlan")
usage_plan.add_api_key(api_key)
usage_plan.add_api_stage(stage=api.deployment_stage)
|
@brad-lucas try to add : "context": {
"@aws-cdk/aws-apigateway:usagePlanKeyOrderInsensitiveId": false
} in your cdk.json file. |
@brad-lucas / @yosefbs I was only able to fix this issue by implementing a fix similar to ghost's (user earlier in this thread). Basically I just made a small change to the logical IDs for the usage plan and for the gateway API stage, in effect forcing CloudFormation to re-create these constructs (as well as a usage plan key). |
❓ General Issue
The Question
Hi there! We migrated a lot of APIs from our on-premise-solution into the AWS API Gateway. To control access to our APIs we configured Cognito (with OAuth Scopes) and API Keys. These API Keys are generated with an array holding a number of subscribers (other stakeholders or applications):
subscriber: ['user1', 'user2', 'user3']
Our code iterates over this array, generates API Keys for each element and adds them to the APIs UsagePlan:
This is the output of a "cdk diff" when adding three freshly new subscriber to a already deployed API:
You can see, that three new API Keys will be added to the UsagePlan. Please note the differences here:
The Logical ID of the first UsagePlanKeyResource differs heavily from the other two. This is a screenshot from the AWS console to make things more clear (the screenshot was taken earlier, thats why the IDs of the cdk diff and the aws console don't match - please understand the screenshot as a visual support):
There are three API Keys and they are all related to the UsagePlan with a UsagePlanKey, with the exception of the first UsagePlanKeys Logical ID being different.
With this issue in mind I could remove user2 and/or user3 from the array with no problems (cdk diff):
But when I try to remove user1 instead, the stack fails because of a replacement:
3/8 | 15:21:45 | UPDATE_IN_PROGRESS | AWS::ApiGateway::UsagePlanKey | test-api/test-api/UsagePlanKeyResource (testapiUsagePlanKeyResource6D2F267F) Requested update requires the creation of a new physical resource; hence creating one. 3/8 | 15:21:45 | UPDATE_FAILED | AWS::ApiGateway::UsagePlanKey | test-api/test-api/UsagePlanKeyResource (testapiUsagePlanKeyResource6D2F267F) ezplnhe139:vxxai7 already exists in stack arn:aws:cloudformation:eu-central-1:318255054997:stack/test-api-stack/63771480-2e4d-11eb-83d4-02886036c898
I can't figure out why this happens. There is no difference in the code regarding the array elements 0,1,...,n. Please note: When adding just one API Key (or just one array element) I can remove it without problems, but the Logical IDs strange look remains.
The replacement of the KeyId happens only if the number of API Keys is greater than one and the to be removed element is the first one. Is this some kind of bug in the cdk?
Any help is greatly appreciated - thanks!
Kind Regards,
Marcus
Environment
The text was updated successfully, but these errors were encountered: