Bladeren bron

protect against nil filter (#3841)

Alex Meijer 1 week geleden
bovenliggende
commit
856462fabd

+ 12 - 0
core/pkg/autocomplete/filter.go

@@ -0,0 +1,12 @@
+package autocomplete
+
+import (
+	"github.com/opencost/opencost/core/pkg/filter"
+	"github.com/opencost/opencost/core/pkg/filter/ast"
+)
+
+// HasFilter reports whether the request carries a user-provided filter.
+// Omitted or empty filters normalize to VoidOp and return false.
+func HasFilter(f filter.Filter) bool {
+	return f != nil && f.Op() != ast.FilterOpVoid
+}

+ 22 - 0
core/pkg/autocomplete/filter_test.go

@@ -0,0 +1,22 @@
+package autocomplete
+
+import (
+	"testing"
+
+	"github.com/opencost/opencost/core/pkg/filter/ast"
+)
+
+func TestHasFilter(t *testing.T) {
+	if HasFilter(nil) {
+		t.Fatal("expected nil filter to be treated as no filter")
+	}
+	if HasFilter(&ast.VoidOp{}) {
+		t.Fatal("expected void filter to be treated as no filter")
+	}
+	if !HasFilter(&ast.EqualOp{
+		Left:  ast.Identifier{Field: ast.NewField("cluster")},
+		Right: "c1",
+	}) {
+		t.Fatal("expected non-void filter to be treated as active filter")
+	}
+}

+ 4 - 0
core/pkg/autocomplete/normalize.go

@@ -3,6 +3,7 @@ package autocomplete
 import (
 	"fmt"
 
+	"github.com/opencost/opencost/core/pkg/filter/ast"
 	"github.com/opencost/opencost/core/pkg/opencost"
 )
 
@@ -57,6 +58,9 @@ func NormalizeRequest(req *Request, validateField FieldValidator, opts Normalize
 	req.Field = field
 	req.Search = SanitizeSearch(req.Search)
 	req.Limit = limit
+	if req.Filter == nil {
+		req.Filter = &ast.VoidOp{}
+	}
 	if opts.EnsureLabelConfig && req.LabelConfig == nil {
 		req.LabelConfig = opencost.NewLabelConfig()
 	}

+ 5 - 1
core/pkg/autocomplete/parse.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/opencost/opencost/core/pkg/filter"
+	"github.com/opencost/opencost/core/pkg/filter/ast"
 	"github.com/opencost/opencost/core/pkg/opencost"
 	"github.com/opencost/opencost/core/pkg/util/httputil"
 )
@@ -53,7 +54,7 @@ func ParseRequest(qp httputil.QueryParams, opts ParseOptions, validateField Fiel
 	}
 
 	filterString := qp.Get("filter", "")
-	var parsedFilter filter.Filter
+	var parsedFilter filter.Filter = &ast.VoidOp{}
 	if filterString != "" {
 		if parseFilter == nil {
 			return nil, fmt.Errorf("%w: invalid 'filter' parameter: filter parser is required", ErrBadRequest)
@@ -62,6 +63,9 @@ func ParseRequest(qp httputil.QueryParams, opts ParseOptions, validateField Fiel
 		if err != nil {
 			return nil, fmt.Errorf("%w: invalid 'filter' parameter: %w", ErrBadRequest, err)
 		}
+		if parsedFilter == nil {
+			parsedFilter = &ast.VoidOp{}
+		}
 	}
 
 	tenantID := qp.Get("tenantId", opts.DefaultTenantID)

+ 20 - 0
core/pkg/autocomplete/request_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/opencost/opencost/core/pkg/filter"
+	"github.com/opencost/opencost/core/pkg/filter/ast"
 	"github.com/opencost/opencost/core/pkg/opencost"
 	"github.com/opencost/opencost/core/pkg/util/httputil"
 )
@@ -42,6 +43,19 @@ func TestNormalizeRequest(t *testing.T) {
 		t.Fatal("expected default label config")
 	}
 
+	nilFilterReq := &Request{
+		TenantID: "t1",
+		Field:    "label",
+		Window:   opencost.NewClosedWindow(start, start.Add(24*time.Hour)),
+	}
+	_, err = NormalizeRequest(nilFilterReq, validateTestField, NormalizeOptions{RequireTenantID: true})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if nilFilterReq.Filter == nil || nilFilterReq.Filter.Op() != ast.FilterOpVoid {
+		t.Fatalf("expected nil filter normalized to void op, got %+v", nilFilterReq.Filter)
+	}
+
 	_, err = NormalizeRequest(nil, validateTestField, NormalizeOptions{})
 	if err == nil || !errors.Is(err, ErrBadRequest) {
 		t.Fatalf("expected nil request error, got %v", err)
@@ -96,6 +110,9 @@ func TestParseRequest(t *testing.T) {
 	if got.Field != "cluster" || got.Search != "ns" || got.TenantID != "t1" {
 		t.Fatalf("unexpected request: %+v", got)
 	}
+	if got.Filter == nil || got.Filter.Op() != ast.FilterOpVoid {
+		t.Fatalf("expected void filter when filter param omitted, got %+v", got.Filter)
+	}
 
 	_, err = ParseRequest(httputil.NewQueryParams(map[string][]string{"field": {"cluster"}}), ParseOptions{}, validateTestField, nil)
 	if err == nil || !errors.Is(err, ErrBadRequest) {
@@ -155,4 +172,7 @@ func TestParseRequest(t *testing.T) {
 	if got.Field != "cluster" {
 		t.Fatalf("unexpected request: %+v", got)
 	}
+	if got.Filter == nil || got.Filter.Op() != ast.FilterOpVoid {
+		t.Fatalf("expected nil parse result normalized to void filter, got %+v", got.Filter)
+	}
 }

+ 1 - 1
pkg/allocation/autocompletequeryservice.go

@@ -18,7 +18,7 @@ func QueryAllocationAutocompleteFromSetRange(asr *opencost.AllocationSetRange, r
 	}
 
 	var matcher opencost.AllocationMatcher
-	if req.Filter != nil {
+	if autocomplete.HasFilter(req.Filter) {
 		compiler := opencost.NewAllocationMatchCompiler(req.LabelConfig)
 		matcher, err = compiler.Compile(req.Filter)
 		if err != nil {

+ 2 - 0
pkg/allocation/autocompletequeryservice_test.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/opencost/opencost/core/pkg/autocomplete"
+	"github.com/opencost/opencost/core/pkg/filter/ast"
 	"github.com/opencost/opencost/core/pkg/opencost"
 )
 
@@ -41,6 +42,7 @@ func TestQueryAllocationAutocompleteFromSetRange(t *testing.T) {
 		Field:  "label",
 		Limit:  10,
 		Window: window,
+		Filter: &ast.VoidOp{},
 	})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)

+ 1 - 1
pkg/asset/autocompletequeryservice.go

@@ -37,7 +37,7 @@ func QueryAssetAutocompleteFromSet(assetSet *opencost.AssetSet, req autocomplete
 	}
 
 	var matcher opencost.AssetMatcher
-	if req.Filter != nil {
+	if autocomplete.HasFilter(req.Filter) {
 		compiler := opencost.NewAssetMatchCompiler()
 		matcher, err = compiler.Compile(req.Filter)
 		if err != nil {

+ 2 - 0
pkg/asset/autocompletequeryservice_test.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/opencost/opencost/core/pkg/autocomplete"
+	"github.com/opencost/opencost/core/pkg/filter/ast"
 	"github.com/opencost/opencost/core/pkg/opencost"
 )
 
@@ -29,6 +30,7 @@ func TestQueryAssetAutocompleteFromSet(t *testing.T) {
 		TenantID: "opencost",
 		Field:    "cluster",
 		Window:   window,
+		Filter:   &ast.VoidOp{},
 	})
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)