fix(appsync): unexpected resolver replacement (aws#23322)

fix(appsync): unstable IDs on resolvers and functions Fixes an issue that would cause unexpected resource replacement for appsync resolvers and functions because of construct nesting and ID generation. Changes `createResolver` and `createFunction` methods on `GraphQlApi` and `DataSource` constructs to require explicitly passing an ID. Additionally changes the scope of the constructs created in `createResolver` and `createFunction` on the `DataSource` construct to be `this.api` instead of `this`. This allows users to change the data sources of resolvers and functions while keeping the IDs stable and avoiding resource replacement. This helps to avoid the `only one resolver per field` error that occurs when deleting a resolver on a field, and adding a new one within the same deployment. BREAKING CHANGE: `DataSource.createResolver`, `DataSource.createFunction`, and `GraphQlApi.createResolver` now require 2 arguments instead of 1. Fixes: aws#13269 ---- ### All Submissions: * [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Construct Runtime Dependencies: * [ ] This PR adds new construct runtime dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-construct-runtime-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho · Feb 22, 2023 · 1aef786 · 1aef786
1 parent b8eb1a1
commit 1aef786
Show file tree

Hide file tree

Showing 44 changed files with 2,823 additions and 2,318 deletions.
diff --git a/packages/@aws-cdk/aws-appsync/README.md b/packages/@aws-cdk/aws-appsync/README.md
@@ -79,15 +79,15 @@ const demoDS = api.addDynamoDbDataSource('demoDataSource', demoTable);
 // Resolver for the Query "getDemos" that scans the DynamoDb table and returns the entire list.
 // Resolver Mapping Template Reference:
 // https://docs.aws.amazon.com/appsync/latest/devguide/resolver-mapping-template-reference-dynamodb.html
-demoDS.createResolver({
+demoDS.createResolver('QueryGetDemosResolver', {
   typeName: 'Query',
   fieldName: 'getDemos',
   requestMappingTemplate: appsync.MappingTemplate.dynamoDbScanTable(),
   responseMappingTemplate: appsync.MappingTemplate.dynamoDbResultList(),
 });
 
 // Resolver for the Mutation "addDemo" that puts the item into the DynamoDb table.
-demoDS.createResolver({
+demoDS.createResolver('MutationAddDemoResolver', {
   typeName: 'Mutation',
   fieldName: 'addDemo',
   requestMappingTemplate: appsync.MappingTemplate.dynamoDbPutItem(
@@ -98,7 +98,7 @@ demoDS.createResolver({
 });
 
 //To enable DynamoDB read consistency with the `MappingTemplate`:
-demoDS.createResolver({
+demoDS.createResolver('QueryGetDemosConsistentResolver', {
   typeName: 'Query',
   fieldName: 'getDemosConsistent',
   requestMappingTemplate: appsync.MappingTemplate.dynamoDbScanTable(true),
@@ -137,7 +137,7 @@ declare const api: appsync.GraphqlApi;
 const rdsDS = api.addRdsDataSource('rds', cluster, secret, 'demos');
 
 // Set up a resolver for an RDS query.
-rdsDS.createResolver({
+rdsDS.createResolver('QueryGetDemosRdsResolver', {
   typeName: 'Query',
   fieldName: 'getDemosRds',
   requestMappingTemplate: appsync.MappingTemplate.fromString(`
@@ -154,7 +154,7 @@ rdsDS.createResolver({
 });
 
 // Set up a resolver for an RDS mutation.
-rdsDS.createResolver({
+rdsDS.createResolver('MutationAddDemoRdsResolver', {
   typeName: 'Mutation',
   fieldName: 'addDemoRds',
   requestMappingTemplate: appsync.MappingTemplate.fromString(`
@@ -244,7 +244,7 @@ const httpDs = api.addHttpDataSource(
   }
 );
 
-httpDs.createResolver({
+httpDs.createResolver('MutationCallStepFunctionResolver', {
   typeName: 'Mutation',
   fieldName: 'callStepFunction',
   requestMappingTemplate: appsync.MappingTemplate.fromFile('request.vtl'),
@@ -275,7 +275,7 @@ const domain = new opensearch.Domain(this, 'Domain', {
 declare const api: appsync.GraphqlApi;
 const ds = api.addOpenSearchDataSource('ds', domain);
 
-ds.createResolver({
+ds.createResolver('QueryGetTestsResolver', {
   typeName: 'Query',
   fieldName: 'getTests',
   requestMappingTemplate: appsync.MappingTemplate.fromString(JSON.stringify({

diff --git a/packages/@aws-cdk/aws-appsync/lib/data-source.ts b/packages/@aws-cdk/aws-appsync/lib/data-source.ts
@@ -132,8 +132,8 @@ export abstract class BaseDataSource extends Construct {
   /**
    * creates a new resolver for this datasource and API using the given properties
    */
-  public createResolver(props: BaseResolverProps): Resolver {
-    return new Resolver(this, `${props.typeName}${props.fieldName}Resolver`, {
+  public createResolver(id: string, props: BaseResolverProps): Resolver {
+    return new Resolver(this.api, id, {
       api: this.api,
       dataSource: this,
       ...props,
@@ -143,8 +143,8 @@ export abstract class BaseDataSource extends Construct {
   /**
    * creates a new appsync function for this datasource and API using the given properties
    */
-  public createFunction(props: BaseAppsyncFunctionProps): AppsyncFunction {
-    return new AppsyncFunction(this, `${props.name}Function`, {
+  public createFunction(id: string, props: BaseAppsyncFunctionProps): AppsyncFunction {
+    return new AppsyncFunction(this.api, id, {
       api: this.api,
       dataSource: this,
       ...props,

diff --git a/packages/@aws-cdk/aws-appsync/lib/graphqlapi-base.ts b/packages/@aws-cdk/aws-appsync/lib/graphqlapi-base.ts
@@ -134,7 +134,7 @@ export interface IGraphqlApi extends IResource {
   /**
    * creates a new resolver for this datasource and API using the given properties
    */
-  createResolver(props: ExtendedResolverProps): Resolver;
+  createResolver(id: string, props: ExtendedResolverProps): Resolver;
 
   /**
    * Add schema dependency if not imported
@@ -285,8 +285,8 @@ export abstract class GraphqlApiBase extends Resource implements IGraphqlApi {
   /**
    * creates a new resolver for this datasource and API using the given properties
    */
-  public createResolver(props: ExtendedResolverProps): Resolver {
-    return new Resolver(this, `${props.typeName}${props.fieldName}Resolver`, {
+  public createResolver(id: string, props: ExtendedResolverProps): Resolver {
+    return new Resolver(this, id, {
       api: this,
       ...props,
     });

diff --git a/packages/@aws-cdk/aws-appsync/test/appsync-caching-config.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync-caching-config.test.ts
@@ -33,7 +33,7 @@ describe('Lambda caching config', () => {
     // WHEN
     const lambdaDS = api.addLambdaDataSource('LambdaDS', func);
 
-    lambdaDS.createResolver({
+    lambdaDS.createResolver('QueryAllPosts', {
       typeName: 'Query',
       fieldName: 'allPosts',
     });
@@ -50,7 +50,7 @@ describe('Lambda caching config', () => {
     // WHEN
     const lambdaDS = api.addLambdaDataSource('LambdaDS', func);
 
-    lambdaDS.createResolver({
+    lambdaDS.createResolver('QueryAllPosts', {
       typeName: 'Query',
       fieldName: 'allPosts',
       cachingConfig: {
@@ -77,7 +77,7 @@ describe('Lambda caching config', () => {
 
     // THEN
     expect(() => {
-      lambdaDS.createResolver({
+      lambdaDS.createResolver('QueryAllPosts', {
         typeName: 'Query',
         fieldName: 'allPosts',
         cachingConfig: {
@@ -95,7 +95,7 @@ describe('Lambda caching config', () => {
 
     // THEN
     expect(() => {
-      lambdaDS.createResolver({
+      lambdaDS.createResolver('QueryAllPosts', {
         typeName: 'Query',
         fieldName: 'allPosts',
         cachingConfig: {
@@ -113,7 +113,7 @@ describe('Lambda caching config', () => {
 
     // THEN
     expect(() => {
-      lambdaDS.createResolver({
+      lambdaDS.createResolver('QueryAllPosts', {
         typeName: 'Query',
         fieldName: 'allPosts',
         cachingConfig: {

diff --git a/packages/@aws-cdk/aws-appsync/test/appsync-lambda.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync-lambda.test.ts
@@ -116,7 +116,7 @@ describe('Lambda Data Source configuration', () => {
       description: 'custom description',
     });
 
-    ds.createResolver({
+    ds.createResolver('TestField', {
       typeName: 'test',
       fieldName: 'field',
     });
@@ -164,4 +164,4 @@ describe('adding lambda data source from imported api', () => {
       ApiId: { 'Fn::GetAtt': ['baseApiCDA4D43A', 'ApiId'] },
     });
   });
-});
+});
diff --git a/packages/@aws-cdk/aws-appsync/test/appsync-mapping-template.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync-mapping-template.test.ts
@@ -34,7 +34,7 @@ describe('Lambda Mapping Templates', () => {
     // WHEN
     const lambdaDS = api.addLambdaDataSource('LambdaDS', func);
 
-    lambdaDS.createResolver({
+    lambdaDS.createResolver('QueryAllPosts', {
       typeName: 'Query',
       fieldName: 'allPosts',
       requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(),
@@ -52,7 +52,7 @@ describe('Lambda Mapping Templates', () => {
     // WHEN
     const lambdaDS = api.addLambdaDataSource('LambdaDS', func);
 
-    lambdaDS.createResolver({
+    lambdaDS.createResolver('PostRelatedPosts', {
       typeName: 'Post',
       fieldName: 'relatedPosts',
       requestMappingTemplate: appsync.MappingTemplate.lambdaRequest('$util.toJson($ctx)', 'BatchInvoke'),
@@ -67,4 +67,4 @@ describe('Lambda Mapping Templates', () => {
       MaxBatchSize: 10,
     });
   });
-});
+});
diff --git a/packages/@aws-cdk/aws-appsync/test/appsync.test.ts b/packages/@aws-cdk/aws-appsync/test/appsync.test.ts
@@ -21,13 +21,13 @@ beforeEach(() => {
 test('appsync should configure pipeline when pipelineConfig has contents', () => {
   // WHEN
   const ds = api.addNoneDataSource('none');
-  const test1 = ds.createFunction({
+  const test1 = ds.createFunction('Test1Function', {
     name: 'test1',
   });
-  const test2 = ds.createFunction({
+  const test2 = ds.createFunction('Test2Function', {
     name: 'test2',
   });
-  api.createResolver({
+  api.createResolver('TestTest2', {
     typeName: 'test',
     fieldName: 'test2',
     pipelineConfig: [test1, test2],
@@ -38,8 +38,8 @@ test('appsync should configure pipeline when pipelineConfig has contents', () =>
     Kind: 'PIPELINE',
     PipelineConfig: {
       Functions: [
-        { 'Fn::GetAtt': ['apinonetest1FunctionEF63046F', 'FunctionId'] },
-        { 'Fn::GetAtt': ['apinonetest2Function615111D0', 'FunctionId'] },
+        { 'Fn::GetAtt': ['apiTest1Function793605E9', 'FunctionId'] },
+        { 'Fn::GetAtt': ['apiTest2FunctionB704A7AD', 'FunctionId'] },
       ],
     },
   });
@@ -48,16 +48,17 @@ test('appsync should configure pipeline when pipelineConfig has contents', () =>
 test('appsync should error when creating pipeline resolver with data source', () => {
   // WHEN
   const ds = api.addNoneDataSource('none');
-  const test1 = ds.createFunction({
+  const test1 = ds.createFunction('Test1Function', {
     name: 'test1',
   });
-  const test2 = ds.createFunction({
+  const test2 = ds.createFunction('Test2Function', {
     name: 'test2',
   });
 
   // THEN
   expect(() => {
-    ds.createResolver({
+    api.createResolver('TestTest2', {
+      dataSource: ds,
       typeName: 'test',
       fieldName: 'test2',
       pipelineConfig: [test1, test2],
@@ -83,7 +84,7 @@ test('appsync should configure resolver as unit when pipelineConfig is empty', (
 
 test('appsync should configure resolver as unit when pipelineConfig is empty array', () => {
   // WHEN
-  api.createResolver({
+  api.createResolver('TestTest2', {
     typeName: 'test',
     fieldName: 'test2',
     pipelineConfig: [],
@@ -237,4 +238,4 @@ test('log retention should not appear when no retention time is specified', () =
 
   // THEN
   Template.fromStack(stack).resourceCountIs('Custom::LogRetention', 0);
-});
+});
diff --git a/packages/@aws-cdk/aws-appsync/test/integ.api-import.ts b/packages/@aws-cdk/aws-appsync/test/integ.api-import.ts
@@ -43,14 +43,14 @@ const testTable = new db.Table(stack, 'TestTable', {
 
 const testDS = api.addDynamoDbDataSource('ds', testTable);
 
-testDS.createResolver({
+testDS.createResolver('QueryGetTests', {
   typeName: 'Query',
   fieldName: 'getTests',
   requestMappingTemplate: appsync.MappingTemplate.dynamoDbScanTable(),
   responseMappingTemplate: appsync.MappingTemplate.dynamoDbResultList(),
 });
 
-testDS.createResolver({
+testDS.createResolver('MutationAddTest', {
   typeName: 'Mutation',
   fieldName: 'addTest',
   requestMappingTemplate: appsync.MappingTemplate.dynamoDbPutItem(appsync.PrimaryKey.partition('id').auto(), appsync.Values.projecting('test')),
@@ -64,7 +64,7 @@ const api2 = appsync.GraphqlApi.fromGraphqlApiAttributes(stack, 'api2', {
 
 const none = api2.addNoneDataSource('none');
 
-const func = none.createFunction({
+const func = none.createFunction('PipelineFunction', {
   name: 'pipeline_function',
   requestMappingTemplate: appsync.MappingTemplate.fromString(JSON.stringify({
     version: '2017-02-28',
@@ -87,4 +87,4 @@ new appsync.Resolver(stack, 'pipeline_resolver', {
   })),
 });
 
-app.synth();
+app.synth();
diff --git a/packages/@aws-cdk/aws-appsync/test/integ.appsync-lambda.ts b/packages/@aws-cdk/aws-appsync/test/integ.appsync-lambda.ts
@@ -47,40 +47,40 @@ const requestPayload = (field: string, { withArgs = false, withSource = false })
 };
 const responseMappingTemplate = appsync.MappingTemplate.lambdaResult();
 
-lambdaDS.createResolver({
+lambdaDS.createResolver('QueryGetPost', {
   typeName: 'Query',
   fieldName: 'getPost',
   requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('getPost', { withArgs: true })),
   responseMappingTemplate,
 });
 
-lambdaDS.createResolver({
+lambdaDS.createResolver('QueryAllPosts', {
   typeName: 'Query',
   fieldName: 'allPosts',
   requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('allPosts', {})),
   responseMappingTemplate,
 });
 
-lambdaDS.createResolver({
+lambdaDS.createResolver('MutationAddPost', {
   typeName: 'Mutation',
   fieldName: 'addPost',
   requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('addPost', { withArgs: true })),
   responseMappingTemplate,
 });
 
-lambdaDS.createResolver({
+lambdaDS.createResolver('PostRelatedPosts', {
   typeName: 'Post',
   fieldName: 'relatedPosts',
   requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('relatedPosts', { withSource: true }), 'BatchInvoke'),
   responseMappingTemplate,
 });
 
-lambdaDS.createResolver({
+lambdaDS.createResolver('PostRelatedPostsMaxBatchSize', {
   typeName: 'Post',
   fieldName: 'relatedPostsMaxBatchSize',
   requestMappingTemplate: appsync.MappingTemplate.lambdaRequest(requestPayload('relatedPostsMaxBatchSize', { withSource: true }), 'BatchInvoke'),
   responseMappingTemplate,
   maxBatchSize: 2,
 });
 
-app.synth();
+app.synth();
diff --git a/...ges/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/aws-appsync-integ.assets.json b/...ges/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/aws-appsync-integ.assets.json
@@ -1,15 +1,15 @@
 {
-  "version": "21.0.0",
+  "version": "22.0.0",
   "files": {
-    "b0462850439179659920597f4327262b24073af4f4969622163b0a295fce1dda": {
+    "de7d932209c6d07c0ba0e631387676246a8182018b2b7423dc18b52baec3e984": {
       "source": {
         "path": "aws-appsync-integ.template.json",
         "packaging": "file"
       },
       "destinations": {
         "current_account-current_region": {
           "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
-          "objectKey": "b0462850439179659920597f4327262b24073af4f4969622163b0a295fce1dda.json",
+          "objectKey": "de7d932209c6d07c0ba0e631387676246a8182018b2b7423dc18b52baec3e984.json",
           "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
         }
       }

diff --git a/...s/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/aws-appsync-integ.template.json b/...s/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/aws-appsync-integ.template.json
@@ -121,7 +121,7 @@
     }
    }
   },
-  "ApitestDataSourceQuerygetTestsResolverA3BBB672": {
+  "ApiQueryGetTestsF8C40170": {
    "Type": "AWS::AppSync::Resolver",
    "Properties": {
     "ApiId": {
@@ -142,7 +142,7 @@
     "ApitestDataSource96AE54D5"
    ]
   },
-  "ApitestDataSourceMutationaddTestResolver36203D6B": {
+  "ApiMutationAddTestBF148084": {
    "Type": "AWS::AppSync::Resolver",
    "Properties": {
     "ApiId": {

diff --git a/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/cdk.out b/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/cdk.out
@@ -1 +1 @@
-{"version":"21.0.0"}
+{"version":"22.0.0"}
diff --git a/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/integ.json b/packages/@aws-cdk/aws-appsync/test/integ.auth-apikey.js.snapshot/integ.json
@@ -1,5 +1,5 @@
 {
-  "version": "21.0.0",
+  "version": "22.0.0",
   "testCases": {
     "integ.auth-apikey": {
       "stacks": [