From bee732b107b62ffcc2acafcaecd181fe2673e814 Mon Sep 17 00:00:00 2001 From: old prince <26155892+LiuFang07@users.noreply.github.com> Date: Wed, 11 Jan 2023 13:05:26 +0800 Subject: [PATCH] Fix: Index structure map[string]string,Mongo resulting in inconsistent results obtained by filtering non-string type by index. (#5274) Signed-off-by: old.prince Signed-off-by: old.prince --- pkg/apiserver/domain/model/application.go | 20 ++++---- pkg/apiserver/domain/model/cluster.go | 4 +- pkg/apiserver/domain/model/env.go | 4 +- pkg/apiserver/domain/model/envbinding.go | 4 +- pkg/apiserver/domain/model/pipeline.go | 8 +-- pkg/apiserver/domain/model/project.go | 4 +- pkg/apiserver/domain/model/system_info.go | 4 +- pkg/apiserver/domain/model/target.go | 4 +- pkg/apiserver/domain/model/user.go | 20 ++++---- pkg/apiserver/domain/model/workflow.go | 11 ++-- pkg/apiserver/domain/service/workflow.go | 4 +- .../infrastructure/datastore/datastore.go | 2 +- .../datastore/kubeapi/kubeapi.go | 28 +++++++++-- .../datastore/kubeapi/kubeapi_test.go | 22 +++++++- pkg/utils/strings.go | 50 +++++++++++++++++++ pkg/utils/strings_test.go | 37 ++++++++++++++ 16 files changed, 174 insertions(+), 52 deletions(-) diff --git a/pkg/apiserver/domain/model/application.go b/pkg/apiserver/domain/model/application.go index 82b2d98a5..78b919d42 100644 --- a/pkg/apiserver/domain/model/application.go +++ b/pkg/apiserver/domain/model/application.go @@ -59,8 +59,8 @@ func (a *Application) PrimaryKey() string { } // Index return custom index -func (a *Application) Index() map[string]string { - index := make(map[string]string) +func (a *Application) Index() map[string]interface{} { + index := make(map[string]interface{}) if a.Name != "" { index["name"] = a.Name } @@ -154,8 +154,8 @@ func (a *ApplicationComponent) PrimaryKey() string { } // Index return custom index -func (a *ApplicationComponent) Index() map[string]string { - index := make(map[string]string) +func (a *ApplicationComponent) Index() map[string]interface{} { + index := make(map[string]interface{}) if a.Name != "" { index["name"] = a.Name } @@ -202,8 +202,8 @@ func (a *ApplicationPolicy) PrimaryKey() string { } // Index return custom index -func (a *ApplicationPolicy) Index() map[string]string { - index := make(map[string]string) +func (a *ApplicationPolicy) Index() map[string]interface{} { + index := make(map[string]interface{}) if a.Name != "" { index["name"] = a.Name } @@ -348,8 +348,8 @@ func (a *ApplicationRevision) PrimaryKey() string { } // Index return custom index -func (a *ApplicationRevision) Index() map[string]string { - index := make(map[string]string) +func (a *ApplicationRevision) Index() map[string]interface{} { + index := make(map[string]interface{}) if a.Version != "" { index["version"] = a.Version } @@ -434,8 +434,8 @@ func (w *ApplicationTrigger) PrimaryKey() string { } // Index return custom index -func (w *ApplicationTrigger) Index() map[string]string { - index := make(map[string]string) +func (w *ApplicationTrigger) Index() map[string]interface{} { + index := make(map[string]interface{}) if w.AppPrimaryKey != "" { index["appPrimaryKey"] = w.AppPrimaryKey } diff --git a/pkg/apiserver/domain/model/cluster.go b/pkg/apiserver/domain/model/cluster.go index 9dd797695..fe0abc598 100644 --- a/pkg/apiserver/domain/model/cluster.go +++ b/pkg/apiserver/domain/model/cluster.go @@ -93,8 +93,8 @@ func (c *Cluster) PrimaryKey() string { } // Index set to nil for list -func (c *Cluster) Index() map[string]string { - index := make(map[string]string) +func (c *Cluster) Index() map[string]interface{} { + index := make(map[string]interface{}) if c.Name != "" { index["name"] = c.Name } diff --git a/pkg/apiserver/domain/model/env.go b/pkg/apiserver/domain/model/env.go index ad3f159a8..b0d6f89f3 100644 --- a/pkg/apiserver/domain/model/env.go +++ b/pkg/apiserver/domain/model/env.go @@ -53,8 +53,8 @@ func (p *Env) PrimaryKey() string { } // Index return custom index -func (p *Env) Index() map[string]string { - index := make(map[string]string) +func (p *Env) Index() map[string]interface{} { + index := make(map[string]interface{}) if p.Name != "" { index["name"] = p.Name } diff --git a/pkg/apiserver/domain/model/envbinding.go b/pkg/apiserver/domain/model/envbinding.go index 9f6a3109d..800ee8deb 100644 --- a/pkg/apiserver/domain/model/envbinding.go +++ b/pkg/apiserver/domain/model/envbinding.go @@ -62,8 +62,8 @@ func (e *EnvBinding) PrimaryKey() string { } // Index return custom index -func (e *EnvBinding) Index() map[string]string { - index := make(map[string]string) +func (e *EnvBinding) Index() map[string]interface{} { + index := make(map[string]interface{}) if e.Name != "" { index["name"] = e.Name } diff --git a/pkg/apiserver/domain/model/pipeline.go b/pkg/apiserver/domain/model/pipeline.go index 242563498..cf39909ee 100644 --- a/pkg/apiserver/domain/model/pipeline.go +++ b/pkg/apiserver/domain/model/pipeline.go @@ -61,8 +61,8 @@ func (p Pipeline) ShortTableName() string { } // Index return custom index -func (p Pipeline) Index() map[string]string { - var index = make(map[string]string) +func (p Pipeline) Index() map[string]interface{} { + var index = make(map[string]interface{}) if p.Project != "" { index["project"] = p.Project } @@ -102,8 +102,8 @@ func (c *PipelineContext) PrimaryKey() string { } // Index return custom index -func (c *PipelineContext) Index() map[string]string { - index := make(map[string]string) +func (c *PipelineContext) Index() map[string]interface{} { + index := make(map[string]interface{}) if c.ProjectName != "" { index["project_name"] = c.ProjectName } diff --git a/pkg/apiserver/domain/model/project.go b/pkg/apiserver/domain/model/project.go index 912c1cfa6..3e500ef9b 100644 --- a/pkg/apiserver/domain/model/project.go +++ b/pkg/apiserver/domain/model/project.go @@ -54,8 +54,8 @@ func (p *Project) PrimaryKey() string { } // Index return custom index -func (p *Project) Index() map[string]string { - index := make(map[string]string) +func (p *Project) Index() map[string]interface{} { + index := make(map[string]interface{}) if p.Name != "" { index["name"] = p.Name } diff --git a/pkg/apiserver/domain/model/system_info.go b/pkg/apiserver/domain/model/system_info.go index d4eb6a192..db1d03179 100644 --- a/pkg/apiserver/domain/model/system_info.go +++ b/pkg/apiserver/domain/model/system_info.go @@ -146,8 +146,8 @@ func (u *SystemInfo) PrimaryKey() string { } // Index return custom index -func (u *SystemInfo) Index() map[string]string { - index := make(map[string]string) +func (u *SystemInfo) Index() map[string]interface{} { + index := make(map[string]interface{}) if u.InstallID != "" { index["installID"] = u.InstallID } diff --git a/pkg/apiserver/domain/model/target.go b/pkg/apiserver/domain/model/target.go index 795fe9ca0..f26b77409 100644 --- a/pkg/apiserver/domain/model/target.go +++ b/pkg/apiserver/domain/model/target.go @@ -48,8 +48,8 @@ func (d *Target) PrimaryKey() string { } // Index return custom index -func (d *Target) Index() map[string]string { - index := make(map[string]string) +func (d *Target) Index() map[string]interface{} { + index := make(map[string]interface{}) if d.Name != "" { index["name"] = d.Name } diff --git a/pkg/apiserver/domain/model/user.go b/pkg/apiserver/domain/model/user.go index 35678047a..1fe00e95e 100644 --- a/pkg/apiserver/domain/model/user.go +++ b/pkg/apiserver/domain/model/user.go @@ -67,8 +67,8 @@ func (u *User) PrimaryKey() string { } // Index return custom index -func (u *User) Index() map[string]string { - index := make(map[string]string) +func (u *User) Index() map[string]interface{} { + index := make(map[string]interface{}) if u.Name != "" { index["name"] = u.Name } @@ -106,8 +106,8 @@ func (u *ProjectUser) PrimaryKey() string { } // Index return custom index -func (u *ProjectUser) Index() map[string]string { - index := make(map[string]string) +func (u *ProjectUser) Index() map[string]interface{} { + index := make(map[string]interface{}) if u.Username != "" { index["username"] = u.Username } @@ -177,8 +177,8 @@ func (r *Role) PrimaryKey() string { } // Index return custom index -func (r *Role) Index() map[string]string { - index := make(map[string]string) +func (r *Role) Index() map[string]interface{} { + index := make(map[string]interface{}) if r.Name != "" { index["name"] = r.Name } @@ -207,8 +207,8 @@ func (p *Permission) PrimaryKey() string { } // Index return custom index -func (p *Permission) Index() map[string]string { - index := make(map[string]string) +func (p *Permission) Index() map[string]interface{} { + index := make(map[string]interface{}) if p.Name != "" { index["name"] = p.Name } @@ -250,8 +250,8 @@ func (p *PermissionTemplate) PrimaryKey() string { } // Index return custom index -func (p *PermissionTemplate) Index() map[string]string { - index := make(map[string]string) +func (p *PermissionTemplate) Index() map[string]interface{} { + index := make(map[string]interface{}) if p.Name != "" { index["name"] = p.Name } diff --git a/pkg/apiserver/domain/model/workflow.go b/pkg/apiserver/domain/model/workflow.go index 5e96b04c1..4be5dc99c 100644 --- a/pkg/apiserver/domain/model/workflow.go +++ b/pkg/apiserver/domain/model/workflow.go @@ -18,7 +18,6 @@ package model import ( "fmt" - "strconv" "time" workflowv1alpha1 "github.com/kubevela/workflow/api/v1alpha1" @@ -88,8 +87,8 @@ func (w *Workflow) PrimaryKey() string { } // Index return custom primary key -func (w *Workflow) Index() map[string]string { - index := make(map[string]string) +func (w *Workflow) Index() map[string]interface{} { + index := make(map[string]interface{}) if w.Name != "" { index["name"] = w.Name } @@ -100,7 +99,7 @@ func (w *Workflow) Index() map[string]string { index["envName"] = w.EnvName } if w.Default != nil { - index["default"] = strconv.FormatBool(*w.Default) + index["default"] = *w.Default } return index @@ -161,8 +160,8 @@ func (w *WorkflowRecord) PrimaryKey() string { } // Index return custom primary key -func (w *WorkflowRecord) Index() map[string]string { - index := make(map[string]string) +func (w *WorkflowRecord) Index() map[string]interface{} { + index := make(map[string]interface{}) if w.Name != "" { index["name"] = w.Name } diff --git a/pkg/apiserver/domain/service/workflow.go b/pkg/apiserver/domain/service/workflow.go index 48aa09e9c..5641ff8bd 100644 --- a/pkg/apiserver/domain/service/workflow.go +++ b/pkg/apiserver/domain/service/workflow.go @@ -866,8 +866,8 @@ func (w *workflowServiceImpl) RollbackRecord(ctx context.Context, appModel *mode if len(revisions) == 0 { return nil, bcode.ErrApplicationNoReadyRevision } - revisionVersion = revisions[0].Index()["version"] - klog.Infof("select lastest complete revision %s", revisions[0].Index()["version"]) + revisionVersion = pkgUtils.ToString(revisions[0].Index()["version"]) + klog.Infof("select lastest complete revision %s", revisionVersion) } var record = &model.WorkflowRecord{ diff --git a/pkg/apiserver/infrastructure/datastore/datastore.go b/pkg/apiserver/infrastructure/datastore/datastore.go index aa5cd09f1..0f1ec846d 100644 --- a/pkg/apiserver/infrastructure/datastore/datastore.go +++ b/pkg/apiserver/infrastructure/datastore/datastore.go @@ -74,7 +74,7 @@ type Entity interface { PrimaryKey() string TableName() string ShortTableName() string - Index() map[string]string + Index() map[string]interface{} } // NewEntity Create a new object based on the input type diff --git a/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi.go b/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi.go index 85e12a930..4b2bb0444 100644 --- a/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi.go +++ b/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi.go @@ -37,6 +37,7 @@ import ( "github.com/oam-dev/kubevela/pkg/apiserver/infrastructure/clients" "github.com/oam-dev/kubevela/pkg/apiserver/infrastructure/datastore" + pkgUtils "github.com/oam-dev/kubevela/pkg/utils" ) type kubeapi struct { @@ -81,7 +82,7 @@ func generateName(entity datastore.Entity) string { func (m *kubeapi) generateConfigMap(entity datastore.Entity) *corev1.ConfigMap { data, _ := json.Marshal(entity) - labels := entity.Index() + labels := convertIndex2Labels(entity.Index()) if labels == nil { labels = make(map[string]string) } @@ -176,7 +177,7 @@ func (m *kubeapi) Put(ctx context.Context, entity datastore.Entity) error { return datastore.ErrTableNameEmpty } // update labels - labels := entity.Index() + labels := convertIndex2Labels(entity.Index()) if labels == nil { labels = make(map[string]string) } @@ -350,8 +351,8 @@ func (m *kubeapi) List(ctx context.Context, entity datastore.Entity, op *datasto rq, _ := labels.NewRequirement(MigrateKey, selection.DoesNotExist, []string{"ok"}) selector = selector.Add(*rq) - - for k, v := range entity.Index() { + metedataLabels := convertIndex2Labels(entity.Index()) + for k, v := range metedataLabels { rq, err := labels.NewRequirement(k, selection.Equals, []string{verifyValue(v)}) if err != nil { return nil, datastore.ErrIndexInvalid @@ -441,7 +442,8 @@ func (m *kubeapi) Count(ctx context.Context, entity datastore.Entity, filterOpti if err != nil { return 0, datastore.NewDBError(err) } - for k, v := range entity.Index() { + metedataLabels := convertIndex2Labels(entity.Index()) + for k, v := range metedataLabels { rq, err := labels.NewRequirement(k, selection.Equals, []string{verifyValue(v)}) if err != nil { return 0, datastore.ErrIndexInvalid @@ -494,3 +496,19 @@ func verifyValue(v string) string { s = strings.ReplaceAll(s, " ", "-") return strings.ToLower(s) } + +func convertIndex2Labels(index map[string]interface{}) map[string]string { + if index == nil { + return nil + } + ret := make(map[string]string, len(index)) + for k, v := range index { + value := pkgUtils.ToString(v) + if value == "" { + klog.Warningf("unable to cast %#v of type %T to string", v, v) + continue + } + ret[k] = pkgUtils.ToString(v) + } + return ret +} diff --git a/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi_test.go b/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi_test.go index 781ef4c09..a1bbda12e 100644 --- a/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi_test.go +++ b/pkg/apiserver/infrastructure/datastore/kubeapi/kubeapi_test.go @@ -72,20 +72,38 @@ var _ = Describe("Test kubeapi datastore driver", func() { err := kubeStore.Put(context.TODO(), &model.Application{Name: "kubevela-app", Description: "this is demo"}) Expect(err).ToNot(HaveOccurred()) }) - It("Test index", func() { + It("Test application index", func() { var app = model.Application{ Name: "test", } selector, err := labels.Parse(fmt.Sprintf("table=%s", app.TableName())) Expect(err).ToNot(HaveOccurred()) Expect(cmp.Diff(app.Index()["name"], "test")).Should(BeEmpty()) - for k, v := range app.Index() { + index := convertIndex2Labels(app.Index()) + for k, v := range index { rq, err := labels.NewRequirement(k, selection.Equals, []string{v}) Expect(err).ToNot(HaveOccurred()) selector = selector.Add(*rq) } Expect(cmp.Diff(selector.String(), "name=test,table=vela_application")).Should(BeEmpty()) }) + It("Test workflow index", func() { + defaultPtr := false + var workflow = model.Workflow{ + Name: "test", + Default: &defaultPtr, + } + selector, err := labels.Parse(fmt.Sprintf("table=%s", workflow.TableName())) + Expect(err).ToNot(HaveOccurred()) + Expect(cmp.Diff(workflow.Index()["name"], "test")).Should(BeEmpty()) + index := convertIndex2Labels(workflow.Index()) + for k, v := range index { + rq, err := labels.NewRequirement(k, selection.Equals, []string{v}) + Expect(err).ToNot(HaveOccurred()) + selector = selector.Add(*rq) + } + Expect(cmp.Diff(selector.String(), "default=false,name=test,table=vela_workflow")).Should(BeEmpty()) + }) It("Test list function", func() { var app model.Application list, err := kubeStore.List(context.TODO(), &app, &datastore.ListOptions{Page: -1}) diff --git a/pkg/utils/strings.go b/pkg/utils/strings.go index 542fed8ac..1f6b3c61e 100644 --- a/pkg/utils/strings.go +++ b/pkg/utils/strings.go @@ -21,6 +21,7 @@ import ( "path" "reflect" "sort" + "strconv" "strings" ) @@ -80,6 +81,55 @@ func SliceIncludeSlice(a, b []string) bool { return true } +// ToString convery an interface to a string type. +func ToString(i interface{}) string { + if i == nil { + return "" + } + v := reflect.ValueOf(i) + if v.Kind() == reflect.Ptr && !v.IsNil() { + v = v.Elem() + } + i = v.Interface() + switch s := i.(type) { + case string: + return s + case bool: + return strconv.FormatBool(s) + case float64: + return strconv.FormatFloat(s, 'f', -1, 64) + case float32: + return strconv.FormatFloat(float64(s), 'f', -1, 32) + case int: + return strconv.Itoa(s) + case int64: + return strconv.FormatInt(s, 10) + case int32: + return strconv.Itoa(int(s)) + case int16: + return strconv.FormatInt(int64(s), 10) + case int8: + return strconv.FormatInt(int64(s), 10) + case uint: + return strconv.FormatUint(uint64(s), 10) + case uint64: + // nolint + return strconv.FormatUint(uint64(s), 10) + case uint32: + return strconv.FormatUint(uint64(s), 10) + case uint16: + return strconv.FormatUint(uint64(s), 10) + case uint8: + return strconv.FormatUint(uint64(s), 10) + case []byte: + return string(s) + case nil: + return "" + default: + return "" + } +} + // MapKey2Array convery map keys to array func MapKey2Array(source map[string]string) []string { var list []string diff --git a/pkg/utils/strings_test.go b/pkg/utils/strings_test.go index 7c68d7e87..5424d21a0 100644 --- a/pkg/utils/strings_test.go +++ b/pkg/utils/strings_test.go @@ -137,3 +137,40 @@ func TestJoinURL(t *testing.T) { } } + +func TestToString(t *testing.T) { + type Obj struct { + k string + } + obj := &Obj{"foo"} + boolPtr := true + caseA := []struct { + input interface{} + expect string + }{ + {int(666), "666"}, + {int8(6), "6"}, + {int16(6), "6"}, + {int32(6), "6"}, + {int64(6), "6"}, + {uint(6), "6"}, + {uint8(6), "6"}, + {uint16(6), "6"}, + {uint32(6), "6"}, + {uint64(6), "6"}, + {float32(3.14), "3.14"}, + {float64(3.14), "3.14"}, + {true, "true"}, + {false, "false"}, + {&boolPtr, "true"}, + {nil, ""}, + {[]byte("one time"), "one time"}, + {"one more time", "one more time"}, + {obj, ""}, + } + + for _, test := range caseA { + v := ToString(test.input) + assert.Equal(t, test.expect, v) + } +}