Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: silence informer errors on unwatchable resources #284

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

sql: silence informer errors on unwatchable resources #284

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d2fc372
bump lasso to include pull #111
moio Dec 3, 2024
fb0a157
Make IsListWatchable public to be reused in other packages
moio Sep 24, 2024
a240682
Let lasso know whether a type is watchable upon requesting a cache
moio Sep 24, 2024
6d0f6d5
Adapt existing tests
moio Dec 3, 2024
bdb70ad
Add a test to check watchability is detected correctly
moio Sep 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ require (
github.com/rancher/apiserver v0.0.0-20241009200134-5a4ecca7b988
github.com/rancher/dynamiclistener v0.6.1-rc.2
github.com/rancher/kubernetes-provider-detector v0.1.5
github.com/rancher/lasso v0.0.0-20240924233157-8f384efc8813
github.com/rancher/lasso v0.0.0-20241202185148-04649f379358
github.com/rancher/norman v0.0.0-20241001183610-78a520c160ab
github.com/rancher/remotedialer v0.3.2
github.com/rancher/wrangler/v3 v3.0.1-rc.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,8 @@ github.com/rancher/dynamiclistener v0.6.1-rc.2 h1:PTKNKcYXZjc/lo40EivRcXuEbCXwjp
github.com/rancher/dynamiclistener v0.6.1-rc.2/go.mod h1:0KhUMHy3VcGMGavTY3i1/Mr8rVM02wFqNlUzjc+Cplg=
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-20240924233157-8f384efc8813 h1:V/LY8pUHZG9Kc+xEDWDOryOnCU6/Q+Lsr9QQEQnshpU=
github.com/rancher/lasso v0.0.0-20240924233157-8f384efc8813/go.mod h1:IxgTBO55lziYhTEETyVKiT8/B5Rg92qYiRmcIIYoPgI=
github.com/rancher/lasso v0.0.0-20241202185148-04649f379358 h1:pJwgJXPt4fi0ysXsJcl28rvxhx/Z/9SNCDwFOEyeGu0=
github.com/rancher/lasso v0.0.0-20241202185148-04649f379358/go.mod h1:IxgTBO55lziYhTEETyVKiT8/B5Rg92qYiRmcIIYoPgI=
github.com/rancher/norman v0.0.0-20241001183610-78a520c160ab h1:ihK6See3y/JilqZlc0CG7NXPN+ue5nY9U7xUZUA8M7I=
github.com/rancher/norman v0.0.0-20241001183610-78a520c160ab/go.mod h1:qX/OG/4wY27xSAcSdRilUBxBumV6Ey2CWpAeaKnBQDs=
github.com/rancher/remotedialer v0.3.2 h1:kstZbRwPS5gPWpGg8VjEHT2poHtArs+Fc317YM8JCzU=
Expand Down
4 changes: 2 additions & 2 deletions pkg/controllers/schema/schemas.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func isListOrGetable(schema *types.APISchema) bool {
return false
}

func isListWatchable(schema *types.APISchema) bool {
func IsListWatchable(schema *types.APISchema) bool {
var (
canList bool
canWatch bool
Expand Down Expand Up @@ -163,7 +163,7 @@ func (h *handler) refreshAll(ctx context.Context) error {

filteredSchemas := map[string]*types.APISchema{}
for _, schema := range schemas {
if isListWatchable(schema) {
if IsListWatchable(schema) {
if preferredTypeExists(schema, schemas) {
continue
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/stores/sqlproxy/proxy_mocks_test.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions pkg/stores/sqlproxy/proxy_store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/rancher/wrangler/v3/pkg/summary"

"github.com/rancher/steve/pkg/attributes"
controllerschema "github.com/rancher/steve/pkg/controllers/schema"
"github.com/rancher/steve/pkg/resources/common"
"github.com/rancher/steve/pkg/resources/virtual"
virtualCommon "github.com/rancher/steve/pkg/resources/virtual/common"
Expand Down Expand Up @@ -184,7 +185,7 @@ type Store struct {
type CacheFactoryInitializer func() (CacheFactory, error)

type CacheFactory interface {
CacheFor(fields [][]string, transform cache.TransformFunc, client dynamic.ResourceInterface, gvk schema.GroupVersionKind, namespaced bool) (factory.Cache, error)
CacheFor(fields [][]string, transform cache.TransformFunc, client dynamic.ResourceInterface, gvk schema.GroupVersionKind, namespaced bool, watchable bool) (factory.Cache, error)
Reset() error
}

Expand Down Expand Up @@ -262,7 +263,7 @@ func (s *Store) initializeNamespaceCache() error {
transformFunc := s.transformBuilder.GetTransformFunc(gvk)

// get the ns informer
nsInformer, err := s.cacheFactory.CacheFor(fields, transformFunc, &tablelistconvert.Client{ResourceInterface: client}, attributes.GVK(&nsSchema), false)
nsInformer, err := s.cacheFactory.CacheFor(fields, transformFunc, &tablelistconvert.Client{ResourceInterface: client}, attributes.GVK(&nsSchema), false, true)
if err != nil {
return err
}
Expand Down Expand Up @@ -702,7 +703,7 @@ func (s *Store) ListByPartitions(apiOp *types.APIRequest, schema *types.APISchem
fields = append(fields, getFieldForGVK(gvk)...)
transformFunc := s.transformBuilder.GetTransformFunc(gvk)

inf, err := s.cacheFactory.CacheFor(fields, transformFunc, &tablelistconvert.Client{ResourceInterface: client}, attributes.GVK(schema), attributes.Namespaced(schema))
inf, err := s.cacheFactory.CacheFor(fields, transformFunc, &tablelistconvert.Client{ResourceInterface: client}, attributes.GVK(schema), attributes.Namespaced(schema), controllerschema.IsListWatchable(schema))
if err != nil {
return nil, 0, "", err
}
Expand Down
101 changes: 94 additions & 7 deletions pkg/stores/sqlproxy/proxy_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestNewProxyStore(t *testing.T) {
nsSchema := baseNSSchema
scc.EXPECT().SetColumns(context.Background(), &nsSchema).Return(nil)
cg.EXPECT().TableAdminClient(nil, &nsSchema, "", &WarningBuffer{}).Return(ri, nil)
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false).Return(c, nil)
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false, true).Return(c, nil)

s, err := NewProxyStore(scc, cg, rn, nil, cf)
assert.Nil(t, err)
Expand Down Expand Up @@ -149,7 +149,7 @@ func TestNewProxyStore(t *testing.T) {
nsSchema := baseNSSchema
scc.EXPECT().SetColumns(context.Background(), &nsSchema).Return(nil)
cg.EXPECT().TableAdminClient(nil, &nsSchema, "", &WarningBuffer{}).Return(ri, nil)
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false).Return(factory.Cache{}, fmt.Errorf("error"))
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false, true).Return(factory.Cache{}, fmt.Errorf("error"))

s, err := NewProxyStore(scc, cg, rn, nil, cf)
assert.Nil(t, err)
Expand Down Expand Up @@ -207,6 +207,7 @@ func TestListByPartitions(t *testing.T) {
Field: "some.field",
},
},
"verbs": []string{"list", "watch"},
}},
}
expectedItems := []unstructured.Unstructured{
Expand Down Expand Up @@ -240,7 +241,7 @@ func TestListByPartitions(t *testing.T) {
assert.Nil(t, err)
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"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema)).Return(c, nil)
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema), true).Return(c, nil)
tb.EXPECT().GetTransformFunc(attributes.GVK(schema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(listToReturn, len(listToReturn.Items), "", nil)
list, total, contToken, err := s.ListByPartitions(req, schema, partitions)
Expand Down Expand Up @@ -277,6 +278,7 @@ func TestListByPartitions(t *testing.T) {
Field: "some.field",
},
},
"verbs": []string{"list", "watch"},
}},
}
expectedItems := []unstructured.Unstructured{
Expand Down Expand Up @@ -343,6 +345,7 @@ func TestListByPartitions(t *testing.T) {
Field: "some.field",
},
},
"verbs": []string{"list", "watch"},
}},
}
expectedItems := []unstructured.Unstructured{
Expand Down Expand Up @@ -380,6 +383,88 @@ func TestListByPartitions(t *testing.T) {
assert.NotNil(t, err)
},
})
tests = append(tests, testCase{
description: "client ListByPartitions() should detect listable-but-unwatchable schema, still work normally",
test: func(t *testing.T) {
nsi := NewMockCache(gomock.NewController(t))
cg := NewMockClientGetter(gomock.NewController(t))
cf := NewMockCacheFactory(gomock.NewController(t))
ri := NewMockResourceInterface(gomock.NewController(t))
bloi := NewMockByOptionsLister(gomock.NewController(t))
tb := NewMockTransformBuilder(gomock.NewController(t))
inf := &informer.Informer{
ByOptionsLister: bloi,
}
c := factory.Cache{
ByOptionsLister: inf,
}
s := &Store{
namespaceCache: nsi,
clientGetter: cg,
cacheFactory: cf,
transformBuilder: tb,
}
var partitions []partition.Partition
req := &types.APIRequest{
Request: &http.Request{
URL: &url.URL{},
},
}
schema := &types.APISchema{
Schema: &schemas.Schema{Attributes: map[string]interface{}{
"columns": []common.ColumnDefinition{
{
Field: "some.field",
},
},
// note: no watch here
"verbs": []string{"list"},
}},
}
expectedItems := []unstructured.Unstructured{
{
Object: map[string]interface{}{
"kind": "apple",
"metadata": map[string]interface{}{
"name": "fuji",
},
"data": map[string]interface{}{
"color": "pink",
},
},
},
}
listToReturn := &unstructured.UnstructuredList{
Items: make([]unstructured.Unstructured, len(expectedItems), len(expectedItems)),
}
gvk := schema2.GroupVersionKind{
Group: "some",
Version: "test",
Kind: "gvk",
}
typeSpecificIndexedFields["some_test_gvk"] = [][]string{{"gvk", "specific", "fields"}}

attributes.SetGVK(schema, gvk)
// ListByPartitions copies point so we need some original record of items to ensure as asserting listToReturn's
// items is equal to the list returned by ListByParititons doesn't ensure no mutation happened
copy(listToReturn.Items, expectedItems)
opts, err := listprocessor.ParseQuery(req, nil)
assert.Nil(t, err)
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
// note also the watchable bool is expected to be false
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema), false).Return(c, nil)

tb.EXPECT().GetTransformFunc(attributes.GVK(schema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
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)
},
})
tests = append(tests, testCase{
description: "client ListByPartitions() with CacheFor() error returned should returned an errors. Should pass fields",
test: func(t *testing.T) {
Expand Down Expand Up @@ -408,6 +493,7 @@ func TestListByPartitions(t *testing.T) {
Field: "some.field",
},
},
"verbs": []string{"list", "watch"},
}},
}
expectedItems := []unstructured.Unstructured{
Expand Down Expand Up @@ -442,7 +528,7 @@ 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
tb.EXPECT().GetTransformFunc(attributes.GVK(schema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema)).Return(factory.Cache{}, fmt.Errorf("error"))
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema), true).Return(factory.Cache{}, fmt.Errorf("error"))

