-
Notifications
You must be signed in to change notification settings - Fork 3
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
Implementing GitHub webhook handler #331
base: fred/approval-service-skeleton-1
Are you sure you want to change the base?
Changes from 1 commit
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,141 @@ | ||
package webhook | ||
|
||
import ( | ||
"fmt" | ||
"log/slog" | ||
"net/http" | ||
|
||
"github.com/google/go-github/v63/github" | ||
) | ||
|
||
// EventHandlerFunc is a function that handles a webhook event. | ||
// The function should return an error if the event could not be handled. | ||
// If the error is not nil, the webhook will respond with a 500 Internal Server Error. | ||
// | ||
// It is important that this is non-blocking and does not perform any long-running operations. | ||
// GitHub will close the connection if the webhook does not respond within 10 seconds. | ||
// | ||
// Example usage: | ||
// | ||
// func(event interface{}) error { | ||
// switch event := event.(type) { | ||
// case *github.CommitCommentEvent: | ||
// processCommitCommentEvent(event) | ||
// case *github.CreateEvent: | ||
// processCreateEvent(event) | ||
// ... | ||
// } | ||
// return nil | ||
// } | ||
type EventHandlerFunc func(event interface{}) error | ||
|
||
// Handler is an implementation of [http.Handler] that handles GitHub webhook events. | ||
type Handler struct { | ||
eventHandler EventHandlerFunc | ||
secretToken []byte | ||
log *slog.Logger | ||
} | ||
|
||
var _ http.Handler = &Handler{} | ||
|
||
type Opt func(*Handler) error | ||
|
||
// WithSecretToken sets the secret token for the webhook. | ||
// The secret token is used to create a hash of the request body, which is sent in the X-Hub-Signature header. | ||
// If not set, the webhook will not verify the signature of the request. | ||
// | ||
// For more information, see: https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries | ||
func WithSecretToken(secretToken []byte) Opt { | ||
return func(p *Handler) error { | ||
p.secretToken = secretToken | ||
return nil | ||
} | ||
} | ||
|
||
// WithLogger sets the logger for the webhook. | ||
func WithLogger(log *slog.Logger) Opt { | ||
return func(p *Handler) error { | ||
p.log = log | ||
return nil | ||
} | ||
} | ||
|
||
var defaultOpts = []Opt{ | ||
WithLogger(slog.Default()), | ||
} | ||
|
||
// NewHandler creates a new webhook handler. | ||
func NewHandler(eventHandler EventHandlerFunc, opts ...Opt) *Handler { | ||
h := Handler{ | ||
eventHandler: eventHandler, | ||
} | ||
for _, opt := range defaultOpts { | ||
opt(&h) | ||
} | ||
|
||
for _, opt := range opts { | ||
opt(&h) | ||
} | ||
return &h | ||
} | ||
|
||
// Headers is a list of special headers that are sent with a webhook request. | ||
// For more information, see: https://docs.github.com/en/webhooks/webhook-events-and-payloads#delivery-headers | ||
type Headers struct { | ||
// GithubHookID is the unique identifier of the webhook. | ||
GithubHookID string | ||
// GithubEvent is the type of event that triggered the delivery. | ||
GithubEvent string | ||
// GithubDelivery is a globally unique identifier (GUID) to identify the event | ||
GithubDelivery string | ||
// GitHubHookInstallationTargetType is the type of resource where the webhook was created. | ||
GitHubHookInstallationTargetType string | ||
// GitHubHookInstallationTargetID is the unique identifier of the resource where the webhook was created. | ||
GitHubHookInstallationTargetID string | ||
|
||
// HubSignature256 is the HMAC hex digest of the response body. | ||
// Is generated with the SHA-256 algorithm with a shared secret used as the HMAC key. | ||
// This header will be sent if the webhook is configured with a secret. | ||
HubSignature256 string | ||
} | ||
|
||
// ServeHTTP handles a webhook request. | ||
func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
// Parse headers for debugging and audit purposes. | ||
var head Headers | ||
head.GithubHookID = r.Header.Get("X-GitHub-Hook-ID") | ||
head.GithubEvent = r.Header.Get("X-GitHub-Event") | ||
head.GithubDelivery = r.Header.Get("X-GitHub-Delivery") | ||
head.GitHubHookInstallationTargetType = r.Header.Get("X-GitHub-Hook-Installation-Target-Type") | ||
head.GitHubHookInstallationTargetID = r.Header.Get("X-GitHub-Hook-Installation-Target-ID") | ||
head.HubSignature256 = r.Header.Get("X-Hub-Signature-256") | ||
|
||
payload, err := github.ValidatePayload(r, h.secretToken) // If secretToken is empty, the signature will not be verified. | ||
if err != nil { | ||
h.log.Warn("webhook validation failed", "headers", head) | ||
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. In all these cases we should probably include some information about the request itself in the log, for debugging purposes. Or maybe the handler (or possibly the ingress gateway) should be able to log the entire request if debug logging is enabled? |
||
http.Error(w, "invalid request", http.StatusBadRequest) | ||
return | ||
} | ||
|
||
event, err := github.ParseWebHook(head.GithubEvent, payload) | ||
if err != nil { | ||
h.log.Error("failed to parse webhook event", "error", err) | ||
http.Error(w, "invalid request", http.StatusBadRequest) | ||
return | ||
} | ||
|
||
if err := h.eventHandler(event); err != nil { | ||
h.log.Error("failed to handle webhook event", "error", err) | ||
http.Error(w, "internal server error", http.StatusInternalServerError) | ||
return | ||
} | ||
|
||
// Respond to the request. | ||
w.WriteHeader(http.StatusOK) | ||
} | ||
|
||
// String returns a string representation of the Headers. | ||
func (h *Headers) String() string { | ||
return fmt.Sprintf("GithubHookID: %s\nGithubEvent: %s\nGithubDelivery: %s\nGitHubHookInstallationTargetType: %s\nGitHubHookInstallationTargetID: %s\nHubSignature256: %s\n", | ||
h.GithubHookID, h.GithubEvent, h.GithubDelivery, h.GitHubHookInstallationTargetType, h.GitHubHookInstallationTargetID, h.HubSignature256) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,19 @@ | ||
package main | ||
|
||
import ( | ||
"context" | ||
"log" | ||
"log/slog" | ||
"net/http" | ||
|
||
"github.com/google/go-github/v69/github" | ||
"github.com/gravitational/shared-workflows/libs/github/webhook" | ||
"github.com/gravitational/trace" | ||
"golang.org/x/sync/errgroup" | ||
) | ||
|
||
var logger = slog.Default() | ||
|
||
// Process: | ||
// 1. Take in events from CI/CD systems | ||
// 2. Extract common information | ||
|
@@ -28,19 +42,22 @@ func main() { | |
eventSources := []EventSource{ | ||
NewGitHubEventSource(processor), | ||
} | ||
|
||
for _, eventSource := range eventSources { | ||
_ = eventSource.Setup() | ||
} | ||
|
||
done := make(chan struct{}) // TODO replace with error? | ||
// 3. Start event sources | ||
eg, ctx := errgroup.WithContext(context.Background()) | ||
for _, eventSource := range eventSources { | ||
_ = eventSource.Run(done) | ||
eg.Go(func() error { | ||
return eventSource.Run(ctx) | ||
}) | ||
} | ||
|
||
// Block until an event source has a fatal error | ||
<-done | ||
close(done) | ||
if err := eg.Wait(); err != nil { | ||
log.Fatal(err) | ||
} | ||
} | ||
|
||
// This contains information needed to process a request | ||
|
@@ -99,12 +116,15 @@ type EventSource interface { | |
Setup() error | ||
|
||
// Handle actual requests. This should not block. | ||
Run(chan struct{}) error | ||
Run(ctx context.Context) error | ||
} | ||
|
||
type GitHubEventSource struct { | ||
processor ApprovalProcessor | ||
// TODO | ||
|
||
deployReviewChan chan *github.DeploymentReviewEvent | ||
addr string | ||
srv *http.Server | ||
} | ||
|
||
func NewGitHubEventSource(processor ApprovalProcessor) *GitHubEventSource { | ||
|
@@ -114,43 +134,73 @@ func NewGitHubEventSource(processor ApprovalProcessor) *GitHubEventSource { | |
// Setup GH client, webhook secret, etc. | ||
// https://github.com/go-playground/webhooks may help here | ||
func (ghes *GitHubEventSource) Setup() error { | ||
// TODO | ||
deployReviewChan := make(chan *github.DeploymentReviewEvent) | ||
ghes.deployReviewChan = deployReviewChan | ||
|
||
mux := http.NewServeMux() | ||
eventProcessor := webhook.EventHandlerFunc(func(event interface{}) error { | ||
switch event := event.(type) { | ||
case *github.DeploymentReviewEvent: | ||
deployReviewChan <- event | ||
return nil | ||
default: | ||
return trace.Errorf("unknown event type: %T", event) | ||
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. This library only exists for Teleport error handling, so we probably shouldn't be using it anymore outside of the product (here and elsewhere) |
||
} | ||
}) | ||
mux.Handle("/webhook", webhook.NewHandler( | ||
eventProcessor, | ||
webhook.WithSecretToken([]byte("secret-token")), // TODO: get from config | ||
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. Maybe this should always take a string? It's a string when configured with GH, and I don't see when we would want to pass a byte array to this func. |
||
webhook.WithLogger(logger), | ||
)) | ||
|
||
ghes.srv = &http.Server{ | ||
Addr: ghes.addr, | ||
Handler: mux, | ||
} | ||
|
||
return nil | ||
} | ||
|
||
// Take incoming events and respond to them | ||
func (ghes *GitHubEventSource) Run(done chan struct{}) error { | ||
// If anything errors, deny the request. For safety, maybe `defer` | ||
// the "response" function? | ||
go func() { | ||
// Notify the service that the listener is completely done. | ||
// Normally this should only be hit if there is a fatal error | ||
defer func() { done <- struct{}{} }() | ||
func (ghes *GitHubEventSource) Run(ctx context.Context) error { | ||
errc := make(chan error) | ||
|
||
// Incoming webhook payloads | ||
// This should be closed by the webhook listener func | ||
payloads := make(chan interface{}) | ||
|
||
// Listen for webhook calls | ||
go ghes.listenForPayloads(payloads, done) | ||
// Start the HTTP server | ||
go func() { | ||
logger.Info("Listening for GitHub Webhooks", "address", ghes.addr) | ||
errc <- ghes.srv.ListenAndServe() | ||
close(errc) | ||
}() | ||
|
||
for payload := range payloads { | ||
go ghes.processWebhookPayload(payload, done) | ||
// Process incoming events | ||
go func() { | ||
defer close(ghes.deployReviewChan) | ||
logger.Info("Starting GitHub event processor") | ||
for { | ||
select { | ||
case <-ctx.Done(): | ||
return | ||
case deployReview := <-ghes.deployReviewChan: | ||
// Process the event | ||
go ghes.processDeploymentReviewEvent(deployReview) | ||
} | ||
} | ||
}() | ||
|
||
return nil | ||
} | ||
|
||
// Listen for incoming webhook events. Register HTTP routes, start server, etc. Long running, blocking. | ||
func (ghes *GitHubEventSource) listenForPayloads(payloads chan interface{}, done chan struct{}) { | ||
// Once a call is received, it should return a 200 response immediately. | ||
var err error | ||
// This will block until an error occurs or the context is done | ||
select { | ||
case err = <-errc: | ||
ghes.srv.Shutdown(context.Background()) // Ignore error - we're already handling one | ||
case <-ctx.Done(): | ||
err = ghes.srv.Shutdown(context.Background()) | ||
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. These should probably take a time-limited shutdown context stemming from |
||
<-errc // flush the error channel to avoid a goroutine leak | ||
} | ||
|
||
// TODO | ||
return trace.Wrap(err) | ||
} | ||
|
||
// Given an event, approve or deny it. This is a long running, blocking function. | ||
func (ghes *GitHubEventSource) processWebhookPayload(payload interface{}, done chan struct{}) { | ||
func (ghes *GitHubEventSource) processDeploymentReviewEvent(payload *github.DeploymentReviewEvent) error { | ||
// Do GitHub-specific checks. Don't approve based off ot this - just deny | ||
// if one fails. | ||
automatedDenial, err := ghes.performAutomatedChecks(payload) | ||
|
@@ -170,6 +220,11 @@ func (ghes *GitHubEventSource) processWebhookPayload(payload interface{}, done c | |
} | ||
|
||
_ = ghes.respondToDeployRequest(true, payload) | ||
return nil | ||
} | ||
|
||
// Given an event, approve or deny it. This is a long running, blocking function. | ||
func (ghes *GitHubEventSource) processWebhookPayload(payload interface{}, done chan struct{}) { | ||
} | ||
|
||
// Turns GH-specific information into "common" information for the approver | ||
|
@@ -183,10 +238,13 @@ func (ghes *GitHubEventSource) convertWebhookPayloadToEvent(payload interface{}) | |
|
||
// Performs approval checks that are GH-specific. This should only be used to deny requests, | ||
// never approve them. | ||
func (ghes *GitHubEventSource) performAutomatedChecks(payload interface{}) (pass bool, err error) { | ||
func (ghes *GitHubEventSource) performAutomatedChecks(payload *github.DeploymentReviewEvent) (pass bool, err error) { | ||
// Verify request is from Gravitational org repo | ||
// Verify request is from Gravitational org member | ||
// See RFD for additional examples | ||
if *payload.Organization.Login != "gravitational" { | ||
return true, nil | ||
} | ||
|
||
return false, nil | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,9 @@ | ||
module github.com/gravitational/shared-workflows/tools/approval-service | ||
|
||
go 1.23.5 | ||
|
||
require ( | ||
github.com/google/go-github/v63 v63.0.0 // indirect | ||
github.com/google/go-github/v69 v69.1.0 // indirect | ||
github.com/google/go-querystring v1.1.0 // indirect | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= | ||
github.com/google/go-github/v63 v63.0.0 h1:13xwK/wk9alSokujB9lJkuzdmQuVn2QCPeck76wR3nE= | ||
github.com/google/go-github/v63 v63.0.0/go.mod h1:IqbcrgUmIcEaioWrGYei/09o+ge5vhffGOcxrO0AfmA= | ||
github.com/google/go-github/v69 v69.1.0 h1:ljzwzEsHsc4qUqyHEJCNA1dMqvoTK3YX2NAaK6iprDg= | ||
github.com/google/go-github/v69 v69.1.0/go.mod h1:xne4jymxLR6Uj9b7J7PyTpkMYstEMMwGZa0Aehh1azM= | ||
github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8= | ||
github.com/google/go-querystring v1.1.0/go.mod h1:Kcdr2DB4koayq7X8pmAG4sNG59So17icRSOU623lUBU= | ||
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= |
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.
I think that we should error if the secret token is empty and a signature is provided. If this happens, we've probably misconfigured something.