feat(code pipeline): support KMS materials for replication stores

[rsmogura]

Overview

This change adds supports for using custom KMS materials
for artifacts replication stores.

It's motivated by enabling customers to use cross-account
Cloud Formation deployments, as specified in
https://github.com/awslabs/aws-refarch-cross-account-pipeline

Changes

Introduce new construct ArtifactStore to represent pipeline artifact store.
If this construct has encryptionMaterialArn set than use it in output
template, when defining pipeline's artifact stores as encryption key.

Motivation

For cross-account deployments artifacts must by encrypted and replicated
using key to which foreign account has an access. By default
aws/s3 key is used which has limited support for setting resource policy.

Currently pipeline was passing the S3 Bucket default KMS key
if it was present, however it only happened in case of default
artifact store. Additionally actions like project build can
use own keys to encrypt artifacts which are not derived
from artifact bucket's default key.

Customers can pass KMS key ARN or key alias ARN. However at current stage CDK doesn't automatically generate those keys, nor manage permissions for those.

In order to make cross-account deployments customers should
follow mentioned
https://github.com/awslabs/aws-refarch-cross-account-pipeline and build
replication (scaffold) stack on their own as follow:

The simplest way could be as follow:

• Create S3 Bucket with known name
  • add build account(s) and target account(s) (i.e. prod, test), to
    resource policy
• Create KMS key
  • add build account(s) and target account (i.e. prod, test), to
    resource policy
• Create KMS key alias with known name
• Pass artifacts stores with imported bucket and KMS key alias ARN
  to pipeline
• Ensure all roles which access or store artifacts has set
  permissions to access KMS

BREAKING CHANGE: Replaced attribute artifactBucket on pipeline props
with artifactsStore. Customers passing buckets will have to construct
ArtifactsStore with bucket, and eventually KMS key.

---

Pull Request Checklist

• [X ] 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
• [ X] 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.

[rix0rrr]

Thanks for the contribution, and helping us solve this use case.

I'm not sure about the direction we're taking though. Instead of introducing the new ArtifactStore concept (though maybe semantically correct), I'm wondering what's missing from the current s3.Bucket to achieve this, and what we can do to fix that instead.

I'm not sure I understand the current issue completely. It seems to have something to do with the KMS key; note that it is possible to import an existing KMS key as well when creating an S3 bucket. If the problem is that we can't specify a KMS key when importing an S3 bucket, or something along those lines, then that seems like the problem we need to fix.

@skinny85, @RomainMuller, opinions?

Also seems like we need a documentation example showing how to set up a cross-account pipeline.

[rsmogura]

Maybe I will start with our use-case.

We have a Devo account in which we build and deploy our service (which is composed from Lambda and ECS cluster all provisioned with CFN templates generated from CDK). We want to deploy to Production account to three regions.

In other words our use case is cross-account and cross-region.

In order to do this, reference architecture suggest to create two roles in Production account and set those roles on CFN actions (one would be an action role, and 2nd one the deployment role), so actually CFN action for production will be executed in context of Production account.

However production roles need to read artifacts from artifacts stores (to read lambda function and generated application templates). Those roles can't do this as there are two additional requirements to be met:

• artifacts buckets have to have resource policy allowing foreign account (or role) to access it (we can achieve this right now);
• as artifacts are encrypted similar thing have to be true for KMS key used to encrypt artifacts (other way read will fail, with S3 access problem which in fact is KMS access).

The default aws/s3 key is AWS managed key (not customer one), and has limited ability to modify its policy. So pipeline should use custom KMS keys (or aliases) to replicate artifacts between regions, only to whitelist Production roles / accounts.

In addition, right now, pipeline will set KMS key only for default artifact store in CFN template, but will not do this for replication stores.

The change like this (with 'action role'), could enable me to generate appropriate buckets, keys, set correct permissions and pass those when required and have desired build stack.

The reason I suggest to use new type ArtifactStore is because S3 buckets encryption keys are separated from keys used for encrypt artifacts and one can use different keys than bucket's default one.

However adding key (or alias, which would be preferred for me) to imported S3 bucket, and passing it to generated CFN template would enable me as well.

The reference architecture is here https://github.com/awslabs/aws-refarch-cross-account-pipeline

For example setup I can try to sync with @skinny85 (offline) to show how actually it could work with both of my changes.

[rix0rrr]

I think the reference architecture you're quoting describes cross-account deployment, but I don't think it describes cross-region deployments, so you must be enhancing it in some way. Is that what the "replication stores" are for?

Still seems to me that when defining buckets (whether the "starter" bucket or a replication bucket), you can set the KMS key that is used by specifying the encryptionKey parameters, which allows you to use a key with a specially prepared resource policy which allows decryption from other accounts.

For example setup I can try to sync with @skinny85 (offline) to show how actually it could work with both of my changes.

That seems like a great idea. @skinny85 is our resident CodeSuite expert.

[skinny85]

@rix0rrr so the issue with the replication Buckets in particular is that they have to live in a different region (obviously, as they are meant for cross-region CodePipeline). Now, because of that, we can't really use IBucket for them, as using it will produce a CFN import / export pair, and CFN exports do not work across regions. For those reasons, we chose to represent Buckets as strings when passing them in the Pipeline Construct's props. For that reason, there's no place to specify the KMS key right now, if you want to encrypt your replication Buckets - which you have to, when combining cross-region and cross-account, as @rsmogura is doing here.

Does this explanation make sense?

[rix0rrr]

@skinny85, that helps, thanks for the explanation.

What do you suppose would be the least invasive way to add support for encryption keys?

[skinny85]

@rsmogura I have a question. In the description, you wrote:

For cross-account deployments artifacts must by encrypted and replicated
using key to which foreign account has an access. By default
aws/s3 key is used which has limited support for setting resource policy.

Why is that? IBucket has a reference to a kms.IEncryptionKey, and IEncryptionKey has a method addToResourcePolicy. Why is that insufficient to manage resource policies for cross-account deployments?

[skinny85]

@rix0rrr I did some digging in this area today, and the preliminary research seems promising. It seems like we are fairly close to being able to use Bucket here. In my simple test, using something like this:

new codepipeline.Pipeline(this, 'Pipeline', {
  crossRegionReplicationBuckets: {
    'us-west-2': s3.Bucket.import(this, 'ImportedUsWest2Bucket', {
      bucketName: 'some-bucket-name',
    },
  },
});

seems to be working fine (after some simple changes in the Pipeline Construct).

What's holding us back is the restriction in "magical" reference logic that you can only reference things in the same region/account. So, you can't do something like this:

const bucketInUsWest2 = new s3.Bucket(stackInUsWest2, 'Bucket', {
  bucketName: 'some-bucket-name',
});

new codepipeline.Pipeline(this, 'Pipeline', {
  crossRegionReplicationBuckets: {
    'us-west-2': bucketInUsWest2,
  },
});

I wonder if we could relax this constraint, and only fail in the case that the resource does not have a physical name defined? Otherwise, we use its ARN.

I believe that would allow us to change the strings we use now in crossRegionReplicationBuckets to IBucket, and manage KMS keys that way. Thoughts?

[rsmogura]

I think it is a nice idea to synthesise bucket ARN from name as it’s completely not region nor account dependant.

From my side I think that only one risk would be if pipeline would like to call grant methods on such bucket, as those would cause update of bucket’s resource policy with not existing roles ARN (cross region stack reference)

For KMS keys it would be more complicated, as I think KMS keys dosen’t have ability to set any name, so their name and ARN is completely random. Solution to this can be generation of KMS alias, however next push back would be that roles policy to access KMS doesn’t allow to pass alias. I.e. pipeline role statement Allow kms:Encrypt arn:...:alias/my-replication-alias will be no-op. I was wondering if this can be solved by using tags on keys.

In my code I generate CFN deployment roles in “prod” stack with known name and than import into pipeline stack. So I would like to have access to KMS key declaration to update it’s resource policy and allow acces from production account.

[skinny85]

In my code I generate CFN deployment roles in “prod” stack with known name and than import into pipeline stack. So I would like to have access to KMS key declaration to update it’s resource policy and allow acces from production account.

And is there anything preventing you from doing that, if cross-account replication Buckets are represented as s3.IBucket instead of the Strings they are now?

[rsmogura]

In my code I generate CFN deployment roles in “prod” stack with known name and than import into pipeline stack. So I would like to have access to KMS key declaration to update it’s resource policy and allow acces from production account.

And is there anything preventing you from doing that, if cross-account replication Buckets are represented as s3.IBucket instead of the Strings they are now?

I think it would be ok, only Bucket.import should be extended to pass KMS material (key or alias) and of course pipeline.ts to represent it in CFN ArtifactStores

[skinny85]

Great. Then I think that's the direction we should be moving to with these changes (not saying you have to do them @rsmogura, to be clear).

[rsmogura]

Cool! If you will have something I can check, it. I want as well to check CN as it may not support KMS in all places.

[rsmogura]

Big thanks!!! It's important set of changes, which will allow to make a huge step for many projects !!! ❤️

[rix0rrr]

I mentioned this in person, but for the record:

I support moving forward by resolving bucketName et. al. directly to physical names instead of {Ref}, if possible:

• Only for the minimal set of resources to unblock cross-region CI/CD right now (which are, I suppose, Bucket, Key and Role?)
• While still retaining the stack dependency relationship (by means of CFN references).

This will unblock an important use case and we don't have an alternative mechanism to do the same.

[eladb]

Hey, any updates on this?

[skinny85]

Continued in: #2977

Let me close this one, and let's move the conversation there.

[skinny85]

This is continued in #3694