Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(cdk): Add AppSync:Api_Key as hot swappable and fix a bug with AppSync.function #27559

Merged
merged 5 commits into from
Oct 24, 2023
Merged

fix(cdk): Add AppSync:Api_Key as hot swappable and fix a bug with AppSync.function #27559

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
89f4b52
fix(cdk): Add AppSync:Api_Key as hotswappable and fix the bug where A…
Amplifiyer Oct 16, 2023
b1167fc
update converting the s3 output to string in all situations
Amplifiyer Oct 16, 2023
c7336e6
Update packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts
Amplifiyer Oct 18, 2023
8794da2
Merge branch 'main' into amplify_hotswap_bugfixes
Amplifiyer Oct 24, 2023
c64e101
Merge branch 'main' into amplify_hotswap_bugfixes
mergify[bot] Oct 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/api/hotswap-deployments.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const RESOURCE_DETECTORS: { [key: string]: HotswapDetector } = {
'AWS::AppSync::Resolver': isHotswappableAppSyncChange,
'AWS::AppSync::FunctionConfiguration': isHotswappableAppSyncChange,
'AWS::AppSync::GraphQLSchema': isHotswappableAppSyncChange,
'AWS::AppSync::ApiKey': isHotswappableAppSyncChange,

'AWS::ECS::TaskDefinition': isHotswappableEcsServiceChange,
'AWS::CodeBuild::Project': isHotswappableCodeBuildProjectChange,
Expand Down
34 changes: 27 additions & 7 deletions packages/aws-cdk/lib/api/hotswap/appsync-mapping-templates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ export async function isHotswappableAppSyncChange(
const isResolver = change.newValue.Type === 'AWS::AppSync::Resolver';
const isFunction = change.newValue.Type === 'AWS::AppSync::FunctionConfiguration';
const isGraphQLSchema = change.newValue.Type === 'AWS::AppSync::GraphQLSchema';

if (!isResolver && !isFunction && !isGraphQLSchema) {
const isAPIKey = change.newValue.Type === 'AWS::AppSync::ApiKey';
if (!isResolver && !isFunction && !isGraphQLSchema && !isAPIKey) {
return [];
}

Expand All @@ -26,6 +26,7 @@ export async function isHotswappableAppSyncChange(
'CodeS3Location',
'Definition',
'DefinitionS3Location',
'Expires',
]);
classifiedChanges.reportNonHotswappablePropertyChanges(ret);

Expand Down Expand Up @@ -60,6 +61,7 @@ export async function isHotswappableAppSyncChange(
responseMappingTemplateS3Location: change.newValue.Properties?.ResponseMappingTemplateS3Location,
code: change.newValue.Properties?.Code,
codeS3Location: change.newValue.Properties?.CodeS3Location,
expires: change.newValue.Properties?.Expires,
};
const evaluatedResourceProperties = await evaluateCfnTemplate.evaluateCfnExpression(sdkProperties);
const sdkRequestObject = transformObjectKeys(evaluatedResourceProperties, lowerCaseFirstCharacter);
Expand All @@ -74,25 +76,34 @@ export async function isHotswappableAppSyncChange(
delete sdkRequestObject.responseMappingTemplateS3Location;
}
if (sdkRequestObject.definitionS3Location) {
sdkRequestObject.definition = await fetchFileFromS3(sdkRequestObject.definitionS3Location, sdk);
sdkRequestObject.definition = (await fetchFileFromS3(sdkRequestObject.definitionS3Location, sdk))?.toString('utf8');
delete sdkRequestObject.definitionS3Location;
}
if (sdkRequestObject.codeS3Location) {
sdkRequestObject.code = await fetchFileFromS3(sdkRequestObject.codeS3Location, sdk);
sdkRequestObject.code = (await fetchFileFromS3(sdkRequestObject.codeS3Location, sdk))?.toString('utf8');
delete sdkRequestObject.codeS3Location;
}

if (isResolver) {
await sdk.appsync().updateResolver(sdkRequestObject).promise();
} else if (isFunction) {

// Function version is only applicable when using VTL and mapping templates
// Runtime only applicable when using code (JS mapping templates)
if (sdkRequestObject.code) {
delete sdkRequestObject.functionVersion;
} else {
delete sdkRequestObject.runtime;
}

const { functions } = await sdk.appsync().listFunctions({ apiId: sdkRequestObject.apiId }).promise();
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(
() => sdk.appsync().updateFunction({ ...sdkRequestObject, functionId: functionId! }).promise(),
3,
5,
'ConcurrentModificationException');
} else {
} else if (isGraphQLSchema) {
let schemaCreationResponse: GetSchemaCreationStatusResponse = await sdk.appsync().startSchemaCreation(sdkRequestObject).promise();
while (schemaCreationResponse.status && ['PROCESSING', 'DELETING'].some(status => status === schemaCreationResponse.status)) {
await sleep(1000); // poll every second
Expand All @@ -104,6 +115,15 @@ export async function isHotswappableAppSyncChange(
if (schemaCreationResponse.status === 'FAILED') {
throw new Error(schemaCreationResponse.details);
}
} else { //isApiKey
if (!sdkRequestObject.id) {
// ApiKeyId is optional in CFN but required in SDK. Grab the KeyId from physicalArn if not available as part of CFN template
const arnParts = physicalName?.split('/');
if (arnParts && arnParts.length === 4) {
sdkRequestObject.id = arnParts[3];
}
Comment on lines +121 to +124
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.Arn.html here and do Arn.split instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems the Arn.split only considers one resourceType or resourceName but not "nested" resource such as in this case arn:aws:appsync:us-east-1:111111111111:apis/apiId/apikeys/api-key-id
Arn.split returns the entire apiId/apikeys/api-key-id as the resourceName which then requires a split anyways.

{
    service: 'appsync',
    resource: 'apis',
    partition: 'aws',
    region: 'us-east-1',
    account: '111111111111',
    resourceName: 'apiId/apikeys/api-key-id',
    sep: '/',
    arnFormat: 'arn:aws:service:region:account:resource/resourceName'
}

Moreover, Arn.split is in aws-cdk-lib and the change will require we bump the dependency from dev to runtime which might cause cyclic dependency issue? Let me know what you think

}
await sdk.appsync().updateApiKey(sdkRequestObject).promise();
}
},
});
Expand All @@ -124,7 +144,7 @@ async function simpleRetry(fn: () => Promise<any>, numOfRetries: number, errorCo
await fn();
} catch (error: any) {
if (error && error.code === errorCodeToRetry && numOfRetries > 0) {
await sleep(500); // wait half a second
await sleep(1000); // wait a whole second
await simpleRetry(fn, numOfRetries - 1, errorCodeToRetry);
} else {
throw error;
Expand Down
230 changes: 230 additions & 0 deletions packages/aws-cdk/test/api/hotswap/appsync-mapping-templates-hotswap-deployments.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,20 @@ import { HotswapMode } from '../../../lib/api/hotswap/common';
let hotswapMockSdkProvider: setup.HotswapMockSdkProvider;
let mockUpdateResolver: (params: AppSync.UpdateResolverRequest) => AppSync.UpdateResolverResponse;
let mockUpdateFunction: (params: AppSync.UpdateFunctionRequest) => AppSync.UpdateFunctionResponse;
let mockUpdateApiKey: (params: AppSync.UpdateApiKeyRequest) => AppSync.UpdateApiKeyResponse;
let mockStartSchemaCreation: (params: AppSync.StartSchemaCreationRequest) => AppSync.StartSchemaCreationResponse;
let mockS3GetObject: (params: S3.GetObjectRequest) => S3.GetObjectOutput;

beforeEach(() => {
hotswapMockSdkProvider = setup.setupHotswapTests();
mockUpdateResolver = jest.fn();
mockUpdateFunction = jest.fn();
mockUpdateApiKey = jest.fn();
mockStartSchemaCreation = jest.fn();
hotswapMockSdkProvider.stubAppSync({
updateResolver: mockUpdateResolver,
updateFunction: mockUpdateFunction,
updateApiKey: mockUpdateApiKey,
startSchemaCreation: mockStartSchemaCreation,
});

Expand Down Expand Up @@ -568,6 +571,127 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
});
});

test('calls the updateFunction() API with function version when it receives both function version and runtime with a mapping template in a Function', async () => {
// GIVEN
const mockListFunctions = jest.fn().mockReturnValue({ 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',
RequestMappingTemplate: '## original request template',
ResponseMappingTemplate: '## original response template',
},
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',
RequestMappingTemplate: '## original request template',
ResponseMappingTemplate: '## new response template',
},
Metadata: {
'aws:asset:path': 'new-path',
},
},
},
},
});

