Skip to content

Commit

Permalink
fix(cli): hotswapping appsync functions fails when API does not retur…
Browse files Browse the repository at this point in the history
…n function on the first page (#31406)

### Issue # (if applicable)

aws-amplify/amplify-category-api#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 aws-amplify/amplify-category-api#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*
  • Loading branch information
dpilch committed Sep 17, 2024
1 parent f60d13b commit 0da4f43
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 2 deletions.
31 changes: 31 additions & 0 deletions packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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');
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
type Query {
listString: [String]
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>) {
const ret = new Array<string>();
for (const child of await fs.readdir(parent, { encoding: 'utf-8' })) {
Expand Down
19 changes: 17 additions & 2 deletions packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -155,3 +155,18 @@ async function simpleRetry(fn: () => Promise<any>, 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<FunctionConfiguration[]> {
const ret = new Array<FunctionConfiguration>();
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;
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down

0 comments on commit 0da4f43

Please sign in to comment.