rancher/steve
1
0
Fork 0
mirror of https://github.com/rancher/steve.git synced 2025-04-27 11:00:48 +00:00
Code Issues Projects Releases Wiki Activity

sql: bugfix: return total resource count correctly (#236)

Browse Source 
* sql: drop dead code

Signed-off-by: Silvio Moioli <silvio@moioli.net>

* sql: bugfix: return total resource count correctly

Signed-off-by: Silvio Moioli <silvio@moioli.net>

* adapt tests

Signed-off-by: Silvio Moioli <silvio@moioli.net>

* adapt mocks

Signed-off-by: Silvio Moioli <silvio@moioli.net>

* TEMP: remove this when bumping lasso to include https://github.com/rancher/lasso/pull/84

Signed-off-by: Silvio Moioli <silvio@moioli.net>

* Use latest lasso instead of fork

---------

Signed-off-by: Silvio Moioli <silvio@moioli.net>
Co-authored-by: Tom Lebreux <tom.lebreux@suse.com>
This commit is contained in:
Silvio Moioli 2024-07-05 22:17:16 +02:00 committed by GitHub
parent 88fd70abbd
commit 0841e03c57
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 71 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
go.mod
View File

@ -22,10 +22,10 @@ require (
github.com/rancher/apiserver v0.0.0-20240604183424-8c448886365e
github.com/rancher/dynamiclistener v0.6.0-rc1
github.com/rancher/kubernetes-provider-detector v0.1.5
github.com/rancher/lasso v0.0.0-20240603075835-701e919d08b7
github.com/rancher/lasso v0.0.0-20240705194423-b2a060d103c1
github.com/rancher/norman v0.0.0-20240604183301-20cd23aadce1
github.com/rancher/remotedialer v0.3.2
github.com/rancher/wrangler/v3 v3.0.0-rc2
github.com/rancher/wrangler/v3 v3.0.0-rc3
github.com/sirupsen/logrus v1.9.3
github.com/stretchr/testify v1.9.0
github.com/urfave/cli v1.22.14

8
go.sum
View File

@ -192,14 +192,14 @@ github.com/rancher/dynamiclistener v0.6.0-rc1 h1:Emwf9o7PMLdQNv4lvFx7xJKxDuDa4Y6
github.com/rancher/dynamiclistener v0.6.0-rc1/go.mod h1:BIPgJ8xFSUyuTyGvRMVt++S1qjD3+7Ptvq1TXl6hcTM=
github.com/rancher/kubernetes-provider-detector v0.1.5 h1:hWRAsWuJOemzGjz/XrbTlM7QmfO4OedvFE3QwXiH60I=
github.com/rancher/kubernetes-provider-detector v0.1.5/go.mod h1:ypuJS7kP7rUiAn330xG46mj+Nhvym05GM8NqMVekpH0=
github.com/rancher/lasso v0.0.0-20240603075835-701e919d08b7 h1:E5AeOkylBXf4APhnHgDvePdtpxOfIjhKnxfjm4sDIEk=
github.com/rancher/lasso v0.0.0-20240603075835-701e919d08b7/go.mod h1:v0FJLrmL4m6zdWfIB0/qo7qN5QIjVMFyvFGaw8uyWsA=
github.com/rancher/lasso v0.0.0-20240705194423-b2a060d103c1 h1:vv1jDlYbd4KhGbPNxmjs8CYgEHUrQm2bMtmULfXJ6iw=
github.com/rancher/lasso v0.0.0-20240705194423-b2a060d103c1/go.mod h1:A/y3BLQkxZXYD60MNDRwAG9WGxXfvd6Z6gWR/a8wPw8=
github.com/rancher/norman v0.0.0-20240604183301-20cd23aadce1 h1:7g0yOiUGfT4zK4N9H0PSijnS/e2YrObi4Gj19JgE1L8=
github.com/rancher/norman v0.0.0-20240604183301-20cd23aadce1/go.mod h1:sGnN5ayvAHLfInOFZ4N1fzZw1IMy3i+9PZA7IxlPsRg=
github.com/rancher/remotedialer v0.3.2 h1:kstZbRwPS5gPWpGg8VjEHT2poHtArs+Fc317YM8JCzU=
github.com/rancher/remotedialer v0.3.2/go.mod h1:Ys004RpJuTLSm+k4aYUCoFiOOad37ubYev3TkOFg/5w=
github.com/rancher/wrangler/v3 v3.0.0-rc2 h1:XGSPPp6GXELqlLvwJp5MsdqyCPu6SCA4UKJ7rQJzE40=
github.com/rancher/wrangler/v3 v3.0.0-rc2/go.mod h1:f54hh7gFkwwbjsieT2b63FowzTU8FvrBonPe//0CIXo=
github.com/rancher/wrangler/v3 v3.0.0-rc3 h1:OqAmpNRZC+aIz5NXBUTqETpC2uKOrNa4xpEY3uo5uk0=
github.com/rancher/wrangler/v3 v3.0.0-rc3/go.mod h1:Dfckuuq7MJk2JWVBDywRlZXMxEyPxHy4XqGrPEzu5Eg=
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE=
github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec/go.mod h1:qqbHyh8v60DhA7CoWK5oRCqLrMHRGoxYCSS9EjAz6Eo=
github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=

11
pkg/stores/sqlpartition/listprocessor/processor.go
View File

@ -44,8 +44,13 @@ type ListOptions struct {
}
type Cache interface {
// ListByOptions returns objects according to the specified list options and partitions
ListByOptions(ctx context.Context, lo informer.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, string, error)
// ListByOptions returns objects according to the specified list options and partitions.
// Specifically:
// - an unstructured list of resources belonging to any of the specified partitions
// - the total number of resources (returned list might be a subset depending on pagination options in lo)
// - a continue token, if there are more pages after the returned one
// - an error instead of all of the above if anything went wrong
ListByOptions(ctx context.Context, lo informer.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
}
// ParseQuery parses the query params of a request and returns a ListOptions.
@ -160,7 +165,7 @@ func getLimit(apiOp *types.APIRequest) int {
func parseNamespaceOrProjectFilters(ctx context.Context, projOrNS string, op informer.Op, namespaceInformer Cache) ([]informer.Filter, error) {
var filters []informer.Filter
for _, pn := range strings.Split(projOrNS, ",") {
uList, _, err := namespaceInformer.ListByOptions(ctx, informer.ListOptions{
uList, _, _, err := namespaceInformer.ListByOptions(ctx, informer.ListOptions{
Filters: []informer.OrFilter{
{
Filters: []informer.Filter{

6
pkg/stores/sqlpartition/listprocessor/processor_test.go
View File

@ -101,7 +101,7 @@ func TestParseQuery(t *testing.T) {
},
},
},
}, []partition.Partition{{Passthrough: true}}, "").Return(list, "", nil)
}, []partition.Partition{{Passthrough: true}}, "").Return(list, len(list.Items), "", nil)
return nsc
},
})
@ -151,7 +151,7 @@ func TestParseQuery(t *testing.T) {
},
},
},
}, []partition.Partition{{Passthrough: true}}, "").Return(nil, "", fmt.Errorf("error"))
}, []partition.Partition{{Passthrough: true}}, "").Return(nil, 0, "", fmt.Errorf("error"))
return nsi
},
})
@ -204,7 +204,7 @@ func TestParseQuery(t *testing.T) {
},
},
},
}, []partition.Partition{{Passthrough: true}}, "").Return(list, "", nil)
}, []partition.Partition{{Passthrough: true}}, "").Return(list, len(list.Items), "", nil)
return nsi
},
})

