-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(pipelines): make CdkPipeline build stage optional #10345
fix(pipelines): make CdkPipeline build stage optional #10345
Conversation
CdkPipeline works perfectly fine without build stage, if already-built cloud assembly is provided in the source stage (e.g. S3 source action). A use case for this is separate CI and CD logic, where CDK synthetization happens within the CI build and the assembly is stored as an artefact to be deployed by a pipeline. This commit makes the build stage optional, so that this use case is possible without a "dummy build stage" to satisfy validation.
@@ -117,12 +117,6 @@ export class CdkPipeline extends Construct { | |||
if (!props.sourceAction && (!props.codePipeline || props.codePipeline.stages.length < 1)) { | |||
throw new Error('You must pass a \'sourceAction\' (or a \'codePipeline\' that already has a Source stage)'); | |||
} | |||
if (!props.synthAction && (!props.codePipeline || props.codePipeline.stages.length < 2)) { |
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.
You are technically correct that this is not necessary, and I see you have a use case where they aren't necessary.
Your use case is outside the 99% though, so I'm hesitant to give up a guard rail that would help all people out there who aren't in your position.
How about a compromise: providing your own pipeline
is an "I know what I'm doing" switch, so we don't require 2 stages in the existing pipeline (but if no pipeline is given, we still require the synthAction?).
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.
Hi @rix0rrr, thanks for your reply.
How about a compromise: providing your own pipeline is an "I know what I'm doing" switch, so we don't require 2 stages in the existing pipeline (but if no pipeline is given, we still require the synthAction?).
I actually believe that's the behaviour this PR introduces: You must provide either sourceAction
or codePipeline
, otherwise the validation fails on line 117. If you provide sourceAction
, then you also need to provide synthAction
, otherwise the validation fails on line 110. That means the only way of not providing the synthAction
is to provide a codePipeline
that already has the Source stage. Does that make sense?
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.
If you like, I am happy to slightly reorder the validation steps to make the dependencies more obvious.
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
In PR #10148, @rix0rrr made it possible to provide a custom CodePipeline pipeline instance to CdkPipeline. This also made the
sourceAction
(Source stage) andsynthAction
(Build stage) props optional.However, validation was added to ensure that if
synthAction
is not provided, the pipeline already contains at least two stages (assuming that would be Source and Build).Logically though, CdkPipeline works perfectly fine without Build stage, if an already-built cloud assembly is provided in the source stage (e.g. S3 source action). A use case for this is, for example, separating CI and CD logic, where CDK synthesis happens within the CI build and the assembly is stored as an artefact to be deployed by a pipeline.
This PR makes the Build stage optional, to allow this use case without a need for a dummy build stage.
Example pipeline code:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license