[appsync]: Data Source breaking change

[asnaseer-resilient]

I just tried the latest CDK v1.60.0 and changed these lines of code in my existing deployment from this:

    const lambdaApiResolverDataSource = graphQlApi.addLambdaDataSource(
      'LambdaApiResolverDataSource',
      `Resolves GraphQL requests for ${product}`,
      lambdaApiResolverFunction
    );

to this:

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

When the deployment ran, it failed with this:

AppSyncConstructGraphQLLambdaApiResolverDataSourceBAD1E2A7
CREATE_FAILED
Data source with name LambdaApiResolverDataSource already exists (Service: AWSAppSync; Status Code: 400; Error Code: BadRequestException; Request ID: 45ade8da-9b90-432e-9620-0de300aaf18a; Proxy: null)

This is for an existing AppSync deployment. This code was working in the previous release I had (v1.59.0).

Reproduction Steps

As described above.

What did you expect to happen?

The deployment should have succeeded.

What actually happened?

The deployment failed with:

AppSyncConstructGraphQLLambdaApiResolverDataSourceBAD1E2A7
CREATE_FAILED
Data source with name LambdaApiResolverDataSource already exists (Service: AWSAppSync; Status Code: 400; Error Code: BadRequestException; Request ID: 45ade8da-9b90-432e-9620-0de300aaf18a; Proxy: null)

Environment

• CLI Version : v1.60.0
• Framework Version:
• Node.js Version: v12.4.0
• OS : Deployment ran on CircleCI
• Language (Version): Typescript 3.9.7

Other

---

This is 🐛 Bug Report

[asnaseer-resilient]

I had to delete my AppSync deployment on AWS and then re-deploy it for it to work again.

[andrestone]

Probably this changed the construct tree which is what is used to generate the correspondent logical ID in the synthesized template, making CloudFormation think you were creating a new data source.

I'm not sure there's something to be fixed here. Maybe make sure such things appear in the "Breaking Changes"?

[asnaseer-resilient]

Thanks for tracking it down - and I agree it should definitely have appeared in the "Breaking Changes".

We have 3 APIs that have been deployed to live and a web app that uses them. This breakage meant that I had to delete the 3 APIs and then re-deploy them which caused their corresponding GraphQL endpoints to change. I then had to update my web app with the new endpoints and re-deploy that.

I realise this is an evolving library and really appreciate all the work everyone has put into this. Many people now use this in production and therefore it would be really appreciated if any changes could be made such that they do not imply a re-deployment of resources - I know this is hard but just something to think about.

[BryanPan342]

@andrestone thanks for the initiative in looking into this

@asnaseer-resilient thanks for the feedback! I apologize, I should have written a better commit description highlighting the potential effects. The AppSync module a couple of months ago used to be a monolithic single file with virtually no unit tests. Currently, I have been trying to clean up the library and make sure it functions similarly to the rest of the CDK.

I definitely keep this in mind as I make changes in the future!

[asnaseer-resilient]

@BryanPan342 Thanks - and don't get me wrong - I really appreciate the amazing work all of you are putting into this library. It is by far the best deployment framework I have ever worked with (and I have been coding for more than 30 years).

[asnaseer-resilient]

BTW: I assume you guys are using semantic versioning which states:

Given a version number MAJOR.MINOR.PATCH, increment the:

MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards compatible manner, and
PATCH version when you make backwards compatible bug fixes.

It will therefore help even more if you always change the MAJOR version when making a breaking change.

[MrArnoldPalmer]

@asnaseer-resilient AppSync is currently an "experimental" module, so breaking changes are common at this point. Check out the docs for more information on our current versioning strategy. Regardless this should have been marked as a breaking change.

Modules in the AWS Construct Library are designated Experimental while we build them; experimental modules may have breaking API changes in any release. After a module is designated Stable, it adheres to semantic versioning, and only major releases can have breaking changes. Each module's stability designation is available on its Overview page in the AWS CDK API Reference. For more information, see Versioning in the CDK Developer Guide.

[andrestone]

@asnaseer-resilient You could safely use the Cfn* classes in production, which are stable across all CDK.

[asnaseer-resilient]

@MrArnoldPalmer Ah - OK - thanks for clarifying this.

@andrestone I prefer to use the CDK classes but thanks for the suggestion.