feat(codedeploy) lambda application and deployment groups #1628

sam-goodwin · 2019-01-29T04:39:58Z

Fixes #1020

Pull Request Checklist

Testing
- Unit test added
- CLI change?: manually run integration tests and paste output as a PR comment
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
Title and Description
- Change type: title prefixed with fix, feat will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

… poll failure with test

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

eladb

looks great!

copy: @skinny85

packages/@aws-cdk/aws-codedeploy/README.md

packages/@aws-cdk/aws-codedeploy/lib/config.ts

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

eladb · 2019-01-29T04:51:55Z

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

+      props.postHook.grantInvoke(serviceRole);
+    }
+
+    (props.alias.node.findChild('Resource') as lambda.CfnAlias).options.updatePolicy = {


We are going to add a cfnresource to L2s so you'd be able to access it without an escape hatch. Feel free to do that now or raise an issue not to forget this escape hatch later.

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

packages/@aws-cdk/aws-codedeploy/test/lambda/integ.deployment-group.ts

skinny85

Generally looks fine, some devil is in the details.

In particular, it is my strong opinion that we should not export a concept of a Compute Platform to our customers. Since Server and Lambda are already separated by different classes, I don't see the point of having a property, nor a type, for that.

packages/@aws-cdk/aws-codedeploy/README.md

packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts

packages/@aws-cdk/aws-codedeploy/lib/server/application.ts

skinny85 · 2019-01-30T00:53:10Z

cc @yubangxi from the CodeDeploy team

sam-goodwin · 2019-01-30T08:28:08Z

packages/@aws-cdk/aws-lambda/lib/alias.ts

-    this.functionName = alias.ref;
-    this.functionArn = alias.aliasArn;
+    this.functionName = `${props.version.lambda.functionArn}:${props.aliasName}`;
+    this.functionArn = `${props.version.lambda.functionArn}:${props.aliasName}`;


Drawing attention to this. Is this a breaking change?

I had to do this or else we can't use an alias's metrics with a deployment group, since it creates a circular dependency:

Alias UpdatePolicy references the deployment group

Deployment group references the Alias's !Ref (via) the alarm and metric.

packages/@aws-cdk/aws-codedeploy/README.md

packages/@aws-cdk/aws-codedeploy/lib/config.ts

eladb · 2019-01-30T12:06:23Z

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

+    let serviceRole: iam.Role | undefined = props.role;
+    if (serviceRole) {
+      if (serviceRole.assumeRolePolicy) {
+        serviceRole.assumeRolePolicy.addStatement(new iam.PolicyStatement()


The "right" thing to do here is to add support in PolicyDocument to conditionally add a statement if a statement with the same SID was not already added and then use some UUID as a SID to avoid duplicates.

skinny85

Some changes to the existing classes that I don't understand + a few minor comments.

packages/@aws-cdk/aws-codedeploy/README.md

packages/@aws-cdk/aws-codedeploy/lib/lambda/application.ts

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

packages/@aws-cdk/aws-codedeploy/lib/server/application.ts

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

skinny85 · 2019-01-31T00:08:52Z

There are also a few places you use @param parent in JSDocs, but the actual name of the first parameter is now scope - probably an artifact of the old naming, but it should be correct in all new code.

skinny85

LGTM, some minor non-blocking comments, almost all of them documentation-related.

packages/@aws-cdk/aws-codedeploy/README.md

skinny85 · 2019-02-04T19:39:16Z

packages/@aws-cdk/aws-codedeploy/README.md

+
+### Lambda Deployment Groups
+
+To enable traffic shifting deployments for Lambda functions, CodeDeploy uses Lambda Aliases, which can balance incoming traffic between two different versions of your function. Before deployment, the alias sends 100% of invokes to the version used in production. When you publish a new version of the function to your stack, CodeDeploy will send a small percentage of traffic to the new version, monitor, and validate before shifting 100% of traffic to the new version.


Can you break these lines on full sentence boundaries? Makes the file read better in text mode, and is later easier to edit (the diffs are cleaner when you break on sentence boundaries).

skinny85 · 2019-02-04T19:40:31Z

packages/@aws-cdk/aws-codedeploy/README.md

+
+#### Pre and Post Hooks
+
+CodeDeploy allows you to run an arbitrary Lambda function before traffic shifting actually starts (PreTraffic Hook) and after it completes (PostTraffic Hook). With either hook, you have the opportunity to run logic that determines whether the deployment must succeed or fail. For example, with PreTraffic hook you could run integration tests against the newly created Lambda version (but not serving traffic). With PostTraffic hook, you could run end-to-end validation checks.


Same note here (break lines on sentence boundaries).

skinny85 · 2019-02-04T19:46:07Z

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

+   * on this deployment group resource.
+   * @param principal to grant permission to
+   */
+  public grantPutLifecycleEventHookExecutionStatus(principal?: iam.IPrincipal) {


Same here (return type).

skinny85 · 2019-02-04T19:47:47Z

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

+}
+
+/**
+ * Properties of a reference to a CodeDeploy EC2/on-premise Deployment Group.


skinny85 · 2019-02-04T19:48:01Z

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

+ * Properties of a reference to a CodeDeploy EC2/on-premise Deployment Group.
+ *
+ * @see ServerDeploymentGroup#import
+ * @see IServerDeploymentGroup#export


Nope & nope :)

skinny85 · 2019-02-04T19:48:21Z

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts

+  application: ILambdaApplication;
+
+  /**
+   * The physical, human-readable name of the CodeDeploy EC2/on-premise Deployment Group


Still not an EC2/on-premise Deployment Group :)

skinny85 · 2019-02-04T19:53:25Z

packages/@aws-cdk/aws-apigateway/test/test.lambda-api.ts

              },
-              "/invocations"
+              ":my-alias/invocations"


The fact that we're changing this worries me a bit as well... maybe consult with somebody that knows a lot about the API Gateway construct whether these changes are OK? I don't feel competent enough in that area to comment on it.

… add lambda application docs

Adds `LambdaApplication` and `LambdaDeploymentGroup` such that an `Alias` to a Lambda `Function` can be deployed via CodeDeploy; supports pre/post hook functions, traffic-shifting and alarm monitoring and rollback.

Sam Goodwin added 6 commits January 28, 2019 15:16

add Lambda CodeDeploy applications and deployment groups

f7ef936

Imporve tests and documentation

4424f97

Add integration test

a55702d

Refactor traffic shifting to deployment config. Add auto rollback and…

6254b9f

… poll failure with test

Replace managed policy with granular permissions

05a486d

Add Lambda details to README

9d02184

sam-goodwin added the @aws-cdk/aws-lambda Related to AWS Lambda label Jan 29, 2019

sam-goodwin requested review from RomainMuller, skinny85 and a team as code owners January 29, 2019 04:39

sam-goodwin commented Jan 29, 2019

View reviewed changes

packages/@aws-cdk/aws-codedeploy/lib/lambda/deployment-group.ts Show resolved Hide resolved

eladb reviewed Jan 29, 2019

View reviewed changes

skinny85 suggested changes Jan 29, 2019

View reviewed changes

Add comments back in and fix deployment option and type as per comments

b15bbbd

Sam Goodwin added 2 commits January 30, 2019 00:17

More updates

304c0b2

Fix up some unneccessary changes and remove comments - cleanups

c3a7fff

sam-goodwin commented Jan 30, 2019

View reviewed changes

Sam Goodwin added 2 commits January 30, 2019 01:14

Fix API gateway alias test

3798120

Add missing lambda handlers

01dbdb1

eladb approved these changes Jan 30, 2019

View reviewed changes

Sam Goodwin added 2 commits January 30, 2019 07:50

Address comments.

f17843f

Fix rename imports ...

094cebb

This was referenced Jan 30, 2019

update Function to support CodeDeploy deployments #1639

Closed

add an example of Lambda with blue/green deployments #1640

Closed

skinny85 reviewed Jan 31, 2019

View reviewed changes

sam-goodwin self-assigned this Feb 4, 2019

Sam Goodwin added 2 commits February 4, 2019 10:54

Updated as per review comments

6291fc4

@param parent -> @param scope

b62ec90

skinny85 approved these changes Feb 4, 2019

View reviewed changes

Sam Goodwin added 3 commits February 4, 2019 12:18

Merge master

8715f00

Update as per comments - add return types, improve wiki new lines and…

d3e4790

… add lambda application docs

s/EC2/Lambda/

50de88a

sam-goodwin merged commit 4c4b015 into master Feb 4, 2019

sam-goodwin deleted the samgood/blue-green branch February 4, 2019 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(codedeploy) lambda application and deployment groups #1628

feat(codedeploy) lambda application and deployment groups #1628

sam-goodwin commented Jan 29, 2019

eladb left a comment

eladb Jan 29, 2019

skinny85 left a comment

skinny85 commented Jan 30, 2019

sam-goodwin Jan 30, 2019

eladb Jan 30, 2019

skinny85 left a comment

skinny85 commented Jan 31, 2019

skinny85 left a comment

skinny85 Feb 4, 2019

skinny85 Feb 4, 2019

skinny85 Feb 4, 2019

skinny85 Feb 4, 2019

skinny85 Feb 4, 2019

skinny85 Feb 4, 2019

skinny85 Feb 4, 2019


		### Lambda Deployment Groups

		To enable traffic shifting deployments for Lambda functions, CodeDeploy uses Lambda Aliases, which can balance incoming traffic between two different versions of your function. Before deployment, the alias sends 100% of invokes to the version used in production. When you publish a new version of the function to your stack, CodeDeploy will send a small percentage of traffic to the new version, monitor, and validate before shifting 100% of traffic to the new version.


		#### Pre and Post Hooks

		CodeDeploy allows you to run an arbitrary Lambda function before traffic shifting actually starts (PreTraffic Hook) and after it completes (PostTraffic Hook). With either hook, you have the opportunity to run logic that determines whether the deployment must succeed or fail. For example, with PreTraffic hook you could run integration tests against the newly created Lambda version (but not serving traffic). With PostTraffic hook, you could run end-to-end validation checks.

feat(codedeploy) lambda application and deployment groups #1628

feat(codedeploy) lambda application and deployment groups #1628

Conversation

sam-goodwin commented Jan 29, 2019

Fixes #1020

Pull Request Checklist

eladb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 commented Jan 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skinny85 left a comment

Choose a reason for hiding this comment

skinny85 commented Jan 31, 2019

skinny85 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment