Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions internal/providers/github/webhook/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,8 @@ func processInstallationRepositoriesAppEvent(
installation,
)
if err != nil {
return nil, err
zerolog.Ctx(ctx).Warn().Err(err).Msg("skipping invalid repository in batch")
continue
}

results = append(results, res)
Expand All @@ -336,15 +337,22 @@ func processInstallationRepositoriesAppEvent(
// be deleted by means of "meta" and "repository" events as
// well.
for _, repo := range event.GetRepositoriesRemoved() {
res := repositoryRemoved(repo)
res, err := repositoryRemoved(repo)
if err != nil {
zerolog.Ctx(ctx).Warn().Err(err).Msg("skipping invalid repository removal in batch")
continue
}
results = append(results, res)
}

return results, nil
}

func repositoryRemoved(repo *repo) *processingResult {
return sendEvaluateRepoMessage(repo, constants.TopicQueueGetEntityAndDelete)
func repositoryRemoved(repo *repo) (*processingResult, error) {
if repo.GetID() == 0 {
return nil, errors.New("invalid repository: id is 0")
}
return sendEvaluateRepoMessage(repo, constants.TopicQueueGetEntityAndDelete), nil
}

func repositoryAdded(
Expand Down
149 changes: 149 additions & 0 deletions internal/providers/github/webhook/app_batch_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
// SPDX-FileCopyrightText: Copyright 2024 The Minder Authors
// SPDX-License-Identifier: Apache-2.0

package webhook

import (
"context"
"encoding/json"
"testing"

"github.com/google/uuid"
"github.com/stretchr/testify/require"
"go.uber.org/mock/gomock"

df "github.com/mindersec/minder/database/mock/fixtures"
"github.com/mindersec/minder/internal/db"
)

func TestProcessInstallationRepositoriesAppEvent_BatchResilience(t *testing.T) {
t.Parallel()

projectID := uuid.New()
providerID := uuid.New()

autoregEnabled := `{"github-app": {}, "auto_registration": {"entities": {"repository": {"enabled": true}}}}`

validRepo := func(id int, name, fullName string) *repo {
idVal := int64(id)
return &repo{
ID: &idVal,
Name: &name,
FullName: &fullName,
}
}

invalidRepo := func() *repo {
// name is empty, triggers repositoryAdded validation error
emptyName := ""
return &repo{
Name: &emptyName,
}
}

zeroIDRepo := func() *repo {
// ID is 0, triggers repositoryRemoved validation error
var zero int64
name := "bad-repo"
return &repo{
ID: &zero,
Name: &name,
}
}

mockInstallation := db.ProviderGithubAppInstallation{
ProjectID: uuid.NullUUID{UUID: projectID, Valid: true},
ProviderID: uuid.NullUUID{UUID: providerID, Valid: true},
}

baseMocks := func(ctrl *gomock.Controller) db.Store {
return df.NewMockStore(
df.WithSuccessfulGetInstallationIDByAppID(mockInstallation, 54321),
df.WithSuccessfulGetProviderByID(
db.Provider{
ID: providerID,
Definition: json.RawMessage(autoregEnabled),
},
providerID,
),
)(ctrl)
}

tests := []struct {
name string
payload *installationRepositoriesEvent
expectedCount int
expectErr bool
}{
{
name: "full batch success",
payload: &installationRepositoriesEvent{
Action: strPtr("added"),
RepositorySelection: strPtr("selected"),
RepositoriesAdded: []*repo{
validRepo(111, "repo-a", "org/repo-a"),
validRepo(222, "repo-b", "org/repo-b"),
},
RepositoriesRemoved: []*repo{
validRepo(333, "repo-c", "org/repo-c"),
},
Installation: &installation{ID: int64Ptr(54321)},
},
expectedCount: 3,
},
{
name: "skip invalid added repo",
payload: &installationRepositoriesEvent{
Action: strPtr("added"),
RepositorySelection: strPtr("selected"),
RepositoriesAdded: []*repo{
validRepo(111, "repo-a", "org/repo-a"),
invalidRepo(), // bad name → skipped
validRepo(333, "repo-c", "org/repo-c"),
},
Installation: &installation{ID: int64Ptr(54321)},
},
expectedCount: 2,
},
{
name: "skip invalid removed repo",
payload: &installationRepositoriesEvent{
Action: strPtr("removed"),
RepositorySelection: strPtr("selected"),
RepositoriesRemoved: []*repo{
validRepo(111, "repo-a", "org/repo-a"),
zeroIDRepo(), // id=0 → skipped
validRepo(333, "repo-c", "org/repo-c"),
},
Installation: &installation{ID: int64Ptr(54321)},
},
expectedCount: 2,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()

ctrl := gomock.NewController(t)
defer ctrl.Finish()

store := baseMocks(ctrl)
payload, err := json.Marshal(tt.payload)
require.NoError(t, err)

results, err := processInstallationRepositoriesAppEvent(
context.Background(), store, payload,
)
if tt.expectErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Len(t, results, tt.expectedCount)
})
}
}

func strPtr(s string) *string { return &s }
func int64Ptr(i int64) *int64 { return &i }
Loading