Skip to content

Commit

Permalink
indexer: Return 4XX status code when Index() returns tarfs.ErrBadFormat
Browse files Browse the repository at this point in the history
Currently when there is any error indexing a manifest we return InternalServerError.
As Clair can be sent layer files that are not valid, we should return
a client error. This will also help unmask real unexpected application
failures.

Signed-off-by: crozzy <joseph.crosland@gmail.com>
  • Loading branch information
crozzy committed Aug 15, 2022
1 parent 9612ee6 commit 8e5d76d
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 1 deletion.
8 changes: 7 additions & 1 deletion httptransport/indexer_v1.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/ldelossa/responserecorder"
"github.com/quay/claircore"
"github.com/quay/claircore/pkg/tarfs"
"github.com/quay/zlog"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"

Expand Down Expand Up @@ -111,7 +112,12 @@ func (h *IndexerV1) indexReport(w http.ResponseWriter, r *http.Request) {
// TODO Do we need some sort of background context embedded in the HTTP
// struct?
report, err := h.srv.Index(ctx, &m)
if err != nil {
switch {
case errors.Is(err, nil):
case errors.Is(err, tarfs.ErrFormat):
apiError(w, http.StatusBadRequest, "failed to start scan: %v", err)
return
default:
apiError(w, http.StatusInternalServerError, "failed to start scan: %v", err)
return
}
Expand Down
44 changes: 44 additions & 0 deletions httptransport/indexer_v1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,57 @@ import (
"testing"

"github.com/quay/claircore"
"github.com/quay/claircore/pkg/tarfs"
"github.com/quay/zlog"
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
"go.opentelemetry.io/otel/trace"

"github.com/quay/clair/v4/indexer"
)

func TestIndexReportBadLayer(t *testing.T) {
ctx := context.Background()
ctx = zlog.Test(ctx, t)

i := &indexer.Mock{
State_: func(ctx context.Context) (string, error) {
return `deadbeef`, nil
},
Index_: func(ctx context.Context, m *claircore.Manifest) (*claircore.IndexReport, error) {
return nil, tarfs.ErrFormat
},
}
v1, err := NewIndexerV1(ctx, "", i, otelhttp.WithTracerProvider(trace.NewNoopTracerProvider()))
if err != nil {
t.Fatal(err)
}
srv := httptest.NewUnstartedServer(v1)
srv.Config.BaseContext = func(_ net.Listener) context.Context { return ctx }
srv.Start()
defer srv.Close()
t.Run("Report", func(t *testing.T) {
ctx := zlog.Test(ctx, t)
const path = `/index_report`
t.Run("POST", func(t *testing.T) {
const body = `{"hash":"sha256:0000000000000000000000000000000000000000000000000000000000000000",` +
`"layers":[{}]}`
req, err := http.NewRequestWithContext(ctx, http.MethodPost, srv.URL+path, strings.NewReader(body))
if err != nil {
t.Fatal(err)
}
res, err := srv.Client().Do(req)
if err != nil {
t.Fatal(err)
}
got, want := res.StatusCode, http.StatusBadRequest
t.Logf("got: %d, want: %d", got, want)
if got != want {
t.Error()
}
})
})
}

func TestIndexerV1(t *testing.T) {
ctx := context.Background()
ctx = zlog.Test(ctx, t)
Expand Down

0 comments on commit 8e5d76d

Please sign in to comment.