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

feat(app-delivery): CI/CD for CDK Stacks #1022

Merged
merged 7 commits into from
Nov 6, 2018
Merged

feat(app-delivery): CI/CD for CDK Stacks #1022

merged 7 commits into from
Nov 6, 2018

Conversation

RomainMuller
Copy link
Contributor

ℹ️ Building & synthesizing the CDK app is left to the user of this construct library, which assumes the user can provide a CodePipeline artifact containing a synthesized app (probably eventually a Cloud Assembly -- see #956).

ℹ️ As much as I don't hate the name, I'm not married to it. If you have a better idea... I'm a taker.

This is a very simple construct that allows one to add actions to a
CodePipeline to deploy a CDK stack.

Currently comes with some pretty strong limitations (deployed stack &
code pipeline must reside in the same account & region, assets are not
supported).

It does however lay the foundation for the API, as the external shape of
the feature shouldn't change (or not much) when amending to support
cross-account deployment and assets.

@RomainMuller
feat(cicd): CI/CD for CDK Stacks
c2d6100 
This is a very simple construct that allows one to add actions to a
CodePipeline to deploy a CDK stack.

Currently comes with some pretty strong limitations (deployed stack &
code pipeline must reside in the same account & region, assets are not
supported).

It does however lay the foundation for the API, as the external shape of
the feature shouldn't change (or not much) when amending to support
cross-account deployment and assets.
eladb
eladb approved these changes Oct 28, 2018
View reviewed changes
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
skinny85
skinny85 reviewed Oct 29, 2018
View reviewed changes
packages/@aws-cdk/cicd/README.md Outdated
# The output artifact is all the files in the `dist` directory
base-directory: dist
files: '**/*'
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this experience. Project in codebuild can also use an inline build spec. Should we put this logic in some reusable utility methods that encapsulate this logic for the languages the CDK supports? A separate method for TypeScript / JavaScript, Java, .NET, etc.

These can be 'fallback' instructions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I guess I can provide some canned buildspecs. I'll have a look into this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking into the canned buildspecs, I realized that it gets awkward once the build image needs to support the buildspec, especially when the standard CodeBuild images don't ... For example, if I want to make a buildspec for a Java CDK project, I'd need to be able to provide an image that has node and a JDK... Right now I'd have to limit support to TypeScript & Javascript, which feels too restrictive.

eladb
eladb approved these changes Oct 30, 2018
View reviewed changes
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/README.md Outdated Show resolved Hide resolved
packages/@aws-cdk/cicd/lib/pipeline-deploy-stack-action.ts Outdated Show resolved Hide resolved
@skinny85
Copy link
Contributor

skinny85 commented Oct 31, 2018 •
edited
Loading

The name cicd is possibly too generic? CI/CD is more than just your CDK code. For example, CodePipeline is in the CI/CD business. How about aws-cdk-cicd?

@RomainMuller
Copy link
Contributor Author

@skinny85 we cannot name this aws-cdk-cicd because the aws- prefix is reserved for L1/2 surface area. This is L3-y. I guess we could name this cdk-cicd, felt that @aws-cdk/cdk-cicd reads quite heavy.

@skinny85
Copy link
Contributor

skinny85 commented Nov 2, 2018

@skinny85 we cannot name this aws-cdk-cicd because the aws- prefix is reserved for L1/2 surface area. This is L3-y. I guess we could name this cdk-cicd, felt that @aws-cdk/cdk-cicd reads quite heavy.

I can understand that. I personally still prefer @aws-cdk/cdk-cicd to @aws-cdk/cicd - I think naming it only cicd misses the fact that this is CI/CD only for CDK packages.

@eladb
Copy link
Contributor

eladb commented Nov 4, 2018

Can't say I have a good idea... I tend to agree with Adam that @aws-cdk/cicd is not clear and quite confusing.

A few alternative options come to mind:

  1. Would it make sense to include these in the codepipeline package? So it feels "native".
  2. Publish it as a library outside the @aws-cdk namespace (like "aws-delivlib"). Then we can give it an awesome name.
  3. Package it together with other high level capabilities related to CDK apps (that we will introduce in the future) - something like @aws-cdk/app-delivery or @aws-cdk/app-management

@RomainMuller
Copy link
Contributor Author

Making this part of @aws-cdk/codepipeline is probably going to make it tough to avoid introducing dependency cycles (that's the primary reason I modeled this as a separate package to begin with).

I kinda like @aws-cdk/app-delivery, @skinny85 does that sound good (at least better) to you as well?

@skinny85
Copy link
Contributor

skinny85 commented Nov 5, 2018

I kinda like @aws-cdk/app-delivery, @skinny85 does that sound good (at least better) to you as well?

Yep, I like it more than @aws-cdk/cicd.

@RomainMuller
Copy link
Contributor Author

Alright, I'll update the PR with the new name (hopefully, shortly).

@RomainMuller RomainMuller merged commit f2fe4e9 into master Nov 6, 2018
@RomainMuller RomainMuller changed the title feat(cicd): CI/CD for CDK Stacks feat(app-delivery): CI/CD for CDK Stacks Nov 6, 2018
@RomainMuller RomainMuller deleted the rmuller/cicd branch November 6, 2018 15:51
rix0rrr pushed a commit that referenced this pull request Nov 6, 2018
v0.15.0
68e18a4 
Bug Fixes
========

* **aws-autoscaling:** allow minSize to be set to 0 ([#1015](#1015)) ([67f7fa1](67f7fa1))
* **aws-codebuild:** correctly pass the timeout property to CFN when creating a Project. ([#1071](#1071)) ([b1322bb](b1322bb))
* **aws-codebuild:** correctly set S3 path when using it as artifact. ([#1072](#1072)) ([f32cba9](f32cba9))
* **aws-kms:** add output value when exporting an encryption key ([#1036](#1036)) ([cb490be](cb490be))
* Switch from `js-yaml` to `yaml` ([#1092](#1092)) ([0b132b5](0b132b5))

Features
========

* don't upload the same asset multiple times ([#1011](#1011)) ([35937b6](35937b6)), closes [#989](#989)
* **app-delivery:** CI/CD for CDK Stacks ([#1022](#1022)) ([f2fe4e9](f2fe4e9))
* add a new construct library for ECS ([#1058](#1058)) ([ae03ddb](ae03ddb))
* **applets:** integrate into toolkit ([#1039](#1039)) ([fdabe95](fdabe95)), closes [#849](#849) [#342](#342) [#291](#291)
* **aws-codecommit:** use CloudWatch Events instead of polling by default in the CodePipeline Action. ([#1026](#1026)) ([d09d30c](d09d30c))
* **aws-dynamodb:** allow specifying partition/sort keys in props ([#1054](#1054)) ([ec87331](ec87331)), closes [#1051](#1051)
* **aws-ec2:** AmazonLinuxImage supports AL2 ([#1081](#1081)) ([97b57a5](97b57a5)), closes [#1062](#1062)
* **aws-lambda:** high level API for event sources ([#1063](#1063)) ([1be3442](1be3442))
* **aws-sqs:** improvements to IAM grants API ([#1052](#1052)) ([6f2475e](6f2475e))
* **codepipeline/cfn:** Use fewer statements for pipeline permissions ([#1009](#1009)) ([8f4c2ab](8f4c2ab))
* **pkglint:** Make sure .snk files are ignored ([#1049](#1049)) ([53c8d76](53c8d76)), closes [#643](#643)
* **toolkit:** deployment ui improvements ([#1067](#1067)) ([c832eaf](c832eaf))
* Update to CloudFormation resource specification v2.11.0

BREAKING CHANGES
========

* The ec2.Connections object has been changed to be able to manage multiple
  security groups. The relevant property has been changed from `securityGroup`
  to `securityGroups` (an array of security group objects).
* **aws-codecommit:** this modifies the default behavior of the CodeCommit
  Action.  It also changes the internal API contract between the
  aws-codepipeline-api module and the CodePipeline Actions in the service
  packages.
* **applets:** The applet schema has changed to allow Multiple applets can be
  define in one file by structuring the files like this:
* **applets:** The applet schema has changed to allow definition of multiple
  applets in the same file.

The schema now looks like this:

    applets:
      MyApplet:
        type: ./my-applet-file
        properties:
          property1: value
          ...
By starting an applet specifier with npm://, applet modules can directly be
referenced in NPM. You can include a version specifier (@1.2.3) to reference
specific versions.
* **aws-sqs:** `queue.grantReceiveMessages` has been removed. It is unlikely
  that this would be sufficient to interact with a queue. Alternatively you can
  use `queue.grantConsumeMessages` or `queue.grant('sqs:ReceiveMessage')` if
  there's a need to only grant this action.
rix0rrr added a commit that referenced this pull request Nov 6, 2018
@rix0rrr
v0.15.0 (#1094)
fbb2067 
Bug Fixes
========

* **aws-autoscaling:** allow minSize to be set to 0 ([#1015](#1015)) ([67f7fa1](67f7fa1))
* **aws-codebuild:** correctly pass the timeout property to CFN when creating a Project. ([#1071](#1071)) ([b1322bb](b1322bb))
* **aws-codebuild:** correctly set S3 path when using it as artifact. ([#1072](#1072)) ([f32cba9](f32cba9))
* **aws-kms:** add output value when exporting an encryption key ([#1036](#1036)) ([cb490be](cb490be))
* Switch from `js-yaml` to `yaml` ([#1092](#1092)) ([0b132b5](0b132b5))

Features
========

* don't upload the same asset multiple times ([#1011](#1011)) ([35937b6](35937b6)), closes [#989](#989)
* **app-delivery:** CI/CD for CDK Stacks ([#1022](#1022)) ([f2fe4e9](f2fe4e9))
* add a new construct library for ECS ([#1058](#1058)) ([ae03ddb](ae03ddb))
* **applets:** integrate into toolkit ([#1039](#1039)) ([fdabe95](fdabe95)), closes [#849](#849) [#342](#342) [#291](#291)
* **aws-codecommit:** use CloudWatch Events instead of polling by default in the CodePipeline Action. ([#1026](#1026)) ([d09d30c](d09d30c))
* **aws-dynamodb:** allow specifying partition/sort keys in props ([#1054](#1054)) ([ec87331](ec87331)), closes [#1051](#1051)
* **aws-ec2:** AmazonLinuxImage supports AL2 ([#1081](#1081)) ([97b57a5](97b57a5)), closes [#1062](#1062)
* **aws-lambda:** high level API for event sources ([#1063](#1063)) ([1be3442](1be3442))
* **aws-sqs:** improvements to IAM grants API ([#1052](#1052)) ([6f2475e](6f2475e))
* **codepipeline/cfn:** Use fewer statements for pipeline permissions ([#1009](#1009)) ([8f4c2ab](8f4c2ab))
* **pkglint:** Make sure .snk files are ignored ([#1049](#1049)) ([53c8d76](53c8d76)), closes [#643](#643)
* **toolkit:** deployment ui improvements ([#1067](#1067)) ([c832eaf](c832eaf))
* Update to CloudFormation resource specification v2.11.0

BREAKING CHANGES
========

* The ec2.Connections object has been changed to be able to manage multiple
  security groups. The relevant property has been changed from `securityGroup`
  to `securityGroups` (an array of security group objects).
* **aws-codecommit:** this modifies the default behavior of the CodeCommit
  Action.  It also changes the internal API contract between the
  aws-codepipeline-api module and the CodePipeline Actions in the service
  packages.
* **applets:** The applet schema has changed to allow Multiple applets can be
  define in one file by structuring the files like this:
* **applets:** The applet schema has changed to allow definition of multiple
  applets in the same file.

The schema now looks like this:

    applets:
      MyApplet:
        type: ./my-applet-file
        properties:
          property1: value
          ...
By starting an applet specifier with npm://, applet modules can directly be
referenced in NPM. You can include a version specifier (@1.2.3) to reference
specific versions.
* **aws-sqs:** `queue.grantReceiveMessages` has been removed. It is unlikely
  that this would be sufficient to interact with a queue. Alternatively you can
  use `queue.grantConsumeMessages` or `queue.grant('sqs:ReceiveMessage')` if
  there's a need to only grant this action.
@NGL321 NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akiym akiym akiym left review comments

@skinny85 skinny85 skinny85 left review comments

@eladb eladb eladb approved these changes

@rix0rrr rix0rrr Awaiting requested review from rix0rrr

Assignees

@skinny85 skinny85

@rix0rrr rix0rrr

@eladb eladb

Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@RomainMuller @skinny85 @eladb @akiym @rix0rrr @NGL321