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

Error in CDK 1.14.0: "Only a PipelineProject can be added to a CodePipeline" #4646

Closed
markusl opened this issue Oct 23, 2019 · 11 comments · Fixed by #4689
Closed

Error in CDK 1.14.0: "Only a PipelineProject can be added to a CodePipeline" #4646

markusl opened this issue Oct 23, 2019 · 11 comments · Fixed by #4689
Assignees
Labels
@aws-cdk/aws-codepipeline Related to AWS CodePipeline bug This issue is a bug.

Comments

@markusl
Copy link
Contributor

markusl commented Oct 23, 2019

I tried updating our CDK stacks to latest version but encountered a new problem with CodePipeline:

cdk/node_modules/@aws-cdk/aws-codebuild/lib/project.ts:766
      throw new Error('Only a PipelineProject can be added to a CodePipeline');
            ^
Error: Only a PipelineProject can be added to a CodePipeline
    at Project.bindToCodePipeline (cdk/node_modules/@aws-cdk/aws-codebuild/lib/project.ts:766:13)
    at CodeBuildAction.bound (cdk/node_modules/@aws-cdk/aws-codepipeline-actions/lib/codebuild/build-action.ts:117:26)
    at CodeBuildAction.bind (cdk/node_modules/@aws-cdk/aws-codepipeline-actions/lib/action.ts:30:17)
    at Pipeline._attachActionToPipeline (cdk/node_modules/@aws-cdk/aws-codepipeline/lib/pipeline.ts:346:37)
    at Stage.attachActionToPipeline (cdk/node_modules/@aws-cdk/aws-codepipeline/lib/stage.ts:136:27)
    at Stage.addAction (cdk/node_modules/@aws-cdk/aws-codepipeline/lib/stage.ts:86:29)
    at new Stage (cdk/node_modules/@aws-cdk/aws-codepipeline/lib/stage.ts:38:12)
    at Pipeline.addStage (cdk/node_modules/@aws-cdk/aws-codepipeline/lib/pipeline.ts:298:19)
    at Object.exports.addSourceAndBuildStages (cdk/pipeline-master.ts:117:12)
    at Object.exports.createBuildOnlyPipeline (cdk/pipeline-build-only.ts:27:25)

Is there something we need to change now with the latest version?

Reproduction Steps

export const addSourceAndBuildStages = (
  pipeline: codepipeline.Pipeline,
  repository: codecommit.IRepository,
  project: codebuild.Project) => {

  const source = new codepipeline.Artifact('Source');
  pipeline.addStage({
      stageName: 'Source',
      actions: [
          new codepipeline_actions.CodeCommitSourceAction({
              actionName: 'Source',
              repository,
              output: source,
              role: codeCommitRole(pipeline.stack),
          }),
      ]
  });
  const buildOutput = new codepipeline.Artifact();
// The problem is caused when adding following stage:
  pipeline.addStage({
      stageName: 'Build',
      actions: [
          new codepipeline_actions.CodeBuildAction({
              actionName: 'Build',
              input: source,
              project,
              outputs: [buildOutput],
          }),
      ]
  });
  return buildOutput;
}

Environment

  • CLI Version : 1.14.0
  • Framework Version: 1.14.0
  • OS : Macbook
  • Language : TypeScript

Other


This is 🐛 Bug Report

@markusl markusl added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 23, 2019
@nmussy
Copy link
Contributor

nmussy commented Oct 23, 2019

Hey @markusl,

It looks like the error was added by @skinny85's PR, #4183. Not sure if it's intended or not.

@skinny85
Copy link
Contributor

Yes, @nmussy is correct. The project you now pass to CodeBuildAction needs to be a PipelineProject instead of just Project.

@SomayaB SomayaB added the @aws-cdk/aws-codepipeline Related to AWS CodePipeline label Oct 23, 2019
@SomayaB SomayaB added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 23, 2019
@markusl
Copy link
Contributor Author

markusl commented Oct 24, 2019

