From a2fc3e06bc8415e311eae066184ac1a1a6f9b5df Mon Sep 17 00:00:00 2001 From: Michelangelo Mori Date: Wed, 15 May 2024 15:51:02 +0200 Subject: [PATCH] Make github webhook easier to extend with new events for auto registration. The idea is to make it easier to extend github webhook with events that are tied to more than one repository, namely "installation" and "installation_repositories", which are necessary for repo auto registration. As part of this change, processing of events is moved inside routines specific to that event. This is work in progress. --- .../controlplane/handlers_githubwebhooks.go | 960 ++++++++++-------- .../handlers_githubwebhooks_test.go | 5 +- 2 files changed, 515 insertions(+), 450 deletions(-) diff --git a/internal/controlplane/handlers_githubwebhooks.go b/internal/controlplane/handlers_githubwebhooks.go index b43b419b53..12d7d88285 100644 --- a/internal/controlplane/handlers_githubwebhooks.go +++ b/internal/controlplane/handlers_githubwebhooks.go @@ -26,8 +26,8 @@ import ( "io" "mime" "net/http" + "reflect" "sort" - "strconv" "strings" "github.com/ThreeDotsLabs/watermill/message" @@ -48,7 +48,6 @@ import ( "github.com/stacklok/minder/internal/providers/github/installations" ghprov "github.com/stacklok/minder/internal/providers/github/service" "github.com/stacklok/minder/internal/repositories" - "github.com/stacklok/minder/internal/util" "github.com/stacklok/minder/internal/verifier/verifyif" pb "github.com/stacklok/minder/pkg/api/protobuf/go/minder/v1" provifv1 "github.com/stacklok/minder/pkg/providers/v1" @@ -77,40 +76,45 @@ func newErrNotHandled(smft string, args ...any) error { // https://docs.github.com/en/webhooks/webhook-events-and-payloads#about-webhook-events-and-payloads var repoEvents = []string{ - "branch_protection_configuration", - "branch_protection_rule", - "code_scanning_alert", - "create", // a tag or branch is created - "member", - "meta", // webhook itself - "repository_vulnerability_alert", - "org_block", - "organization", - "public", + "branch_protection_configuration", // not in go-github + "repository_advisory", // not in go-github + "repository_ruleset", // not in go-github + "secret_scanning_alert_location", // not in go-github + "branch_protection_rule", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#BranchProtectionRuleEvent + "code_scanning_alert", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#CodeScanningAlertEvent + "create", // a tag or branch is created // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#CreateEvent + "member", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#MemberEvent + "meta", // webhook itself // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#MetaEvent + "repository_vulnerability_alert", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#RepositoryVulnerabilityAlertEvent + "org_block", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#OrgBlockEvent + "organization", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#OrganizationEvent + "public", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#PublicEvent // listening to push makes sure we evaluate on pushes to branches we need to check, but might be too noisy // for topic branches - "push", - "repository", - "repository_advisory", - "repository_import", - "repository_ruleset", - "secret_scanning_alert", - "secret_scanning_alert_location", - "security_advisory", - "security_and_analysis", - "team", - "team_add", + "push", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#PushEvent + "repository", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#RepositoryEvent + "repository_import", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#RepositoryImportEvent + "secret_scanning_alert", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#SecretScanningAlertEvent + "security_advisory", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#SecurityAdvisoryEvent + "security_and_analysis", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#SecurityAndAnalysisEvent + "team", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#TeamEvent + "team_add", // https://pkg.go.dev/github.com/google/go-github/v62@v62.0.0/github#TeamAddEvent } // WebhookActionEventDeleted is the action for a deleted event const ( + WebhookActionEventAdded = "added" WebhookActionEventDeleted = "deleted" WebhookActionEventOpened = "opened" WebhookActionEventClosed = "closed" WebhookActionEventSynchronize = "synchronize" - WebhookActionEventPublished = "published" ) +type processingResult struct { + topic string + eiw *entities.EntityInfoWrapper +} + func entityFromWebhookEventTypeKey(m *message.Message) pb.Entity { key := m.Metadata.Get(events.GithubWebhookEventTypeKey) switch { @@ -118,6 +122,9 @@ func entityFromWebhookEventTypeKey(m *message.Message) pb.Entity { return pb.Entity_ENTITY_ARTIFACTS case key == "pull_request": return pb.Entity_ENTITY_PULL_REQUESTS + case key == "installation_repositories": + // not sure if it has to be considered a "repo event" + return pb.Entity_ENTITY_REPOSITORIES // temporary hack case slices.Contains(repoEvents, key): return pb.Entity_ENTITY_REPOSITORIES } @@ -128,6 +135,7 @@ func entityFromWebhookEventTypeKey(m *message.Message) pb.Entity { // HandleGitHubAppWebhook handles incoming GitHub App webhooks func (s *Server) HandleGitHubAppWebhook() http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() wes := &metrics.WebhookEventState{ Typ: "unknown", Accepted: false, @@ -145,18 +153,22 @@ func (s *Server) HandleGitHubAppWebhook() http.HandlerFunc { } wes.Typ = github.WebHookType(r) + event, err := github.ParseWebHook(github.WebHookType(r), rawWBPayload) + if err != nil { + log.Error().Err(err).Msg("Error parsing github webhook message") + } if wes.Typ == "ping" { - logPingReceivedEvent(r.Context(), rawWBPayload) + s.processPingEvent(ctx, event.(*github.PingEvent)) wes.Error = false return } m := message.NewMessage(uuid.New().String(), nil) m.Metadata.Set(events.ProviderDeliveryIdKey, github.DeliveryID(r)) - m.Metadata.Set(events.ProviderSourceKey, "https://api.github.com/") // TODO: handle other sources + // TODO: handle other sources + m.Metadata.Set(events.ProviderSourceKey, "https://api.github.com/") m.Metadata.Set(events.GithubWebhookEventTypeKey, wes.Typ) - ctx := r.Context() l := zerolog.Ctx(ctx).With(). Str("webhook-event-type", m.Metadata[events.GithubWebhookEventTypeKey]). Str("providertype", m.Metadata[events.ProviderTypeKey]). @@ -218,11 +230,6 @@ func (s *Server) HandleGitHubWebHook() http.HandlerFunc { } wes.Typ = github.WebHookType(r) - if wes.Typ == "ping" { - logPingReceivedEvent(r.Context(), rawWBPayload) - wes.Error = false - return - } // TODO: extract sender and event time from payload portably m := message.NewMessage(uuid.New().String(), nil) @@ -240,79 +247,458 @@ func (s *Server) HandleGitHubWebHook() http.HandlerFunc { l.Debug().Msg("parsing event") - // Parse the webhook event and construct a message for the event router - if err := s.parseGithubEventForProcessing(rawWBPayload, m); err != nil { - wes = handleParseError(wes.Typ, err) + event, err := github.ParseWebHook(github.WebHookType(r), rawWBPayload) + if err != nil { + l.Error().Err(err).Msg("Error parsing github webhook message") + } + + var res *processingResult + var processingErr error + switch event := event.(type) { + case *github.PingEvent: + // For ping events, we do not set wes.Accepted + // to true because they're not relevant + // business events. + wes.Error = false + processingErr = s.processPingEvent(ctx, event) + case *github.PackageEvent: + // This is an artifact-related event, and can + // only trigger a reconciliation. + res, processingErr = s.processPackageEvent(ctx, event, m) + case *github.InstallationEvent: + wes.Accepted = true + res, processingErr = s.processInstallationEvent(ctx, event, m) + case *github.InstallationRepositoriesEvent: + // This event occurs whenever the github app + // is installed or modified, changing it from + // allowing access to "all" repos to a + // "selected" list, or vice versa. + // + // We handle this as a mass registration or + // deletion of repositories. + wes.Accepted = true + res, processingErr = s.processInstallationRepositoriesEvent(ctx, event, m) + case *github.MetaEvent: + // As per github documentation, MetaEvent is + // triggered when the webhook that this event + // is configured on is deleted. + // + // Our action here is to de-register the + // related repo. + wes.Accepted = true + res, processingErr = s.processMetaEvent(ctx, event, m) + case *github.BranchProtectionRuleEvent, + *github.CodeScanningAlertEvent, + *github.CreateEvent, + *github.MemberEvent, + *github.OrgBlockEvent, + *github.OrganizationEvent, + *github.PublicEvent, + *github.PushEvent, + *github.RepositoryEvent, + *github.RepositoryImportEvent, + *github.RepositoryVulnerabilityAlertEvent, + *github.SecretScanningAlertEvent, + *github.SecurityAdvisoryEvent, + *github.SecurityAndAnalysisEvent, + *github.TeamAddEvent, + *github.TeamEvent: + // All these events are related to a repo and + // usually contain an action. They all trigger + // a reconciliation or, in some cases, a + // deletion. + // + // This routine requires a bit of manual + // dispatch because (a) different event types + // access the repo differently and (b) some + // event types don't have an explicit action. + wes.Accepted = true + res, processingErr = s.processGenericRepositoryEvent(ctx, event, m) + default: + l.Info().Msgf("webhook event %s not handled", wes.Typ) + } + + if processingErr != nil { + wes = handleParseError(wes.Typ, processingErr) if wes.Error { w.WriteHeader(http.StatusInternalServerError) + return } else { w.WriteHeader(http.StatusOK) + return } - return } - wes.Accepted = true - l.Info().Str("message-id", m.UUID).Msg("publishing event for execution") - // Channel the event based on the webhook action - var watermillTopic string - if shouldIssueDeletionEvent(m) { - watermillTopic = events.TopicQueueReconcileEntityDelete - } else { - watermillTopic = events.TopicQueueEntityEvaluate - } + // res is null only when a ping event occurred. + if res != nil { + if err := res.eiw.ToMessage(m); err != nil { + wes.Error = true + log.Printf("Error creating event: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } - // Publish the message to the event router - if err := s.evt.Publish(watermillTopic, m); err != nil { - wes.Error = true - log.Printf("Error publishing message: %v", err) - w.WriteHeader(http.StatusInternalServerError) - return + // Publish the message to the event router + if err := s.evt.Publish(res.topic, m); err != nil { + wes.Error = true + log.Printf("Error publishing message: %v", err) + w.WriteHeader(http.StatusInternalServerError) + return + } } // We successfully published the message wes.Error = false w.WriteHeader(http.StatusOK) + return } } -// logPingReceivedEvent logs the type of token used to authenticate the webhook. The idea is to log a link between the -// repo and the token type. Since this is done only for the ping event, we can assume that the sender is the app that -// installed the webhook on the repository. -func logPingReceivedEvent(ctx context.Context, rawWHPayload []byte) { +// processPingEvent logs the type of token used to authenticate the +// webhook. The idea is to log a link between the repo and the token +// type. Since this is done only for the ping event, we can assume +// that the sender is the app that installed the webhook on the +// repository. +func (s *Server) processPingEvent( + ctx context.Context, + event *github.PingEvent, +) error { l := zerolog.Ctx(ctx).With().Logger() - var payload map[string]any - err := json.Unmarshal(rawWHPayload, &payload) - if err == nil { - repoInfo, ok := payload["repository"].(map[string]any) - if ok { - // Log the repository ID and URL if available - repoID, err := parseRepoID(repoInfo["id"]) - if err == nil { - l = l.With().Int64("github-repository-id", repoID).Logger() - } - repoUrl := repoInfo["html_url"].(string) - l = l.With().Str("github-repository-url", repoUrl).Logger() + if event.GetRepo() != nil { + l = l.With().Int64("github-repository-id", event.GetRepo().GetID()).Logger() + l = l.With().Str("github-repository-url", event.GetRepo().GetHTMLURL()).Logger() + } + if event.GetSender() != nil { + l = l.With().Str("sender-login", event.GetSender().GetLogin()).Logger() + l = l.With().Str("github-repository-url", event.GetSender().GetHTMLURL()).Logger() + if strings.Contains(event.GetSender().GetHTMLURL(), "github.com/apps") { + l = l.With().Str("sender-token-type", "github-app").Logger() + } else { + l = l.With().Str("sender-token-type", "oauth-app").Logger() } + } - // During the ping event, the sender corresponds to the app that installed the webhook on the repository - if payload["sender"] != nil { - // Log the sender if available - senderLogin, err := util.JQReadFrom[string](ctx, ".sender.login", payload) - if err == nil { - l = l.With().Str("sender-login", senderLogin).Logger() - } - senderHTMLUrl, err := util.JQReadFrom[string](ctx, ".sender.html_url", payload) - if err == nil { - if strings.Contains(senderHTMLUrl, "github.com/apps") { - l = l.With().Str("sender-token-type", "github-app").Logger() - } else { - l = l.With().Str("sender-token-type", "oauth-app").Logger() - } - } + l.Debug().Msg("ping received") + return nil +} + +func (s *Server) processPackageEvent( + ctx context.Context, + event *github.PackageEvent, + msg *message.Message, +) (*processingResult, error) { + if event.GetPackage() == nil || event.GetRepo() == nil { + log.Printf("could not determine relevant entity for event. Skipping execution.") + return nil, nil // this is awkward + } + + if event.GetPackage().GetOwner() == nil { + return nil, errors.New("could not determine articfact owner") + } + + dbrepo, err := s.fetchRepo(ctx, event.GetRepo()) + if err != nil { + return nil, err + } + + provider, err := s.providerManager.InstantiateFromID(ctx, dbrepo.ProviderID) + if err != nil { + log.Printf("error instantiating provider: %v", err) + return nil, err + } + + cli, err := provifv1.As[provifv1.GitHub](provider) + if err != nil { + log.Printf("error instantiating provider: %v", err) + return nil, err + } + + tempArtifact, err := gatherArtifact(ctx, cli, event) + if err != nil { + return nil, fmt.Errorf("error gathering versioned artifact: %w", err) + } + + dbArtifact, err := s.store.UpsertArtifact(ctx, db.UpsertArtifactParams{ + RepositoryID: uuid.NullUUID{ + UUID: dbrepo.ID, + Valid: true, + }, + ArtifactName: tempArtifact.GetName(), + ArtifactType: tempArtifact.GetTypeLower(), + ArtifactVisibility: tempArtifact.Visibility, + ProjectID: dbrepo.ProjectID, + ProviderID: dbrepo.ProviderID, + ProviderName: dbrepo.Provider, + }) + if err != nil { + return nil, fmt.Errorf("error upserting artifact: %w", err) + } + + _, pbArtifact, err := artifacts.GetArtifact(ctx, s.store, dbrepo.ProjectID, dbArtifact.ID) + if err != nil { + return nil, fmt.Errorf("error getting artifact with versions: %w", err) + } + // TODO: wrap in a function + pbArtifact.Versions = tempArtifact.Versions + + eiw := entities.NewEntityInfoWrapper(). + WithArtifact(pbArtifact). + WithProviderID(dbrepo.ProviderID). + WithProjectID(dbrepo.ProjectID). + WithRepositoryID(dbrepo.ID). + WithArtifactID(dbArtifact.ID). + WithActionEvent(event.GetAction()) + + return &processingResult{topic: events.TopicQueueEntityEvaluate, eiw: eiw}, nil +} + +func (s *Server) processGenericRepositoryEvent( + ctx context.Context, + event any, + msg *message.Message, +) (*processingResult, error) { + var repo *github.Repository + if getter := reflect.ValueOf(event).MethodByName("GetRepo"); getter.IsValid() { + repo = getter.Call([]reflect.Value{})[0].Interface().(*github.Repository) + } else if getter := reflect.ValueOf(event).MethodByName("GetRepository"); getter.IsValid() { + repo = getter.Call([]reflect.Value{})[0].Interface().(*github.Repository) + } else { + // TODO mic I believe this was a log and not an error + return nil, fmt.Errorf("event %T does not have a repo associated", event) + } + + action := "" + if getter := reflect.ValueOf(event).MethodByName("GetAction"); getter.IsValid() { + action = getter.Call([]reflect.Value{})[0].Interface().(string) + } + + return s.innerProcessGenericRepositoryEvent( + ctx, + action, + repo, + msg, + ) +} + +// processRepositoryEvent handles events coming from the webhook of a +// specific repo. +// +// More precisely, it does +// - looks the repository up in the DB, short-circuiting in case +// it's unknown +// - if repo is known, issues the right operation given the event's +// action +func (s *Server) processRepositoryEvent( + ctx context.Context, + event *github.RepositoryEvent, + msg *message.Message, +) (*processingResult, error) { + // Check fields mandatory for processing the event + if event.GetAction() == "" { + return nil, errors.New("event action is null") + } + if event.GetRepo() == nil { + return nil, errRepoNotFound + } + + return s.innerProcessGenericRepositoryEvent( + ctx, + event.GetAction(), + event.GetRepo(), + msg, + ) +} + +func (s *Server) innerProcessGenericRepositoryEvent( + ctx context.Context, + action string, + repo *github.Repository, + msg *message.Message, +) (*processingResult, error) { + if repo.GetID() == 0 { + return nil, errors.New("event repo id is null") + } + + log.Printf("handling event for repository %d", repo.GetID()) + + dbrepo, err := s.fetchRepo(ctx, repo) + if err != nil { + return nil, err + } + + // protobufs are our API, so we always execute on these instead of the DB directly. + pbRepo := repositories.PBRepositoryFromDB(*dbrepo) + eiw := entities.NewEntityInfoWrapper(). + WithProviderID(dbrepo.ProviderID). + WithRepository(pbRepo). + WithProjectID(dbrepo.ProjectID). + WithRepositoryID(dbrepo.ID). + WithActionEvent(action) + + topic := events.TopicQueueEntityEvaluate + if action == "deleted" { + topic = events.TopicQueueReconcileEntityDelete + } + if action == "created" { + // TODO find right topic + // topic = events.TopicQueueReconcileRepoInit + } + + return &processingResult{topic: topic, eiw: eiw}, nil +} + +// processMetaEvent handles events related to the webhook itself. As +// per GitHub's documentation, the only possible action is "delete", +// in which case we have to de-register the related repo. +func (s *Server) processMetaEvent( + ctx context.Context, + event *github.MetaEvent, + msg *message.Message, +) (*processingResult, error) { + // Check fields mandatory for processing the event + if event.GetAction() != "delete" { + // "delete" is the only allowed action for "meta" + // events + return nil, errors.New(`event action is not "delete"`) + } + if event.GetRepo() == nil { + return nil, errRepoNotFound + } + if event.GetRepo().GetID() == 0 { + return nil, errors.New("event repo id is null") + } + + log.Printf("handling event for repository %d", event.GetRepo().GetID()) + + dbrepo, err := s.fetchRepo(ctx, event.GetRepo()) + if err != nil { + return nil, err + } + + if dbrepo.WebhookID.Valid { + // Check if the payload webhook ID matches the one we + // have stored in the DB for this repository + if event.GetHookID() != dbrepo.WebhookID.Int64 { + // This means we got a deleted event for a + // webhook ID that doesn't correspond to the + // one we have stored in the DB. + return nil, newErrNotHandled("meta event with action %s not handled, hook ID %d does not match stored webhook ID %d", + event.GetAction(), + event.GetHookID(), + dbrepo.WebhookID.Int64, + ) } } - l.Debug().Msg("ping received") + // If we get this far it means we got a deleted event for a + // webhook ID that corresponds to the one we have stored in + // the DB. We will remove the repo from the DB, so we can + // proceed with the deletion event for this entity + // (repository). + + // TODO: perhaps handle this better by trying to re-create the + // webhook if it was deleted manually + + // protobufs are our API, so we always execute on these instead of the DB directly. + repo := repositories.PBRepositoryFromDB(*dbrepo) + eiw := entities.NewEntityInfoWrapper(). + WithProviderID(dbrepo.ProviderID). + WithRepository(repo). + WithProjectID(dbrepo.ProjectID). + WithRepositoryID(dbrepo.ID). + WithActionEvent(event.GetAction()) + + return &processingResult{topic: events.TopicQueueReconcileEntityDelete, eiw: eiw}, nil +} + +// processInstallationEvent processes events related to changes to the +// list of repositories accessible by the GitHub app. +// +// There are two possible actions, "created" or "deleted", but both +// struct fields mapping to "repositories_added" can be accessed +// regardless. +func (s *Server) processInstallationEvent( + ctx context.Context, + event *github.InstallationEvent, + msg *message.Message, +) (*processingResult, error) { + // Check fields mandatory for processing the event + if event.GetAction() == "" { + return nil, errors.New(`invalid event action`) + } + + // TODO check if provider/installation is configured for + // auto-registration + + for _, repo := range event.Repositories { + log.Printf("ADD %+v", repo) + } + + return nil, errors.New("TODO") +} + +// processInstallationRepositoriesEvent processes events related to +// changes to the list of repositories accessible by the GitHub app. +// +// There are two possible actions, "added" or "removed", but both +// struct fields mapping to "repositories_added" and +// "repositories_removed" can be accessed regardless. +func (s *Server) processInstallationRepositoriesEvent( + ctx context.Context, + event *github.InstallationRepositoriesEvent, + msg *message.Message, +) (*processingResult, error) { + // Check fields mandatory for processing the event + if event.GetAction() != "added" && + event.GetAction() != "removed" { + return nil, errors.New(`event action is neither "added" nor "removed"`) + } + + // maybe do something with event.GetRepositorySelection() + // which can be either "all" or "selected" + + // TODO check if provider/installation is configured for + // auto-registration + + for _, repo := range event.RepositoriesAdded { + log.Printf("ADD %+v", repo) + } + for _, repo := range event.RepositoriesRemoved { + log.Printf("DEL %+v", repo) + } + + return nil, errors.New("TODO") +} + +func (s *Server) fetchRepo( + ctx context.Context, + repo *github.Repository, +) (*db.Repository, error) { + dbrepo, err := s.store.GetRepositoryByRepoID(ctx, repo.GetID()) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + log.Printf("repository %d not found", repo.GetID()) + // no use in continuing if the repository doesn't exist + return nil, fmt.Errorf("repository %d not found: %w", + repo.GetID(), + errRepoNotFound, + ) + } + return nil, fmt.Errorf("error getting repository: %w", err) + } + + if repo.GetPrivate() { + if !features.ProjectAllowsPrivateRepos(ctx, s.store, dbrepo.ProjectID) { + return nil, errRepoIsPrivate + } + } + + if dbrepo.ProjectID.String() == "" { + return nil, fmt.Errorf("no project found for repository %s/%s: %w", + dbrepo.RepoOwner, dbrepo.RepoName, errRepoNotFound) + } + + return &dbrepo, nil } // NoopWebhookHandler is a no-op handler for webhooks @@ -478,266 +864,34 @@ func (_ *Server) parseGithubAppEventForProcessing( return nil } -func (s *Server) parseGithubEventForProcessing( - rawWHPayload []byte, - msg *message.Message, -) error { - ent := entityFromWebhookEventTypeKey(msg) - if ent == pb.Entity_ENTITY_UNSPECIFIED { - return newErrNotHandled("event %s not handled", msg.Metadata.Get(events.GithubWebhookEventTypeKey)) - } - - ctx := context.Background() - - var payload map[string]any - if err := json.Unmarshal(rawWHPayload, &payload); err != nil { - return fmt.Errorf("error unmarshalling payload: %w", err) - } - - // get information about the repository from the payload - dbRepo, err := getRepoInformationFromPayload(ctx, s.store, payload) - if err != nil { - return fmt.Errorf("error getting repo information from payload: %w", err) - } - - var action string // explicit declaration to use the default value - action, err = util.JQReadFrom[string](ctx, ".action", payload) - if err != nil && !errors.Is(err, util.ErrNoValueFound) { - return fmt.Errorf("error getting action from payload: %w", err) - } - - // determine if the payload is an artifact published event - // TODO: this needs to be managed via signals - if ent == pb.Entity_ENTITY_ARTIFACTS && action == WebhookActionEventPublished { - return s.parseArtifactPublishedEvent(ctx, payload, msg, dbRepo, dbRepo.ProviderID, action) - } else if ent == pb.Entity_ENTITY_PULL_REQUESTS { - return s.parsePullRequestModEvent(ctx, payload, msg, dbRepo, dbRepo.ProviderID, action) - } else if ent == pb.Entity_ENTITY_REPOSITORIES { - return parseRepoEvent(payload, msg, dbRepo, action) - } - - return newErrNotHandled("event %s with action %s not handled", - msg.Metadata.Get(events.GithubWebhookEventTypeKey), action) -} - -func parseRepoEvent( - whPayload map[string]any, - msg *message.Message, - dbrepo db.Repository, - action string, -) error { - if action == WebhookActionEventDeleted { - // Find out what kind of repository event we are dealing with - if whPayload["hook"] != nil || whPayload["hook_id"] != nil { - // Having these means it's a repository event related to the webhook itself, i.e., deleted, created, etc. - // Get the webhook ID from the payload - whID := whPayload["hook_id"].(float64) - if dbrepo.WebhookID.Valid { - // Check if the payload webhook ID matches the one we have stored in the DB for this repository - if int64(whID) != dbrepo.WebhookID.Int64 { - // This means we got a deleted event for a webhook ID that doesn't correspond to the one we have stored in the DB. - return newErrNotHandled("delete event %s with action %s not handled, hook ID %d does not match stored webhook ID %d", - msg.Metadata.Get(events.GithubWebhookEventTypeKey), action, int64(whID), dbrepo.WebhookID.Int64) - } - } - // This means we got a deleted event for a webhook ID that corresponds to the one we have stored in the DB. - // We will remove the repo from the DB, so we can proceed with the deletion event for this entity (repository) - // TODO: perhaps handle this better by trying to re-create the webhook if it was deleted manually - } - // If we don't have hook/hook_id, continue with the deletion event for this entity (repository) - } - - // protobufs are our API, so we always execute on these instead of the DB directly. - repo := repositories.PBRepositoryFromDB(dbrepo) - eiw := entities.NewEntityInfoWrapper(). - WithProviderID(dbrepo.ProviderID). - WithRepository(repo). - WithProjectID(dbrepo.ProjectID). - WithRepositoryID(dbrepo.ID). - WithActionEvent(action) - - return eiw.ToMessage(msg) -} - -func (s *Server) parseArtifactPublishedEvent( - ctx context.Context, - whPayload map[string]any, - msg *message.Message, - dbrepo db.Repository, - providerID uuid.UUID, - action string, -) error { - // we need to have information about package and repository - if whPayload["package"] == nil || whPayload["repository"] == nil { - log.Printf("could not determine relevant entity for event. Skipping execution.") - return nil - } - - provider, err := s.providerManager.InstantiateFromID(ctx, providerID) - if err != nil { - log.Printf("error instantiating provider: %v", err) - return err - } - - cli, err := provifv1.As[provifv1.GitHub](provider) - if err != nil { - log.Printf("error instantiating provider: %v", err) - return err - } - - tempArtifact, err := gatherArtifact(ctx, cli, whPayload) - if err != nil { - return fmt.Errorf("error gathering versioned artifact: %w", err) - } - - dbArtifact, err := s.store.UpsertArtifact(ctx, db.UpsertArtifactParams{ - RepositoryID: uuid.NullUUID{ - UUID: dbrepo.ID, - Valid: true, - }, - ArtifactName: tempArtifact.GetName(), - ArtifactType: tempArtifact.GetTypeLower(), - ArtifactVisibility: tempArtifact.Visibility, - ProjectID: dbrepo.ProjectID, - ProviderID: dbrepo.ProviderID, - ProviderName: dbrepo.Provider, - }) - if err != nil { - return fmt.Errorf("error upserting artifact: %w", err) - } - - _, pbArtifact, err := artifacts.GetArtifact(ctx, s.store, dbrepo.ProjectID, dbArtifact.ID) - if err != nil { - return fmt.Errorf("error getting artifact with versions: %w", err) - } - // TODO: wrap in a function - pbArtifact.Versions = tempArtifact.Versions - - eiw := entities.NewEntityInfoWrapper(). - WithArtifact(pbArtifact). - WithProviderID(dbrepo.ProviderID). - WithProjectID(dbrepo.ProjectID). - WithRepositoryID(dbrepo.ID). - WithArtifactID(dbArtifact.ID). - WithActionEvent(action) - - return eiw.ToMessage(msg) -} - -func (s *Server) parsePullRequestModEvent( +// This routine assumes that all necessary validation is performed on +// the upper layer and accesses package and repo without checking for +// nulls. +func gatherArtifactInfo( ctx context.Context, - whPayload map[string]any, - msg *message.Message, - dbrepo db.Repository, - providerID uuid.UUID, - action string, -) error { - provider, err := s.providerManager.InstantiateFromID(ctx, providerID) - if err != nil { - log.Printf("error instantiating provider: %v", err) - return err - } - - cli, err := provifv1.As[provifv1.GitHub](provider) - if err != nil { - log.Printf("error instantiating provider: %v", err) - return err - } - - prEvalInfo, err := getPullRequestInfoFromPayload(ctx, whPayload) - if err != nil { - return fmt.Errorf("error getting pull request information from payload: %w", err) - } - - dbPr, err := s.reconcilePrWithDb(ctx, dbrepo, prEvalInfo) - if errors.Is(err, errNotHandled) { - return err - } else if err != nil { - return fmt.Errorf("error reconciling PR with DB: %w", err) - } - - err = updatePullRequestInfoFromProvider(ctx, cli, dbrepo, prEvalInfo) - if err != nil { - return fmt.Errorf("error updating pull request information from provider: %w", err) - } - - log.Printf("evaluating PR %+v", prEvalInfo) - - eiw := entities.NewEntityInfoWrapper(). - WithPullRequest(prEvalInfo). - WithPullRequestID(dbPr.ID). - WithProviderID(dbrepo.ProviderID). - WithProjectID(dbrepo.ProjectID). - WithRepositoryID(dbrepo.ID). - WithActionEvent(action) - - return eiw.ToMessage(msg) -} - -func extractArtifactFromPayload(ctx context.Context, payload map[string]any) (*pb.Artifact, error) { - artifactName, err := util.JQReadFrom[string](ctx, ".package.name", payload) - if err != nil { - return nil, err - } - artifactType, err := util.JQReadFrom[string](ctx, ".package.package_type", payload) - if err != nil { - return nil, err - } - ownerLogin, err := util.JQReadFrom[string](ctx, ".package.owner.login", payload) - if err != nil { - return nil, err - } - repoName, err := util.JQReadFrom[string](ctx, ".repository.full_name", payload) - if err != nil { - return nil, err + client provifv1.GitHub, + event *github.PackageEvent, +) (*pb.Artifact, error) { + owner := "" + if event.GetPackage().GetOwner() != nil { + owner = event.GetPackage().GetOwner().GetLogin() } artifact := &pb.Artifact{ - Owner: ownerLogin, - Name: artifactName, - Type: artifactType, - Repository: repoName, + Owner: owner, + Name: event.GetPackage().GetName(), + Type: event.GetPackage().GetPackageType(), + Repository: event.GetRepo().GetFullName(), // visibility and createdAt are not in the payload, we need to get it with a REST call } - return artifact, nil -} - -func extractArtifactVersionFromPayload(ctx context.Context, payload map[string]any) (*pb.ArtifactVersion, error) { - packageVersionId, err := util.JQReadFrom[float64](ctx, ".package.package_version.id", payload) - if err != nil { - return nil, err - } - packageVersionSha, err := util.JQReadFrom[string](ctx, ".package.package_version.version", payload) - if err != nil { - return nil, err - } - tag, err := util.JQReadFrom[string](ctx, ".package.package_version.container_metadata.tag.name", payload) - if err != nil { - return nil, err - } - - version := &pb.ArtifactVersion{ - VersionId: int64(packageVersionId), - Tags: []string{tag}, - Sha: packageVersionSha, - } - - return version, nil -} - -func gatherArtifactInfo( - ctx context.Context, - client provifv1.GitHub, - payload map[string]any, -) (*pb.Artifact, error) { - artifact, err := extractArtifactFromPayload(ctx, payload) - if err != nil { - return nil, fmt.Errorf("error extracting artifact from payload: %w", err) - } - // we also need to fill in the visibility which is not in the payload - ghArtifact, err := client.GetPackageByName(ctx, artifact.Owner, string(verifyif.ArtifactTypeContainer), artifact.Name) + ghArtifact, err := client.GetPackageByName( + ctx, + artifact.Owner, + string(verifyif.ArtifactTypeContainer), + artifact.Name, + ) if err != nil { return nil, fmt.Errorf("error extracting artifact from repo: %w", err) } @@ -746,21 +900,35 @@ func gatherArtifactInfo( return artifact, nil } +// This routine assumes that all necessary validation is performed on +// the upper layer and accesses package and repo without checking for +// nulls. func gatherArtifactVersionInfo( ctx context.Context, cli provifv1.GitHub, - payload map[string]any, + event *github.PackageEvent, artifactOwnerLogin, artifactName string, ) (*pb.ArtifactVersion, error) { - version, err := extractArtifactVersionFromPayload(ctx, payload) - if err != nil { - return nil, fmt.Errorf("error extracting artifact version from payload: %w", err) + if event.GetPackage().GetPackageVersion() == nil { + return nil, errors.New("Package version is nil") + } + + pv := event.GetPackage().GetPackageVersion() + version := &pb.ArtifactVersion{ + VersionId: pv.GetID(), + Tags: []string{pv.GetTagName()}, + Sha: pv.GetVersion(), } // not all information is in the payload, we need to get it from the container registry // and/or GH API - err = updateArtifactVersionFromRegistry(ctx, cli, artifactOwnerLogin, artifactName, version) - if err != nil { + if err := updateArtifactVersionFromRegistry( + ctx, + cli, + artifactOwnerLogin, + artifactName, + version, + ); err != nil { return nil, fmt.Errorf("error getting upstream information for artifact version: %w", err) } @@ -770,14 +938,14 @@ func gatherArtifactVersionInfo( func gatherArtifact( ctx context.Context, cli provifv1.GitHub, - payload map[string]any, + event *github.PackageEvent, ) (*pb.Artifact, error) { - artifact, err := gatherArtifactInfo(ctx, cli, payload) + artifact, err := gatherArtifactInfo(ctx, cli, event) if err != nil { return nil, fmt.Errorf("error gatherinfo artifact info: %w", err) } - version, err := gatherArtifactVersionInfo(ctx, cli, payload, artifact.Owner, artifact.Name) + version, err := gatherArtifactVersionInfo(ctx, cli, event, artifact.Owner, artifact.Name) if err != nil { return nil, fmt.Errorf("error extracting artifact from payload: %w", err) } @@ -814,38 +982,6 @@ func updateArtifactVersionFromRegistry( return nil } -func getPullRequestInfoFromPayload( - ctx context.Context, - payload map[string]any, -) (*pb.PullRequest, error) { - prUrl, err := util.JQReadFrom[string](ctx, ".pull_request.url", payload) - if err != nil { - return nil, fmt.Errorf("error getting pull request url from payload: %w", err) - } - - prNumber, err := util.JQReadFrom[float64](ctx, ".pull_request.number", payload) - if err != nil { - return nil, fmt.Errorf("error getting pull request number from payload: %w", err) - } - - prAuthorId, err := util.JQReadFrom[float64](ctx, ".pull_request.user.id", payload) - if err != nil { - return nil, fmt.Errorf("error getting pull request author ID from payload: %w", err) - } - - action, err := util.JQReadFrom[string](ctx, ".action", payload) - if err != nil { - return nil, fmt.Errorf("error getting action from payload: %w", err) - } - - return &pb.PullRequest{ - Url: prUrl, - Number: int64(prNumber), - AuthorId: int64(prAuthorId), - Action: action, - }, nil -} - func (s *Server) reconcilePrWithDb( ctx context.Context, dbrepo db.Repository, @@ -903,75 +1039,3 @@ func updatePullRequestInfoFromProvider( prEvalInfo.RepoName = dbrepo.RepoName return nil } - -func getRepoInformationFromPayload( - ctx context.Context, - store db.Store, - payload map[string]any, -) (db.Repository, error) { - repoInfo, ok := payload["repository"].(map[string]any) - if !ok { - return db.Repository{}, fmt.Errorf("unable to determine repository for event: %w", errRepoNotFound) - } - - id, err := parseRepoID(repoInfo["id"]) - if err != nil { - return db.Repository{}, fmt.Errorf("error parsing repository ID: %w", err) - } - - // At this point, we're unsure what the project ID is, so we need to look it up. - // It's the same case for the provider. We can gather this information from the - // repository ID. - dbrepo, err := store.GetRepositoryByRepoID(ctx, id) - if err != nil { - if errors.Is(err, sql.ErrNoRows) { - log.Printf("repository %d not found", id) - // no use in continuing if the repository doesn't exist - return db.Repository{}, fmt.Errorf("repository %d not found: %w", id, errRepoNotFound) - } - return db.Repository{}, fmt.Errorf("error getting repository: %w", err) - } - - if dbrepo.ProjectID.String() == "" { - return db.Repository{}, fmt.Errorf("no project found for repository %s/%s: %w", - dbrepo.RepoOwner, dbrepo.RepoName, errRepoNotFound) - } - - // ignore processing webhooks for private repositories - isPrivate, ok := repoInfo["private"].(bool) - if ok { - if isPrivate && !features.ProjectAllowsPrivateRepos(ctx, store, dbrepo.ProjectID) { - return db.Repository{}, errRepoIsPrivate - } - } - - log.Printf("handling event for repository %d", id) - - return dbrepo, nil -} - -func parseRepoID(repoID any) (int64, error) { - switch v := repoID.(type) { - case int32: - return int64(v), nil - case int64: - return v, nil - case float64: - return int64(v), nil - case string: - // convert string to int - return strconv.ParseInt(v, 10, 64) - default: - return 0, fmt.Errorf("unknown type for repoID: %T", v) - } -} - -func shouldIssueDeletionEvent(m *message.Message) bool { - return m.Metadata.Get(entities.ActionEventKey) == WebhookActionEventDeleted && - deletionOfRelevantType(m) -} - -func deletionOfRelevantType(m *message.Message) bool { - return m.Metadata.Get(events.GithubWebhookEventTypeKey) == "repository" || - m.Metadata.Get(events.GithubWebhookEventTypeKey) == "meta" -} diff --git a/internal/controlplane/handlers_githubwebhooks_test.go b/internal/controlplane/handlers_githubwebhooks_test.go index bfec3ffbdb..4af0fe75fe 100644 --- a/internal/controlplane/handlers_githubwebhooks_test.go +++ b/internal/controlplane/handlers_githubwebhooks_test.go @@ -269,6 +269,7 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() { go server.ListenAndServe() event := github.MetaEvent{ + Action: github.String("delete"), Repo: &github.Repository{ ID: github.Int64(12345), Name: github.String("stacklok/minder"), @@ -287,7 +288,7 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() { req.Header.Add("X-GitHub-Event", "meta") req.Header.Add("X-GitHub-Delivery", "12345") req.Header.Add("Content-Type", "application/json") - req.Header.Add("X-Hub-Signature-256", "sha256=ab22bd9a3712e444e110c8088011fd827143ed63ba8655f07e76ed1a0f05edd1") + req.Header.Add("X-Hub-Signature-256", "sha256=a6a32e346c5bc576858a40da9ea951873a49c7b9e6fccabb6ec65cad37baa3a3") resp, err := httpDoWithRetry(client, req) require.NoError(t, err, "failed to make request") // We expect OK since we don't want to leak information about registered repositories @@ -310,7 +311,7 @@ func (s *UnitTestSuite) TestHandleWebHookRepository() { req.Header.Add("X-GitHub-Event", "meta") req.Header.Add("X-GitHub-Delivery", "12345") req.Header.Add("Content-Type", "application/json") - req.Header.Add("X-Hub-Signature-256", "sha256=ab22bd9a3712e444e110c8088011fd827143ed63ba8655f07e76ed1a0f05edd1") + req.Header.Add("X-Hub-Signature-256", "sha256=a6a32e346c5bc576858a40da9ea951873a49c7b9e6fccabb6ec65cad37baa3a3") _, err = prevCredsFile.Seek(0, 0) require.NoError(t, err, "failed to seek to beginning of temporary file")