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.

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

Setup APIGW CloudWatch role via custom resource #6531

Merged
merged 14 commits into from
Aug 14, 2019
7 changes: 5 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ before_script:
# More info on below line: https://www.davidpashley.com/articles/writing-robust-shell-scripts/#idm5413512
- set -e

script:
- npm test -- -b # Bail after first fail

# Ensure to fail build if deploy fails, Travis doesn't ensure that: https://github.com/travis-ci/travis-ci/issues/921
before_deploy:
# Remove eventual old npm logs
Expand All @@ -65,15 +68,15 @@ jobs:
script:
- npm run prettier-check-updated
- npm run lint-updated
- npm test
- npm test -- -b

# master branch and version tags
- name: 'Lint, Unit Tests - Linux - Node.js v12'
if: type != pull_request
node_js: 12
script:
- npm run lint
- npm test
- npm test -- -b

- name: 'Unit Tests - Windows - Node.js v12'
os: windows
Expand Down
3 changes: 3 additions & 0 deletions lib/plugins/aws/customResources/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
parserOptions: { ecmaVersion: 2017 },
};
13 changes: 11 additions & 2 deletions lib/plugins/aws/customResources/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function copyCustomResources(srcDirPath, destDirPath) {
}

function installDependencies(dirPath) {
return childProcess.execAsync(`npm install --prefix ${dirPath}`, { stdio: 'ignore' });
return childProcess.execAsync(`npm install --prefix ${dirPath}`);
}

function addCustomResourceToService(resourceName, iamRoleStatements) {
Expand Down Expand Up @@ -47,9 +47,18 @@ function addCustomResourceToService(resourceName, iamRoleStatements) {
FunctionName = `${funcPrefix}-${this.provider.naming.getCustomResourceEventBridgeHandlerFunctionName()}`;
Handler = 'eventBridge/handler.handler';
customResourceFunctionLogicalId = this.provider.naming.getCustomResourceEventBridgeHandlerFunctionLogicalId();
} else if (resourceName === 'apiGatewayCloudWatchRole') {
FunctionName = `${funcPrefix}-${this.provider.naming.getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionName()}`;
Handler = 'apiGatewayCloudWatchRole/handler.handler';
customResourceFunctionLogicalId = this.provider.naming.getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionLogicalId();
} else {
return BbPromise.reject(`No implementation found for Custom Resource "${resourceName}"`);
}
if (FunctionName.length > 64) {
return BbPromise.reject(
new Error(`Resolved custom resource function name '${FunctionName}' is too long`)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this happen if the service + stage name combo is too long? Is there anything we could do to help the user, or are they stuck?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would this happen if the service + stage name combo is too long?

I believe so.

This change is purely about exposing that error earlier than it happens now (during CF deployment).

I plan to refactor a custom resources handling, and with that I'll ensure we're more reliable here, but I don't want to focus on that in context of that PR

);
}

// TODO: check every once in a while if external packages are still necessary
this.serverless.cli.log('Installing dependencies for custom CloudFormation resources...');
Expand Down Expand Up @@ -127,7 +136,7 @@ function addCustomResourceToService(resourceName, iamRoleStatements) {
'Fn::GetAtt': [customResourcesRoleLogicalId, 'Arn'],
},
Runtime: 'nodejs10.x',
Timeout: 6,
Timeout: 180,
},
DependsOn: [customResourcesRoleLogicalId],
};
Expand Down
6 changes: 3 additions & 3 deletions lib/plugins/aws/customResources/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ describe('#addCustomResourceToService()', () => {
'Fn::GetAtt': ['IamRoleCustomResourcesLambdaExecution', 'Arn'],
},
Runtime: 'nodejs10.x',
Timeout: 6,
Timeout: 180,
},
DependsOn: ['IamRoleCustomResourcesLambdaExecution'],
});
Expand All @@ -149,7 +149,7 @@ describe('#addCustomResourceToService()', () => {
'Fn::GetAtt': ['IamRoleCustomResourcesLambdaExecution', 'Arn'],
},
Runtime: 'nodejs10.x',
Timeout: 6,
Timeout: 180,
},
DependsOn: ['IamRoleCustomResourcesLambdaExecution'],
});
Expand All @@ -168,7 +168,7 @@ describe('#addCustomResourceToService()', () => {
'Fn::GetAtt': ['IamRoleCustomResourcesLambdaExecution', 'Arn'],
},
Runtime: 'nodejs10.x',
Timeout: 6,
Timeout: 180,
},
DependsOn: ['IamRoleCustomResourcesLambdaExecution'],
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
'use strict';

const ApiGateway = require('aws-sdk/clients/apigateway');
const Iam = require('aws-sdk/clients/iam');
const { getEnvironment, handlerWrapper, wait } = require('../utils');

function handler(event, context) {
if (event.RequestType === 'Create') {
return create(event, context);
} else if (event.RequestType === 'Update') {
return update(event, context);
} else if (event.RequestType === 'Delete') {
return remove(event, context);
}
throw new Error(`Unhandled RequestType ${event.RequestType}`);
}

