Add support for CloudFormation actions in CodePipeline #509
Conversation
--- Author's note: This work mostly belongs to Max Hall (@hallmaxw) It was at some point commented out during a refactor - this is just adding it back. Most of the credit should go to him. --- Anywho - this adds the ability for customers to use CloudForamtion actions in CodePipeline. This follows the same pattern as previous CodePipeline actions by making it it's own package.
| import { ArtifactPath, Pipeline, Stage } from '@aws-cdk/aws-codepipeline'; | ||
| import { Role } from '@aws-cdk/aws-iam'; | ||
| import { PolicyStatement, ServicePrincipal } from '@aws-cdk/cdk'; | ||
| import { CreateReplaceChangeSet, ExecuteChangeSet } from '../lib/pipeline-action'; |
There was a problem hiding this comment.
- This is not how users would use this module.
- I don't think there's a need to explicitly call out all the imports here.
- Also, we have been standardizing around namespaced imports:
import cdk = require('@aws-cdk/cdk'), etc.
| stackName, | ||
| changeSetName, | ||
| roleArn: changeSetExecRole.roleArn, | ||
| templatePath: new ArtifactPath(source.artifact, 'some_template.yaml'), |
There was a problem hiding this comment.
This aspect is probably worth a bit of explaining in the README. Who put some_template.yaml there? Try to think from a user's perspective what would they need to quickly make this work for their use case.
| @@ -0,0 +1,50 @@ | |||
| ## AWS CodePipline Actions for AWS CloudFormation | |||
|
|
|||
| This module contains an Action that allows you to use CloudFormation actions from CodePipeline. | |||
There was a problem hiding this comment.
README requires a bit more work 😄 Where are the gifs?
Also, I think it will be useful to list all the action types and when would someone want to use them
| import { ArtifactPath, DeployAction, Stage, } from '@aws-cdk/aws-codepipeline'; | ||
| import { RoleArn } from '@aws-cdk/aws-iam'; | ||
|
|
||
| export enum CloudFormationCapabilities { |
| NamedIAM = 'CAPABILITY_NAMED_IAM' | ||
| } | ||
|
|
||
| export interface CloudFormationCommonOptions { |
There was a problem hiding this comment.
Missing @default descriptions
| } | ||
|
|
||
| /** | ||
| * Executes a change set. |
There was a problem hiding this comment.
Add some docs about when would someone want to use this
|
|
||
| /** | ||
| * The path to the template to pass to CloudFormation during the change set operation. | ||
| */ |
| * The arn of the IAM service role that AWS CloudFormation assumes | ||
| * when it operates on resources in the specified stack. | ||
| */ | ||
| roleArn: RoleArn; |
There was a problem hiding this comment.
This should be Role and optional (default should be to auto-create a role by the construct)
There was a problem hiding this comment.
Tricky. Normally when we follow this pattern, we autocreate a Role with the right permissions.
In this case though, the "right permissions" are *:*, because there's no telling what the CloudFormation might do. Except... autocreating a role that has *:* feels risky. So the better solution might be to punt it to the consumer, and have them knowingly opt in.
In the same vein, I wonder if we should default the IAM capabilities to... you know... work. Everyone's going to want this and hence specify the IAM capabilities. Should we make people opt out to improve convenience in the 99% case?
There was a problem hiding this comment.
We could still create a role with no statements and require that the user explicitly call addToRolePolicy with whatever permissions they want.
As sugar, we can implement a props flag such as adminRights: boolean which will auto-add the "/" and set the IAM capabilities to all
| * The arn of the IAM service role that AWS CloudFormation assumes | ||
| * when it operates on resources in the specified stack. | ||
| */ | ||
| roleArn: RoleArn; |
| * The arn of the IAM service role that AWS CloudFormation assumes | ||
| * when it operates on resources in the specified stack. | ||
| */ | ||
| roleArn: RoleArn; |
|
Superseded by #525 |
Author's note: This work mostly belongs to Max Hall (@hallmaxw)
It was at some point commented out during a refactor - this is just adding it back.
Most of the credit should go to him.
Anywho - this adds the ability for customers to use CloudForamtion actions in CodePipeline.
This follows the same pattern as previous CodePipeline actions by making it it's own package.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.