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

app-sync-alpha: 2.55.0-alpha.0 >= cannot migrate to new createResolver without downtime #24108

Closed
jscott-resilient opened this issue Feb 10, 2023 · 12 comments
Labels
@aws-cdk/aws-appsync Related to AWS AppSync

Comments

@jscott-resilient
Copy link

Describe the bug

Now that createResolver requires a resource ID, when building the resolver resource we get the "Only one resolver is allowed per field." error as the logical ID is different and it tries to create the "new" resolver before deleting the old one. This is even the case when using the same typeName:fieldName formatting.

Is it possible to use the new createResolver method without having downtime?

Hope this has a simple solution! But if not, would appreciate some guidance on how to move over.

Thanks for the time!

Joe

Expected Behavior

When using the new createResolver which now takes in the ID parameter:

const lambdaApiResolverDataSource = graphQlApi.addLambdaDataSource(`LambdaApiResolverDataSource`, lambdaApiResolverFunction, {
      name: 'name',
      description: 'Resolves GraphQL requests for audit items'
    });

lambdaApiResolverDataSource.createResolver(CDK_UTILS.graphQlResolver('Query', 'getAuditItems'));

to

const lambdaApiResolverDataSource = graphQlApi.addLambdaDataSource(`LambdaApiResolverDataSource`, lambdaApiResolverFunction, {
      name: 'name',
      description: 'Resolves GraphQL requests for audit items'
    });

lambdaApiResolverDataSource.createResolver('QuerygetAuditItemsResolver', CDK_UTILS.graphQlResolver('Query', 'getAuditItems'));

It shouldn't create a new resolver when keeping the id parameter the same as the current logical id.

Current Behavior

Throws "Only one resolver is allowed per field." exception when trying to move over to new createResolver method

Reproduction Steps

Shown in expected behavior

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.59

Framework Version

No response

Node.js Version

14.15.4

OS

macOS 13.2

Language

Typescript

Language Version

4.7.4

Other information

No response

@jscott-resilient jscott-resilient added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 10, 2023
@github-actions github-actions bot added the @aws-cdk/aws-appsync Related to AWS AppSync label Feb 10, 2023
@peterwoodworth
Copy link
Contributor

The PR changed the scope in which the Resolver is created, so without overrides you won't be able to work around this.

With an override, you could try the following:

const resolver = lambdaApiResolverDataSource.createResolver(CDK_UTILS.graphQlResolver('Query', 'getAuditItems'));
(resolver.node.defaultChild as CfnResolver).overrideLogicalId('<Your old logical ID>')

This is an alpha module, so that means breaking changes may occur from time to time. I hope this helps you work around this breaking change we made!

@peterwoodworth peterwoodworth added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 10, 2023
@github-actions
Copy link

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Feb 13, 2023
@jscott-resilient
Copy link
Author

Thanks for the quick response! We can use your suggestion and create new resolvers to move over to (with different field names), so not the end of the world!

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Feb 13, 2023
@kadishmal
Copy link

Oh, I was bitten by this issue. We have a production application and deleting and recreating the resolvers is not a good option for us. Overriding logical IDs is not practical because our construct that wraps around the resolver creation logic is being used across a number of CDK projects, and we cannot hardcode each project's each resolver's logical ID in the sharable construct. Damn... This is not good. In the CDK v2 migration guide it explicitly mentioned that logical ID change is not expected.

Is there not-alpha version of this AppSync module for CDK v2?

@peterwoodworth
Copy link
Contributor

This isn't related to v1 -> v2 migration, it just has to deal with the alpha module making a breaking change while it was still in alpha

Appsync has since graduated as of 2.60.0, and is now considered stable e5b0230

@github-actions
Copy link

github-actions bot commented May 1, 2023

⚠️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.

@kadishmal
Copy link

kadishmal commented May 1, 2023

This is exactly related to v1 to v2 migration. Even when using 2.77.0 of aws-cdk-lib/aws-appsync stable module, it changes the ID of the resolver.

v1: betaGraphQLbetaGraphQLAPIbetaWarehousesAPILambdaDatasourceMutationcreateWarehouseResolver5880A2CC
v2: betaGraphQLbetaGraphQLAPIbetaWarehousesAPILambdaDatasourceMutationcreateWarehouseResolverD4CBA40F

The suffix has changed from 5880A2CC to D4CBA40F even when no structural change to constructs or their parent stacks has occurred. The problem is that only Resolver resources have changed while all other resource logical IDs are still the same. @peterwoodworth, is it expected?

I see that the resource paths in the metadata have change:

v1: betaServiceAPIStack/betaGraphQL/betaGraphQLAPI/betaWarehousesAPILambdaDatasource/MutationcreateWarehouseResolver/Resource
v2: betaServiceAPIStack/betaGraphQL/betaGraphQLAPI/betaWarehousesAPILambdaDatasourceMutationcreateWarehouseResolver/Resource

Basically, the v2 resource path does not separate resources with a forward slash, instead it somehow merged two resources paths into one which perhaps is causing the logical ID change. I've tried to fix that by manually adding a forward slash into the new resolver ID (in v1 the resolver did not require an ID), but it did not help since it converted the slash into two dashes --.

betaServiceAPIStack/betaGraphQL/betaGraphQLAPI/betaWarehousesAPILambdaDatasource--MutationcreateWarehouseResolver/Resource

Is there a way I can make the resource path return to its previous shape?

@peterwoodworth
Copy link
Contributor

peterwoodworth commented May 1, 2023

As mentioned above, this was a change made to our alpha module, where breaking changes are to be expected from time to time. You might be experiencing this in a migration from v1 to v2, but the root cause would be unrelated to the difference between major versions. You can see the PR here #23322 that is responsible for these changes. The Logical ID is changing because the scope in which these resources are declared changed.

I don't believe there's any sort of workaround that doesn't involve either staying at 2.54.0 which is the last version before this change, or migrating and requiring you to override the logical IDs. It might be possible if instead of calling the createResolver or createFunction methods, you instead manually create the Resolver or AppsyncFunction constructs exactly how the source code did before

@kadishmal
Copy link

Got it! Good suggestions. Let me think through them and see what I should do to reduce the risks. Thank you.

@bandreit
Copy link

@kadishmal do you have any updates on this? what were your conclusions? I'm currently dealing with the same thing.

@PGuimarais
Copy link

@kadishmal do you have any updates on this? what were your conclusions? I'm currently dealing with the same thing.

Facing the same issue when upgrading from cdk v1 to v2

@bandreit
Copy link

bandreit commented Aug 2, 2023

@PGuimarais The solution represented here: #13269 (comment) worked for me as a work-around:

const resolverId = `${typeName}${fieldName}Resolver`;
  // eslint-disable-next-line no-new
  new Resolver(this.lambdaDataSource, resolverId, {
    api: this.api,
    dataSource: this.lambdaDataSource,
    typeName,
    fieldName,
  });

This does not create new resolvers and you can upgrade without any downtime.

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
Projects
None yet
Development

No branches or pull requests

5 participants