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

fix: race condition provisioning default apigw responses #731

Merged
merged 1 commit into from
Aug 17, 2022
+35 −59
Merged

fix: race condition provisioning default apigw responses #731

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
94 changes: 35 additions & 59 deletions ...ategory-api/src/provider-utils/awscloudformation/cdk-stack-builder/apigw-stack-builder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/* eslint-disable jsdoc/require-param-description */
/* eslint-disable no-new */
/* eslint-disable no-use-before-define */
/* eslint-disable @typescript-eslint/explicit-function-return-type */
/* eslint-disable no-underscore-dangle */
import * as apigw from '@aws-cdk/aws-apigateway';
import * as iam from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
Expand All @@ -6,7 +11,9 @@ import { $TSObject, JSONUtilities } from 'amplify-cli-core';
import _ from 'lodash';
import { v4 as uuid } from 'uuid';
import { ADMIN_QUERIES_NAME } from '../../../category-constants';
import { AmplifyApigwResourceTemplate, ApigwInputs, ApigwPathPolicy, Path, PermissionSetting } from './types';
import {
AmplifyApigwResourceTemplate, ApigwInputs, ApigwPathPolicy, Path, PermissionSetting,
} from './types';

const CFN_TEMPLATE_FORMAT_VERSION = '2010-09-09';
const ROOT_CFN_DESCRIPTION = 'API Gateway Resource for AWS Amplify CLI';
Expand Down Expand Up @@ -34,58 +41,33 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
this.templateOptions.description = ROOT_CFN_DESCRIPTION;
}

/**
*
* @param props
* @param logicalId
*/
addCfnOutput(props: cdk.CfnOutputProps, logicalId: string): void {
this.validateLogicalId(logicalId);
new cdk.CfnOutput(this, logicalId, props);
}

/**
*
* @param props
* @param logicalId
*/
addCfnMapping(props: cdk.CfnMappingProps, logicalId: string): void {
this.validateLogicalId(logicalId);
new cdk.CfnMapping(this, logicalId, props);
}

/**
*
* @param props
* @param logicalId
*/
addCfnCondition(props: cdk.CfnConditionProps, logicalId: string): void {
this.validateLogicalId(logicalId);
new cdk.CfnCondition(this, logicalId, props);
}

/**
*
* @param props
* @param logicalId
*/
addCfnResource(props: cdk.CfnResourceProps, logicalId: string): void {
this.validateLogicalId(logicalId);
new cdk.CfnResource(this, logicalId, props);
}

/**
*
* @param props
* @param logicalId
*/
addCfnLambdaPermissionResource(props: lambda.CfnPermissionProps, logicalId: string): void {
this.validateLogicalId(logicalId);
new lambda.CfnPermission(this, logicalId, props);
}

