-
Notifications
You must be signed in to change notification settings - Fork 15
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
SHIP : Git event-driven build executions #41
base: main
Are you sure you want to change the base?
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,300 @@ | ||
<!-- | ||
Copyright The Shipwright Contributors | ||
|
||
SPDX-License-Identifier: Apache-2.0 | ||
--> | ||
|
||
--- | ||
title: event-driven-builds | ||
authors: | ||
- "@sbose787" | ||
|
||
reviewers: | ||
- "@gmontero" | ||
- "@adamkaplan" | ||
- "@ImJasonH" | ||
- "@SaschaSchwarze0" | ||
- "@HeavyWombat" | ||
|
||
approvers: | ||
- "@adamkaplan" | ||
- "@SaschaSchwarze0" | ||
|
||
|
||
|
||
|
||
creation-date: 2021-11-03 | ||
status: implementable | ||
|
||
--- | ||
|
||
# Git Event-driven triggering of Shipwright Builds | ||
|
||
|
||
## Release Signoff Checklist | ||
|
||
- [x] Enhancement is `implementable` | ||
- [ ] Design details are appropriately documented from clear requirements | ||
- [ ] Test plan is defined | ||
- [ ] Graduation criteria for dev preview, tech preview, GA | ||
- [ ] User-facing documentation is created in [docs](/docs/) | ||
|
||
## Open Questions [optional] | ||
|
||
1. Almost every user would need an exposed `Service`, how do we create a vendor-agnostic `Ingress` object ? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like something for a Helm Chart? Users could choose between Ingress and Route. Since we don't have a Helm Chart yet, installation instructions should do it. |
||
|
||
|
||
## Summary | ||
|
||
Trigger the execution of an image build based on a commit/push event from a relevant source code repository. | ||
|
||
|
||
|
||
## Motivation | ||
|
||
This enhancement proposal aims to provide an API to enable users to express the intent of having their builds triggered by events from a Git Repository. | ||
|
||
|
||
### Goals | ||
|
||
* Build a technology-agostic user experience for Git-triggered build executions. | ||
* Add support for 'reacting' to events from Github and Gitlab repositories. | ||
|
||
|
||
|
||
### Non-Goals | ||
|
||
* Support events from generic git servers. The implementation should be extensible to support those in a non-breaking manner. | ||
* Creation of the vendor-specific Ingress resources to expose the webhook URL. | ||
|
||
|
||
## Proposal | ||
|
||
|
||
### User Stories [optional] | ||
|
||
Detail the things that people will be able to do if this is implemented. Include as much detail as | ||
possible so that people can understand the "how" of the system. The goal here is to make this feel | ||
real for users without getting bogged down. | ||
|
||
#### Story 1 | ||
As a user, I would like to define a `Build` and trigger the execution of the same upon pushes to my Git repository. | ||
|
||
#### Story 2 | ||
As a user, I would like to define a `Build` and the execution of the same for pushes to any branch in that Git repository. | ||
|
||
#### Story 3 | ||
As a user, I would like to configure a secure webhook URL for triggering `Builds`. | ||
|
||
|
||
### Implementation Notes | ||
|
||
Upon specification of the **new** API field `.spec.webhook`, the Shipwright Build Controller will do the needful to generate a webhook URL and | ||
provide the information on the same in the `status` of the `Build` resource. | ||
|
||
|
||
``` | ||
spec: | ||
... | ||
... | ||
webhook: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could generalize this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 ..... image change triggers could be a peer type like git/scm webhook triggers ... and a build can have multiple "trigger types" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm good with the change you've put in for this @sbose78 |
||
type: github | ||
imageTagPolicy: short_sha # optional, allowed values: 'short_sha' , 'branch'. Defaults to 'branch'. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure if mixing in the concept of image tag policies here into the webhook is correct. Imo an image tag policy should be outside of it as it also makes sense as a standalone concept. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. This was mainly brought it to address the issue of receiving events for all branches. I wonder if we should probably support:
We could definitely de-scope things and say, we'll only act on events which come in from the .spec.source.revision only. My over-enthusiastic self wanted to dabble with more than that. |
||
secretRef: # optional, will be genereated if not specified. | ||
name: my-webhook-secret. | ||
Comment on lines
+97
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if optional is reasonable here because if we generate it, then the user will need to decode it to get the webhook token to then store it in GitHub, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct, this is more of a knob of convenience :) |
||
``` | ||
|
||
The `.status` sub-resource would contain the information | ||
|
||
``` | ||
status: | ||
webhook: | ||
status: live | ||
reason: "" # to be populated in case of error. | ||
secretRef: # mandatory field, in-secure not an option. | ||
name: _user_specified_or_generated | ||
serviceRef: # kubernetes service which needs to be exposed. | ||
name: _name_of_the_recieveing_webhook_traffic | ||
``` | ||
|
||
Here's what a full Build resource would look like : | ||
``` | ||
kind: Build | ||
metadata: | ||
name: buildpack-nodejs-build | ||
spec: | ||
source: | ||
url: https://github.com/shipwright-io/sample-nodejs | ||
contextDir: source-build | ||
strategy: | ||
name: buildpacks-v3 | ||
kind: ClusterBuildStrategy | ||
output: | ||
image: docker.io/${REGISTRY_ORG}/sample-nodejs:latest | ||
credentials: | ||
name: push-secret | ||
webhook: | ||
type: github | ||
imageTagPolicy: short_sha # optional, allowed values: 'short_sha' , 'branch'. Defaults to 'branch'. | ||
secretRef: # optional, will be genereated if not specified. | ||
name: my-webhook-secret. | ||
status: | ||
... | ||
... | ||
webhook: | ||
status: live | ||
reason: "" # to be populated in case of error. | ||
secretRef: # mandatory field, in-secure not an option. | ||
name: _user_specified_or_generated | ||
serviceRef: # kubernetes service which needs to be exposed. | ||
name: _name_of_the_recieveing_webhook_traffic | ||
``` | ||
|
||
#### Pre-requisities | ||
|
||
The following items ( part of "Bill of materials" ) would need to be shipped with the Shipwright installation so that they could be consumed in the webhook-generation process: | ||
|
||
1. A `ClusterTriggerBinding` which exposes `$(body.head_commit.id)` and `$(body.ref)` from the webhook payload. | ||
|
||
``` | ||
apiVersion: triggers.tekton.dev/v1alpha1 | ||
kind: ClusterTriggerBinding | ||
metadata: | ||
name: github-shipwright-webhook | ||
spec: | ||
params: | ||
- name: commit | ||
value: $(body.head_commit.id) | ||
- name: branch | ||
value: $(body.ref) | ||
``` | ||
|
||
Similar `ClusterTriggerBinding`s need to be shipped for `Gitlab` and `BitBucket`. | ||
|
||
2. A "Custom Tekton Task" controller with the following behaviour: | ||
* _watches_ `Tekton` `Run` resources referencing Shipwright's `Build` resources. | ||
* Expects the commit ID and branch name in the `params`. | ||
* Based on the above information, the controller would generate the following: | ||
* A `Build` with the revision and output image tag overwritten with the above information based on the optional `.spec.webhook.imageTagPolicy`. | ||
* A `BuildRun` referencing the above `Build`. | ||
|
||
|
||
|
||
#### Webhook endpoint creation process | ||
|
||
|
||
Upon creation of a `Build` resource by the user, the following would occur: | ||
|
||
1. The Shipwright Build Controller would create a `TriggerTemplate` that would map the information coming in from the event into the "parameters" | ||
expected by the `Run` resource ie, the branch name & the commit ID. | ||
|
||
``` | ||
|
||
### Generated by the `Build` reconciler. | ||
|
||
apiVersion: triggers.tekton.dev/v1alpha1 | ||
kind: TriggerTemplate | ||
metadata: | ||
name: my-build-name | ||
spec: | ||
params: | ||
- name: branch | ||
- name: commit | ||
resourceTemplates: | ||
- apiVersion: tekton.dev/v1alpha1 | ||
kind: Run | ||
metadata: | ||
generateName: build-execution- | ||
spec: | ||
ref: | ||
apiVersion: shipwright.io/v1alpha1 | ||
kind: Build | ||
name: my-build-bame | ||
timeout: 3000s | ||
params: | ||
- name: branch | ||
value: $(tt.params.branch) | ||
- name: commit | ||
value: $(tt.params.commit) | ||
``` | ||
|
||
2. The Shipwright Build Controller would create a `EventListener` under-the-hood per build. | ||
|
||
``` | ||
### Generated by the `Build` reconciler. | ||
|
||
apiVersion: triggers.tekton.dev/v1alpha1 | ||
kind: EventListener | ||
metadata: | ||
name: name-of-the-build | ||
spec: | ||
serviceAccountName: pipeline | ||
triggers: | ||
- bindings: | ||
- ref: github-shipwright-webhook | ||
template: | ||
name: name-of-the-build | ||
|
||
``` | ||
|
||
And that's it, you may now go ahead and | ||
|
||
### Test Plan | ||
|
||
To be filled. | ||
|
||
### Release Criteria | ||
|
||
To be filled. | ||
|
||
#### Removing a deprecated feature [if necessary] | ||
|
||
N/A | ||
|
||
#### Upgrade Strategy [if necessary] | ||
|
||
N/A | ||
|
||
### Risks and Mitigations | ||
|
||
1. This feature opens up the possiblility of triggering Build executions for branches which weren't explicitly specified. | ||
|
||
This isn't a risk per se since only repository committers would have the permissions needed to push branches. Therefore, | ||
as long as the image is tagged appropriately to indiciate that it is not built off 'main' ( or the branch specified iniially by the user in the `Build`), | ||
this should not be a problem. | ||
|
||
While the enhancement could have been scoped to only deal with the branch/revision explicitly specified in the `Build` resource, | ||
doing the same would have have excluded some key use cases where users push to a different branch before merging to 'main'. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, but what is the user's expectation? I'd say if I specify a Build with a source revision set to main, that I only want commits into main to trigger a BuildRun. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did go in that direction and quickly found it to be too cumbersome to have to define a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. We'll have to think about a good solution. Let's look at the scenario first: in my experience, repositories typically only have one or two major branches (main and develop) - having two Builds for this would be okayish for me because of how we use those branches internally (the images go into different repos). The other branches you have in mind, would not that have been pull requests ? Because that would make sense for me as a scenario: define branch A in the build and define that pull requests for that branch should also be built. Another scenario where branches do not play any role would be to build a tag once it gets pushed. |
||
|
||
2. Exposing a webhook URL enables creation of pods ( ie, processes on the node ) by actors who may not necessarily have access to the cluster. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As part of the installation instructions, we must include instructions for the cluster admin to only allow access to certain IP addresses. This filtering ability completely depend on the cluster capabilities, like for instance when using a service mesh in place. |
||
|
||
This does open up an attack vector given the execution of the build is done using the configured service account. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Given we are dealing with external access, we need to be clear that it does require special attention from the cluster admin. |
||
|
||
* Webhook-driven builds are being designed to be secure by default - the usage of a webhook secret is mandatory. | ||
* The resulting `BuildRun` would be annotated with relevant metadata from the webhook event so that it's easy to trace the actor responsible for the trigger. | ||
|
||
|
||
## Drawbacks | ||
|
||
1. Is this even a Shipwright concern or should this be the concern of a general-purpose CI ? | ||
( to be filled ) | ||
|
||
|
||
|
||
|
||
## Alternatives | ||
|
||
Similar to the `Drawbacks` section the `Alternatives` section is used to highlight and record other | ||
possible approaches to delivering the value proposed by an enhancement. | ||
Comment on lines
+308
to
+309
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How much simpler would it be if we implement our own webhook endpoint that directly consumes the event data from GitHub or GitLab? I personally think, it is much simpler. And it will be trivial to implement logic there that checks the branch of the payload and compares it with what is defined in the Build. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean - write a simple http service which understands the payload and creates corresponding Build/BuildRuns? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. |
||
|
||
## Infrastructure Needed [optional] | ||
|
||
Use this section if you need things from the project. Examples include a new subproject, repos | ||
requested, github details, and/or testing infrastructure. | ||
|
||
Listing these here allows the community to get the process for these resources started right away. | ||
|
||
## Implementation History | ||
|
||
Major milestones in the life cycle of a proposal should be tracked in `Implementation History`. | ||
|
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.
Little typo here, the extra
7
at the end.