From 52a28ff25a8ac1b2d830505af212ba2c7c5759be Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Wed, 11 Sep 2024 08:12:23 -0600 Subject: [PATCH 1/8] fix: list all appsync functions by iterating through the paginated list --- .../api/hotswap/appsync-mapping-templates.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts index a5a2156409458..786d9ef4b68b7 100644 --- a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts +++ b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts @@ -1,4 +1,4 @@ -import { GetSchemaCreationStatusRequest, GetSchemaCreationStatusResponse } from 'aws-sdk/clients/appsync'; +import { GetSchemaCreationStatusRequest, GetSchemaCreationStatusResponse, ListFunctionsResponse, FunctionConfiguration } from 'aws-sdk/clients/appsync'; import { ChangeHotswapResult, classifyChanges, HotswappableChangeCandidate, lowerCaseFirstCharacter, transformObjectKeys } from './common'; import { ISDK } from '../aws-auth'; @@ -96,7 +96,7 @@ export async function isHotswappableAppSyncChange( delete sdkRequestObject.runtime; } - const { functions } = await sdk.appsync().listFunctions({ apiId: sdkRequestObject.apiId }).promise(); + const functions = await getAppSyncFunctions(sdk, sdkRequestObject.apiId!, undefined); const { functionId } = functions?.find(fn => fn.name === physicalName) ?? {}; // Updating multiple functions at the same time or along with graphql schema results in `ConcurrentModificationException` await simpleRetry( @@ -155,3 +155,18 @@ async function simpleRetry(fn: () => Promise, numOfRetries: number, errorCo async function sleep(ms: number) { return new Promise(ok => setTimeout(ok, ms)); } + +/** + * Get all functions for a given AppSync API by iterating through the paginated list of functions + */ +async function getAppSyncFunctions(sdk: ISDK, apiId: string, nextToken: string | undefined): Promise { + const ret = new Array(); + return sdk.appsync().listFunctions({ apiId, nextToken }).promise() + .then(async (listFunctionsResponse: ListFunctionsResponse) => { + ret.push(...(listFunctionsResponse.functions ?? [])); + if (listFunctionsResponse.nextToken) { + ret.push(...await getAppSyncFunctions(sdk, apiId, listFunctionsResponse.nextToken)); + } + return ret; + }); +} From 7c08cfce9d0afb7fc34bf08504834937c8983eb6 Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Wed, 11 Sep 2024 09:38:41 -0600 Subject: [PATCH 2/8] test: add unit test for list AppSync functions page iteration --- ...ping-templates-hotswap-deployments.test.ts | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts index ca45a75f031f1..700924c69f906 100644 --- a/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts @@ -871,6 +871,89 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot } }); + silentTest('calls the updateFunction() API with functionId when function is listed on second page', async () => { + // GIVEN + const mockListFunctions = jest + .fn() + .mockReturnValueOnce({ + functions: [{ name: 'other-function', functionId: 'other-functionId' }], + nextToken: 'nexttoken', + }) + .mockReturnValueOnce({ + functions: [{ name: 'my-function', functionId: 'functionId' }], + }); + hotswapMockSdkProvider.stubAppSync({ + listFunctions: mockListFunctions, + updateFunction: mockUpdateFunction, + }); + + setup.setCurrentCfnStackTemplate({ + Resources: { + AppSyncFunction: { + Type: 'AWS::AppSync::FunctionConfiguration', + Properties: { + Name: 'my-function', + ApiId: 'apiId', + DataSourceName: 'my-datasource', + FunctionVersion: '2018-05-29', + Runtime: 'APPSYNC_JS', + Code: 'old test code', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + AppSyncFunction: { + Type: 'AWS::AppSync::FunctionConfiguration', + Properties: { + Name: 'my-function', + ApiId: 'apiId', + DataSourceName: 'my-datasource', + FunctionVersion: '2018-05-29', + Runtime: 'APPSYNC_JS', + Code: 'new test code', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + const deployStackResult = + await hotswapMockSdkProvider.tryHotswapDeployment( + hotswapMode, + cdkStackArtifact, + ); + + // THEN + expect(deployStackResult).not.toBeUndefined(); + expect(mockListFunctions).toHaveBeenCalledTimes(2); + expect(mockListFunctions).toHaveBeenCalledWith({ + apiId: 'apiId', + nextToken: undefined, + }); + expect(mockListFunctions).toHaveBeenCalledWith({ + apiId: 'apiId', + nextToken: 'nexttoken', + }); + expect(mockUpdateFunction).toHaveBeenCalledWith({ + apiId: 'apiId', + dataSourceName: 'my-datasource', + functionId: 'functionId', + runtime: 'APPSYNC_JS', + name: 'my-function', + code: 'new test code', + }); + }); + silentTest('calls the startSchemaCreation() API when it receives only a definition difference in a graphql schema', async () => { // GIVEN mockStartSchemaCreation = jest.fn().mockReturnValueOnce({ status: 'SUCCESS' }); From f9e5da3654c6356dba084fae82fe708ad39543b3 Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Wed, 11 Sep 2024 13:51:35 -0600 Subject: [PATCH 3/8] test: add integ tests for appsync many function hotswap --- .../cli-integ/resources/cdk-apps/app/app.js | 29 +++++++++++++++++ .../cdk-apps/app/appsync.hotswap.graphql | 3 ++ .../tests/cli-integ-tests/cli.integtest.ts | 31 +++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/appsync.hotswap.graphql diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index d094055795e27..f452a810cbcd4 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -637,6 +637,34 @@ class BuiltinLambdaStack extends cdk.Stack { } } +class AppSyncHotswapStack extends cdk.Stack { + constructor(parent, id, props) { + super(parent, id, props); + + const api = new appsync.GraphqlApi(this, "Api", { + name: "appsync-hotswap", + definition: appsync.Definition.fromFile(path.join(__dirname, 'appsync.hotswap.graphql')), + authorizationConfig: { + defaultAuthorization: { + authorizationType: appsync.AuthorizationType.IAM, + }, + }, + }); + + const noneDataSource = api.addNoneDataSource("none"); + // create 50 appsync functions to hotswap + for (const i of Array(50).keys()) { + const appsyncFunction = new appsync.AppsyncFunction(this, `Function${i}`, { + name: `appsync_function${i}`, + api, + dataSource: noneDataSource, + requestMappingTemplate: appsync.MappingTemplate.fromString(process.env.DYNAMIC_APPSYNC_PROPERTY_VALUE ?? "$util.toJson({})"), + responseMappingTemplate: appsync.MappingTemplate.fromString('$util.toJson({})'), + }); + } + } +} + const app = new cdk.App({ context: { '@aws-cdk/core:assetHashSalt': process.env.CODEBUILD_BUILD_ID, // Force all assets to be unique, but consistent in one build @@ -674,6 +702,7 @@ switch (stackSet) { new LambdaStack(app, `${stackPrefix}-lambda`); new LambdaHotswapStack(app, `${stackPrefix}-lambda-hotswap`); new EcsHotswapStack(app, `${stackPrefix}-ecs-hotswap`); + new AppSyncHotswapStack(app, `${stackPrefix}-appsync-hotswap`); new DockerStack(app, `${stackPrefix}-docker`); new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/appsync.hotswap.graphql b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/appsync.hotswap.graphql new file mode 100644 index 0000000000000..808e3421471ab --- /dev/null +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/appsync.hotswap.graphql @@ -0,0 +1,3 @@ +type Query { + listString: [String] +} \ No newline at end of file diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 8ebfcc0752716..d24958b4a6e87 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -2176,6 +2176,37 @@ integTest( }), ); +integTest('hotswap deployment supports AppSync APIs with many functions', + withDefaultFixture(async (fixture) => { + // GIVEN + const stackArn = await fixture.cdkDeploy('appsync-hotswap', { + captureStderr: false, + }); + + // WHEN + const deployOutput = await fixture.cdkDeploy('appsync-hotswap', { + options: ['--hotswap'], + captureStderr: true, + onlyStderr: true, + modEnv: { + DYNAMIC_APPSYNC_PROPERTY_VALUE: '$util.qr($ctx.stash.put("newTemplate", []))\n$util.toJson({})', + }, + }); + + const response = await fixture.aws.cloudFormation.send( + new DescribeStacksCommand({ + StackName: stackArn, + }), + ); + + expect(response.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE'); + // assert all 50 functions were hotswapped + for (const i of Array(50).keys()) { + expect(deployOutput).toContain(`AWS::AppSync::FunctionConfiguration appsync_function${i} hotswapped!`); + } + }), +); + async function listChildren(parent: string, pred: (x: string) => Promise) { const ret = new Array(); for (const child of await fs.readdir(parent, { encoding: 'utf-8' })) { From 100b6d8907706926184393048bcf962f5281e4ac Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Thu, 12 Sep 2024 11:20:53 -0600 Subject: [PATCH 4/8] fix: remove unnecessary bang --- packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts index 786d9ef4b68b7..8359da774368e 100644 --- a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts +++ b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts @@ -96,7 +96,7 @@ export async function isHotswappableAppSyncChange( delete sdkRequestObject.runtime; } - const functions = await getAppSyncFunctions(sdk, sdkRequestObject.apiId!, undefined); + const functions = await getAppSyncFunctions(sdk, sdkRequestObject.apiId, undefined); const { functionId } = functions?.find(fn => fn.name === physicalName) ?? {}; // Updating multiple functions at the same time or along with graphql schema results in `ConcurrentModificationException` await simpleRetry( From 94f0d88e6c7573dc1c9c7692cc0c8801e7435b6b Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Fri, 13 Sep 2024 11:45:01 -0600 Subject: [PATCH 5/8] test: add missing import statement --- .../@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index f452a810cbcd4..b5a25d27f7b7f 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -10,6 +10,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { var iam = require('@aws-cdk/aws-iam'); var sns = require('@aws-cdk/aws-sns'); var sqs = require('@aws-cdk/aws-sqs'); + var appsync = require('@aws-cdk/aws-appsync'); var lambda = require('@aws-cdk/aws-lambda'); var sso = require('@aws-cdk/aws-sso'); var docker = require('@aws-cdk/aws-ecr-assets'); From 4630ea3fcf33efd71362d6b010f508a7d873e079 Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Fri, 13 Sep 2024 12:16:38 -0600 Subject: [PATCH 6/8] test: fix import statement --- .../@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index b5a25d27f7b7f..b728d4849f44d 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -10,10 +10,10 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { var iam = require('@aws-cdk/aws-iam'); var sns = require('@aws-cdk/aws-sns'); var sqs = require('@aws-cdk/aws-sqs'); - var appsync = require('@aws-cdk/aws-appsync'); var lambda = require('@aws-cdk/aws-lambda'); var sso = require('@aws-cdk/aws-sso'); var docker = require('@aws-cdk/aws-ecr-assets'); + var appsync = require('@aws-cdk/aws-appsync'); } else { var cdk = require('aws-cdk-lib'); var { @@ -29,6 +29,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { aws_sqs: sqs, aws_lambda: lambda, aws_ecr_assets: docker, + aws_appsync: appsync, Stack } = require('aws-cdk-lib'); } From 36b02367dda5b8b36d27fc7bd905dd860927fdac Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Fri, 13 Sep 2024 13:29:20 -0600 Subject: [PATCH 7/8] test: fix assertion string --- .../cli-integ/tests/cli-integ-tests/cli.integtest.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index d24958b4a6e87..4f62c15d62482 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -2202,7 +2202,7 @@ integTest('hotswap deployment supports AppSync APIs with many functions', expect(response.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE'); // assert all 50 functions were hotswapped for (const i of Array(50).keys()) { - expect(deployOutput).toContain(`AWS::AppSync::FunctionConfiguration appsync_function${i} hotswapped!`); + expect(deployOutput).toContain(`AWS::AppSync::FunctionConfiguration 'appsync_function${i}' hotswapped!`); } }), ); From bba2dcc447ef394bc5ba1061e29702eacc8070fa Mon Sep 17 00:00:00 2001 From: Kaizen Conroy <36202692+kaizencc@users.noreply.github.com> Date: Tue, 17 Sep 2024 12:55:27 -0400 Subject: [PATCH 8/8] Apply suggestions from code review --- packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts index 8359da774368e..91dcba4461f78 100644 --- a/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts +++ b/packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts @@ -96,7 +96,7 @@ export async function isHotswappableAppSyncChange( delete sdkRequestObject.runtime; } - const functions = await getAppSyncFunctions(sdk, sdkRequestObject.apiId, undefined); + const functions = await getAppSyncFunctions(sdk, sdkRequestObject.apiId); const { functionId } = functions?.find(fn => fn.name === physicalName) ?? {}; // Updating multiple functions at the same time or along with graphql schema results in `ConcurrentModificationException` await simpleRetry( @@ -159,7 +159,7 @@ async function sleep(ms: number) { /** * Get all functions for a given AppSync API by iterating through the paginated list of functions */ -async function getAppSyncFunctions(sdk: ISDK, apiId: string, nextToken: string | undefined): Promise { +async function getAppSyncFunctions(sdk: ISDK, apiId: string, nextToken?: string): Promise { const ret = new Array(); return sdk.appsync().listFunctions({ apiId, nextToken }).promise() .then(async (listFunctionsResponse: ListFunctionsResponse) => {