9
pkg/stores/sqlpartition/listprocessor/proxy_mocks_test.go
View File

@ -38,13 +38,14 @@ func (m *MockCache) EXPECT() *MockCacheMockRecorder {
}
// ListByOptions mocks base method.
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 informer.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, string, error) {
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 informer.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByOptions", arg0, arg1, arg2, arg3)
ret0, _ := ret[0].(*unstructured.UnstructuredList)
ret1, _ := ret[1].(string)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
ret1, _ := ret[1].(int)
ret2, _ := ret[2].(string)
ret3, _ := ret[3].(error)
return ret0, ret1, ret2, ret3
}
// ListByOptions indicates an expected call of ListByOptions.

9
pkg/stores/sqlpartition/partition_mocks_test.go
View File

@ -138,13 +138,14 @@ func (mr *MockUnstructuredStoreMockRecorder) Delete(arg0, arg1, arg2 interface{}
}
// ListByPartitions mocks base method.
func (m *MockUnstructuredStore) ListByPartitions(arg0 *types.APIRequest, arg1 *types.APISchema, arg2 []partition.Partition) ([]unstructured.Unstructured, string, error) {
func (m *MockUnstructuredStore) ListByPartitions(arg0 *types.APIRequest, arg1 *types.APISchema, arg2 []partition.Partition) ([]unstructured.Unstructured, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByPartitions", arg0, arg1, arg2)
ret0, _ := ret[0].([]unstructured.Unstructured)
ret1, _ := ret[1].(string)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
ret1, _ := ret[1].(int)
ret2, _ := ret[2].(string)
ret3, _ := ret[3].(error)
return ret0, ret1, ret2, ret3
}
// ListByPartitions indicates an expected call of ListByPartitions.

2
pkg/stores/sqlpartition/partitioner.go
View File

@ -28,7 +28,7 @@ type UnstructuredStore interface {
Update(apiOp *types.APIRequest, schema *types.APISchema, data types.APIObject, id string) (*unstructured.Unstructured, []types.Warning, error)
Delete(apiOp *types.APIRequest, schema *types.APISchema, id string) (*unstructured.Unstructured, []types.Warning, error)
ListByPartitions(apiOp *types.APIRequest, schema *types.APISchema, partitions []partition.Partition) ([]unstructured.Unstructured, string, error)
ListByPartitions(apiOp *types.APIRequest, schema *types.APISchema, partitions []partition.Partition) ([]unstructured.Unstructured, int, string, error)
WatchByPartitions(apiOp *types.APIRequest, schema *types.APISchema, wr types.WatchRequest, partitions []partition.Partition) (chan watch.Event, error)
}

4
pkg/stores/sqlpartition/store.go
View File

@ -76,12 +76,12 @@ func (s *Store) List(apiOp *types.APIRequest, schema *types.APISchema) (types.AP
store := s.Partitioner.Store()
list, continueToken, err := store.ListByPartitions(apiOp, schema, partitions)
list, total, continueToken, err := store.ListByPartitions(apiOp, schema, partitions)
if err != nil {
return result, err
}
result.Count = len(list)
result.Count = total
for _, item := range list {
item := item.DeepCopy()

4
pkg/stores/sqlpartition/store_test.go
View File

@ -88,7 +88,7 @@ func TestList(t *testing.T) {
}
p.EXPECT().All(req, schema, "list", "").Return(partitions, nil)
p.EXPECT().Store().Return(us)
us.EXPECT().ListByPartitions(req, schema, partitions).Return(uListToReturn, "", nil)
us.EXPECT().ListByPartitions(req, schema, partitions).Return(uListToReturn, len(uListToReturn), "", nil)
l, err := s.List(req, schema)
assert.Nil(t, err)
assert.Equal(t, expectedAPIObjList, l)
@ -125,7 +125,7 @@ func TestList(t *testing.T) {
partitions := make([]partition.Partition, 0)
p.EXPECT().All(req, schema, "list", "").Return(partitions, nil)
p.EXPECT().Store().Return(us)
us.EXPECT().ListByPartitions(req, schema, partitions).Return(nil, "", fmt.Errorf("error"))
us.EXPECT().ListByPartitions(req, schema, partitions).Return(nil, 0, "", fmt.Errorf("error"))
_, err := s.List(req, schema)
assert.NotNil(t, err)
},

9
pkg/stores/sqlproxy/proxy_mocks_test.go
View File

@ -45,13 +45,14 @@ func (m *MockCache) EXPECT() *MockCacheMockRecorder {
}
// ListByOptions mocks base method.
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 informer.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, string, error) {
func (m *MockCache) ListByOptions(arg0 context.Context, arg1 informer.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByOptions", arg0, arg1, arg2, arg3)
ret0, _ := ret[0].(*unstructured.UnstructuredList)
ret1, _ := ret[1].(string)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
ret1, _ := ret[1].(int)
ret2, _ := ret[2].(string)
ret3, _ := ret[3].(error)
return ret0, ret1, ret2, ret3
}
// ListByOptions indicates an expected call of ListByOptions.

32
pkg/stores/sqlproxy/proxy_store.go
View File

@ -92,9 +92,13 @@ type SchemaColumnSetter interface {
}
type Cache interface {
// ListByOptions returns objects according to the specified list options and partitions
// see ListOptionIndexer.ListByOptions
ListByOptions(ctx context.Context, lo informer.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, string, error)
// ListByOptions returns objects according to the specified list options and partitions.
// Specifically:
// - an unstructured list of resources belonging to any of the specified partitions
// - the total number of resources (returned list might be a subset depending on pagination options in lo)
// - a continue token, if there are more pages after the returned one
// - an error instead of all of the above if anything went wrong
ListByOptions(ctx context.Context, lo informer.ListOptions, partitions []partition.Partition, namespace string) (*unstructured.UnstructuredList, int, string, error)
}
// WarningBuffer holds warnings that may be returned from the kubernetes api
@ -603,35 +607,39 @@ func (s *Store) Delete(apiOp *types.APIRequest, schema *types.APISchema, id stri
return obj, buffer, nil
}
// ListByPartitions returns an unstructured list of resources belonging to any of the specified partitions
func (s *Store) ListByPartitions(apiOp *types.APIRequest, schema *types.APISchema, partitions []partition.Partition) ([]unstructured.Unstructured, string, error) {
// ListByPartitions returns:
// - an unstructured list of resources belonging to any of the specified partitions
// - the total number of resources (returned list might be a subset depending on pagination options in apiOp)
// - a continue token, if there are more pages after the returned one
// - an error instead of all of the above if anything went wrong
func (s *Store) ListByPartitions(apiOp *types.APIRequest, schema *types.APISchema, partitions []partition.Partition) ([]unstructured.Unstructured, int, string, error) {
opts, err := listprocessor.ParseQuery(apiOp, s.namespaceCache)
if err != nil {
return nil, "", err
return nil, 0, "", err
}
// warnings from inside the informer are discarded
buffer := WarningBuffer{}
client, err := s.clientGetter.TableAdminClient(apiOp, schema, "", &buffer)
if err != nil {
return nil, "", err
return nil, 0, "", err
}
fields := getFieldsFromSchema(schema)
fields = append(fields, getFieldForGVK(attributes.GVK(schema))...)
inf, err := s.cacheFactory.CacheFor(fields, &tablelistconvert.Client{ResourceInterface: client}, attributes.GVK(schema), attributes.Namespaced(schema))
if err != nil {
return nil, "", err
return nil, 0, "", err
}
list, continueToken, err := inf.ListByOptions(apiOp.Context(), opts, partitions, apiOp.Namespace)
list, total, continueToken, err := inf.ListByOptions(apiOp.Context(), opts, partitions, apiOp.Namespace)
if err != nil {
if errors.Is(err, informer.InvalidColumnErr) {
return nil, "", apierror.NewAPIError(validation.InvalidBodyContent, err.Error())
return nil, 0, "", apierror.NewAPIError(validation.InvalidBodyContent, err.Error())
}
return nil, "", err
return nil, 0, "", err
}
return list.Items, continueToken, nil
return list.Items, total, continueToken, nil
}
// WatchByPartitions returns a channel of events for a list or resource belonging to any of the specified partitions

17
pkg/stores/sqlproxy/proxy_store_test.go
View File

@ -231,10 +231,11 @@ func TestListByPartitions(t *testing.T) {
cg.EXPECT().TableAdminClient(req, schema, "", &WarningBuffer{}).Return(ri, nil)
// This tests that fields are being extracted from schema columns and the type specific fields map
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {"gvk", "specific", "fields"}}, &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema)).Return(c, nil)
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(listToReturn, "", nil)
list, contToken, err := s.ListByPartitions(req, schema, partitions)
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(listToReturn, len(listToReturn.Items), "", nil)
list, total, contToken, err := s.ListByPartitions(req, schema, partitions)
assert.Nil(t, err)
assert.Equal(t, expectedItems, list)
assert.Equal(t, len(expectedItems), total)
assert.Equal(t, "", contToken)
},
})
@ -293,11 +294,11 @@ func TestListByPartitions(t *testing.T) {
// items is equal to the list returned by ListByParititons doesn't ensure no mutation happened
copy(listToReturn.Items, expectedItems)
nsi.EXPECT().ListByOptions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, "", fmt.Errorf("error")).Times(2)
nsi.EXPECT().ListByOptions(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, 0, "", fmt.Errorf("error")).Times(2)
_, err := listprocessor.ParseQuery(req, nsi)
assert.NotNil(t, err)
_, _, err = s.ListByPartitions(req, schema, partitions)
_, _, _, err = s.ListByPartitions(req, schema, partitions)
assert.NotNil(t, err)
},
})
@ -360,7 +361,7 @@ func TestListByPartitions(t *testing.T) {
assert.Nil(t, err)
cg.EXPECT().TableAdminClient(req, schema, "", &WarningBuffer{}).Return(nil, fmt.Errorf("error"))
_, _, err = s.ListByPartitions(req, schema, partitions)
_, _, _, err = s.ListByPartitions(req, schema, partitions)
assert.NotNil(t, err)
},
})
@ -425,7 +426,7 @@ func TestListByPartitions(t *testing.T) {
// This tests that fields are being extracted from schema columns and the type specific fields map
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {"gvk", "specific", "fields"}}, &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema)).Return(factory.Cache{}, fmt.Errorf("error"))
_, _, err = s.ListByPartitions(req, schema, partitions)
_, _, _, err = s.ListByPartitions(req, schema, partitions)
assert.NotNil(t, err)
},
})
@ -496,9 +497,9 @@ func TestListByPartitions(t *testing.T) {
cg.EXPECT().TableAdminClient(req, schema, "", &WarningBuffer{}).Return(ri, nil)
// This tests that fields are being extracted from schema columns and the type specific fields map
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {"gvk", "specific", "fields"}}, &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema)).Return(c, nil)
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(nil, "", fmt.Errorf("error"))
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(nil, 0, "", fmt.Errorf("error"))
_, _, err = s.ListByPartitions(req, schema, partitions)
_, _, _, err = s.ListByPartitions(req, schema, partitions)
assert.NotNil(t, err)
},
})

9
pkg/stores/sqlproxy/sql_informer_mocks_test.go
View File

@ -38,13 +38,14 @@ func (m *MockByOptionsLister) EXPECT() *MockByOptionsListerMockRecorder {
}
// ListByOptions mocks base method.
func (m *MockByOptionsLister) ListByOptions(arg0 context.Context, arg1 informer.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, string, error) {
func (m *MockByOptionsLister) ListByOptions(arg0 context.Context, arg1 informer.ListOptions, arg2 []partition.Partition, arg3 string) (*unstructured.UnstructuredList, int, string, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "ListByOptions", arg0, arg1, arg2, arg3)
ret0, _ := ret[0].(*unstructured.UnstructuredList)
ret1, _ := ret[1].(string)
ret2, _ := ret[2].(error)
return ret0, ret1, ret2
ret1, _ := ret[1].(int)
ret2, _ := ret[2].(string)
ret3, _ := ret[3].(error)
return ret0, ret1, ret2, ret3
}
// ListByOptions indicates an expected call of ListByOptions.