From d0a82f841e514380cecbc8cb661108dc3060cdb7 Mon Sep 17 00:00:00 2001 From: mmatur Date: Wed, 22 Jul 2026 15:38:43 +0200 Subject: [PATCH 1/2] feat: manage plugin blacklist via dedicated endpoints --- cmd/serve/serve.go | 22 +++++ pkg/db/db.go | 9 ++ pkg/db/mongodb/blacklistdb.go | 97 +++++++++++++++++++++ pkg/db/mongodb/blacklistdb_test.go | 59 +++++++++++++ pkg/db/mongodb/bootstrap.go | 14 ++++ pkg/handlers/blacklist.go | 129 ++++++++++++++++++++++++++++ pkg/handlers/blacklist_test.go | 130 +++++++++++++++++++++++++++++ pkg/handlers/dbmock_test.go | 16 ++++ pkg/handlers/handlers.go | 4 + 9 files changed, 480 insertions(+) create mode 100644 pkg/db/mongodb/blacklistdb.go create mode 100644 pkg/db/mongodb/blacklistdb_test.go create mode 100644 pkg/handlers/blacklist.go create mode 100644 pkg/handlers/blacklist_test.go diff --git a/cmd/serve/serve.go b/cmd/serve/serve.go index 5c955b8..14ef120 100644 --- a/cmd/serve/serve.go +++ b/cmd/serve/serve.go @@ -54,6 +54,7 @@ func run(ctx context.Context, cfg Config) error { r.Handle("/public/", buildPublicRouter(handler)) r.Handle("/internal/", buildInternalRouter(handler)) + r.Handle("/internal/blacklist", buildInternalBlacklistRouter(handler)) r.Handle("/external/", buildExternalRouter(handler)) r.HandleFunc("/live", healthChecker.Live) r.HandleFunc("/ready", healthChecker.Ready) @@ -88,6 +89,27 @@ func buildInternalRouter(handler handlers.Handlers) http.Handler { return http.StripPrefix("/internal", r) } +// buildInternalBlacklistRouter serves the blacklist endpoints on a dedicated +// handler mounted at /internal/blacklist. It is kept separate from the internal +// httprouter to avoid a route conflict between the static "/blacklist" segment +// and the existing "/:uuid" wildcard. +func buildInternalBlacklistRouter(handler handlers.Handlers) http.Handler { + next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + switch req.Method { + case http.MethodGet: + handler.ListBlacklist(rw, req) + case http.MethodPost: + handler.AddToBlacklist(rw, req) + case http.MethodDelete: + handler.DeleteFromBlacklist(rw, req) + default: + handlers.JSONError(rw, http.StatusMethodNotAllowed, http.StatusText(http.StatusMethodNotAllowed)) + } + }) + + return otelhttp.NewHandler(next, "internal_blacklist") +} + func buildExternalRouter(handler handlers.Handlers) http.Handler { r := httprouter.New() diff --git a/pkg/db/db.go b/pkg/db/db.go index 9fe9ca5..54d3bf8 100644 --- a/pkg/db/db.go +++ b/pkg/db/db.go @@ -27,6 +27,15 @@ type Plugin struct { UseUnsafe bool `json:"useUnsafe,omitempty" bson:"useUnsafe"` } +// BlacklistEntry represents a repository excluded from the plugin scraping. +// The Repository is the GitHub full name (owner/repo) and is the unique key. +type BlacklistEntry struct { + Repository string `json:"repository" bson:"repository"` + Reason string `json:"reason,omitempty" bson:"reason"` + Author string `json:"author,omitempty" bson:"author"` + CreatedAt time.Time `json:"createdAt" bson:"createdAt"` +} + // PluginHash The plugin hash tuple. type PluginHash struct { Name string `json:"name,omitempty" bson:"name"` diff --git a/pkg/db/mongodb/blacklistdb.go b/pkg/db/mongodb/blacklistdb.go new file mode 100644 index 0000000..2c1c088 --- /dev/null +++ b/pkg/db/mongodb/blacklistdb.go @@ -0,0 +1,97 @@ +package mongodb + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/traefik/plugin-service/pkg/db" + "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/mongo/options" +) + +const ( + blacklistCollName = "blacklist" + blacklistRepositoryKey = "repository" +) + +// ListBlacklist returns all the blacklisted repositories, sorted by repository. +func (m *MongoDB) ListBlacklist(ctx context.Context) ([]db.BlacklistEntry, error) { + ctx, span := m.tracer.Start(ctx, "db_list_blacklist") + defer span.End() + + opts := &options.FindOptions{} + opts.SetSort(bson.D{{Key: blacklistRepositoryKey, Value: 1}}) + + cursor, err := m.client.Collection(blacklistCollName).Find(ctx, bson.D{}, opts) + if err != nil { + span.RecordError(err) + + return nil, fmt.Errorf("unable to find blacklist entries: %w", err) + } + + entries := []db.BlacklistEntry{} + + if err = cursor.All(ctx, &entries); err != nil { + span.RecordError(err) + + return nil, fmt.Errorf("unable to unmarshal blacklist entries: %w", err) + } + + return entries, nil +} + +// UpsertBlacklist creates or updates a blacklist entry, keyed by repository. +// CreatedAt is only set on insert. +func (m *MongoDB) UpsertBlacklist(ctx context.Context, entry db.BlacklistEntry) (db.BlacklistEntry, error) { + ctx, span := m.tracer.Start(ctx, "db_upsert_blacklist") + defer span.End() + + filter := bson.D{{Key: blacklistRepositoryKey, Value: entry.Repository}} + + update := bson.D{ + {Key: "$set", Value: bson.D{ + {Key: "reason", Value: entry.Reason}, + {Key: "author", Value: entry.Author}, + }}, + {Key: "$setOnInsert", Value: bson.D{ + {Key: "repository", Value: entry.Repository}, + {Key: "createdAt", Value: time.Now().Truncate(time.Millisecond)}, + }}, + } + + opts := &options.FindOneAndUpdateOptions{} + opts.SetUpsert(true) + opts.SetReturnDocument(options.After) + + var updated db.BlacklistEntry + if err := m.client.Collection(blacklistCollName).FindOneAndUpdate(ctx, filter, update, opts).Decode(&updated); err != nil { + span.RecordError(err) + + return db.BlacklistEntry{}, fmt.Errorf("unable to upsert blacklist entry: %w", err) + } + + return updated, nil +} + +// DeleteBlacklist removes the blacklist entry for the given repository. +func (m *MongoDB) DeleteBlacklist(ctx context.Context, repository string) error { + ctx, span := m.tracer.Start(ctx, "db_delete_blacklist") + defer span.End() + + filter := bson.D{{Key: blacklistRepositoryKey, Value: repository}} + + res, err := m.client.Collection(blacklistCollName).DeleteOne(ctx, filter) + if err != nil { + span.RecordError(err) + + return err + } + + if res.DeletedCount == 0 { + return db.NotFoundError{Err: errors.New(repository)} + } + + return nil +} diff --git a/pkg/db/mongodb/blacklistdb_test.go b/pkg/db/mongodb/blacklistdb_test.go new file mode 100644 index 0000000..e52204f --- /dev/null +++ b/pkg/db/mongodb/blacklistdb_test.go @@ -0,0 +1,59 @@ +package mongodb + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/traefik/plugin-service/pkg/db" +) + +func TestMongoDB_Blacklist(t *testing.T) { + ctx := context.Background() + store, _ := createDatabase(t, nil) + + // Empty at first. + entries, err := store.ListBlacklist(ctx) + require.NoError(t, err) + assert.Empty(t, entries) + + // Insert. + created, err := store.UpsertBlacklist(ctx, db.BlacklistEntry{ + Repository: "deas/teectl", + Reason: "Not a plugin", + Author: "alice", + }) + require.NoError(t, err) + assert.Equal(t, "deas/teectl", created.Repository) + assert.False(t, created.CreatedAt.IsZero()) + + // Upsert keeps CreatedAt and updates reason/author. + updated, err := store.UpsertBlacklist(ctx, db.BlacklistEntry{ + Repository: "deas/teectl", + Reason: "Still not a plugin", + Author: "bob", + }) + require.NoError(t, err) + assert.Equal(t, "Still not a plugin", updated.Reason) + assert.Equal(t, "bob", updated.Author) + assert.Equal(t, created.CreatedAt, updated.CreatedAt) + + // List returns the single entry. + entries, err = store.ListBlacklist(ctx) + require.NoError(t, err) + require.Len(t, entries, 1) + assert.Equal(t, "deas/teectl", entries[0].Repository) + + // Delete. + require.NoError(t, store.DeleteBlacklist(ctx, "deas/teectl")) + + // Delete missing returns NotFoundError. + err = store.DeleteBlacklist(ctx, "deas/teectl") + assert.True(t, errors.As(err, &db.NotFoundError{})) + + entries, err = store.ListBlacklist(ctx) + require.NoError(t, err) + assert.Empty(t, entries) +} diff --git a/pkg/db/mongodb/bootstrap.go b/pkg/db/mongodb/bootstrap.go index 96d9202..99c2bda 100644 --- a/pkg/db/mongodb/bootstrap.go +++ b/pkg/db/mongodb/bootstrap.go @@ -44,5 +44,19 @@ func (m *MongoDB) Bootstrap() error { return fmt.Errorf("unable to create indexes: %w", err) } + blacklistModels := []mongo.IndexModel{ + { + Options: &options.IndexOptions{ + Name: new("_uniq_repository"), + Unique: new(true), + }, + Keys: bson.D{{Key: blacklistRepositoryKey, Value: 1}}, + }, + } + + if _, err := m.client.Collection(blacklistCollName).Indexes().CreateMany(context.Background(), blacklistModels); err != nil { + return fmt.Errorf("unable to create blacklist indexes: %w", err) + } + return nil } diff --git a/pkg/handlers/blacklist.go b/pkg/handlers/blacklist.go new file mode 100644 index 0000000..ca15c78 --- /dev/null +++ b/pkg/handlers/blacklist.go @@ -0,0 +1,129 @@ +package handlers + +import ( + "encoding/json" + "errors" + "io" + "net/http" + "regexp" + + "github.com/rs/zerolog/log" + "github.com/traefik/plugin-service/pkg/db" +) + +// repositoryRegexp matches a GitHub full name: owner/repo (exactly one slash, no spaces). +var repositoryRegexp = regexp.MustCompile(`^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`) + +// ListBlacklist lists the blacklisted repositories. +func (h Handlers) ListBlacklist(rw http.ResponseWriter, req *http.Request) { + ctx, span := h.tracer.Start(req.Context(), "handler_list_blacklist") + defer span.End() + + rw.Header().Set("Content-Type", "application/json") + + entries, err := h.store.ListBlacklist(ctx) + if err != nil { + span.RecordError(err) + log.Error().Err(err).Msg("Error fetching blacklist") + JSONInternalServerError(rw) + + return + } + + if err := json.NewEncoder(rw).Encode(entries); err != nil { + span.RecordError(err) + log.Error().Err(err).Msg("Failed to encode blacklist response") + JSONInternalServerError(rw) + + return + } +} + +// AddToBlacklist adds (or updates) a repository in the blacklist. +func (h Handlers) AddToBlacklist(rw http.ResponseWriter, req *http.Request) { + ctx, span := h.tracer.Start(req.Context(), "handler_add_to_blacklist") + defer span.End() + + rw.Header().Set("Content-Type", "application/json") + + body, err := io.ReadAll(req.Body) + if err != nil { + span.RecordError(err) + log.Error().Err(err).Msg("Error reading body for blacklist creation") + JSONError(rw, http.StatusBadRequest, err.Error()) + + return + } + + entry := db.BlacklistEntry{} + if err = json.Unmarshal(body, &entry); err != nil { + span.RecordError(err) + log.Error().Err(err).Msg("Error decoding blacklist entry") + JSONError(rw, http.StatusBadRequest, err.Error()) + + return + } + + if !repositoryRegexp.MatchString(entry.Repository) { + JSONError(rw, http.StatusBadRequest, "invalid repository, expected owner/repo") + + return + } + + logger := log.With().Str("repository", entry.Repository).Logger() + + created, err := h.store.UpsertBlacklist(ctx, entry) + if err != nil { + span.RecordError(err) + logger.Error().Err(err).Msg("Error persisting blacklist entry") + JSONInternalServerError(rw) + + return + } + + rw.WriteHeader(http.StatusOK) + + if err := json.NewEncoder(rw).Encode(created); err != nil { + span.RecordError(err) + logger.Error().Err(err).Msg("Error sending blacklist response") + JSONInternalServerError(rw) + + return + } +} + +// DeleteFromBlacklist removes a repository from the blacklist. +// The repository is passed as a query parameter: ?repository=owner/repo. +func (h Handlers) DeleteFromBlacklist(rw http.ResponseWriter, req *http.Request) { + ctx, span := h.tracer.Start(req.Context(), "handler_delete_from_blacklist") + defer span.End() + + rw.Header().Set("Content-Type", "application/json") + + repository := req.URL.Query().Get("repository") + if repository == "" { + JSONError(rw, http.StatusBadRequest, "missing repository query parameter") + + return + } + + logger := log.With().Str("repository", repository).Logger() + + err := h.store.DeleteBlacklist(ctx, repository) + if err != nil { + span.RecordError(err) + + if errors.As(err, &db.NotFoundError{}) { + NotFound(rw, req) + + return + } + + logger.Error().Err(err).Msg("Failed to delete blacklist entry") + JSONInternalServerError(rw) + + return + } + + rw.WriteHeader(http.StatusNoContent) +} diff --git a/pkg/handlers/blacklist_test.go b/pkg/handlers/blacklist_test.go new file mode 100644 index 0000000..3df7eb7 --- /dev/null +++ b/pkg/handlers/blacklist_test.go @@ -0,0 +1,130 @@ +package handlers + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/traefik/plugin-service/pkg/db" +) + +func TestHandlers_ListBlacklist(t *testing.T) { + entries := []db.BlacklistEntry{ + {Repository: "containous/plugintestxxx", Reason: "Crash piceus", Author: "alice", CreatedAt: time.Date(2020, 1, 1, 1, 0, 0, 0, time.UTC)}, + {Repository: "deas/teectl", Reason: "Not a plugin", Author: "bob", CreatedAt: time.Date(2020, 1, 1, 1, 0, 0, 0, time.UTC)}, + } + + testDB := mockDB{ + listBlacklistFn: func(_ context.Context) ([]db.BlacklistEntry, error) { + return entries, nil + }, + } + + rw := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/internal/blacklist", http.NoBody) + + New(testDB, nil, nil).ListBlacklist(rw, req) + + require.Equal(t, http.StatusOK, rw.Code) + assert.JSONEq(t, `[ + {"repository":"containous/plugintestxxx","reason":"Crash piceus","author":"alice","createdAt":"2020-01-01T01:00:00Z"}, + {"repository":"deas/teectl","reason":"Not a plugin","author":"bob","createdAt":"2020-01-01T01:00:00Z"} + ]`, rw.Body.String()) +} + +func TestHandlers_AddToBlacklist(t *testing.T) { + var got db.BlacklistEntry + + testDB := mockDB{ + upsertBlacklistFn: func(_ context.Context, entry db.BlacklistEntry) (db.BlacklistEntry, error) { + got = entry + entry.CreatedAt = time.Date(2020, 1, 1, 1, 0, 0, 0, time.UTC) + return entry, nil + }, + } + + rw := httptest.NewRecorder() + body := `{"repository":"deas/teectl","reason":"Not a plugin","author":"alice"}` + req := httptest.NewRequest(http.MethodPost, "/internal/blacklist", strings.NewReader(body)) + + New(testDB, nil, nil).AddToBlacklist(rw, req) + + require.Equal(t, http.StatusOK, rw.Code) + assert.Equal(t, "deas/teectl", got.Repository) + assert.Equal(t, "Not a plugin", got.Reason) + assert.Equal(t, "alice", got.Author) +} + +func TestHandlers_AddToBlacklist_invalidRepository(t *testing.T) { + testDB := mockDB{ + upsertBlacklistFn: func(_ context.Context, _ db.BlacklistEntry) (db.BlacklistEntry, error) { + t.Fatal("store should not be called on invalid input") + return db.BlacklistEntry{}, nil + }, + } + + for _, repository := range []string{"", "no-slash", "too/many/slashes", "with space/repo"} { + rw := httptest.NewRecorder() + body := `{"repository":"` + repository + `"}` + req := httptest.NewRequest(http.MethodPost, "/internal/blacklist", strings.NewReader(body)) + + New(testDB, nil, nil).AddToBlacklist(rw, req) + + assert.Equalf(t, http.StatusBadRequest, rw.Code, "repository %q", repository) + } +} + +func TestHandlers_DeleteFromBlacklist(t *testing.T) { + var deleted string + + testDB := mockDB{ + deleteBlacklistFn: func(_ context.Context, repository string) error { + deleted = repository + return nil + }, + } + + rw := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist?repository=deas/teectl", http.NoBody) + + New(testDB, nil, nil).DeleteFromBlacklist(rw, req) + + require.Equal(t, http.StatusNoContent, rw.Code) + assert.Equal(t, "deas/teectl", deleted) +} + +func TestHandlers_DeleteFromBlacklist_missingRepository(t *testing.T) { + testDB := mockDB{ + deleteBlacklistFn: func(_ context.Context, _ string) error { + t.Fatal("store should not be called without a repository") + return nil + }, + } + + rw := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist", http.NoBody) + + New(testDB, nil, nil).DeleteFromBlacklist(rw, req) + + assert.Equal(t, http.StatusBadRequest, rw.Code) +} + +func TestHandlers_DeleteFromBlacklist_notFound(t *testing.T) { + testDB := mockDB{ + deleteBlacklistFn: func(_ context.Context, _ string) error { + return db.NotFoundError{} + }, + } + + rw := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist?repository=deas/teectl", http.NoBody) + + New(testDB, nil, nil).DeleteFromBlacklist(rw, req) + + assert.Equal(t, http.StatusNotFound, rw.Code) +} diff --git a/pkg/handlers/dbmock_test.go b/pkg/handlers/dbmock_test.go index 4d0a838..0bc162b 100644 --- a/pkg/handlers/dbmock_test.go +++ b/pkg/handlers/dbmock_test.go @@ -19,6 +19,10 @@ type mockDB struct { createHashFn func(ctx context.Context, module, version, hash string) (db.PluginHash, error) updateHashVerifiedFn func(ctx context.Context, module, version, hash string, verified bool) (db.PluginHash, error) getHashByNameFn func(ctx context.Context, module, version string) (db.PluginHash, error) + + listBlacklistFn func(ctx context.Context) ([]db.BlacklistEntry, error) + upsertBlacklistFn func(ctx context.Context, entry db.BlacklistEntry) (db.BlacklistEntry, error) + deleteBlacklistFn func(ctx context.Context, repository string) error } func (m mockDB) Get(ctx context.Context, id string) (db.Plugin, error) { @@ -64,3 +68,15 @@ func (m mockDB) UpdateHashVerified(ctx context.Context, module, version, hash st func (m mockDB) GetHashByName(ctx context.Context, module, version string) (db.PluginHash, error) { return m.getHashByNameFn(ctx, module, version) } + +func (m mockDB) ListBlacklist(ctx context.Context) ([]db.BlacklistEntry, error) { + return m.listBlacklistFn(ctx) +} + +func (m mockDB) UpsertBlacklist(ctx context.Context, entry db.BlacklistEntry) (db.BlacklistEntry, error) { + return m.upsertBlacklistFn(ctx, entry) +} + +func (m mockDB) DeleteBlacklist(ctx context.Context, repository string) error { + return m.deleteBlacklistFn(ctx, repository) +} diff --git a/pkg/handlers/handlers.go b/pkg/handlers/handlers.go index df62720..7ff4a27 100644 --- a/pkg/handlers/handlers.go +++ b/pkg/handlers/handlers.go @@ -36,6 +36,10 @@ type PluginStorer interface { CreateHash(ctx context.Context, module, version, hash string) (db.PluginHash, error) UpdateHashVerified(ctx context.Context, module, version, hash string, verified bool) (db.PluginHash, error) GetHashByName(ctx context.Context, module, version string) (db.PluginHash, error) + + ListBlacklist(ctx context.Context) ([]db.BlacklistEntry, error) + UpsertBlacklist(ctx context.Context, entry db.BlacklistEntry) (db.BlacklistEntry, error) + DeleteBlacklist(ctx context.Context, repository string) error } // Handlers a set of handlers. From 6e02ec38d912094d32d7a4ccf4e62ed332f74ee9 Mon Sep 17 00:00:00 2001 From: mmatur Date: Fri, 24 Jul 2026 17:45:30 +0200 Subject: [PATCH 2/2] review: address feedback --- cmd/serve/serve.go | 29 +++++++--------------- pkg/db/mongodb/blacklistdb.go | 9 +++---- pkg/db/mongodb/blacklistdb_test.go | 3 +-- pkg/db/mongodb/bootstrap.go | 2 +- pkg/handlers/blacklist.go | 40 ++++++++++++------------------ pkg/handlers/blacklist_test.go | 26 ++++++------------- pkg/handlers/handlers.go | 12 +++++---- 7 files changed, 45 insertions(+), 76 deletions(-) diff --git a/cmd/serve/serve.go b/cmd/serve/serve.go index 14ef120..2eb8ac0 100644 --- a/cmd/serve/serve.go +++ b/cmd/serve/serve.go @@ -54,8 +54,9 @@ func run(ctx context.Context, cfg Config) error { r.Handle("/public/", buildPublicRouter(handler)) r.Handle("/internal/", buildInternalRouter(handler)) - r.Handle("/internal/blacklist", buildInternalBlacklistRouter(handler)) r.Handle("/external/", buildExternalRouter(handler)) + + registerInternalBlacklistRoutes(r, handler) r.HandleFunc("/live", healthChecker.Live) r.HandleFunc("/ready", healthChecker.Ready) @@ -89,25 +90,13 @@ func buildInternalRouter(handler handlers.Handlers) http.Handler { return http.StripPrefix("/internal", r) } -// buildInternalBlacklistRouter serves the blacklist endpoints on a dedicated -// handler mounted at /internal/blacklist. It is kept separate from the internal -// httprouter to avoid a route conflict between the static "/blacklist" segment -// and the existing "/:uuid" wildcard. -func buildInternalBlacklistRouter(handler handlers.Handlers) http.Handler { - next := http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - switch req.Method { - case http.MethodGet: - handler.ListBlacklist(rw, req) - case http.MethodPost: - handler.AddToBlacklist(rw, req) - case http.MethodDelete: - handler.DeleteFromBlacklist(rw, req) - default: - handlers.JSONError(rw, http.StatusMethodNotAllowed, http.StatusText(http.StatusMethodNotAllowed)) - } - }) - - return otelhttp.NewHandler(next, "internal_blacklist") +// registerInternalBlacklistRoutes registers the blacklist endpoints directly on +// the ServeMux: they cannot live in the internal httprouter because the static +// "blacklist" segment conflicts with its "/:uuid" wildcard routes. +func registerInternalBlacklistRoutes(r *http.ServeMux, handler handlers.Handlers) { + r.Handle("GET /internal/blacklist", otelhttp.NewHandler(http.HandlerFunc(handler.ListBlacklist), "internal_list_blacklist")) + r.Handle("POST /internal/blacklist", otelhttp.NewHandler(http.HandlerFunc(handler.AddToBlacklist), "internal_add_to_blacklist")) + r.Handle("DELETE /internal/blacklist/{owner}/{repo}", otelhttp.NewHandler(http.HandlerFunc(handler.DeleteFromBlacklist), "internal_delete_from_blacklist")) } func buildExternalRouter(handler handlers.Handlers) http.Handler { diff --git a/pkg/db/mongodb/blacklistdb.go b/pkg/db/mongodb/blacklistdb.go index 2c1c088..3e3ca54 100644 --- a/pkg/db/mongodb/blacklistdb.go +++ b/pkg/db/mongodb/blacklistdb.go @@ -28,7 +28,7 @@ func (m *MongoDB) ListBlacklist(ctx context.Context) ([]db.BlacklistEntry, error if err != nil { span.RecordError(err) - return nil, fmt.Errorf("unable to find blacklist entries: %w", err) + return nil, fmt.Errorf("finding blacklist entries: %w", err) } entries := []db.BlacklistEntry{} @@ -36,7 +36,7 @@ func (m *MongoDB) ListBlacklist(ctx context.Context) ([]db.BlacklistEntry, error if err = cursor.All(ctx, &entries); err != nil { span.RecordError(err) - return nil, fmt.Errorf("unable to unmarshal blacklist entries: %w", err) + return nil, fmt.Errorf("unmarshalling blacklist entries: %w", err) } return entries, nil @@ -56,7 +56,6 @@ func (m *MongoDB) UpsertBlacklist(ctx context.Context, entry db.BlacklistEntry) {Key: "author", Value: entry.Author}, }}, {Key: "$setOnInsert", Value: bson.D{ - {Key: "repository", Value: entry.Repository}, {Key: "createdAt", Value: time.Now().Truncate(time.Millisecond)}, }}, } @@ -69,7 +68,7 @@ func (m *MongoDB) UpsertBlacklist(ctx context.Context, entry db.BlacklistEntry) if err := m.client.Collection(blacklistCollName).FindOneAndUpdate(ctx, filter, update, opts).Decode(&updated); err != nil { span.RecordError(err) - return db.BlacklistEntry{}, fmt.Errorf("unable to upsert blacklist entry: %w", err) + return db.BlacklistEntry{}, fmt.Errorf("upserting blacklist entry: %w", err) } return updated, nil @@ -86,7 +85,7 @@ func (m *MongoDB) DeleteBlacklist(ctx context.Context, repository string) error if err != nil { span.RecordError(err) - return err + return fmt.Errorf("deleting blacklist entry: %w", err) } if res.DeletedCount == 0 { diff --git a/pkg/db/mongodb/blacklistdb_test.go b/pkg/db/mongodb/blacklistdb_test.go index e52204f..f6a5120 100644 --- a/pkg/db/mongodb/blacklistdb_test.go +++ b/pkg/db/mongodb/blacklistdb_test.go @@ -2,7 +2,6 @@ package mongodb import ( "context" - "errors" "testing" "github.com/stretchr/testify/assert" @@ -51,7 +50,7 @@ func TestMongoDB_Blacklist(t *testing.T) { // Delete missing returns NotFoundError. err = store.DeleteBlacklist(ctx, "deas/teectl") - assert.True(t, errors.As(err, &db.NotFoundError{})) + require.ErrorAs(t, err, &db.NotFoundError{}) entries, err = store.ListBlacklist(ctx) require.NoError(t, err) diff --git a/pkg/db/mongodb/bootstrap.go b/pkg/db/mongodb/bootstrap.go index 99c2bda..860abb8 100644 --- a/pkg/db/mongodb/bootstrap.go +++ b/pkg/db/mongodb/bootstrap.go @@ -55,7 +55,7 @@ func (m *MongoDB) Bootstrap() error { } if _, err := m.client.Collection(blacklistCollName).Indexes().CreateMany(context.Background(), blacklistModels); err != nil { - return fmt.Errorf("unable to create blacklist indexes: %w", err) + return fmt.Errorf("creating blacklist indexes: %w", err) } return nil diff --git a/pkg/handlers/blacklist.go b/pkg/handlers/blacklist.go index ca15c78..80afda4 100644 --- a/pkg/handlers/blacklist.go +++ b/pkg/handlers/blacklist.go @@ -1,6 +1,7 @@ package handlers import ( + "context" "encoding/json" "errors" "io" @@ -14,6 +15,13 @@ import ( // repositoryRegexp matches a GitHub full name: owner/repo (exactly one slash, no spaces). var repositoryRegexp = regexp.MustCompile(`^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$`) +// BlacklistStorer is capable of storing the plugin blacklist. +type BlacklistStorer interface { + ListBlacklist(ctx context.Context) ([]db.BlacklistEntry, error) + UpsertBlacklist(ctx context.Context, entry db.BlacklistEntry) (db.BlacklistEntry, error) + DeleteBlacklist(ctx context.Context, repository string) error +} + // ListBlacklist lists the blacklisted repositories. func (h Handlers) ListBlacklist(rw http.ResponseWriter, req *http.Request) { ctx, span := h.tracer.Start(req.Context(), "handler_list_blacklist") @@ -49,16 +57,16 @@ func (h Handlers) AddToBlacklist(rw http.ResponseWriter, req *http.Request) { body, err := io.ReadAll(req.Body) if err != nil { span.RecordError(err) - log.Error().Err(err).Msg("Error reading body for blacklist creation") + log.Error().Err(err).Msg("Unable to read body for adding entry in the blacklist") JSONError(rw, http.StatusBadRequest, err.Error()) return } - entry := db.BlacklistEntry{} + var entry db.BlacklistEntry if err = json.Unmarshal(body, &entry); err != nil { span.RecordError(err) - log.Error().Err(err).Msg("Error decoding blacklist entry") + log.Error().Err(err).Msg("Unable to decode blacklist entry") JSONError(rw, http.StatusBadRequest, err.Error()) return @@ -75,17 +83,15 @@ func (h Handlers) AddToBlacklist(rw http.ResponseWriter, req *http.Request) { created, err := h.store.UpsertBlacklist(ctx, entry) if err != nil { span.RecordError(err) - logger.Error().Err(err).Msg("Error persisting blacklist entry") + logger.Error().Err(err).Msg("Unable to persist blacklist entry") JSONInternalServerError(rw) return } - rw.WriteHeader(http.StatusOK) - if err := json.NewEncoder(rw).Encode(created); err != nil { span.RecordError(err) - logger.Error().Err(err).Msg("Error sending blacklist response") + logger.Error().Err(err).Msg("Unable to send blacklist response") JSONInternalServerError(rw) return @@ -93,32 +99,18 @@ func (h Handlers) AddToBlacklist(rw http.ResponseWriter, req *http.Request) { } // DeleteFromBlacklist removes a repository from the blacklist. -// The repository is passed as a query parameter: ?repository=owner/repo. +// The repository is passed in the path: DELETE /internal/blacklist/{owner}/{repo}. func (h Handlers) DeleteFromBlacklist(rw http.ResponseWriter, req *http.Request) { ctx, span := h.tracer.Start(req.Context(), "handler_delete_from_blacklist") defer span.End() - rw.Header().Set("Content-Type", "application/json") - - repository := req.URL.Query().Get("repository") - if repository == "" { - JSONError(rw, http.StatusBadRequest, "missing repository query parameter") - - return - } + repository := req.PathValue("owner") + "/" + req.PathValue("repo") logger := log.With().Str("repository", repository).Logger() err := h.store.DeleteBlacklist(ctx, repository) - if err != nil { + if err != nil && !errors.As(err, &db.NotFoundError{}) { span.RecordError(err) - - if errors.As(err, &db.NotFoundError{}) { - NotFound(rw, req) - - return - } - logger.Error().Err(err).Msg("Failed to delete blacklist entry") JSONInternalServerError(rw) diff --git a/pkg/handlers/blacklist_test.go b/pkg/handlers/blacklist_test.go index 3df7eb7..b18373f 100644 --- a/pkg/handlers/blacklist_test.go +++ b/pkg/handlers/blacklist_test.go @@ -90,7 +90,9 @@ func TestHandlers_DeleteFromBlacklist(t *testing.T) { } rw := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist?repository=deas/teectl", http.NoBody) + req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist/deas/teectl", http.NoBody) + req.SetPathValue("owner", "deas") + req.SetPathValue("repo", "teectl") New(testDB, nil, nil).DeleteFromBlacklist(rw, req) @@ -98,22 +100,6 @@ func TestHandlers_DeleteFromBlacklist(t *testing.T) { assert.Equal(t, "deas/teectl", deleted) } -func TestHandlers_DeleteFromBlacklist_missingRepository(t *testing.T) { - testDB := mockDB{ - deleteBlacklistFn: func(_ context.Context, _ string) error { - t.Fatal("store should not be called without a repository") - return nil - }, - } - - rw := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist", http.NoBody) - - New(testDB, nil, nil).DeleteFromBlacklist(rw, req) - - assert.Equal(t, http.StatusBadRequest, rw.Code) -} - func TestHandlers_DeleteFromBlacklist_notFound(t *testing.T) { testDB := mockDB{ deleteBlacklistFn: func(_ context.Context, _ string) error { @@ -122,9 +108,11 @@ func TestHandlers_DeleteFromBlacklist_notFound(t *testing.T) { } rw := httptest.NewRecorder() - req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist?repository=deas/teectl", http.NoBody) + req := httptest.NewRequest(http.MethodDelete, "/internal/blacklist/deas/teectl", http.NoBody) + req.SetPathValue("owner", "deas") + req.SetPathValue("repo", "teectl") New(testDB, nil, nil).DeleteFromBlacklist(rw, req) - assert.Equal(t, http.StatusNotFound, rw.Code) + assert.Equal(t, http.StatusNoContent, rw.Code) } diff --git a/pkg/handlers/handlers.go b/pkg/handlers/handlers.go index 7ff4a27..1730d01 100644 --- a/pkg/handlers/handlers.go +++ b/pkg/handlers/handlers.go @@ -36,22 +36,24 @@ type PluginStorer interface { CreateHash(ctx context.Context, module, version, hash string) (db.PluginHash, error) UpdateHashVerified(ctx context.Context, module, version, hash string, verified bool) (db.PluginHash, error) GetHashByName(ctx context.Context, module, version string) (db.PluginHash, error) +} - ListBlacklist(ctx context.Context) ([]db.BlacklistEntry, error) - UpsertBlacklist(ctx context.Context, entry db.BlacklistEntry) (db.BlacklistEntry, error) - DeleteBlacklist(ctx context.Context, repository string) error +// Storer is capable of storing plugins and the plugin blacklist. +type Storer interface { + PluginStorer + BlacklistStorer } // Handlers a set of handlers. type Handlers struct { - store PluginStorer + store Storer goProxy *goproxy.Client gh *github.Client tracer trace.Tracer } // New creates all HTTP handlers. -func New(store PluginStorer, goProxy *goproxy.Client, gh *github.Client) Handlers { +func New(store Storer, goProxy *goproxy.Client, gh *github.Client) Handlers { return Handlers{ store: store, goProxy: goProxy,