_, _, _, err = s.ListByPartitions(req, schema, partitions)
assert.NotNil(t, err)
Expand Down Expand Up @@ -483,6 +569,7 @@ func TestListByPartitions(t *testing.T) {
Field: "some.field",
},
},
"verbs": []string{"list", "watch"},
}},
}
expectedItems := []unstructured.Unstructured{
Expand Down Expand Up @@ -516,7 +603,7 @@ func TestListByPartitions(t *testing.T) {
assert.Nil(t, err)
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"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema)).Return(c, nil)
cf.EXPECT().CacheFor([][]string{{"some", "field"}, {`id`}, {`metadata`, `state`, `name`}, {"gvk", "specific", "fields"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(schema), attributes.Namespaced(schema), true).Return(c, nil)
bloi.EXPECT().ListByOptions(req.Context(), opts, partitions, req.Namespace).Return(nil, 0, "", fmt.Errorf("error"))
tb.EXPECT().GetTransformFunc(attributes.GVK(schema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })

Expand Down Expand Up @@ -558,7 +645,7 @@ func TestReset(t *testing.T) {
cf.EXPECT().Reset().Return(nil)
cs.EXPECT().SetColumns(gomock.Any(), gomock.Any()).Return(nil)
cg.EXPECT().TableAdminClient(nil, &nsSchema, "", &WarningBuffer{}).Return(ri, nil)
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false).Return(nsc2, nil)
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false, true).Return(nsc2, nil)
tb.EXPECT().GetTransformFunc(attributes.GVK(&nsSchema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
err := s.Reset()
assert.Nil(t, err)
Expand Down Expand Up @@ -661,7 +748,7 @@ func TestReset(t *testing.T) {
cf.EXPECT().Reset().Return(nil)
cs.EXPECT().SetColumns(gomock.Any(), gomock.Any()).Return(nil)
cg.EXPECT().TableAdminClient(nil, &nsSchema, "", &WarningBuffer{}).Return(ri, nil)
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false).Return(factory.Cache{}, fmt.Errorf("error"))
cf.EXPECT().CacheFor([][]string{{`id`}, {`metadata`, `state`, `name`}, {"metadata", "labels[field.cattle.io/projectId]"}}, gomock.Any(), &tablelistconvert.Client{ResourceInterface: ri}, attributes.GVK(&nsSchema), false, true).Return(factory.Cache{}, fmt.Errorf("error"))
tb.EXPECT().GetTransformFunc(attributes.GVK(&nsSchema)).Return(func(obj interface{}) (interface{}, error) { return obj, nil })
err := s.Reset()
assert.NotNil(t, err)
Expand Down
Loading