async function create(event, context) {
const { AccountId: accountId } = getEnvironment(context);

const apiGatewayPushToCloudWatchLogsPolicyArn =
'arn:aws:iam::aws:policy/service-role/AmazonAPIGatewayPushToCloudWatchLogs';
const roleArn = `arn:aws:iam::${accountId}:role/serverlessApiGatewayCloudWatchRole`;

const apiGateway = new ApiGateway();

const assignedRoleArn = (await apiGateway.getAccount().promise()).cloudwatchRoleArn;
const roleName = roleArn.slice(roleArn.lastIndexOf('/') + 1);

const iam = new Iam();

const attachedPolicies = await (async () => {
try {
return (await iam.listAttachedRolePolicies({ RoleName: roleName }).promise())
.AttachedPolicies;
} catch (error) {
if (error.code === 'NoSuchEntity') {
// Role doesn't exist yet, create;
await iam
.createRole({
AssumeRolePolicyDocument: JSON.stringify({
Version: '2012-10-17',
Statement: [
{
Effect: 'Allow',
Principal: {
Service: ['apigateway.amazonaws.com'],
},
Action: ['sts:AssumeRole'],
},
],
}),
Path: '/',
RoleName: roleName,
})
.promise();
return [];
}
throw error;
}
})();

if (
!attachedPolicies.some(policy => policy.PolicyArn === apiGatewayPushToCloudWatchLogsPolicyArn)
) {
await iam
.attachRolePolicy({
PolicyArn: apiGatewayPushToCloudWatchLogsPolicyArn,
RoleName: roleName,
})
.promise();
}

if (assignedRoleArn === roleArn) return null;

const updateAccount = async (counter = 1) => {
try {
await apiGateway
.updateAccount({
patchOperations: [
{
op: 'replace',
path: '/cloudwatchRoleArn',
value: roleArn,
},
],
})
.promise();
} catch (error) {
if (counter < 10) {
// Observed fails with errors marked as non-retryable. Still they're outcome of
// temporary state where just created AWS role is not being ready for use (yet)
await wait(10000);
return updateAccount(++counter);
}
throw error;
}
return null;
};

return updateAccount();
}

function update() {
// No actions
}

function remove() {
// No actions
}

module.exports = {
handler: handlerWrapper(handler, 'CustomResouceApiGatewayAccountCloudWatchRole'),
medikoo marked this conversation as resolved.
Show resolved Hide resolved
};
5 changes: 5 additions & 0 deletions lib/plugins/aws/customResources/resources/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,15 @@ function handlerWrapper(handler, PhysicalResourceId) {
};
}

function wait(ms) {
return new Promise(resolve => setTimeout(resolve, ms));
}

module.exports = {
logger,
response,
getEnvironment,
getLambdaArn,
handlerWrapper,
wait,
};
20 changes: 14 additions & 6 deletions lib/plugins/aws/lib/naming.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,12 +297,6 @@ module.exports = {
getApiGatewayLogGroupLogicalId() {
return 'ApiGatewayLogGroup';
},
getApiGatewayLogsRoleLogicalId() {
return 'IamRoleApiGatewayLogs';
},
getApiGatewayAccountLogicalId() {
return 'ApiGatewayAccount';
},

// S3
getDeploymentBucketLogicalId() {
Expand Down Expand Up @@ -504,4 +498,18 @@ module.exports = {
getCustomResourceEventBridgeResourceLogicalId(functionName, idx) {
return `${this.getNormalizedFunctionName(functionName)}CustomEventBridge${idx}`;
},
// API Gateway Account Logs Write Role
getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionName() {
return 'custom-resource-apigw-cw-role';
},
getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionLogicalId() {
return this.getLambdaLogicalId(
`${this.getNormalizedFunctionName(
this.getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionName()
)}`
);
},
getCustomResourceApiGatewayAccountCloudWatchRoleResourceLogicalId() {
return 'CustomApiGatewayAccountCloudWatchRole';
},
};
36 changes: 24 additions & 12 deletions lib/plugins/aws/lib/naming.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -468,18 +468,6 @@ describe('#naming()', () => {
});
});

describe('#getApiGatewayLogsRoleLogicalId()', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want tests for the new naming functions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

it('should return the API Gateway logs IAM role logical id', () => {
expect(sdk.naming.getApiGatewayLogsRoleLogicalId()).to.equal('IamRoleApiGatewayLogs');
});
});

describe('#getApiGatewayAccountLogicalId()', () => {
it('should return the API Gateway account logical id', () => {
expect(sdk.naming.getApiGatewayAccountLogicalId()).to.equal('ApiGatewayAccount');
});
});

describe('#getDeploymentBucketLogicalId()', () => {
it('should return "ServerlessDeploymentBucket"', () => {
expect(sdk.naming.getDeploymentBucketLogicalId()).to.equal('ServerlessDeploymentBucket');
Expand Down Expand Up @@ -811,4 +799,28 @@ describe('#naming()', () => {
).to.equal('MyDashfunctionCustomEventBridge1');
});
});

describe('#getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionName()', () => {
it('should return the nane of the APIGW Account CloudWatch role custom resouce handler function', () => {
expect(
sdk.naming.getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionName()
).to.equal('custom-resource-apigw-cw-role');
});
});

describe('#getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionLogicalId()', () => {
it('should return the logical id of the APIGW Account CloudWatch role custom resouce handler function', () => {
expect(
sdk.naming.getCustomResourceApiGatewayAccountCloudWatchRoleHandlerFunctionLogicalId()
).to.equal('CustomDashresourceDashapigwDashcwDashroleLambdaFunction');
});
});

describe('#getCustomResourceApiGatewayAccountCloudWatchRoleResourceLogicalId()', () => {
it('should return the logical id of the APIGW Account CloudWatch role custom resouce', () => {
expect(
sdk.naming.getCustomResourceApiGatewayAccountCloudWatchRoleResourceLogicalId()
).to.equal('CustomApiGatewayAccountCloudWatchRole');
});
});
});
Loading