// WHEN
const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact);

// THEN
expect(deployStackResult).not.toBeUndefined();
expect(mockUpdateFunction).toHaveBeenCalledWith({
apiId: 'apiId',
dataSourceName: 'my-datasource',
functionId: 'functionId',
functionVersion: '2018-05-29',
name: 'my-function',
requestMappingTemplate: '## original request template',
responseMappingTemplate: '## new response template',
});
});

test('calls the updateFunction() API with runtime when it receives both function version and runtime with code in a Function', async () => {
// GIVEN
const mockListFunctions = jest.fn().mockReturnValue({ 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(mockUpdateFunction).toHaveBeenCalledWith({
apiId: 'apiId',
dataSourceName: 'my-datasource',
functionId: 'functionId',
runtime: 'APPSYNC_JS',
name: 'my-function',
code: 'new test code',
});
});

test('calls the updateFunction() API when it receives only a mapping template s3 location difference in a Function', async () => {
// GIVEN
mockS3GetObject = jest.fn().mockImplementation(async () => {
Expand Down Expand Up @@ -1032,4 +1156,110 @@ describe.each([HotswapMode.FALL_BACK, HotswapMode.HOTSWAP_ONLY])('%p mode', (hot
apiId: 'apiId',
});
});

test('calls the updateApiKey() API when it receives only a expires property difference in an AppSync ApiKey', async () => {
// GIVEN
setup.setCurrentCfnStackTemplate({
Resources: {
AppSyncApiKey: {
Type: 'AWS::AppSync::ApiKey',
Properties: {
ApiId: 'apiId',
Expires: 1000,
Id: 'key-id',
},
Metadata: {
'aws:asset:path': 'old-path',
},
},
},
});
setup.pushStackResourceSummaries(
setup.stackSummaryOf(
'AppSyncApiKey',
'AWS::AppSync::ApiKey',
'arn:aws:appsync:us-east-1:111111111111:apis/apiId/apikeys/api-key-id',
),
);
const cdkStackArtifact = setup.cdkStackArtifactOf({
template: {
Resources: {
AppSyncApiKey: {
Type: 'AWS::AppSync::ApiKey',
Properties: {
ApiId: 'apiId',
Expires: 1001,
Id: 'key-id',
},
Metadata: {
'aws:asset:path': 'new-path',
},
},
},
},
});

// WHEN
const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact);

// THEN
expect(deployStackResult).not.toBeUndefined();
expect(mockUpdateApiKey).toHaveBeenCalledWith({
apiId: 'apiId',
expires: 1001,
id: 'key-id',
});
});

test('calls the updateApiKey() API when it receives only a expires property difference and no api-key-id in an AppSync ApiKey', async () => {
// GIVEN
setup.setCurrentCfnStackTemplate({
Resources: {
AppSyncApiKey: {
Type: 'AWS::AppSync::ApiKey',
Properties: {
ApiId: 'apiId',
Expires: 1000,
},
Metadata: {
'aws:asset:path': 'old-path',
},
},
},
});
setup.pushStackResourceSummaries(
setup.stackSummaryOf(
'AppSyncApiKey',
'AWS::AppSync::ApiKey',
'arn:aws:appsync:us-east-1:111111111111:apis/apiId/apikeys/api-key-id',
),
);
const cdkStackArtifact = setup.cdkStackArtifactOf({
template: {
Resources: {
AppSyncApiKey: {
Type: 'AWS::AppSync::ApiKey',
Properties: {
ApiId: 'apiId',
Expires: 1001,
},
Metadata: {
'aws:asset:path': 'new-path',
},
},
},
},
});

// WHEN
const deployStackResult = await hotswapMockSdkProvider.tryHotswapDeployment(hotswapMode, cdkStackArtifact);

// THEN
expect(deployStackResult).not.toBeUndefined();
expect(mockUpdateApiKey).toHaveBeenCalledWith({
apiId: 'apiId',
expires: 1001,
id: 'api-key-id',
});
});
});
Loading