Skip to content

Commit

Permalink
Added Installation Info Wrapper.
Browse files Browse the repository at this point in the history
This decouples processing of "installation" events from the production
of the message for further processing. Implementation is inspered to
EntityInfoWrapper.
  • Loading branch information
blkt committed May 28, 2024
1 parent e6f9a2e commit a26fafd
Show file tree
Hide file tree
Showing 6 changed files with 671 additions and 85 deletions.
42 changes: 27 additions & 15 deletions internal/controlplane/fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package controlplane

import (
"bytes"
"context"
"crypto/hmac"
"crypto/sha256"
"encoding/hex"
Expand Down Expand Up @@ -60,14 +61,20 @@ var eventTypes = [23]string{
"pull_request",
}

// FuzzGithubEventParsers tests Minders two GH event parsers:
// FuzzGithubEventParsers tests Minder's GH event parsers:
//
// 1: parseGithubEventForProcessing
// 2: parseGithubAppEventForProcessing
// - processPingEvent
// - processRelevantRepositoryEvent
// - processRepositoryEvent
// - processPackageEvent
// - processInstallationAppEvent
//
// It also tests validatePayloadSignature given it contains a fair
// amount of logic specific to Minder that depends on external input.
//
// The fuzzer does not validate return values of the parsers. It tests if any
// input can cause code-level issues.
func FuzzGithubEventParsers(f *testing.F) {
func FuzzGitHubEventParsers(f *testing.F) {
f.Fuzz(func(t *testing.T, rawWHPayload []byte, target, eventEnum uint) {
mac := hmac.New(sha256.New, []byte("test"))
mac.Write(rawWHPayload)
Expand Down Expand Up @@ -114,22 +121,27 @@ func FuzzGithubEventParsers(f *testing.F) {
whConfig := &server.WebhookConfig{WebhookSecretFile: whSecretFile.Name()}

s := &Server{}
ctx := context.Background()

switch target % 2 {
switch target % 6 {
case 0:
payload, err := validatePayloadSignature(req, whConfig)
if err != nil {
return
}
//nolint:gosec // The fuzzer does not validate the return values
s.parseGithubEventForProcessing(payload, m)
s.processInstallationAppEvent(ctx, rawWHPayload)
case 1:
payload, err := github.ValidatePayload(req, []byte(secret))
if err != nil {
return
}
//nolint:gosec // The fuzzer does not validate the return values
s.parseGithubAppEventForProcessing(payload, m)
s.processRelevantRepositoryEvent(ctx, rawWHPayload)
case 2:
//nolint:gosec // The fuzzer does not validate the return values
s.processRepositoryEvent(ctx, rawWHPayload)
case 3:
//nolint:gosec // The fuzzer does not validate the return values
s.processPackageEvent(ctx, rawWHPayload)
case 4:
//nolint:gosec // The fuzzer does not validate the return values
s.processPingEvent(ctx, rawWHPayload)
case 5:
//nolint:gosec // The fuzzer does not validate the return values
validatePayloadSignature(req, whConfig)
}
})
}
50 changes: 32 additions & 18 deletions internal/controlplane/handlers_githubwebhooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,20 @@ func (i *installation) GetID() int64 {
return 0
}

// processingResult struct contains the sole information necessary to
// send a message out from the handler, namely a destination topic and
// an object that knows how to "convert itself" to a watermill message.
//
// It is supposed to just be an easy, uniform way of returning
// results.
type processingResult struct {
// destination topic
topic string
eiw *entities.EntityInfoWrapper
// wrapper object for repository, pull-request, and artifact
// (package) events.
eiw *entities.EntityInfoWrapper
// wrapper object for installation (app) events
iiw *installations.InstallationInfoWrapper
}

// HandleGitHubAppWebhook handles incoming GitHub App webhooks
Expand Down Expand Up @@ -350,7 +361,7 @@ func (s *Server) HandleGitHubAppWebhook() http.HandlerFunc {
wes.Error = false
s.processPingEvent(ctx, rawWBPayload)
case "installation":
res, processingErr = s.processInstallationAppEvent(ctx, rawWBPayload, m)
res, processingErr = s.processInstallationAppEvent(ctx, rawWBPayload)
default:
l.Info().Msgf("webhook event %s not handled", wes.Typ)
}
Expand All @@ -367,10 +378,15 @@ func (s *Server) HandleGitHubAppWebhook() http.HandlerFunc {
}

if res != nil {
wes.Accepted = true
l.Info().Str("message-id", m.UUID).Msg("publishing event for execution")
if err := res.iiw.ToMessage(m); err != nil {
wes.Error = true
log.Printf("Error creating event: %v", err)
w.WriteHeader(http.StatusInternalServerError)
return
}

if err := s.evt.Publish(installations.ProviderInstallationTopic, m); err != nil {
if err := s.evt.Publish(res.topic, m); err != nil {
wes.Error = true
log.Printf("Error publishing message: %v", err)
w.WriteHeader(http.StatusInternalServerError)
Expand Down Expand Up @@ -450,15 +466,19 @@ func (s *Server) HandleGitHubWebHook() http.HandlerFunc {
// reconciliation or, in some cases, a deletion.
case "repository", "meta":
res, processingErr = s.processRelevantRepositoryEvent(ctx, rawWBPayload)
case "branch_protection_rule",
case "branch_protection_configuration",
"branch_protection_rule",
"code_scanning_alert",
"create",
"member",
"public",
"push",
"repository_advisory",
"repository_import",
"repository_ruleset",
"repository_vulnerability_alert",
"secret_scanning_alert",
"secret_scanning_alert_location",
"security_advisory",
"security_and_analysis",
"team",
Expand All @@ -471,11 +491,6 @@ func (s *Server) HandleGitHubWebHook() http.HandlerFunc {
case "pull_request":
res, processingErr = s.processPullRequestEvent(ctx, rawWBPayload)
// This event is not currently handled.
case "branch_protection_configuration",
"repository_advisory",
"repository_ruleset",
"secret_scanning_alert_location":
l.Info().Msgf("webhook events %s not handled", wes.Typ)
case "org_block",
"organization":
l.Info().Msgf("webhook events %s do not contain repo", wes.Typ)
Expand All @@ -486,6 +501,8 @@ func (s *Server) HandleGitHubWebHook() http.HandlerFunc {
wes.Accepted = false
wes.Error = false
s.processPingEvent(ctx, rawWBPayload)
default:
l.Info().Msgf("webhook event %s not handled", wes.Typ)
}

if processingErr != nil {
Expand Down Expand Up @@ -841,11 +858,10 @@ func (s *Server) processPullRequestEvent(
// the app itself as well as the list of accessible repositories.
//
// There are several possible actions, but in the current user flows
// we only process.
// we only process deletion.
func (_ *Server) processInstallationAppEvent(
_ context.Context,
payload []byte,
msg *message.Message,
) (*processingResult, error) {
var event *installationEvent
if err := json.Unmarshal(payload, &event); err != nil {
Expand Down Expand Up @@ -877,15 +893,13 @@ func (_ *Server) processInstallationAppEvent(
return nil, fmt.Errorf("error marshalling payload: %w", err)
}

// We could use something like an EntityInfoWrapper here as
// well.
installations.ProviderInstanceRemovedMessage(
msg,
db.ProviderClassGithubApp,
payloadBytes)
iiw := installations.NewInstallationInfoWrapper().
WithProviderClass(db.ProviderClassGithubApp).
WithPayload(payloadBytes)

res := &processingResult{
topic: installations.ProviderInstallationTopic,
iiw: iiw,
}

return res, nil
Expand Down
Loading

0 comments on commit a26fafd

Please sign in to comment.