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

feat(pipelines): allow disabling of KMS keys #10396

Merged
merged 12 commits into from
Sep 29, 2020
Merged
135 changes: 105 additions & 30 deletions packages/@aws-cdk/aws-codepipeline/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,22 @@ const pipeline = new codepipeline.Pipeline(this, 'MyFirstPipeline', {
});
```

Be aware that in the default configuration, the `Pipeline` construct creates
an AWS Key Management Service (AWS KMS) Customer Master Key (CMK) for you to
encrypt the artifacts in the artifact bucket, which incurs a cost of
**$1/month**. This default configuration is necessary to allow cross-account
actions.

If you do not intend to perform cross-account deployments, you can disable
the creation of the Customer Master Keys by passing `crossAccountKeys: false`
when defining the Pipeline:

```ts
const pipeline = new codepipeline.Pipeline(this, 'MyFirstPipeline', {
crossAccountKeys: false,
});
```

### Stages

You can provide Stages when creating the Pipeline:
Expand Down Expand Up @@ -80,16 +96,101 @@ or you can use the `IStage.addAction()` method to mutate an existing Stage:
sourceStage.addAction(someAction);
```

### Cross-account CodePipelines

> Cross-account Pipeline actions require that the Pipeline has *not* been
> created with `crossAccountKeys: false`.

Most pipeline Actions accept an AWS resource object to operate on. For example:

* `S3DeployAction` accepts an `s3.IBucket`.
* `CodeBuildAction` accepts a `codebuild.IProject`.
* etc.

These resources can be either newly defined (`new s3.Bucket(...)`) or imported
(`s3.Bucket.fromBucketAttributes(...)`) and identify the resource that should
be changed.

These resources can be in different accounts than the pipeline itself. For
example, the following action deploys to an imported S3 bucket from a
different account:

```typescript
stage.addAction(new codepipeline_actions.S3DeployAction({
bucket: s3.Bucket.fromBucketAttributes(this, 'Bucket', {
account: '123456789012',
// ...
}),
// ...
}));
```

Actions that don't accept a resource object accept an explicit `account` parameter:

```typescript
stage.addAction(new codepipeline_actions.CloudFormationCreateUpdateStackAction({
account: '123456789012',
// ...
}));
```

The `Pipeline` construct automatically defines an **IAM Role** for you in the
target account which the pipeline will assume to perform that action. This
Role will be defined in a **support stack** named
`<PipelineStackName>-support-<account>`, that will automatically be deployed
before the stack containing the pipeline.

If you do not want to use the generated role, you can also explicitly pass a
`role` when creating the action. In that case, the action will operate in the
account the role belongs to:

```ts
stage.addAction(new codepipeline_actions.CloudFormationCreateUpdateStackAction({
// ...
role: iam.Role.fromRoleArn(this, 'ActionRole', '...'),
}));
```

### Cross-region CodePipelines

You can also use the cross-region feature to deploy resources
into a different region than your Pipeline is in.
Similar to how you set up a cross-account Action, the AWS resource object you
pass to actions can also be in different *Regions*. For example, the
following Action deploys to an imported S3 bucket from a different Region:

```typescript
stage.addAction(new codepipeline_actions.S3DeployAction({
bucket: s3.Bucket.fromBucketAttributes(this, 'Bucket', {
region: 'us-west-1',
// ...
}),
// ...
}));
```

It works like this:
Actions that don't take an AWS resource will accept an explicit `region`
parameter:

```typescript
const pipeline = new codepipeline.Pipeline(this, 'MyFirstPipeline', {
stage.addAction(new codepipeline_actions.CloudFormationCreateUpdateStackAction({
// ...
region: 'us-west-1',
}));
```

The `Pipeline` construct automatically defines a **replication bucket** for
you in the target region, which the pipeline will replicate artifacts to and
from. This Bucket will be defined in a **support stack** named
`<PipelineStackName>-support-<region>`, that will automatically be deployed
before the stack containing the pipeline.

If you don't want to use these support stacks, and already have buckets in
place to serve as replication buckets, you can supply these at Pipeline definition
time using the `crossRegionReplicationBuckets` parameter. Example:

```ts
const pipeline = new codepipeline.Pipeline(this, 'MyFirstPipeline', { /* ... */ });
// ...

crossRegionReplicationBuckets: {
// note that a physical name of the replication Bucket must be known at synthesis time
'us-west-1': s3.Bucket.fromBucketAttributes(this, 'UsWest1ReplicationBucket', {
Expand All @@ -101,32 +202,6 @@ const pipeline = new codepipeline.Pipeline(this, 'MyFirstPipeline', {
}),
},
});

// later in the code...
new codepipeline_actions.CloudFormationCreateUpdateStackAction({
actionName: 'CFN_US_West_1',
// ...
region: 'us-west-1',
});
```

This way, the `CFN_US_West_1` Action will operate in the `us-west-1` region,
regardless of which region your Pipeline is in.

If you don't provide a bucket for a region (other than the Pipeline's region)
that you're using for an Action,
there will be a new Stack, called `<nameOfYourPipelineStack>-support-<region>`,
defined for you, containing a replication Bucket.
This new Stack will depend on your Pipeline Stack,
so deploying the Pipeline Stack will deploy the support Stack(s) first.
Example:

```bash
$ cdk ls
MyMainStack
MyMainStack-support-us-west-1
$ cdk deploy MyMainStack
# output of cdk deploy here...
```

