-
Notifications
You must be signed in to change notification settings - Fork 79
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
CLI 12.10+ breaks GraphQL Queries #3006
Comments
@teckapps How are you overriding table names in your project? Please provide same schema of the model and the code block you have to override a table name. |
My schema is split in multiple files. I have e.g. these two files: PublicSettingsUtilities.graphqltype PublicSetting @model PublicTutorials.graphqltype PublicTutorialTopic @model type PublicTutorialItem @model type PublicTutorialMediaResource @model enum PublicTutorialMediaResourceType { My overrides looks like this override.tsimport { AmplifyApiGraphQlResourceStackTemplate } from '@aws-amplify/cli-extensibility-helper'; export function override(resources: AmplifyApiGraphQlResourceStackTemplate) { |
Hi @AnilMaktala. I'm getting an DiagnoseReportUploadError every time I try. Is there any other way I can send you the .zip file? Or what might be the cause of the 'DiagnoseReportUploadError'? (I haven't found anything in the docs) amplify diagnose ✅ Report saved: /var/folders/t_/_3hxz6vx7kv0fscmqsdq017w0000gp/T/WineCloud/report-1731440812145.zip ✔ Send Report (y/N) · yes |
Hey @AnilMaktala, yes. You can find me as danmedia93 |
@teckapps I've sent you a message on Discord. Please share the zip file there. |
Hi @teckapps, I'm looking at this and haven't been able to reproduce using an API created with 12.9.0 and then updated after an upgrade to 12.10.0. Client operations succeed, and the table policy looks as expected. Can you paste the IAM policy for your
|
Hi @palpatim, thanks for looking into this. Here are the (account redacted) policy statements. For what I can see, there is a problem with the DynamoBB Policy, as it does not reference the correct (override) table name but the name with the random id and environment at the end. The strange thing is, that currently all works as expected in my dev environment. The default policy references the correct table name. I guess I would have to upgrade the cli and see if the IAMRoleDefautPolicy then changes as well? Modify permissions in DynamoDBAccess71ABE5AE
Modify permissions in PublicFeedbackItemIAMRoleDefaultPolicyA7CFE05B
Trusted entities
Let me know how to proceed. |
A couple of things are puzzling me: 1. Timeline We made a change to the way DDB policies are handled in a86c816, which was released in Amplify CLI 12.12.0 in May 2024. However, that doesn't match up with the behavior you're reporting that any version >= 12.10.0 is causing the issue. I'm investigating this anyway because it's the most likely culprit, but the timeline inconsistency bothers me. 2. Behavior I saw the same policy structure in my dev environment as well -- the DynamoDBAccess policy resources are named with the default Amplify naming convention, while the PublicFeedbackItemIAMRoleDefaultPolicy has the overridden name. The thing is, it shouldn't matter! :) When AppSync adopts the role, it would gain the union of those permissions, and as long as no other permission explicitly denied access to Followups
(In case it's not clear, at this point I'm kind of throwing things at the wall to see what sticks, because something isn't quite adding up for me. :( ) |
Hi @palpatim. You might be right about the timeline, I might have messed up the versions when testing the issue (sorry for that). I can confirm that everything still works fine with 12.10.0. But it definitely breaks with 12.12.0 and 12.13.1. First my follow ups you asked for: Now to my findings after more than 2hours up- & downgrading the cli:
So the problem seems, that somehow the '...IAMRoleDefaultPolicy...' is missing. Deployed and correct working with 12.9.0 & 12.10.0: Deployed and not working with 12.12.0 & 12.13.1: |
Thanks for the additional context. This is confirming what I'm seeing, which is that the |
Commit a86c816 optimized table roles to reduce the number of individual resources created in a stack, and to improve deployment time. One of the changes was to manually craft the table role with the same permissions as would be granted by the CDK default, and add the role to the stack with a Unfortunately, the routine constructs the role resources using the table name components (model name, API ID, and environment name) to craft the table name, without considering table name overrides. By contrast, the default CDK handling attaches the role directly to the table object, so the table name is resolved correctly at deploy time. We'll need to figure out the right way to handle this. The good news is that we can scope this to just Gen 1 projects -- Gen 2 projects have different ways of handling overrides, and we're currently thinking about the right way of handling them. Even so, we need to make sure that any change we make doesn't regress any existing Gen 1 customers on CLI version >= 12.12.0. We'll keep working on this and update here when we have more info. In the meantime, you should be able to workaround by overriding the Amplify-created override.ts import { AmplifyApiGraphQlResourceStackTemplate } from "@aws-amplify/cli-extensibility-helper";
const AWS_REGION = "YOUR AWS REGION";
const AWS_ACCOUNT_ID = "YOUR AWS ACCOUNT ID";
export function override(resources: AmplifyApiGraphQlResourceStackTemplate) {
addTableOverrideToResources(resources, "PublicFeedbackItem", "PublicFeedbackItemOverride");
}
/**
* Updates the resources in place by adding a table name override for the
* specified model name, and applying appropriate policy resource updates.
*/
const addTableOverrideToResources = (
resources: AmplifyApiGraphQlResourceStackTemplate,
modelName: string,
newTableName: string
): void => {
resources.models[modelName].modelDDBTable.tableName = newTableName;
resources.models[modelName].modelIamRole.policies = [{
policyName: 'DynamoDBAccessOverride',
policyDocument: {
Version: "2012-10-17",
Statement: [
{
Action: [
"dynamodb:BatchGetItem",
"dynamodb:BatchWriteItem",
"dynamodb:ConditionCheckItem",
"dynamodb:DeleteItem",
"dynamodb:DescribeTable",
"dynamodb:GetItem",
"dynamodb:GetRecords",
"dynamodb:GetShardIterator",
"dynamodb:PutItem",
"dynamodb:Query",
"dynamodb:Scan",
"dynamodb:UpdateItem",
],
Resource: [
`arn:aws:dynamodb:${AWS_REGION}:${AWS_ACCOUNT_ID}:table/${newTableName}`,
`arn:aws:dynamodb:${AWS_REGION}:${AWS_ACCOUNT_ID}:table/${newTableName}/index/*`,
],
Effect: "Allow",
},
],
}
}];
}; |
Thank you, just tested it and it works for me! Just to give you heads up: I already played around with some Gen2 Projects in the last couple of months and I came across one issue with the overrides: I probably won't be able to deploy a sandbox in the same AWS Account as my dev environment using my override names. For new projects this won't be an issue because I wouldn't use the overrides anymore. But at some point I will hopefully be able to migrate my Gen1 project to Gen2. But obviously I will still have my tables with the override names per account. So in order to deploy a sandbox environment in the same AWS account and region as my dev environment, it would be necessary to attach a suffix "-SandboxABCXYZ" to my table names - but only in the sandbox deployment, not in dev and not in prod. My project is in production for 1.5 years now, so I can't just remove the overrides. Sorry for the hassle, had I known that before I wouldn't have used the override names... |
No apologies necessary! We're working on a Gen 2 migration guide and hopefully we'll be able to address the Gen 2 override issue at the same time. In the meantime, I'm leaving this issue open so we can solve this in Ampilfy code rather than making you install the workaround. |
The fix for this (#3075) has been merged to our Gen 1 branch. It will be packaged up in the next version of the Amplify CLI. |
Updated Amplify Data package version has been propagated to CLI configs: aws-amplify/amplify-cli#14048 and is staged for that package's next release. |
Temporarily revert changes in Gen 1 branch, and Amplify Data package version bump in CLI as we're working on investigating root cause of certain IAM role error in test. The Amplify Data package version change may need to be delayed to next CLI package's release. |
How did you install the Amplify CLI?
npm
If applicable, what version of Node.js are you using?
v20.14.0
Amplify CLI Version
12.9.0
What operating system are you using?
macOS 15.1
Did you make any manual changes to the cloud resources managed by Amplify? Please describe the changes made.
No. The only "manual" change I made was overriding my DynamoDB table names using amplify override api. I guess this could be the reason, because the error appends a role name like 'IAfe82d0-2aokgojt5jbq7dzxznmrvbqgwi-dev' to my TableName, just as it would be if I wouldn't use overrides.
Describe the bug
My GraphQL queries break after updating to CLI 12.10 and later (tried up to 12.13).
If I push my project using an CLI version > 12.9 and attempt to perform list or query operations from the client side, I get errors like this:
"User: arn:aws:sts::[ACCOUNT_ID]:assumed-role/PublicFeedbackItemIAfe82d0-2aokgojt5jbq7dzxznmrvbqgwi-dev/APPSYNC_ASSUME_ROLE is not authorized to perform: dynamodb:Query on resource: arn:aws:dynamodb:eu-central-1:[ACCOUNT_ID]:table/PublicFeedbackItem/index/byStatus because no identity-based policy allows the dynamodb:Query action (Service: DynamoDb, Status Code: 400, Request ID: KL839V8TGVEMTOOJUV75SNSOD3VV4KQNSO5AEMVJF66Q9ASUAAJG)"
Expected behavior
My queries should work without the DynamoDB exception error when using a cli version > 12.9
Reproduction steps
1.) Override table names in a current project
2.) Deploy project
3.) Update to cli >= 12.10
I think I created my project about two years ago and never had problems until now.
Project Identifier
DiagnoseReportUploadError
✖ Sending zip
Log output
Additional information
No response
Before submitting, please confirm:
The text was updated successfully, but these errors were encountered: