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

Support encryption key in an ArtifactStore of CodePipeline when bucket is imported #3138

Closed
kadishmal opened this issue Jul 1, 2019 · 3 comments · Fixed by #3694
Closed
Assignees
Labels
@aws-cdk/aws-codepipeline Related to AWS CodePipeline @aws-cdk/aws-s3 Related to Amazon S3 feature-request A feature should be added or improved.

Comments

@kadishmal
Copy link

  • CDK CLI Version: 0.36.0
  • Module Version: 0.26.0
  • OS: macOS 0.13.6
  • Language: TypeScript

Currently it is not possible to set encryption key for an ArtifactStore of CodePipeline based on an imported S3 bucket. This is required for cross account code pipeline setup. For instance:

// The bucket is created in the same account but a different stack prior to this current stack.
// The S3 bucket has encryption enabled with a custom KMS key created in that same stack.
const bucket = Bucket.fromBucketName(this, 'ArtifactBucket', S3BucketName);

// The pipeline is being created in the second stack in the same account.
const pipeline = new Pipeline(this, 'code-pipeline', {
  artifactBucket: bucket,
  stages: [
     pipelineSourceStage,
     pipelineBuildStage
  ]
});

This generates the following CFN template.

  codepipeline5D0077E6:
    Type: AWS::CodePipeline::Pipeline
    Properties:
      RoleArn:
        Fn::GetAtt:
          - codepipelineRole2DBF2AC8
          - Arn
      Stages:
        - Actions:
            - ActionTypeId:
                Category: Source
                Owner: AWS
                Provider: CodeCommit
                Version: "1"
              Configuration:
                RepositoryName: web
                BranchName: master
                PollForSourceChanges: false
              InputArtifacts: []
              Name: CodeCommit
              OutputArtifacts:
                - Name: SourceArtifact
              RoleArn: arn:aws:iam::123456789101:role/ToolsAcctCodePipelineCodeCommitRole
              RunOrder: 1
          Name: Source
        - Actions:
            - ActionTypeId:
                Category: Build
                Owner: AWS
                Provider: CodeBuild
                Version: "1"
              Configuration:
                ProjectName:
                  Ref: codebuild2BA0470D
              InputArtifacts:
                - Name: SourceArtifact
              Name: CodeBuild
              OutputArtifacts: []
              RunOrder: 1
          Name: Build
      ArtifactStore:
        Location: artifactbucket
        Type: S3
    DependsOn:
      - codepipelineRoleDefaultPolicyAAA5ADDF
      - codepipelineRole2DBF2AC8
    Metadata:
      aws:cdk:path: ToolsCodePipelineStack/code-pipeline/Resource

Note that ArtifactStore doesn't have any encryption related properties.

CDK should be able to detect automatically (or at least provide an option so that developers can indicate) that a bucket has default encryption set which uses the a custom KMS key.

The ideal definition for the second stack should have the following instead:

"ArtifactStore": {
  "Type": "S3",
  "Location": {
    "Ref": "S3Bucket"
  },
  "EncryptionKey": {
    "Id": {
      "Ref": "CMKARN"
    },
    "Type": "KMS"
  }
}
@kadishmal kadishmal added the needs-triage This issue or PR still needs to be triaged. label Jul 1, 2019
@kadishmal
Copy link
Author

A workaround is:

// Imported bucket doesn't allow setting an encryption key (it's readonly), but it is required for the ArtifactStore.
(bucket as any).encryptionKey = encryptionKey;

@NGL321 NGL321 added @aws-cdk/aws-codepipeline Related to AWS CodePipeline feature-request A feature should be added or improved. @aws-cdk/aws-s3 Related to Amazon S3 and removed needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2019
@NGL321
Copy link
Contributor

NGL321 commented Jul 1, 2019

Hi @kadishmal,

Thank you for bringing this to our attention!
We are working hard to stabilize the CDK APIs and tuning them to meet our consistency guidelines. While we work on getting the APIs aligned with our guidelines, we are pausing work on most community feature-requests.

We expect to get back to work on community PRs within a few weeks.

@skinny85
Copy link
Contributor

skinny85 commented Jul 2, 2019

Thanks for bringing this to our attention @kadishmal!

I believe the solution is to add IEncryptionKey to BucketAttributes, like this:

export interface BucketAttributes {
  // old stuff
  readonly bucketArn?: string;
  readonly bucketName?: string;
  readonly bucketDomainName?: string;
  readonly bucketWebsiteUrl?: string;
  readonly bucketRegionalDomainName?: string;
  readonly bucketDualStackDomainName?: string;
  readonly bucketWebsiteNewUrlFormat?: boolean;

  // new
  encryptionKey?: kms.Key;
}

And then, if it has been passed, assign it to the IBucket that fromBucketAttributes returns.

@skinny85 skinny85 self-assigned this Aug 12, 2019
@eladb eladb assigned eladb and unassigned skinny85 and eladb Aug 12, 2019
skinny85 added a commit that referenced this issue Sep 11, 2019
This changes the scaffolding stack logic for the cross-region CodePipelines to include a KMS key and alias as part of it, which are required if an action is simultaneously cross-region and cross-account. We also change to use the KMS key ID instead of the key ARN when rendering the ArtifactStores property.

We also add an alias to the default pipeline artifact bucket.

This required a bunch of changes to the KMS and S3 modules:

* Alias now implements IKey
* Added the keyId property to IKey
* Added removalPolicy property to Alias
* Granting permissions to a key works if the principal belongs to a stack that is a dependent of the key stack
* Allow specifying a key when importing a bucket

Fixes #52
Concerns #1584
Fixes #2517
Fixes #2569
Concerns #3275
Fixes #3138
Fixes #3388
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-codepipeline Related to AWS CodePipeline @aws-cdk/aws-s3 Related to Amazon S3 feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants