-
Notifications
You must be signed in to change notification settings - Fork 30
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
watchbot binaries only built for the Head commit in a push? #268
Comments
We should also investigate if this is related to the events that the CodePipeline webhook is triggered for. Currently, it's triggered on |
@freenerd and I paired on this today, and here are some findings:
This did not work as expected, since there's a filter tag applied on the GitHub source, currently created on code-pipeline-helper. We want to modify the code to use this filter only if the
@rclark, I see a couple of workarounds here:
|
This is a really good thing to learn about CodePipeline as it would pertain to any kind of CI pipeline that you might use it for -- I was curious how/whether it backlogs work or otherwise handles concurrency. Thanks for figuring it out! So let's talk about code-pipeline-helper. This library is a shim for CodePipeline's unacceptable intergration-with-github system. To create a pipeline you have to provide it with a github personal access token (strike one) in plain-text (strike two), and if you're generating the pipeline in CloudFormation, that means the personal token exists in plain-text either in the template itself or as a stack parameter (strike three). While building out this shim, I found that CodePipeline's new webhooks (which aren't supported by CloudFormation, strike 4) have this At the end of the day I don't want us to spend very much time "improving" on this shim. This is why the code is sitting in one of my personal repositories. Consider what value CodePipeline is offering you in this situation:
I would argue that given CodePipeline's challenges, and given the fact that this particular CI system doesn't have more than one step, it might not be the right tool for the job. We might be better of with a proven system like we use elsewhere of github app event --> hookshot --> codebuild. And all that said, I would advocate that we backlog changes here for now. Tagged releases ought to be rare enough that, knowing about CodePipeline concurrency issues, we ought to be able to anticipate and make sure that we get the tagged binaries that we expect until we can make a smoother integration. |
@rclark 👍 thanks for outlining the issues with CodePipeline and
Yep, except in our case there's no dependency on any branch, since tags are global to a repository. When we experimented with the filtered branch set to
I think the issue is that CodePipeline seems to be closed for development, in general, even ignoring this current PR. So if anyone at Mapbox were to want to develop using code-pipeline-helper, and modify the code-base for a specific use-case, they currently cannot, since the CodeBucket and CodeKey values are not configurable in master, and the S3 buckets and prefixes containing the code are closed off. Do you see an issue with allowing for one of these? Should we not use code-pipeline-helper (And CodePipeline?) at Mapbox considering the security and support unavailability flaws? |
@freenerd and I are going to pair on the |
Was looking to diagnose why v4.9.1 binaries weren't built - and I'm thinking the webhook is only pushing the head commit in a
git push
to codepipeline? Or CodePipeline is only meant to build the head commit?If true - not sure that this is necessarily a bug; but it's something to be aware of in any case.
Code pipeline ran for bfa1544 and 7fd2475 but couldn't find a record of dd75368, the commit with the tags.
cc: @mapbox/platform-engine-room
The text was updated successfully, but these errors were encountered: