fix(cr-gateway): Remove GitHub API requirement from CR server. #255
Conversation
technosophos
force-pushed
the
fix/cr-doesnt-use-github-api
branch
from
22763d3
to
0b4a23a
Compare
| b := &brigade.Build{ | ||
| ProjectID: proj.ID, | ||
| Type: "image_push", | ||
| Provider: "dockerhub", | ||
| Commit: commit, | ||
| Payload: payload, | ||
| Script: brigadeJS, |
There was a problem hiding this comment.
Can we really just defer determining the script to be executed for a build like this?
As far as I understand, brigade-controller as of today doesn't have an ability to populate missing brigade.js(=script in a secret representing an brigade event) - it looks like we have to populate it on gateway-side.
I'm writing this referring the followings:
- https://github.com/Azure/brigade/search?utf8=%E2%9C%93&q=syncSecret&type=
- https://github.com/Azure/brigade/search?utf8=%E2%9C%93&q=Script&type=
Please correct me if I'm missing something!
There was a problem hiding this comment.
Just noticed that brigade does handle this case here.
Sorry for bothering!
For anyone interested, in nutshell brigade seems to create a vcs-sidecar(as an init-container) to populate /vcs/brigade.js only when script isn't included in an event. Then, the brigade-worker runs either /etc/brigade/script(corresponds to the script included in a brigade.Build=secret) or /vcs/brigade.js according to this code. So, if we omit a script in a brigade.Build coming from the cr-gateway, the worker just tries to git-clone the necessary script=brigade.js from the vcs repo. No problem.
There was a problem hiding this comment.
Thanks for taking the time to look through all of that.
| @@ -12,8 +12,7 @@ cloneURL: "https://github.com/deis/empty-testbed.git" | |||
| sharedSecret: "IBrakeForSeaBeasts" | |||
|
|
|||
| # OPTIONAL: Use this to have Brigade update your project about the build. | |||
| # You probably want this if you want pull requests or commits to show | |||
| # the build status. | |||
| # This is REQUIRED for the GitHub gateway, but optional otherwise. | |||
Relates to #225