-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(codepipeline-actions): Add CAPABILITY_AUTO_EXPAND (#2851) #2852
Changes from 3 commits
5f1c779
501ece9
61efbad
50a7fa7
e7526dd
845d485
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,21 +97,21 @@ export = nodeunit.testCase({ | |
stage: selfUpdateStage1, | ||
stack: pipelineStack, | ||
input: selfUpdatingStack.synthesizedApp, | ||
capabilities: cfn.CloudFormationCapabilities.NamedIAM, | ||
capabilities: [cfn.CloudFormationCapabilities.NamedIAM], | ||
adminPermissions: false, | ||
}); | ||
new PipelineDeployStackAction(pipelineStack, 'DeployStack', { | ||
stage: selfUpdateStage2, | ||
stack: stackWithNoCapability, | ||
input: selfUpdatingStack.synthesizedApp, | ||
capabilities: cfn.CloudFormationCapabilities.None, | ||
capabilities: [cfn.CloudFormationCapabilities.None], | ||
adminPermissions: false, | ||
}); | ||
new PipelineDeployStackAction(pipelineStack, 'DeployStack2', { | ||
stage: selfUpdateStage3, | ||
stack: stackWithAnonymousCapability, | ||
input: selfUpdatingStack.synthesizedApp, | ||
capabilities: cfn.CloudFormationCapabilities.AnonymousIAM, | ||
capabilities: [cfn.CloudFormationCapabilities.AnonymousIAM], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would like to have an additional test that uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a couple more tests. |
||
adminPermissions: false, | ||
}); | ||
expect(pipelineStack).to(haveResource('AWS::CodePipeline::Pipeline', hasPipelineAction({ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,7 +141,7 @@ export interface CloudFormationDeployActionProps extends CloudFormationActionPro | |
* @see https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-iam-template.html#using-iam-capabilities | ||
* @default None, unless `adminPermissions` is true | ||
*/ | ||
readonly capabilities?: cloudformation.CloudFormationCapabilities; | ||
readonly capabilities?: cloudformation.CloudFormationCapabilities[]; | ||
|
||
/** | ||
* Whether to grant full permissions to CloudFormation while deploying this template. | ||
|
@@ -221,12 +221,12 @@ export abstract class CloudFormationDeployAction extends CloudFormationAction { | |
|
||
constructor(props: CloudFormationDeployActionProps, configuration: any) { | ||
const capabilities = props.adminPermissions && props.capabilities === undefined | ||
? cloudformation.CloudFormationCapabilities.NamedIAM | ||
? [cloudformation.CloudFormationCapabilities.NamedIAM] | ||
: props.capabilities; | ||
super(props, { | ||
...configuration, | ||
// None evaluates to empty string which is falsey and results in undefined | ||
Capabilities: (capabilities && capabilities.toString()) || undefined, | ||
Capabilities: parseCapabilities(capabilities), | ||
RoleArn: cdk.Lazy.stringValue({ produce: () => this.deploymentRole.roleArn }), | ||
ParameterOverrides: cdk.Lazy.stringValue({ produce: () => Stack.of(this.scope).toJsonString(props.parameterOverrides) }), | ||
TemplateConfiguration: props.templateConfiguration ? props.templateConfiguration.location : undefined, | ||
|
@@ -540,3 +540,14 @@ interface StatementTemplate { | |
} | ||
|
||
type StatementCondition = { [op: string]: { [attribute: string]: string } }; | ||
|
||
function parseCapabilities(capabilities: any): any { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we avoid using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, sorry for missing out on this one. I've updated the function. |
||
if (capabilities === undefined) { | ||
return undefined; | ||
} else if (capabilities.length === 1) { | ||
const capability = capabilities.toString(); | ||
return (capability === '') ? undefined : capability; | ||
} else if (capabilities.length > 1) { | ||
return capabilities.join(','); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this should not contain
AutoExpand
, too.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, makes sense, I've added it. Please let me know if the changes are acceptable.