From 0da4f4347b0852b7eb7742504ae613a9c14b5958 Mon Sep 17 00:00:00 2001 From: Dane Pilcher Date: Tue, 17 Sep 2024 11:34:01 -0600 Subject: [PATCH] fix(cli): hotswapping appsync functions fails when API does not return function on the first page (#31406) ### Issue # (if applicable) https://github.com/aws-amplify/amplify-category-api/issues/2307 ### Reason for this change Hotswap for AppSync functions sometimes fails with `Deployment failed: MissingRequiredParameter: Missing required key 'functionId' in params`. This is because the SDK list function only retrieves the first page. In APIs where there are many functions it is possible for the hotswapped function to not be contained in the first page. This results in the previously mentioned error because the `functionId` is never retrieved. ### Description of changes List all AppSync functions for a given API by iterating through the pages with the `nextToken`. The change was modeled after a similar request [here](https://github.com/aws/aws-cdk/blob/1e203753519e10e19ef0db87e1382377b609bcaa/packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts#L23-L36). ### Description of how you validated changes * Unit tests * Integ tests * Manual testing in app from https://github.com/aws-amplify/amplify-category-api/issues/2307 ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../cli-integ/resources/cdk-apps/app/app.js | 31 +++++++ .../cdk-apps/app/appsync.hotswap.graphql | 3 + .../tests/cli-integ-tests/cli.integtest.ts | 31 +++++++ .../api/hotswap/appsync-mapping-templates.ts | 19 ++++- ...ping-templates-hotswap-deployments.test.ts | 83 +++++++++++++++++++ 5 files changed, 165 insertions(+), 2 deletions(-) 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..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 @@ -13,6 +13,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { 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 { @@ -28,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'); } @@ -637,6 +639,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 +704,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..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 @@ -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' })) { 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..91dcba4461f78 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); 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): 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; + }); +} 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' });