fix(appsync): Source APIs are not auto-merged by default #27025
Conversation
Sets a broad set of permissions for Merged AppSync API Execution Role: `appsync:*` to allow auto-merging both by default and by setting `mergeType` to `AUTO_MERGE`.
github-actions
bot
added
the
beginning-contributor
github-actions
bot
added
bug
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
@jumic Can you have a look at this? |
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing |
|
Hi @rix0rrr. Yep, I can do that. I'll update permissions with only required ones. |
|
@ndejaco2 also replied in the PR regarding wrong permissions: Please also check this comment. I'm on vacation without notebook, so I can't test any fixes. |
|
I am hoping to submit a change that will fix the bugs introduced here as well as refactor the change so that the source api association is itself an L2 construct that can also be used outside the merged api definition in case teams do that in different stacks. I plan to send a PR for this shortly. |
|
I sent PR which will better address this: #27121 |
|
Closing in favor of #27121 |
As part of supporting AppSync Merged APIs, this change introduces a standalone SourceApiAssociation construct for declaring a source api association between a source API and a Merged API. Why do we need a standalone construct? * There are two potential deployment models when dealing with separate stacks/pipelines between the source API and Merged API: 1. Push model where the source API owners manage the association in their stack 2. Pull model where the associations are managed in the Merged API stack. * Having a standalone construct gives developers more flexibility while still handling all the IAM permission handling in a single place. * Developers can continue to use the GraphQLApi construct and declare the source api configuration all within a single construct as before. But, if they want to have the source api association as a standalone object this change gives them flexibility I also fixed two issues related to IAM: 1. The resource for appsync:SourceGraphQL needs both the source api arn and the source api arn + "/*" to get all top level fields. 2. The merged api execution role also needs appsync:StartSchemaMerge if the association is using AUTO_MERGE. The fix here is preferred over existing PR: #27025 Closes #26986 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Creating Merged API does not auto-merge Source API's because it's Execution Role doesn't have enough permissions.
Gives broader (
appsync:*) set of permissions for Merged API Execution Role to fix the issue.Closes #26986.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license