Skip to content

Commit a1f5923

Browse files
authored
clickhouse(depstore): return error instead of panic in GetDependencies (#8172)
### Motivation - The ClickHouse storage backend is selectable and its dependency reader previously used `panic("not implemented")`, which can crash the query service when invoked and cause a DoS. This change prevents process termination by returning an error. ### Description - Replace the panic in `(*Reader).GetDependencies` with a returned error. - Update the unit test in `internal/storage/v2/clickhouse/depstore/reader_test.go` to assert a `nil` result and the expected error instead of expecting a panic. ### Testing - `make fmt` - `make lint` - `make test` Signed-off-by: Jonah Kowall <jkowall@kowall.net>
1 parent a272adc commit a1f5923

2 files changed

Lines changed: 7 additions & 4 deletions

File tree

internal/storage/v2/clickhouse/depstore/reader.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package depstore
55

66
import (
77
"context"
8+
"errors"
89

910
"github.com/jaegertracing/jaeger-idl/model/v1"
1011
"github.com/jaegertracing/jaeger/internal/storage/v2/api/depstore"
@@ -19,5 +20,5 @@ func NewDependencyReader() *Reader {
1920
}
2021

2122
func (*Reader) GetDependencies(context.Context, depstore.QueryParameters) ([]model.DependencyLink, error) {
22-
panic("not implemented")
23+
return nil, errors.New("clickhouse dependency reader is not implemented")
2324
}

internal/storage/v2/clickhouse/depstore/reader_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88
"testing"
99

10+
"github.com/stretchr/testify/assert"
1011
"github.com/stretchr/testify/require"
1112

1213
"github.com/jaegertracing/jaeger/internal/storage/v2/api/depstore"
@@ -17,7 +18,8 @@ func TestReader_GetDependencies(t *testing.T) {
1718
ctx := context.Background()
1819
query := depstore.QueryParameters{}
1920

20-
require.Panics(t, func() {
21-
reader.GetDependencies(ctx, query)
22-
})
21+
dependencies, err := reader.GetDependencies(ctx, query)
22+
23+
require.Nil(t, dependencies)
24+
assert.EqualError(t, err, "clickhouse dependency reader is not implemented")
2325
}

0 commit comments

Comments
 (0)