I see. For us this is a breaking change and require us to update our stacks and pipelines. We previously used just Project which worked fine for our use.

The nasty thing is that when updating all pipelines, we again hit the #4465 and need to manually deploy all pipelines one-by-one to avoid hitting the resource limit.

@PeterBengtson
Copy link

The CDK team must stop mishandling semantic versioning. This is a breaking change, one of far too many. It's a problem.

@SomayaB SomayaB removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Oct 24, 2019
@fulghum
Copy link
Contributor

fulghum commented Oct 24, 2019

Apologies that this caused some unintended churn for customers. We have discussed backwards compatibility on the team a lot and understand that it is a critical expectation for all GA modules in the AWS Construct Library. I followed up on this specific change with the team and we are looking at removing the type check error for the next release.

Please let us know if you are hit by other backwards incompatible changes in GA construct library modules. We strive hard to ensure we don't make breaking changes and if we've missed something, we want to know so we can improve our process.

@markusl
Copy link
Contributor Author

markusl commented Oct 25, 2019

When we quite mechanically converted our pipelines to CDK, we weren't aware of differences between PipelineProject and Project . TBH it still is not quite clear to me and the documentation does not help: "A representation of a CodeBuild Project" vs. "A convenience class for CodeBuild Projects that are used in CodePipeline.".

The versioning scheme is not problematic for us, and neither is minor change in code but since #4465 this became quite much work for not a clear reason. If you need customers to move to the proper construct, would it be possible to add a some kind of warning first before enforcing build-breaking change?

@eladb
Copy link
Contributor

eladb commented Oct 25, 2019

I think the validation should be that it is a Project with a CodePipeline source and not necessarily a PipelineProject, which is just a convenience wrapper for a Project with a CodePipeline source.

@skinny85
Copy link
Contributor

@PeterBengtson @markusl apologies for the breakage. This was a result of us misunderstanding how the interaction between CodeBuild and CodePipeline works. We were under the impression that if you added a CodeBuild project to a CodePipeline with source that had a value other than CODEPIPELINE, the resulting pipeline would fail at runtime. That's why we added the validation - so that you would get feedback earlier in the development process. We thought there's no valid use case for having a project with source other than CODEPIPELINE added to a pipeline, because it wouldn't have worked anyway.

What we failed to realize was that actually neither CodeBuild nor CodePipeline do any validation of this. What happens is that the execution of the project triggered from the pipeline simply overrides any source set on the project level, and simply passes the source from the CodePipeline's S3 bucket.

I've submitted a PR reverting this validation here: #4689 . It will be released in 1.15.0.

Again, apologies for the bad experience.

@mergify mergify bot closed this as completed in #4689 Oct 26, 2019
mergify bot pushed a commit that referenced this issue Oct 26, 2019
@PeterBengtson
Copy link

PeterBengtson commented Oct 26, 2019

Thanks for reversing this, but I really think the problem is wider and involves the development methodology and design philosophy choices made by the CDK team in areas such as framework abstraction, generality and consistency. It also has to do with how you introduce changes. All the trouble you've given us when you've changed existing features – in all areas, not just with pipelines – could very easily have been avoided if you've simply put the choice in your users' hands. That the CDK team doesn't do this by default is very worrying.

@eladb
Copy link
Contributor

eladb commented Oct 26, 2019

Can you please provide some more examples? We are extremely intentional about breaking changes to the API, but I am wondering if there are other aspects of “breaking behavior” that we are missing.

@PeterBengtson
Copy link

Sure. I'm preparing a letter to you and Adam about these things; it should be ready by Monday. My team and I are working in Fintech, in a highly controlled PCI+ environment where every change is scrutinised in detail by several internal and external security bodies. Everything you do to our infrastructure without informing us beforehand, or without giving us a choice, creates Jira tickets and meetings. Had the CDK team kept to the real meaning of semver, we would have no extra work.

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 bug This issue is a bug.
Projects
None yet
7 participants