// eslint-disable-next-line jsdoc/require-description
/**
*
* @param props
* @param logicalId
* @param value optional value which will be stored in parameters.json
Expand All @@ -109,24 +91,21 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
this._seenLogicalIds.add(logicalId);
}

// eslint-disable-next-line class-methods-use-this
private _craftPolicyDocument(apiResourceName: string, pathName: string, supportedOperations: string[]) {
const paths = [pathName, appendToUrlPath(pathName, '*')];
const resources = paths.flatMap(path =>
supportedOperations.map(op =>
cdk.Fn.join('', [
'arn:aws:execute-api:',
cdk.Fn.ref('AWS::Region'),
':',
cdk.Fn.ref('AWS::AccountId'),
':',
cdk.Fn.ref(apiResourceName),
'/',
cdk.Fn.conditionIf('ShouldNotCreateEnvResources', 'Prod', cdk.Fn.ref('env')).toString(),
op,
path,
]),
),
);
const resources = paths.flatMap((path) => supportedOperations.map((op) => cdk.Fn.join('', [
'arn:aws:execute-api:',
cdk.Fn.ref('AWS::Region'),
':',
cdk.Fn.ref('AWS::AccountId'),
':',
cdk.Fn.ref(apiResourceName),
'/',
cdk.Fn.conditionIf('ShouldNotCreateEnvResources', 'Prod', cdk.Fn.ref('env')).toString(),
op,
path,
])));

return new iam.PolicyDocument({
statements: [
Expand Down Expand Up @@ -158,9 +137,7 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
_.set(this.policies, [pathName, 'groups', groupName], iamPolicy);
}

renderCloudFormationTemplate = (): string => {
return JSONUtilities.stringify(this._toCloudFormation());
};
renderCloudFormationTemplate = (): string => JSONUtilities.stringify(this._toCloudFormation());

generateAdminQueriesStack = (resourceName: string, authResourceName: string) => {
this._constructCfnPaths(resourceName);
Expand Down Expand Up @@ -266,26 +243,26 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
},
},
});
new apigw.CfnGatewayResponse(this, `${resourceName}Default4XXResponse`, {
const default4xx = new apigw.CfnGatewayResponse(this, `${resourceName}Default4XXResponse`, {
responseType: 'DEFAULT_4XX',
restApiId: cdk.Fn.ref(resourceName),
responseParameters: defaultCorsGatewayResponseParams,
});
new apigw.CfnGatewayResponse(this, `${resourceName}Default5XXResponse`, {
const default5xx = new apigw.CfnGatewayResponse(this, `${resourceName}Default5XXResponse`, {
responseType: 'DEFAULT_5XX',
restApiId: cdk.Fn.ref(resourceName),
responseParameters: defaultCorsGatewayResponseParams,
});

this._setDeploymentResource(resourceName);
this._setDeploymentResource(resourceName, [default4xx, default5xx]);
};

private _constructCfnPaths(resourceName: string) {
const addedFunctionPermissions = new Set();
for (const [pathName, path] of Object.entries(this._props.paths)) {
let lambdaPermissionLogicalId: string;
if (resourceName === ADMIN_QUERIES_NAME) {
this.paths[`/{proxy+}`] = getAdminQueriesPathObject(path.lambdaFunction);
this.paths['/{proxy+}'] = getAdminQueriesPathObject(path.lambdaFunction);
lambdaPermissionLogicalId = `${ADMIN_QUERIES_NAME}APIGWPolicyForLambda`;
} else {
this.paths[pathName] = createPathObject(path);
Expand Down Expand Up @@ -316,26 +293,25 @@ export class AmplifyApigwResourceStack extends cdk.Stack implements AmplifyApigw
}
}

private _setDeploymentResource = (resourceName: string) => {
private _setDeploymentResource = (apiName: string, dependencies: cdk.CfnResource[] = []) => {
const [shortId] = uuid().split('-');
this.deploymentResource = new apigw.CfnDeployment(this, `DeploymentAPIGW${resourceName}${shortId}`, {
this.deploymentResource = new apigw.CfnDeployment(this, `DeploymentAPIGW${apiName}${shortId}`, {
description: 'The Development stage deployment of your API.',
stageName: cdk.Fn.conditionIf('ShouldNotCreateEnvResources', 'Prod', cdk.Fn.ref('env')).toString(),
restApiId: cdk.Fn.ref(resourceName),
restApiId: cdk.Fn.ref(apiName),
});
dependencies.forEach((dep) => this.deploymentResource.addDependsOn(dep));
};
}

const appendToUrlPath = (path: string, postfix: string) => {
return path.charAt(path.length - 1) === '/' ? `${path}${postfix}` : `${path}/${postfix}`;
};
const appendToUrlPath = (path: string, postfix: string) => (path.charAt(path.length - 1) === '/' ? `${path}${postfix}` : `${path}/${postfix}`);

const getAdminQueriesPathObject = (lambdaFunctionName: string) => ({
options: {
consumes: ['application/json'],
produces: ['application/json'],
responses: {
'200': {
200: {
description: '200 response',
schema: {
$ref: '#/definitions/Empty',
Expand Down Expand Up @@ -418,7 +394,7 @@ const createPathObject = (path: Path) => {
consumes: ['application/json'],
produces: ['application/json'],
responses: {
'200': response200,
200: response200,
},
'x-amazon-apigateway-integration': {
responses: {
Expand All @@ -445,7 +421,7 @@ const createPathObject = (path: Path) => {
},
],
responses: {
'200': {
200: {
description: '200 response',
schema: {
$ref: '#/definitions/ResponseSchema',
Expand Down