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

watchbot binaries only built for the Head commit in a push? #268

Open
brendanmcfarland opened this issue Jul 13, 2018 · 5 comments
Open
Labels

Comments

@brendanmcfarland
Copy link
Contributor

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.

screenshot 2018-07-13 16 13 41

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

@arunasank
Copy link
Contributor

We should also investigate if this is related to the events that the CodePipeline webhook is triggered for. Currently, it's triggered on push events, but we must investigate if the push event is triggered only on the HEAD commit when a series of commits are pushed together.

@arunasank
Copy link
Contributor

@freenerd and I paired on this today, and here are some findings:

  • The GitHub webhook builds on HEAD commits alone - so when a series of commits are pushed together, the CodePipeline build is triggered for the latest commit. If the tag commit was not the latest commit, a build is not triggered, and therefore, a binary not generated.
  • To resolve this, we:
    • Updated the webhook triggers to be triggered on "Branch/Tag" creation instead of on the push event
    • Tried to create an event by committing to this repo, and then creating a tag and pushing the tag.

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 Branch property is set when the code-pipeline-helper custom resource is created, but when we tried to do this, we were blocked by two things:

  • Permissions to update the code pipeline code, and update the code-pipeline-helper-production stack seem to only be allowed for @rclark currently, since accessing the code-pipeline-helper S3 bucket which contains the lambda code for the code-pipeline custom resource is disallowed, even though the Lambda code itself is accessible. Registering this package with a library like stork will necessitate making the CodeBucket and CodeKey configurable since code-pipeline helper is a public repository. So currently, @freenerd and I are blocked from modifying the code there and testing the new changes on the code-pipeline-helper stacks. 😅

@rclark, I see a couple of workarounds here:

  • Creating a private fork of code-pipeline-helper, registering it with the Mapbox stork github app, and creating all stacks and modification using this modified code.
  • Making the CodeBucket and CodeKey configurable, so more folks can collaborate on the code-pipeline-helper repository

@rclark
Copy link
Contributor

rclark commented Jul 24, 2018

when a series of commits are pushed together, the CodePipeline build is triggered for the latest commit

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 .filter setting. What is confusing about that is that the filter could possibly not match the branch specified in the Source action (see here and here). These two things could be contradictory, and I don't know what the outcome would be. I believe that's exactly what you did which didn't work as expected.

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:

  1. Its hooking up a repository to a CI pipeline (though we have to struggle to make that work)
  2. The pipeline has one single action: run a codebuild project that builds and uploads the binary

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.

@arunasank
Copy link
Contributor

arunasank commented Jul 25, 2018

@rclark 👍 thanks for outlining the issues with CodePipeline and code-pipeline-helper. Perhaps it's best if we do away with CodePipeline and use another event chain like the one you have outlined, or even clarify this in the docs to clarify the dependence between the webook, the tag and the binary creation.

While building out this shim, I found that CodePipeline's new webhooks (which aren't supported by CloudFormation, strike 4) have this .filter setting. What is confusing about that is that the filter could possibly not match the branch specified in the Source action (see here and here). These two things could be contradictory, and I don't know what the outcome would be. I believe that's exactly what you did which didn't work as expected.

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 master and the CodePipeline webhook set to trigger whenever a tag was created, we did not see an event when the global tag was created.

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.

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?

@arunasank
Copy link
Contributor

@rclark 👍 thanks for outlining the issues with CodePipeline and code-pipeline-helper. Perhaps it's best if we do away with CodePipeline and use another event chain like the one you have outlined, or even clarify this in the docs to clarify the dependence between the webook, the tag and the binary creation.

@freenerd and I are going to pair on the github app event --> hookshot --> codebuild chain tomorrow, and timebox a fix for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants