From 2e4ee872d98baf4ad8f1525b389644e771225993 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Thu, 16 Feb 2023 15:32:29 -0800 Subject: [PATCH 1/9] Add support for OR filters Currently, multiple filters can be appended on the query string, and each subsequent filter is ANDed with the set. Items that pass through the filter set must match every filter in the set. This change adds support for OR filters. A single filter key can specify multiple filters, separated by ','. An item that passes this filter can match any filter in the set. For example, this filter matches items that have either "name" or "namespace" that match "example": ?filter=metadata.name=example,metadata.namespace=example This filter matches items that have "name" that matches either "foo" or "bar": ?filter=metadata.name=foo,metadata.name=bar Specifying more than one filter key in the query still ANDs each inner filter set together. This set of filters can match either a name of "foo" or "bar", but must in all cases match namespace "abc": ?filter=metadata.name=foo,metadata.name=bar&filter=metadata.namespace=abc --- README.md | 8 +- .../partition/listprocessor/processor.go | 76 +++- .../partition/listprocessor/processor_test.go | 357 ++++++++++++++++-- pkg/stores/partition/store_test.go | 156 ++++++++ 4 files changed, 547 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 8343c5a..935b79e 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,13 @@ Example, filtering by object name: /v1/{type}?filter=metadata.name=foo ``` -Filters are ANDed together, so an object must match all filters to be +One filter can list multiple possible fields to match, these are ORed together: + +``` +/v1/{type}?filter=metadata.name=foo,metadata.namespace=foo +``` + +Stacked filters are ANDed together, so an object must match all filters to be included in the list. ``` diff --git a/pkg/stores/partition/listprocessor/processor.go b/pkg/stores/partition/listprocessor/processor.go index 67cace3..75bdee4 100644 --- a/pkg/stores/partition/listprocessor/processor.go +++ b/pkg/stores/partition/listprocessor/processor.go @@ -21,13 +21,14 @@ const ( pageSizeParam = "pagesize" pageParam = "page" revisionParam = "revision" + orOp = "," ) // ListOptions represents the query parameters that may be included in a list request. type ListOptions struct { ChunkSize int Resume string - Filters []Filter + Filters []OrFilter Sort Sort Pagination Pagination Revision string @@ -47,6 +48,25 @@ func (f Filter) String() string { return field + "=" + f.match } +// OrFilter represents a set of possible fields to filter by, where an item may match any filter in the set to be included in the result. +type OrFilter struct { + filters []Filter +} + +// String returns the filter as a query string. +func (f OrFilter) String() string { + var fields strings.Builder + for i, field := range f.filters { + fields.WriteString(strings.Join(field.field, ".")) + fields.WriteByte('=') + fields.WriteString(field.match) + if i < len(f.filters)-1 { + fields.WriteByte(',') + } + } + return fields.String() +} + // SortOrder represents whether the list should be ascending or descending. type SortOrder int @@ -102,19 +122,36 @@ func ParseQuery(apiOp *types.APIRequest) *ListOptions { q := apiOp.Request.URL.Query() cont := q.Get(continueParam) filterParams := q[filterParam] - filterOpts := []Filter{} + filterOpts := []OrFilter{} for _, filters := range filterParams { - filter := strings.Split(filters, "=") - if len(filter) != 2 { - continue + orFilters := strings.Split(filters, orOp) + orFilter := OrFilter{} + for _, filter := range orFilters { + filter := strings.Split(filter, "=") + if len(filter) != 2 { + continue + } + orFilter.filters = append(orFilter.filters, Filter{field: strings.Split(filter[0], "."), match: filter[1]}) } - filterOpts = append(filterOpts, Filter{field: strings.Split(filter[0], "."), match: filter[1]}) + filterOpts = append(filterOpts, orFilter) } // sort the filter fields so they can be used as a cache key in the store + for _, orFilter := range filterOpts { + sort.Slice(orFilter.filters, func(i, j int) bool { + fieldI := strings.Join(orFilter.filters[i].field, ".") + fieldJ := strings.Join(orFilter.filters[j].field, ".") + return fieldI < fieldJ + }) + } sort.Slice(filterOpts, func(i, j int) bool { - fieldI := strings.Join(filterOpts[i].field, ".") - fieldJ := strings.Join(filterOpts[j].field, ".") - return fieldI < fieldJ + var fieldI, fieldJ strings.Builder + for _, f := range filterOpts[i].filters { + fieldI.WriteString(strings.Join(f.field, ".")) + } + for _, f := range filterOpts[j].filters { + fieldJ.WriteString(strings.Join(f.field, ".")) + } + return fieldI.String() < fieldJ.String() }) sortOpts := Sort{} sortKeys := q.Get(sortParam) @@ -174,7 +211,7 @@ func getLimit(apiOp *types.APIRequest) int { // FilterList accepts a channel of unstructured objects and a slice of filters and returns the filtered list. // Filters are ANDed together. -func FilterList(list <-chan []unstructured.Unstructured, filters []Filter) []unstructured.Unstructured { +func FilterList(list <-chan []unstructured.Unstructured, filters []OrFilter) []unstructured.Unstructured { result := []unstructured.Unstructured{} for items := range list { for _, item := range items { @@ -215,14 +252,14 @@ func matchesOne(obj map[string]interface{}, filter Filter) bool { } case []interface{}: filter = Filter{field: subField, match: filter.match} - if matchesAny(typedVal, filter) { + if matchesOneInList(typedVal, filter) { return true } } return false } -func matchesAny(obj []interface{}, filter Filter) bool { +func matchesOneInList(obj []interface{}, filter Filter) bool { for _, v := range obj { switch typedItem := v.(type) { case string, int, bool: @@ -235,7 +272,7 @@ func matchesAny(obj []interface{}, filter Filter) bool { return true } case []interface{}: - if matchesAny(typedItem, filter) { + if matchesOneInList(typedItem, filter) { return true } } @@ -243,9 +280,18 @@ func matchesAny(obj []interface{}, filter Filter) bool { return false } -func matchesAll(obj map[string]interface{}, filters []Filter) bool { +func matchesAny(obj map[string]interface{}, filter OrFilter) bool { + for _, f := range filter.filters { + if matches := matchesOne(obj, f); matches { + return true + } + } + return false +} + +func matchesAll(obj map[string]interface{}, filters []OrFilter) bool { for _, f := range filters { - if !matchesOne(obj, f) { + if !matchesAny(obj, f) { return false } } diff --git a/pkg/stores/partition/listprocessor/processor_test.go b/pkg/stores/partition/listprocessor/processor_test.go index dfc5f9d..2f0ba64 100644 --- a/pkg/stores/partition/listprocessor/processor_test.go +++ b/pkg/stores/partition/listprocessor/processor_test.go @@ -11,7 +11,7 @@ func TestFilterList(t *testing.T) { tests := []struct { name string objects [][]unstructured.Unstructured - filters []Filter + filters []OrFilter want []unstructured.Unstructured }{ { @@ -42,10 +42,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color"}, - match: "pink", + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -101,14 +105,22 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color"}, - match: "pink", + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + }, + }, }, { - field: []string{"metadata", "name"}, - match: "honey", + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "honey", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -153,10 +165,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color"}, - match: "purple", + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "purple", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -189,7 +205,7 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{}, + filters: []OrFilter{}, want: []unstructured.Unstructured{ { Object: map[string]interface{}{ @@ -254,10 +270,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"spec", "volumes"}, - match: "hostPath", + filters: []Filter{ + { + field: []string{"spec", "volumes"}, + match: "hostPath", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -301,10 +321,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "productType"}, - match: "tablet", + filters: []Filter{ + { + field: []string{"data", "productType"}, + match: "tablet", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -326,10 +350,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "color", "shade"}, - match: "green", + filters: []Filter{ + { + field: []string{"data", "color", "shade"}, + match: "green", + }, + }, }, }, want: []unstructured.Unstructured{}, @@ -384,10 +412,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "colors"}, - match: "yellow", + filters: []Filter{ + { + field: []string{"data", "colors"}, + match: "yellow", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -496,10 +528,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "varieties", "color"}, - match: "red", + filters: []Filter{ + { + field: []string{"data", "varieties", "color"}, + match: "red", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -625,10 +661,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "attributes"}, - match: "black", + filters: []Filter{ + { + field: []string{"data", "attributes"}, + match: "black", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -752,10 +792,14 @@ func TestFilterList(t *testing.T) { }, }, }, - filters: []Filter{ + filters: []OrFilter{ { - field: []string{"data", "attributes", "green"}, - match: "plantain", + filters: []Filter{ + { + field: []string{"data", "attributes", "green"}, + match: "plantain", + }, + }, }, }, want: []unstructured.Unstructured{ @@ -781,6 +825,251 @@ func TestFilterList(t *testing.T) { }, }, }, + { + name: "single or filter, filter on one value", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "pink", + }, + { + field: []string{"data", "color"}, + match: "pink", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + }, + }, + { + name: "single or filter, filter on different value", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "pink", + }, + { + field: []string{"metadata", "name"}, + match: "pom", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + }, + }, + }, + }, + { + name: "single or filter, no matches", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "blue", + }, + { + field: []string{"metadata", "name"}, + match: "watermelon", + }, + }, + }, + }, + want: []unstructured.Unstructured{}, + }, + { + name: "and-ed or filters", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + "data": map[string]interface{}{ + "flavor": "sweet", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + "flavor": "sweet", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "grapefruit", + }, + "data": map[string]interface{}{ + "color": "pink", + "data": map[string]interface{}{ + "flavor": "bitter", + }, + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "pink", + }, + { + field: []string{"data", "color"}, + match: "pink", + }, + }, + }, + { + filters: []Filter{ + { + field: []string{"data", "flavor"}, + match: "sweet", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pink-lady", + }, + "data": map[string]interface{}{ + "flavor": "sweet", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "pomegranate", + }, + "data": map[string]interface{}{ + "color": "pink", + "flavor": "sweet", + }, + }, + }, + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/pkg/stores/partition/store_test.go b/pkg/stores/partition/store_test.go index 72d668f..8d466a4 100644 --- a/pkg/stores/partition/store_test.go +++ b/pkg/stores/partition/store_test.go @@ -278,6 +278,9 @@ func TestList(t *testing.T) { apiOps: []*types.APIRequest{ newRequest("filter=data.color=green", "user1"), newRequest("filter=data.color=green&filter=metadata.name=bramley", "user1"), + newRequest("filter=data.color=green,data.color=pink", "user1"), + newRequest("filter=data.color=green,data.color=pink&filter=metadata.name=fuji", "user1"), + newRequest("filter=data.color=green,data.color=pink&filter=metadata.name=crispin", "user1"), }, access: []map[string]string{ { @@ -286,6 +289,15 @@ func TestList(t *testing.T) { { "user1": "roleA", }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, }, partitions: map[string][]Partition{ "user1": { @@ -318,6 +330,23 @@ func TestList(t *testing.T) { newApple("bramley").toObj(), }, }, + { + Count: 3, + Objects: []types.APIObject{ + newApple("fuji").toObj(), + newApple("granny-smith").toObj(), + newApple("bramley").toObj(), + }, + }, + { + Count: 1, + Objects: []types.APIObject{ + newApple("fuji").toObj(), + }, + }, + { + Count: 0, + }, }, }, { @@ -1739,6 +1768,133 @@ func TestList(t *testing.T) { {"green": 2}, }, }, + { + name: "pagination with or filters", + apiOps: []*types.APIRequest{ + newRequest("filter=metadata.name=el,data.color=el&pagesize=2", "user1"), + newRequest("filter=metadata.name=el,data.color=el&pagesize=2&page=2&revision=42", "user1"), + newRequest("filter=metadata.name=el,data.color=el&pagesize=2&page=3&revision=42", "user1"), + }, + access: []map[string]string{ + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + }, + partitions: map[string][]Partition{ + "user1": { + mockPartition{ + name: "all", + }, + }, + }, + objects: map[string]*unstructured.UnstructuredList{ + "all": { + Object: map[string]interface{}{ + "metadata": map[string]interface{}{ + "resourceVersion": "42", + }, + }, + Items: []unstructured.Unstructured{ + newApple("fuji").Unstructured, + newApple("granny-smith").Unstructured, + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + want: []types.APIObjectList{ + { + Count: 3, + Pages: 2, + Revision: "42", + Objects: []types.APIObject{ + newApple("red-delicious").toObj(), + newApple("golden-delicious").toObj(), + }, + }, + { + Count: 3, + Pages: 2, + Revision: "42", + Objects: []types.APIObject{ + newApple("crispin").toObj(), + }, + }, + { + Count: 3, + Pages: 2, + Revision: "42", + }, + }, + wantCache: []mockCache{ + { + contents: map[cacheKey]*unstructured.UnstructuredList{ + { + chunkSize: 100000, + filters: "data.color=el,metadata.name=el", + pageSize: 2, + accessID: getAccessID("user1", "roleA"), + resourcePath: "/apples", + revision: "42", + }: { + Items: []unstructured.Unstructured{ + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + }, + { + contents: map[cacheKey]*unstructured.UnstructuredList{ + { + chunkSize: 100000, + filters: "data.color=el,metadata.name=el", + pageSize: 2, + accessID: getAccessID("user1", "roleA"), + resourcePath: "/apples", + revision: "42", + }: { + Items: []unstructured.Unstructured{ + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + }, + { + contents: map[cacheKey]*unstructured.UnstructuredList{ + { + chunkSize: 100000, + filters: "data.color=el,metadata.name=el", + pageSize: 2, + accessID: getAccessID("user1", "roleA"), + resourcePath: "/apples", + revision: "42", + }: { + Items: []unstructured.Unstructured{ + newApple("red-delicious").Unstructured, + newApple("golden-delicious").Unstructured, + newApple("crispin").Unstructured, + }, + }, + }, + }, + }, + wantListCalls: []map[string]int{ + {"all": 1}, + {"all": 1}, + {"all": 1}, + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { From 61a39906f934ddff552f811b1eec5efd5f20f661 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Mon, 6 Mar 2023 16:10:50 -0800 Subject: [PATCH 2/9] Add support for NOT filters This change adds support for excluding results using the != operator. Example to exclude all results with name "example": ?filter=metadata.name!=example Include all results from namespace "example" but exclude those with name "example": ?filter=metadata.namespace=example&metadata.name!=example Exclude results with name "foo" OR exclude results with name "bar" (effectively includes results of both types): ?filter=metadata.name!=foo,metadata.name!=bar --- .../partition/listprocessor/processor.go | 24 +- .../partition/listprocessor/processor_test.go | 652 ++++++++++++++++++ pkg/stores/partition/store_test.go | 33 + 3 files changed, 705 insertions(+), 4 deletions(-) diff --git a/pkg/stores/partition/listprocessor/processor.go b/pkg/stores/partition/listprocessor/processor.go index 75bdee4..a316a53 100644 --- a/pkg/stores/partition/listprocessor/processor.go +++ b/pkg/stores/partition/listprocessor/processor.go @@ -2,6 +2,7 @@ package listprocessor import ( + "regexp" "sort" "strconv" "strings" @@ -24,6 +25,15 @@ const ( orOp = "," ) +var opReg = regexp.MustCompile(`[!]?=`) + +type op string + +const ( + eq op = "" + notEq op = "!=" +) + // ListOptions represents the query parameters that may be included in a list request. type ListOptions struct { ChunkSize int @@ -40,6 +50,7 @@ type ListOptions struct { type Filter struct { field []string match string + op op } // String returns the filter as a query string. @@ -127,11 +138,15 @@ func ParseQuery(apiOp *types.APIRequest) *ListOptions { orFilters := strings.Split(filters, orOp) orFilter := OrFilter{} for _, filter := range orFilters { - filter := strings.Split(filter, "=") + var op op + if strings.Contains(filter, "!=") { + op = "!=" + } + filter := opReg.Split(filter, -1) if len(filter) != 2 { continue } - orFilter.filters = append(orFilter.filters, Filter{field: strings.Split(filter[0], "."), match: filter[1]}) + orFilter.filters = append(orFilter.filters, Filter{field: strings.Split(filter[0], "."), match: filter[1], op: op}) } filterOpts = append(filterOpts, orFilter) } @@ -251,7 +266,7 @@ func matchesOne(obj map[string]interface{}, filter Filter) bool { return true } case []interface{}: - filter = Filter{field: subField, match: filter.match} + filter = Filter{field: subField, match: filter.match, op: filter.op} if matchesOneInList(typedVal, filter) { return true } @@ -282,7 +297,8 @@ func matchesOneInList(obj []interface{}, filter Filter) bool { func matchesAny(obj map[string]interface{}, filter OrFilter) bool { for _, f := range filter.filters { - if matches := matchesOne(obj, f); matches { + matches := matchesOne(obj, f) + if (matches && f.op == eq) || (!matches && f.op == notEq) { return true } } diff --git a/pkg/stores/partition/listprocessor/processor_test.go b/pkg/stores/partition/listprocessor/processor_test.go index 2f0ba64..a5e7c96 100644 --- a/pkg/stores/partition/listprocessor/processor_test.go +++ b/pkg/stores/partition/listprocessor/processor_test.go @@ -1070,6 +1070,658 @@ func TestFilterList(t *testing.T) { }, }, }, + { + name: "not filter", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + }, + "data": map[string]interface{}{ + "color": "green", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + op: "!=", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + }, + "data": map[string]interface{}{ + "color": "green", + }, + }, + }, + }, + }, + { + name: "or'ed not filter", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + }, + "data": map[string]interface{}{ + "color": "green", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + op: "!=", + }, + { + field: []string{"data", "color"}, + match: "green", + op: "!=", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + }, + "data": map[string]interface{}{ + "color": "green", + }, + }, + }, + }, + }, + { + name: "mixed or'ed filter", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + }, + "data": map[string]interface{}{ + "color": "green", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + op: "!=", + }, + { + field: []string{"metadata", "name"}, + match: "fuji", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + }, + "data": map[string]interface{}{ + "color": "green", + }, + }, + }, + }, + }, + { + name: "anded and or'ed mixed equality filter", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + }, + "data": map[string]interface{}{ + "color": "green", + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "fuji", + op: "!=", + }, + }, + }, + { + filters: []Filter{ + { + field: []string{"data", "color"}, + match: "pink", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + }, + "data": map[string]interface{}{ + "color": "pink", + }, + }, + }, + }, + }, + { + name: "match string array with not", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "apple", + }, + "data": map[string]interface{}{ + "colors": []interface{}{ + "pink", + "red", + "green", + "yellow", + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "berry", + }, + "data": map[string]interface{}{ + "colors": []interface{}{ + "blue", + "red", + "black", + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "banana", + }, + "data": map[string]interface{}{ + "colors": []interface{}{ + "yellow", + }, + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"data", "colors"}, + match: "yellow", + op: "!=", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "berry", + }, + "data": map[string]interface{}{ + "colors": []interface{}{ + "blue", + "red", + "black", + }, + }, + }, + }, + }, + }, + { + name: "match object array with not", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "apple", + }, + "data": map[string]interface{}{ + "varieties": []interface{}{ + map[string]interface{}{ + "name": "fuji", + "color": "pink", + }, + map[string]interface{}{ + "name": "granny-smith", + "color": "green", + }, + map[string]interface{}{ + "name": "red-delicious", + "color": "red", + }, + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "berry", + }, + "data": map[string]interface{}{ + "varieties": []interface{}{ + map[string]interface{}{ + "name": "blueberry", + "color": "blue", + }, + map[string]interface{}{ + "name": "raspberry", + "color": "red", + }, + map[string]interface{}{ + "name": "blackberry", + "color": "black", + }, + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "banana", + }, + "data": map[string]interface{}{ + "varieties": []interface{}{ + map[string]interface{}{ + "name": "cavendish", + "color": "yellow", + }, + map[string]interface{}{ + "name": "plantain", + "color": "green", + }, + }, + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"data", "varieties", "color"}, + match: "red", + op: "!=", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "banana", + }, + "data": map[string]interface{}{ + "varieties": []interface{}{ + map[string]interface{}{ + "name": "cavendish", + "color": "yellow", + }, + map[string]interface{}{ + "name": "plantain", + "color": "green", + }, + }, + }, + }, + }, + }, + }, + { + name: "match nested array with not", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "apple", + }, + "data": map[string]interface{}{ + "attributes": []interface{}{ + []interface{}{ + "pink", + "green", + "red", + "purple", + }, + []interface{}{ + "fuji", + "granny-smith", + "red-delicious", + "black-diamond", + }, + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "berry", + }, + "data": map[string]interface{}{ + "attributes": []interface{}{ + []interface{}{ + "blue", + "red", + "black", + }, + []interface{}{ + "blueberry", + "raspberry", + "blackberry", + }, + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "banana", + }, + "data": map[string]interface{}{ + "attributes": []interface{}{ + []interface{}{ + "yellow", + "green", + }, + []interface{}{ + "cavendish", + "plantain", + }, + }, + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"data", "attributes"}, + match: "black", + op: "!=", + }, + }, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "banana", + }, + "data": map[string]interface{}{ + "attributes": []interface{}{ + []interface{}{ + "yellow", + "green", + }, + []interface{}{ + "cavendish", + "plantain", + }, + }, + }, + }, + }, + }, + }, + { + name: "match nested object array with mixed equality", + objects: [][]unstructured.Unstructured{ + { + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "apple", + }, + "data": map[string]interface{}{ + "attributes": []interface{}{ + []interface{}{ + map[string]interface{}{ + "pink": "fuji", + }, + map[string]interface{}{ + "green": "granny-smith", + }, + map[string]interface{}{ + "pink": "honeycrisp", + }, + }, + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "berry", + }, + "data": map[string]interface{}{ + "attributes": []interface{}{ + []interface{}{ + map[string]interface{}{ + "blue": "blueberry", + }, + map[string]interface{}{ + "red": "raspberry", + }, + map[string]interface{}{ + "black": "blackberry", + }, + }, + }, + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "fruit", + "metadata": map[string]interface{}{ + "name": "banana", + }, + "data": map[string]interface{}{ + "attributes": []interface{}{ + []interface{}{ + map[string]interface{}{ + "yellow": "cavendish", + }, + map[string]interface{}{ + "green": "plantain", + }, + }, + }, + }, + }, + }, + }, + }, + filters: []OrFilter{ + { + filters: []Filter{ + { + field: []string{"data", "attributes", "green"}, + match: "plantain", + op: "!=", + }, + { + field: []string{"data", "attributes", "green"}, + match: "granny-smith", + }, + }, + }, + { + filters: []Filter{ + { + field: []string{"metadata", "name"}, + match: "banana", + }, + }, + }, + }, + want: []unstructured.Unstructured{}, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { diff --git a/pkg/stores/partition/store_test.go b/pkg/stores/partition/store_test.go index 8d466a4..fdd666c 100644 --- a/pkg/stores/partition/store_test.go +++ b/pkg/stores/partition/store_test.go @@ -281,6 +281,9 @@ func TestList(t *testing.T) { newRequest("filter=data.color=green,data.color=pink", "user1"), newRequest("filter=data.color=green,data.color=pink&filter=metadata.name=fuji", "user1"), newRequest("filter=data.color=green,data.color=pink&filter=metadata.name=crispin", "user1"), + newRequest("filter=data.color!=green", "user1"), + newRequest("filter=data.color!=green,metadata.name=granny-smith", "user1"), + newRequest("filter=data.color!=green&filter=metadata.name!=crispin", "user1"), }, access: []map[string]string{ { @@ -298,6 +301,15 @@ func TestList(t *testing.T) { { "user1": "roleA", }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, }, partitions: map[string][]Partition{ "user1": { @@ -347,6 +359,27 @@ func TestList(t *testing.T) { { Count: 0, }, + { + Count: 2, + Objects: []types.APIObject{ + newApple("fuji").toObj(), + newApple("crispin").toObj(), + }, + }, + { + Count: 3, + Objects: []types.APIObject{ + newApple("fuji").toObj(), + newApple("granny-smith").toObj(), + newApple("crispin").toObj(), + }, + }, + { + Count: 1, + Objects: []types.APIObject{ + newApple("fuji").toObj(), + }, + }, }, }, { From 13ac720b0ed2b48f6e09e623e9ec9de8b53f3499 Mon Sep 17 00:00:00 2001 From: "renovate-rancher[bot]" <119870437+renovate-rancher[bot]@users.noreply.github.com> Date: Tue, 18 Apr 2023 11:24:39 +0000 Subject: [PATCH 3/9] Add initial Renovate configuration --- .github/renovate.json | 9 +++++++++ .github/workflows/renovate.yml | 25 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 .github/renovate.json create mode 100644 .github/workflows/renovate.yml diff --git a/.github/renovate.json b/.github/renovate.json new file mode 100644 index 0000000..6d7f10c --- /dev/null +++ b/.github/renovate.json @@ -0,0 +1,9 @@ +{ + "extends": [ + "github>rancher/renovate-config#release" + ], + "baseBranches": [ + "master" + ], + "prHourlyLimit": 2 +} diff --git a/.github/workflows/renovate.yml b/.github/workflows/renovate.yml new file mode 100644 index 0000000..abade6b --- /dev/null +++ b/.github/workflows/renovate.yml @@ -0,0 +1,25 @@ +name: Renovate +on: + workflow_dispatch: + inputs: + logLevel: + description: "Override default log level" + required: false + default: "info" + type: string + overrideSchedule: + description: "Override all schedules" + required: false + default: "false" + type: string + # Run twice in the early morning (UTC) for initial and follow up steps (create pull request and merge) + schedule: + - cron: '30 4,6 * * *' + +jobs: + call-workflow: + uses: rancher/renovate-config/.github/workflows/renovate.yml@release + with: + logLevel: ${{ inputs.logLevel || 'info' }} + overrideSchedule: ${{ github.event.inputs.overrideSchedule == 'true' && '{''schedule'':null}' || '' }} + secrets: inherit From 3db0918eb8c4d8dfc4f1bb75f4ac84485b81a60e Mon Sep 17 00:00:00 2001 From: Ricardo Weir Date: Wed, 10 May 2023 15:47:27 -0700 Subject: [PATCH 4/9] Change cache default to false --- pkg/stores/partition/store.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/stores/partition/store.go b/pkg/stores/partition/store.go index 185a6cb..4c25022 100644 --- a/pkg/stores/partition/store.go +++ b/pkg/stores/partition/store.go @@ -29,7 +29,7 @@ const ( // Not related to the total size in memory of the cache, as any item could take any amount of memory. cacheSizeEnv = "CATTLE_REQUEST_CACHE_SIZE_INT" defaultCacheSize = 1000 - // Set to non-empty to disable list request caching entirely. + // Set to "false" to enable list request caching. cacheDisableEnv = "CATTLE_REQUEST_CACHE_DISABLED" ) @@ -60,7 +60,7 @@ func NewStore(partitioner Partitioner, asl accesscontrol.AccessSetLookup) *Store Partitioner: partitioner, asl: asl, } - if v := os.Getenv(cacheDisableEnv); v == "" { + if v := os.Getenv(cacheDisableEnv); v == "false" { s.listCache = cache.NewLRUExpireCache(cacheSize) } return s From 0cabe8de10ce7a6c4a56bd91623941f47e24dd13 Mon Sep 17 00:00:00 2001 From: Ricardo Weir Date: Wed, 10 May 2023 16:21:20 -0700 Subject: [PATCH 5/9] Update tests --- pkg/stores/partition/store_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/stores/partition/store_test.go b/pkg/stores/partition/store_test.go index fdd666c..a139db0 100644 --- a/pkg/stores/partition/store_test.go +++ b/pkg/stores/partition/store_test.go @@ -1941,8 +1941,8 @@ func TestList(t *testing.T) { } } asl := &mockAccessSetLookup{userRoles: test.access} - if test.disableCache { - t.Setenv("CATTLE_REQUEST_CACHE_DISABLED", "Y") + if !test.disableCache { + t.Setenv("CATTLE_REQUEST_CACHE_DISABLED", "false") } store := NewStore(mockPartitioner{ stores: stores, @@ -2024,7 +2024,6 @@ func TestListByRevision(t *testing.T) { }, }, asl) req := newRequest("", "user1") - t.Setenv("CATTLE_REQUEST_CACHE_DISABLED", "Y") got, gotErr := store.List(req, schema) assert.Nil(t, gotErr) From 84dedac146d49c3a51507019e1a7b7f9e1dc3192 Mon Sep 17 00:00:00 2001 From: Colleen Murphy Date: Wed, 10 May 2023 15:31:01 -0700 Subject: [PATCH 6/9] Add projectsornamespaces query parameter Add a new query parameter to filter resources by their namespace or their namespace's project. This parameter is separate from the existing `filter` parameter. Filter by a comma-separated list of projects and/or namespaces with: ?projectsornamespaces=p1,n1,n2 The result can be negated with the ! operator: ?projectsornamespaces!=p1,n1,n2 --- README.md | 30 + pkg/resources/common/formatter.go | 6 +- pkg/resources/schema.go | 6 +- pkg/server/server.go | 2 +- .../partition/listprocessor/processor.go | 108 ++- .../partition/listprocessor/processor_test.go | 670 ++++++++++++++++++ pkg/stores/partition/store.go | 16 +- pkg/stores/partition/store_test.go | 181 ++++- pkg/stores/proxy/proxy_store.go | 4 +- 9 files changed, 985 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 935b79e..49347ae 100644 --- a/README.md +++ b/README.md @@ -117,6 +117,36 @@ item is included in the list. /v1/{type}?filter=spec.containers.image=alpine ``` +#### `projectsornamespaces` + +Resources can also be filtered by the Rancher projects their namespaces belong +to. Since a project isn't an intrinsic part of the resource itself, the filter +parameter for filtering by projects is separate from the main `filter` +parameter. This query parameter is only applicable when steve is runnning in +concert with Rancher. + +The list can be filtered by either projects or namespaces or both. + +Filtering by a single project or a single namespace: + +``` +/v1/{type}?projectsornamespaces=p1 +``` + +Filtering by multiple projects or namespaces is done with a comma separated +list. A resource matching any project or namespace in the list is included in +the result: + +``` +/v1/{type}?projectsornamespaces=p1,n1,n2 +``` + +The list can be negated to exclude results: + +``` +/v1/{type}?projectsornamespaces!=p1,n1,n2 +``` + #### `sort` Only applicable to list requests (`/v1/{type}` and `/v1/{type}/{namespace}`). diff --git a/pkg/resources/common/formatter.go b/pkg/resources/common/formatter.go index 2922481..6ac320a 100644 --- a/pkg/resources/common/formatter.go +++ b/pkg/resources/common/formatter.go @@ -11,6 +11,7 @@ import ( "github.com/rancher/steve/pkg/stores/proxy" "github.com/rancher/steve/pkg/summarycache" "github.com/rancher/wrangler/pkg/data" + corecontrollers "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/rancher/wrangler/pkg/slice" "github.com/rancher/wrangler/pkg/summary" "k8s.io/apimachinery/pkg/api/meta" @@ -21,9 +22,10 @@ import ( func DefaultTemplate(clientGetter proxy.ClientGetter, summaryCache *summarycache.SummaryCache, - asl accesscontrol.AccessSetLookup) schema.Template { + asl accesscontrol.AccessSetLookup, + namespaceCache corecontrollers.NamespaceCache) schema.Template { return schema.Template{ - Store: metricsStore.NewMetricsStore(proxy.NewProxyStore(clientGetter, summaryCache, asl)), + Store: metricsStore.NewMetricsStore(proxy.NewProxyStore(clientGetter, summaryCache, asl, namespaceCache)), Formatter: formatter(summaryCache), } } diff --git a/pkg/resources/schema.go b/pkg/resources/schema.go index ff3fee1..148f9e2 100644 --- a/pkg/resources/schema.go +++ b/pkg/resources/schema.go @@ -19,6 +19,7 @@ import ( steveschema "github.com/rancher/steve/pkg/schema" "github.com/rancher/steve/pkg/stores/proxy" "github.com/rancher/steve/pkg/summarycache" + corecontrollers "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/client-go/discovery" ) @@ -46,9 +47,10 @@ func DefaultSchemaTemplates(cf *client.Factory, baseSchemas *types.APISchemas, summaryCache *summarycache.SummaryCache, lookup accesscontrol.AccessSetLookup, - discovery discovery.DiscoveryInterface) []schema.Template { + discovery discovery.DiscoveryInterface, + namespaceCache corecontrollers.NamespaceCache) []schema.Template { return []schema.Template{ - common.DefaultTemplate(cf, summaryCache, lookup), + common.DefaultTemplate(cf, summaryCache, lookup, namespaceCache), apigroups.Template(discovery), { ID: "configmap", diff --git a/pkg/server/server.go b/pkg/server/server.go index 788e0fc..06f1c0e 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -145,7 +145,7 @@ func setup(ctx context.Context, server *Server) error { summaryCache := summarycache.New(sf, ccache) summaryCache.Start(ctx) - for _, template := range resources.DefaultSchemaTemplates(cf, server.BaseSchemas, summaryCache, asl, server.controllers.K8s.Discovery()) { + for _, template := range resources.DefaultSchemaTemplates(cf, server.BaseSchemas, summaryCache, asl, server.controllers.K8s.Discovery(), server.controllers.Core.Namespace().Cache()) { sf.AddTemplate(template) } diff --git a/pkg/stores/partition/listprocessor/processor.go b/pkg/stores/partition/listprocessor/processor.go index a316a53..f028e5a 100644 --- a/pkg/stores/partition/listprocessor/processor.go +++ b/pkg/stores/partition/listprocessor/processor.go @@ -10,19 +10,24 @@ import ( "github.com/rancher/apiserver/pkg/types" "github.com/rancher/wrangler/pkg/data" "github.com/rancher/wrangler/pkg/data/convert" + corecontrollers "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) const ( - defaultLimit = 100000 - continueParam = "continue" - limitParam = "limit" - filterParam = "filter" - sortParam = "sort" - pageSizeParam = "pagesize" - pageParam = "page" - revisionParam = "revision" - orOp = "," + defaultLimit = 100000 + continueParam = "continue" + limitParam = "limit" + filterParam = "filter" + sortParam = "sort" + pageSizeParam = "pagesize" + pageParam = "page" + revisionParam = "revision" + projectsOrNamespacesVar = "projectsornamespaces" + projectIDFieldLabel = "field.cattle.io/projectId" + + orOp = "," + notOp = "!" ) var opReg = regexp.MustCompile(`[!]?=`) @@ -36,12 +41,13 @@ const ( // ListOptions represents the query parameters that may be included in a list request. type ListOptions struct { - ChunkSize int - Resume string - Filters []OrFilter - Sort Sort - Pagination Pagination - Revision string + ChunkSize int + Resume string + Filters []OrFilter + Sort Sort + Pagination Pagination + Revision string + ProjectsOrNamespaces ProjectsOrNamespacesFilter } // Filter represents a field to filter by. @@ -127,11 +133,21 @@ func (p Pagination) PageSize() int { return p.pageSize } +type ProjectsOrNamespacesFilter struct { + filter map[string]struct{} + op op +} + // ParseQuery parses the query params of a request and returns a ListOptions. func ParseQuery(apiOp *types.APIRequest) *ListOptions { - chunkSize := getLimit(apiOp) + opts := ListOptions{} + + opts.ChunkSize = getLimit(apiOp) + q := apiOp.Request.URL.Query() cont := q.Get(continueParam) + opts.Resume = cont + filterParams := q[filterParam] filterOpts := []OrFilter{} for _, filters := range filterParams { @@ -168,6 +184,8 @@ func ParseQuery(apiOp *types.APIRequest) *ListOptions { } return fieldI.String() < fieldJ.String() }) + opts.Filters = filterOpts + sortOpts := Sort{} sortKeys := q.Get(sortParam) if sortKeys != "" { @@ -191,6 +209,8 @@ func ParseQuery(apiOp *types.APIRequest) *ListOptions { } } } + opts.Sort = sortOpts + var err error pagination := Pagination{} pagination.pageSize, err = strconv.Atoi(q.Get(pageSizeParam)) @@ -201,15 +221,29 @@ func ParseQuery(apiOp *types.APIRequest) *ListOptions { if err != nil { pagination.page = 1 } + opts.Pagination = pagination + revision := q.Get(revisionParam) - return &ListOptions{ - ChunkSize: chunkSize, - Resume: cont, - Filters: filterOpts, - Sort: sortOpts, - Pagination: pagination, - Revision: revision, + opts.Revision = revision + + projectsOptions := ProjectsOrNamespacesFilter{} + var op op + projectsOrNamespaces := q.Get(projectsOrNamespacesVar) + if projectsOrNamespaces == "" { + projectsOrNamespaces = q.Get(projectsOrNamespacesVar + notOp) + if projectsOrNamespaces != "" { + op = notEq + } } + if projectsOrNamespaces != "" { + projectsOptions.filter = make(map[string]struct{}) + for _, pn := range strings.Split(projectsOrNamespaces, ",") { + projectsOptions.filter[pn] = struct{}{} + } + projectsOptions.op = op + opts.ProjectsOrNamespaces = projectsOptions + } + return &opts } // getLimit extracts the limit parameter from the request or sets a default of 100000. @@ -360,3 +394,31 @@ func PaginateList(list []unstructured.Unstructured, p Pagination) ([]unstructure } return list[offset : offset+p.pageSize], pages } + +func FilterByProjectsAndNamespaces(list []unstructured.Unstructured, projectsOrNamespaces ProjectsOrNamespacesFilter, namespaceCache corecontrollers.NamespaceCache) []unstructured.Unstructured { + if len(projectsOrNamespaces.filter) == 0 { + return list + } + result := []unstructured.Unstructured{} + for _, obj := range list { + namespaceName := obj.GetNamespace() + if namespaceName == "" { + continue + } + namespace, err := namespaceCache.Get(namespaceName) + if namespace == nil || err != nil { + continue + } + projectLabel, _ := namespace.GetLabels()[projectIDFieldLabel] + _, matchesProject := projectsOrNamespaces.filter[projectLabel] + _, matchesNamespace := projectsOrNamespaces.filter[namespaceName] + matches := matchesProject || matchesNamespace + if projectsOrNamespaces.op == eq && matches { + result = append(result, obj) + } + if projectsOrNamespaces.op == notEq && !matches { + result = append(result, obj) + } + } + return result +} diff --git a/pkg/stores/partition/listprocessor/processor_test.go b/pkg/stores/partition/listprocessor/processor_test.go index a5e7c96..d17c0ca 100644 --- a/pkg/stores/partition/listprocessor/processor_test.go +++ b/pkg/stores/partition/listprocessor/processor_test.go @@ -3,8 +3,12 @@ package listprocessor import ( "testing" + corecontrollers "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" ) func TestFilterList(t *testing.T) { @@ -2567,3 +2571,669 @@ func TestPaginateList(t *testing.T) { }) } } + +func TestFilterByProjectsAndNamespaces(t *testing.T) { + tests := []struct { + name string + objects []unstructured.Unstructured + filter ProjectsOrNamespacesFilter + want []unstructured.Unstructured + }{ + { + name: "filter by one project", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n2", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "p-abcde": struct{}{}, + }, + op: eq, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + }, + }, + { + name: "filter by multiple projects", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "p-abcde": struct{}{}, + "p-fghij": struct{}{}, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + }, + }, + { + name: "filter by one namespace", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n2", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "n1": struct{}{}, + }, + op: eq, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + }, + }, + { + name: "filter by multiple namespaces", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "n1": struct{}{}, + "n2": struct{}{}, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + }, + }, + { + name: "filter by namespaces and projects", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "n1": struct{}{}, + "p-fghij": struct{}{}, + }, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + }, + }, + { + name: "no matches", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "foobar": struct{}{}, + }, + }, + want: []unstructured.Unstructured{}, + }, + { + name: "no filters", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{}, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + }, + { + name: "filter by one project negated", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "p-abcde": struct{}{}, + }, + op: notEq, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + }, + { + name: "filter by multiple projects negated", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "p-abcde": struct{}{}, + "p-fghij": struct{}{}, + }, + op: notEq, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + }, + { + name: "filter by one namespace negated", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n2", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "n1": struct{}{}, + }, + op: notEq, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n2", + }, + }, + }, + }, + }, + { + name: "filter by multiple namespaces negated", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "n1": struct{}{}, + "n2": struct{}{}, + }, + op: notEq, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + }, + { + name: "filter by namespaces and projects negated", + objects: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "fuji", + "namespace": "n1", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "honeycrisp", + "namespace": "n2", + }, + }, + }, + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + filter: ProjectsOrNamespacesFilter{ + filter: map[string]struct{}{ + "n1": struct{}{}, + "p-fghij": struct{}{}, + }, + op: notEq, + }, + want: []unstructured.Unstructured{ + { + Object: map[string]interface{}{ + "kind": "apple", + "metadata": map[string]interface{}{ + "name": "granny-smith", + "namespace": "n3", + }, + }, + }, + }, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := FilterByProjectsAndNamespaces(test.objects, test.filter, mockNamespaceCache{}) + assert.Equal(t, test.want, got) + }) + } +} + +var namespaces = map[string]*corev1.Namespace{ + "n1": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n1", + Labels: map[string]string{ + "field.cattle.io/projectId": "p-abcde", + }, + }, + }, + "n2": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n2", + Labels: map[string]string{ + "field.cattle.io/projectId": "p-fghij", + }, + }, + }, + "n3": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n3", + Labels: map[string]string{ + "field.cattle.io/projectId": "p-klmno", + }, + }, + }, + "n4": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n4", + }, + }, +} + +type mockNamespaceCache struct{} + +func (m mockNamespaceCache) Get(name string) (*corev1.Namespace, error) { + return namespaces[name], nil +} + +func (m mockNamespaceCache) List(selector labels.Selector) ([]*corev1.Namespace, error) { + panic("not implemented") +} +func (m mockNamespaceCache) AddIndexer(indexName string, indexer corecontrollers.NamespaceIndexer) { + panic("not implemented") +} +func (m mockNamespaceCache) GetByIndex(indexName, key string) ([]*corev1.Namespace, error) { + panic("not implemented") +} diff --git a/pkg/stores/partition/store.go b/pkg/stores/partition/store.go index 185a6cb..482d838 100644 --- a/pkg/stores/partition/store.go +++ b/pkg/stores/partition/store.go @@ -13,6 +13,7 @@ import ( "github.com/rancher/apiserver/pkg/types" "github.com/rancher/steve/pkg/accesscontrol" "github.com/rancher/steve/pkg/stores/partition/listprocessor" + corecontrollers "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/sirupsen/logrus" "golang.org/x/sync/errgroup" "k8s.io/apimachinery/pkg/api/meta" @@ -42,13 +43,14 @@ type Partitioner interface { // Store implements types.Store for partitions. type Store struct { - Partitioner Partitioner - listCache *cache.LRUExpireCache - asl accesscontrol.AccessSetLookup + Partitioner Partitioner + listCache *cache.LRUExpireCache + asl accesscontrol.AccessSetLookup + namespaceCache corecontrollers.NamespaceCache } // NewStore creates a types.Store implementation with a partitioner and an LRU expiring cache for list responses. -func NewStore(partitioner Partitioner, asl accesscontrol.AccessSetLookup) *Store { +func NewStore(partitioner Partitioner, asl accesscontrol.AccessSetLookup, namespaceCache corecontrollers.NamespaceCache) *Store { cacheSize := defaultCacheSize if v := os.Getenv(cacheSizeEnv); v != "" { sizeInt, err := strconv.Atoi(v) @@ -57,8 +59,9 @@ func NewStore(partitioner Partitioner, asl accesscontrol.AccessSetLookup) *Store } } s := &Store{ - Partitioner: partitioner, - asl: asl, + Partitioner: partitioner, + asl: asl, + namespaceCache: namespaceCache, } if v := os.Getenv(cacheDisableEnv); v == "" { s.listCache = cache.NewLRUExpireCache(cacheSize) @@ -203,6 +206,7 @@ func (s *Store) List(apiOp *types.APIRequest, schema *types.APISchema) (types.AP listToCache := &unstructured.UnstructuredList{ Items: list, } + list = listprocessor.FilterByProjectsAndNamespaces(list, opts.ProjectsOrNamespaces, s.namespaceCache) c := lister.Continue() if c != "" { listToCache.SetContinue(c) diff --git a/pkg/stores/partition/store_test.go b/pkg/stores/partition/store_test.go index fdd666c..6d557bd 100644 --- a/pkg/stores/partition/store_test.go +++ b/pkg/stores/partition/store_test.go @@ -12,9 +12,13 @@ import ( "github.com/rancher/apiserver/pkg/types" "github.com/rancher/steve/pkg/accesscontrol" + corecontrollers "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/rancher/wrangler/pkg/schemas" "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/authentication/user" "k8s.io/apiserver/pkg/endpoints/request" @@ -1928,6 +1932,118 @@ func TestList(t *testing.T) { {"all": 1}, }, }, + { + name: "with project filters", + apiOps: []*types.APIRequest{ + newRequest("projectsornamespaces=p-abcde", "user1"), + newRequest("projectsornamespaces=p-abcde,p-fghij", "user1"), + newRequest("projectsornamespaces=p-abcde,n2", "user1"), + newRequest("projectsornamespaces!=p-abcde", "user1"), + newRequest("projectsornamespaces!=p-abcde,p-fghij", "user1"), + newRequest("projectsornamespaces!=p-abcde,n2", "user1"), + newRequest("projectsornamespaces=foobar", "user1"), + newRequest("projectsornamespaces!=foobar", "user1"), + }, + access: []map[string]string{ + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + { + "user1": "roleA", + }, + }, + partitions: map[string][]Partition{ + "user1": { + mockPartition{ + name: "all", + }, + }, + }, + objects: map[string]*unstructured.UnstructuredList{ + "all": { + Items: []unstructured.Unstructured{ + newApple("fuji").withNamespace("n1").Unstructured, + newApple("granny-smith").withNamespace("n1").Unstructured, + newApple("bramley").withNamespace("n2").Unstructured, + newApple("crispin").withNamespace("n3").Unstructured, + }, + }, + }, + want: []types.APIObjectList{ + { + Count: 2, + Objects: []types.APIObject{ + newApple("fuji").withNamespace("n1").toObj(), + newApple("granny-smith").withNamespace("n1").toObj(), + }, + }, + { + Count: 3, + Objects: []types.APIObject{ + newApple("fuji").withNamespace("n1").toObj(), + newApple("granny-smith").withNamespace("n1").toObj(), + newApple("bramley").withNamespace("n2").toObj(), + }, + }, + { + Count: 3, + Objects: []types.APIObject{ + newApple("fuji").withNamespace("n1").toObj(), + newApple("granny-smith").withNamespace("n1").toObj(), + newApple("bramley").withNamespace("n2").toObj(), + }, + }, + { + Count: 2, + Objects: []types.APIObject{ + newApple("bramley").withNamespace("n2").toObj(), + newApple("crispin").withNamespace("n3").toObj(), + }, + }, + { + Count: 1, + Objects: []types.APIObject{ + newApple("crispin").withNamespace("n3").toObj(), + }, + }, + { + Count: 1, + Objects: []types.APIObject{ + newApple("crispin").withNamespace("n3").toObj(), + }, + }, + { + Count: 0, + }, + { + Count: 4, + Objects: []types.APIObject{ + newApple("fuji").withNamespace("n1").toObj(), + newApple("granny-smith").withNamespace("n1").toObj(), + newApple("bramley").withNamespace("n2").toObj(), + newApple("crispin").withNamespace("n3").toObj(), + }, + }, + }, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { @@ -1947,7 +2063,7 @@ func TestList(t *testing.T) { store := NewStore(mockPartitioner{ stores: stores, partitions: test.partitions, - }, asl) + }, asl, mockNamespaceCache{}) for i, req := range test.apiOps { got, gotErr := store.List(req, schema) assert.Nil(t, gotErr) @@ -2022,7 +2138,7 @@ func TestListByRevision(t *testing.T) { }, }, }, - }, asl) + }, asl, mockNamespaceCache{}) req := newRequest("", "user1") t.Setenv("CATTLE_REQUEST_CACHE_DISABLED", "Y") @@ -2215,9 +2331,15 @@ func newApple(name string) apple { } func (a apple) toObj() types.APIObject { + meta := a.Object["metadata"].(map[string]interface{}) + id := meta["name"].(string) + ns, ok := meta["namespace"] + if ok { + id = ns.(string) + "/" + id + } return types.APIObject{ Type: "apple", - ID: a.Object["metadata"].(map[string]interface{})["name"].(string), + ID: id, Object: &a.Unstructured, } } @@ -2229,6 +2351,11 @@ func (a apple) with(data map[string]string) apple { return a } +func (a apple) withNamespace(namespace string) apple { + a.Object["metadata"].(map[string]interface{})["namespace"] = namespace + return a +} + type mockAccessSetLookup struct { accessID string userRoles []map[string]string @@ -2251,3 +2378,51 @@ func getAccessID(user, role string) string { h := sha256.Sum256([]byte(user + role)) return string(h[:]) } + +var namespaces = map[string]*corev1.Namespace{ + "n1": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n1", + Labels: map[string]string{ + "field.cattle.io/projectId": "p-abcde", + }, + }, + }, + "n2": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n2", + Labels: map[string]string{ + "field.cattle.io/projectId": "p-fghij", + }, + }, + }, + "n3": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n3", + Labels: map[string]string{ + "field.cattle.io/projectId": "p-klmno", + }, + }, + }, + "n4": &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "n4", + }, + }, +} + +type mockNamespaceCache struct{} + +func (m mockNamespaceCache) Get(name string) (*corev1.Namespace, error) { + return namespaces[name], nil +} + +func (m mockNamespaceCache) List(selector labels.Selector) ([]*corev1.Namespace, error) { + panic("not implemented") +} +func (m mockNamespaceCache) AddIndexer(indexName string, indexer corecontrollers.NamespaceIndexer) { + panic("not implemented") +} +func (m mockNamespaceCache) GetByIndex(indexName, key string) ([]*corev1.Namespace, error) { + panic("not implemented") +} diff --git a/pkg/stores/proxy/proxy_store.go b/pkg/stores/proxy/proxy_store.go index b32dfc6..13b18da 100644 --- a/pkg/stores/proxy/proxy_store.go +++ b/pkg/stores/proxy/proxy_store.go @@ -19,6 +19,7 @@ import ( metricsStore "github.com/rancher/steve/pkg/stores/metrics" "github.com/rancher/steve/pkg/stores/partition" "github.com/rancher/wrangler/pkg/data" + corecontrollers "github.com/rancher/wrangler/pkg/generated/controllers/core/v1" "github.com/rancher/wrangler/pkg/schemas/validation" "github.com/rancher/wrangler/pkg/summary" "github.com/sirupsen/logrus" @@ -85,7 +86,7 @@ type Store struct { } // NewProxyStore returns a wrapped types.Store. -func NewProxyStore(clientGetter ClientGetter, notifier RelationshipNotifier, lookup accesscontrol.AccessSetLookup) types.Store { +func NewProxyStore(clientGetter ClientGetter, notifier RelationshipNotifier, lookup accesscontrol.AccessSetLookup, namespaceCache corecontrollers.NamespaceCache) types.Store { return &errorStore{ Store: &WatchRefresh{ Store: partition.NewStore( @@ -96,6 +97,7 @@ func NewProxyStore(clientGetter ClientGetter, notifier RelationshipNotifier, loo }, }, lookup, + namespaceCache, ), asl: lookup, }, From 27aa1b48aee2786af03044a9c380886d5180b52e Mon Sep 17 00:00:00 2001 From: Silvio Moioli Date: Wed, 7 Jun 2023 09:53:30 +0200 Subject: [PATCH 7/9] Fix drone validate build go1.19 provides go=1.19.9, the right symbol for the major version is golang(API) Signed-off-by: Silvio Moioli --- .drone.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.drone.yml b/.drone.yml index fe090d8..488f067 100644 --- a/.drone.yml +++ b/.drone.yml @@ -35,7 +35,7 @@ steps: - name: validate image: registry.suse.com/bci/bci-base:15.4 commands: - - zypper in -y go=1.19 git tar gzip make + - zypper in -y "golang(API)=1.19" git tar gzip make - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v1.49.0 - mv ./bin/golangci-lint /usr/local/bin/golangci-lint - GOBIN=/usr/local/bin go install github.com/golang/mock/mockgen@v1.6.0 From 7010a5e6c703468f9f95fdcf3377a3c6c96525fb Mon Sep 17 00:00:00 2001 From: Silvio Moioli Date: Tue, 6 Jun 2023 16:50:02 +0200 Subject: [PATCH 8/9] proxy_store: improve error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This prevents a goroutine leak when item.Object is a `runtime.Object` but not a `metav1.Object`, as in that case `WatchNames`’s `for` loop will quit early and subsequent calls to `returnErr` will remain parked forever. This helps with https://github.com/rancher/rancher/issues/41225 Fuller explanation in https://github.com/rancher/steve/pull/107 --- pkg/stores/proxy/proxy_store.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/stores/proxy/proxy_store.go b/pkg/stores/proxy/proxy_store.go index 13b18da..52b0cc7 100644 --- a/pkg/stores/proxy/proxy_store.go +++ b/pkg/stores/proxy/proxy_store.go @@ -311,7 +311,6 @@ func (s *Store) listAndWatch(apiOp *types.APIRequest, client dynamic.ResourceInt rowToObject(obj) result <- watch.Event{Type: watch.Modified, Object: obj} } else { - logrus.Debugf("notifier watch error: %v", err) returnErr(errors.Wrapf(err, "notifier watch error: %v", err), result) } } @@ -323,7 +322,6 @@ func (s *Store) listAndWatch(apiOp *types.APIRequest, client dynamic.ResourceInt for event := range watcher.ResultChan() { if event.Type == watch.Error { if status, ok := event.Object.(*metav1.Status); ok { - logrus.Debugf("event watch error: %s", status.Message) returnErr(fmt.Errorf("event watch error: %s", status.Message), result) } else { logrus.Debugf("event watch error: could not decode event object %T", event.Object) @@ -363,12 +361,22 @@ func (s *Store) WatchNames(apiOp *types.APIRequest, schema *types.APISchema, w t go func() { defer close(result) for item := range c { + if item.Type == watch.Error { + if status, ok := item.Object.(*metav1.Status); ok { + logrus.Debugf("WatchNames received error: %s", status.Message) + } else { + logrus.Debugf("WatchNames received error: %v", item) + } + continue + } m, err := meta.Accessor(item.Object) if err != nil { - return + logrus.Debugf("WatchNames cannot process unexpected object: %s", err) + continue } - if item.Type != watch.Error && names.Has(m.GetName()) { + + if names.Has(m.GetName()) { result <- item } } From c53bd62e9c5f46b63af26178e5a7249365cfdbdc Mon Sep 17 00:00:00 2001 From: Ricardo Weir Date: Thu, 8 Jun 2023 19:46:32 -0700 Subject: [PATCH 9/9] Add tests --- pkg/stores/proxy/proxy_store_test.go | 104 +++++++++++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 pkg/stores/proxy/proxy_store_test.go diff --git a/pkg/stores/proxy/proxy_store_test.go b/pkg/stores/proxy/proxy_store_test.go new file mode 100644 index 0000000..75c290c --- /dev/null +++ b/pkg/stores/proxy/proxy_store_test.go @@ -0,0 +1,104 @@ +package proxy + +import ( + "fmt" + "net/http" + "testing" + "time" + + "github.com/pkg/errors" + "github.com/rancher/apiserver/pkg/types" + "github.com/rancher/steve/pkg/client" + "github.com/rancher/wrangler/pkg/schemas" + "github.com/stretchr/testify/assert" + "golang.org/x/sync/errgroup" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + schema2 "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/rest" + clientgotesting "k8s.io/client-go/testing" +) + +var c *watch.FakeWatcher + +type testFactory struct { + *client.Factory + + fakeClient *fake.FakeDynamicClient +} + +func TestWatchNamesErrReceive(t *testing.T) { + testClientFactory, err := client.NewFactory(&rest.Config{}, false) + assert.Nil(t, err) + + fakeClient := fake.NewSimpleDynamicClient(runtime.NewScheme()) + c = watch.NewFakeWithChanSize(5, true) + defer c.Stop() + errMsgsToSend := []string{"err1", "err2", "err3"} + c.Add(&v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "testsecret1"}}) + for index := range errMsgsToSend { + c.Error(&metav1.Status{ + Message: errMsgsToSend[index], + }) + } + c.Add(&v1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "testsecret2"}}) + fakeClient.PrependWatchReactor("*", func(action clientgotesting.Action) (handled bool, ret watch.Interface, err error) { + return true, c, nil + }) + testStore := Store{ + clientGetter: &testFactory{Factory: testClientFactory, + fakeClient: fakeClient, + }, + } + apiSchema := &types.APISchema{Schema: &schemas.Schema{Attributes: map[string]interface{}{"table": "something"}}} + wc, err := testStore.WatchNames(&types.APIRequest{Namespace: "", Schema: apiSchema, Request: &http.Request{}}, apiSchema, types.WatchRequest{}, sets.NewString("testsecret1", "testsecret2")) + assert.Nil(t, err) + + eg := errgroup.Group{} + eg.Go(func() error { return receiveUntil(wc, 5*time.Second) }) + + err = eg.Wait() + assert.Nil(t, err) + + assert.Equal(t, 0, len(c.ResultChan()), "Expected all secrets to have been received") +} + +func (t *testFactory) TableAdminClientForWatch(ctx *types.APIRequest, schema *types.APISchema, namespace string, warningHandler rest.WarningHandler) (dynamic.ResourceInterface, error) { + return t.fakeClient.Resource(schema2.GroupVersionResource{}), nil +} + +func receiveUntil(wc chan watch.Event, d time.Duration) error { + timer := time.NewTicker(d) + defer timer.Stop() + secretNames := []string{"testsecret1", "testsecret2"} + for { + select { + case event, ok := <-wc: + if !ok { + return errors.New("watch chan should not have been closed") + } + + if event.Type == watch.Error { + return errors.New(fmt.Sprintf("watch chan should not have sent events of type [%s]", watch.Error)) + } + secret, ok := event.Object.(*v1.Secret) + if !ok { + continue + } + if secret.Name == secretNames[0] { + secretNames = secretNames[1:] + } + if len(secretNames) == 0 { + return nil + } + continue + case <-timer.C: + return errors.New("timed out waiting to receiving objects from chan") + } + } +}