Skip to content
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-appsync: "Auto-merge failed" when creating Merged AppSync API with Source API merge type set AUTO_MERGE #26986

Closed
frixaco opened this issue Sep 2, 2023 · 2 comments · Fixed by #27121
Labels
@aws-cdk/aws-appsync Related to AWS AppSync bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@frixaco
Copy link

frixaco commented Sep 2, 2023

Describe the bug

Hi and thank you for your work on the library!

Merged API Execution Role does not automatically merge the Source API, even with merge_type=appsync.MergeType.AUTO_MERGE.
If I understood correctly, the policy added to this role only allows appsync:SourceGraphQL.

Expected Behavior

Creating Merged AppSync API should, by default and when merge_type is specifically set as AUTO_MERGE, automatically merge Source AppSync APIs.

Current Behavior

Default behaviour for Source APIs does not work. Manually setting merge_type to AUTO_MERGE also does not work.

Reproduction Steps

endpoint1_lambda = lambda_.Function(
    self,
    id="endpoint1-lambda",
    function_name="endpoint1-lambda",
    runtime=lambda_.Runtime.PYTHON_3_11,
    handler="app.lambda_handler",
    code=lambda_.Code.from_asset(
        "./lambda_functions/endpoint1",
    ),
)

source_api_1 = appsync.GraphqlApi(
    self,
    id="source-api-1",
    name="source-api-1",
    definition=appsync.Definition.from_schema(
        appsync.SchemaFile.from_asset(
            "./graphql_schemas/sourceApi1.graphql"
        )
    ),
)
endpoint1_lambda_ds = appsync.LambdaDataSource(
    self,
    id="endpoint1-lambda-ds",
    lambda_function=endpoint1_lambda,
    api=source_api_1,
)

appsync.Resolver(
    self,
    id="endpoint1-resolver",
    api=source_api_1,
    type_name="Query",
    field_name="endpoint1",
    data_source=endpoint1_lambda_ds,
    code=appsync.Code.from_asset(
        "./asd_cdk/graphql_unit_resolvers/base.js",
    ),
    runtime=appsync.FunctionRuntime.JS_1_0_0,
)

merged_api = appsync.GraphqlApi(
    self,
    id="merged-api",
    name="merged-api",
    definition=appsync.Definition.from_source_apis(
        source_apis=[
            appsync.SourceApi(
                source_api=source_api_1,
                merge_type=appsync.MergeType.AUTO_MERGE, # this doesn't work
            )
        ],
    ),
)

Possible Solution

Manually creating Execution Role fixes the issue:

...
merged_api = appsync.GraphqlApi(
    self,
    id="merged-api",
    name="merged-api",
    definition=appsync.Definition.from_source_apis(
        source_apis=[
            appsync.SourceApi(
                source_api=source_api_1,
                merge_type=appsync.MergeType.AUTO_MERGE,
            )
        ],
        merged_api_execution_role=iam.Role( # <=====
            self,
            id="merged-api-execution-role",
            assumed_by=iam.ServicePrincipal("appsync.amazonaws.com"),
            inline_policies={
                "appsync": iam.PolicyDocument(
                    statements=[
                        iam.PolicyStatement(
                            resources=["*"],
                            actions=["appsync:*"],
                        )
                    ]
                )
            },
        ),
    ),
)

Additional Information/Context

Extending allowed list of actions should fix the issue. ['appsync:*] instead of ['appsync:SourceGraphQL']?

CDK CLI Version

2.94.0 (build 987c329)

Framework Version

No response

Node.js Version

v18.16.0

OS

MacOS

Language

Python

Language Version

Python (3.11.5)

Other information

CleanShot 2023-09-02 at 16 40 45@2x
@frixaco frixaco added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 2, 2023
@github-actions github-actions bot added the @aws-cdk/aws-appsync Related to AWS AppSync label Sep 2, 2023
@frixaco frixaco changed the title aws-appsync: "Auto-merge failed" when creating Merged AppSync API aws-appsync: "Auto-merge failed" when creating Merged AppSync API with Source API merge type set AUTO_MERGE Sep 2, 2023
@peterwoodworth peterwoodworth added good first issue Related to contributions. See CONTRIBUTING.md p2 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Sep 5, 2023
@peterwoodworth
Copy link
Contributor

Makes sense, thanks for the investigation and letting us know! Feel free to help us out more by contributing

@mergify mergify bot closed this as completed in #27121 Sep 20, 2023
mergify bot pushed a commit that referenced this issue Sep 20, 2023
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*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-appsync Related to AWS AppSync bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
2 participants