See [the AWS docs here](https://docs.aws.amazon.com/codepipeline/latest/userguide/actions-create-cross-region.html)
Expand Down
36 changes: 36 additions & 0 deletions packages/@aws-cdk/aws-codepipeline/lib/_enhanced_action.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { Aws, ResourceEnvironment } from '@aws-cdk/core';
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't our standard to use lib/private/ for private classes. What's the _ prefix in the file name?

Copy link
Contributor Author

@rix0rrr rix0rrr Sep 21, 2020

Choose a reason for hiding this comment

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

We have both (I started the lib/private convention, Elad prefers the _prefix.ts convention). At the moment my opinion is that one or a few files can have a _ prefix. If there grow to be too many, move to a /private/ folder.

Copy link
Contributor

Choose a reason for hiding this comment

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

Feels weird. We should standardize to one of the two.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on moving this to private/ (there are a few different private files in this module already, maybe it makes sense to move those there as well).

import { IAction } from './action';

/**
* Helper routines to work with Actions
*
* Can't put these on Action themselves since we only have an interface
* and every library would need to reimplement everything (there is no
* `ActionBase`).
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 introduce it now and add env() as its first member?
How many such instances are there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could have, but since we have a ticket for properly refactoring aws-codepipeline anyway to support CDKP and other extensions better in the future, I'd rather tackle it as part of that work.

*
* So here go the members that should have gone onto the Action class
* but can't.
*
* It was probably my own idea but I don't want it anymore:
* https://github.com/aws/aws-cdk/issues/10393
*/
export class EnhancedAction {
constructor(private readonly action: IAction) {
}

/**
* The environment this action runs in
*/
public get env(): ResourceEnvironment {
Copy link
Contributor

Choose a reason for hiding this comment

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

Have to say, I don't love this class. I feel like this would be better served as a helper function, or private method in Pipeline.

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 is effectively a helper function, except modeled as a richer class instead of a static method. The following:

new EnhancedAction(action).env

ActionHelpers.env(action)

Are effectively the same thing. Except the first feels "better" to me. It's more OO. It's like Rich Wrappers in Scala (except explicit instead of implicit).

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 agree that a class whose only possible usage is new EnhancedAction(action).env is more OO. Classes need to have behavior, state, methods, to be considered good OO. This reminds me of the argument that instead of this:

interface Whatevs {
  value: string;
}

, you should do this:

class Whatevs {
  private _value: string;
  public get value(): string { return this._value; }
  public set value(value: string): void { this._value = value; }
}

because it's "more OO".

ActionHelpers.env(action)

I don't actually think this should be a static method. This should be a private method of the Pipeline class, which is the entity that is actually concerned with the algorithm used to determine this (as it's what determines the support Stack algorithm).

return {
account: (this.action.actionProperties.role?.env.account
?? this.action.actionProperties?.resource?.env.account
?? this.action.actionProperties.account
?? Aws.ACCOUNT_ID),
region: (this.action.actionProperties.resource?.env.region
?? this.action.actionProperties.region
?? Aws.REGION),
};
}

};
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-codepipeline/lib/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ export interface ActionConfig {
readonly configuration?: any;
}

/**
* A Pipeline Action
*/
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
export interface IAction {
readonly actionProperties: ActionProperties;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,39 @@ function lastNCharacters(str: string, n: number) {
return str.substring(startIndex);
}

/**
* Props for the support stack
*/
export interface CrossRegionSupportConstructProps {
/**
* Whether to create the KMS CMK
*
* (Required for cross-account deployments)
*
* @default true
*/
readonly createKmsKey?: boolean;
}

export class CrossRegionSupportConstruct extends cdk.Construct {
public readonly replicationBucket: s3.IBucket;

constructor(scope: cdk.Construct, id: string) {
constructor(scope: cdk.Construct, id: string, props: CrossRegionSupportConstructProps = {}) {
super(scope, id);

const encryptionKey = new kms.Key(this, 'CrossRegionCodePipelineReplicationBucketEncryptionKey', {
removalPolicy: cdk.RemovalPolicy.DESTROY,
});
const encryptionAlias = new AliasWithShorterGeneratedName(this, 'CrossRegionCodePipelineReplicationBucketEncryptionAlias', {
targetKey: encryptionKey,
aliasName: cdk.PhysicalName.GENERATE_IF_NEEDED,
removalPolicy: cdk.RemovalPolicy.DESTROY,
});
const createKmsKey = props.createKmsKey ?? true;

let encryptionAlias;
if (createKmsKey) {
const encryptionKey = new kms.Key(this, 'CrossRegionCodePipelineReplicationBucketEncryptionKey', {
removalPolicy: cdk.RemovalPolicy.DESTROY,
});
encryptionAlias = new AliasWithShorterGeneratedName(this, 'CrossRegionCodePipelineReplicationBucketEncryptionAlias', {
targetKey: encryptionKey,
aliasName: cdk.PhysicalName.GENERATE_IF_NEEDED,
removalPolicy: cdk.RemovalPolicy.DESTROY,
});
}
this.replicationBucket = new s3.Bucket(this, 'CrossRegionCodePipelineReplicationBucket', {
bucketName: cdk.PhysicalName.GENERATE_IF_NEEDED,
encryptionKey: encryptionAlias,
rix0rrr marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -73,6 +92,15 @@ export interface CrossRegionSupportStackProps {
readonly account: string;

readonly synthesizer: cdk.IStackSynthesizer | undefined;

/**
* Whether or not to create a KMS key in the support stack
*
* (Required for cross-account deployments)
*
* @default true
*/
readonly createKmsKey?: boolean;
}

/**
Expand All @@ -95,7 +123,9 @@ export class CrossRegionSupportStack extends cdk.Stack {
synthesizer: props.synthesizer,
});

const crossRegionSupportConstruct = new CrossRegionSupportConstruct(this, 'Default');
const crossRegionSupportConstruct = new CrossRegionSupportConstruct(this, 'Default', {
createKmsKey: props.createKmsKey,
});
this.replicationBucket = crossRegionSupportConstruct.replicationBucket;
}
}
Expand Down
Loading