From 38099cadf9c94d315cea82435e26ab95abbd5054 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Mon, 9 Sep 2024 09:28:00 +0545 Subject: [PATCH 1/3] feat: support include/exclude for labels filter in config summary --- query/config.go | 16 +++++- tests/config_summary_test.go | 99 ++++++++++++++++++++++++++-------- tests/fixtures/dummy/config.go | 1 + 3 files changed, 94 insertions(+), 22 deletions(-) diff --git a/query/config.go b/query/config.go index b2c6a4e0..fe7fbc72 100644 --- a/query/config.go +++ b/query/config.go @@ -218,8 +218,22 @@ func (t ConfigSummaryRequest) healthClause() []clause.Expression { } func (t *ConfigSummaryRequest) filterClause(q *gorm.DB) *gorm.DB { + var orClause *gorm.DB for k, v := range t.Filter { - q = q.Where("config_items.labels @> ?", types.JSONStringMap{k: v}) + if strings.HasPrefix(v, "!") { + excludeValue := strings.TrimPrefix(v, "!") + q = q.Where("NOT (config_items.labels @> ?)", types.JSONStringMap{k: excludeValue}) + } else { + if orClause == nil { + orClause = q + } + + orClause = orClause.Or("config_items.labels @> ?", types.JSONStringMap{k: v}) + } + } + + if orClause != nil { + q = q.Where(orClause) } return q diff --git a/tests/config_summary_test.go b/tests/config_summary_test.go index 54e8963c..25dccd21 100644 --- a/tests/config_summary_test.go +++ b/tests/config_summary_test.go @@ -48,29 +48,82 @@ var _ = ginkgo.Describe("Config Summary Search", ginkgo.Ordered, func() { } }) - ginkgo.It("should filter by labels", func() { - request := query.ConfigSummaryRequest{ - Filter: map[string]string{ - "environment": "production", - }, - } - response, err := query.ConfigSummary(DefaultContext, request) - Expect(err).To(BeNil()) + ginkgo.Context("labels filter", func() { + ginkgo.It("should filter by labels", func() { + request := query.ConfigSummaryRequest{ + Filter: map[string]string{ + "environment": "production", + }, + } - var output []map[string]any - err = json.Unmarshal(response, &output) - Expect(err).To(BeNil()) + response, err := query.ConfigSummary(DefaultContext, request) + Expect(err).To(BeNil()) - types := lo.Map(output, func(item map[string]any, _ int) string { - return item["type"].(string) + var output []map[string]any + err = json.Unmarshal(response, &output) + Expect(err).To(BeNil()) + + types := lo.Map(output, func(item map[string]any, _ int) string { + return item["type"].(string) + }) + withLabels := lo.Filter(dummy.AllDummyConfigs, func(item models.ConfigItem, _ int) bool { + return lo.FromPtr(item.Labels)["environment"] == "production" + }) + expected := lo.Uniq(lo.Map(withLabels, func(item models.ConfigItem, _ int) string { + return lo.FromPtr(item.Type) + })) + Expect(types).To(ConsistOf(expected)) }) - withLabels := lo.Filter(dummy.AllDummyConfigs, func(item models.ConfigItem, _ int) bool { - return lo.FromPtr(item.Labels)["environment"] == "production" + + ginkgo.It("should filter by multiple labels", func() { + request := query.ConfigSummaryRequest{ + Filter: map[string]string{ + "environment": "production", + "cluster": "demo", + }, + } + + response, err := query.ConfigSummary(DefaultContext, request) + Expect(err).To(BeNil()) + + var output []map[string]any + err = json.Unmarshal(response, &output) + Expect(err).To(BeNil()) + + types := lo.Map(output, func(item map[string]any, _ int) string { + return item["type"].(string) + }) + withLabels := lo.Filter(dummy.AllDummyConfigs, func(item models.ConfigItem, _ int) bool { + return lo.FromPtr(item.Labels)["environment"] == "production" || lo.FromPtr(item.Labels)["cluster"] == "demo" + }) + expected := lo.Uniq(lo.Map(withLabels, func(item models.ConfigItem, _ int) string { + return lo.FromPtr(item.Type) + })) + Expect(types).To(ConsistOf(expected)) + }) + + ginkgo.It("should handle exclude queries", func() { + request := query.ConfigSummaryRequest{ + Filter: map[string]string{ + "environment": "production", + "account": "!flanksource", + "owner": "!team-1", + "database": "!logistics", + }, + } + + response, err := query.ConfigSummary(DefaultContext, request) + Expect(err).To(BeNil()) + + var output []map[string]any + err = json.Unmarshal(response, &output) + Expect(err).To(BeNil()) + + types := lo.Map(output, func(item map[string]any, _ int) string { + return item["type"].(string) + }) + Expect(types).To(ConsistOf([]string{"Logistics::UI::Deployment", "Logistics::Worker::Deployment"})) }) - expected := lo.Uniq(lo.Map(withLabels, func(item models.ConfigItem, _ int) string { - return lo.FromPtr(item.Type) - })) - Expect(types).To(ConsistOf(expected)) }) ginkgo.Context("should query changes by range", func() { @@ -79,7 +132,9 @@ var _ = ginkgo.Describe("Config Summary Search", ginkgo.Ordered, func() { Changes: query.ConfigSummaryRequestChanges{ Since: "7d", }, - Filter: lo.FromPtr(dummy.EKSCluster.Labels), + Filter: map[string]string{ + "eks_version": "1.27", + }, } response, err := query.ConfigSummary(DefaultContext, request) Expect(err).To(BeNil()) @@ -97,7 +152,9 @@ var _ = ginkgo.Describe("Config Summary Search", ginkgo.Ordered, func() { Changes: query.ConfigSummaryRequestChanges{ Since: "5y", }, - Filter: lo.FromPtr(dummy.EKSCluster.Labels), + Filter: map[string]string{ + "eks_version": "1.27", + }, } response, err := query.ConfigSummary(DefaultContext, request) Expect(err).To(BeNil()) diff --git a/tests/fixtures/dummy/config.go b/tests/fixtures/dummy/config.go index 8f150165..e3fb8cd2 100644 --- a/tests/fixtures/dummy/config.go +++ b/tests/fixtures/dummy/config.go @@ -21,6 +21,7 @@ var EKSCluster = models.ConfigItem{ "cluster": "aws", "environment": "production", "telemetry": "enabled", + "eks_version": "1.27", }), } From a66dea6db4a706c271f67df927e0734e0df6d2bc Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Mon, 9 Sep 2024 19:09:47 +0545 Subject: [PATCH 2/3] feat: support multiple valeus per label on config summary search --- query/config.go | 17 ++++++++++++----- tests/config_summary_test.go | 29 ++++++++++++++++++++++++----- upstream/commands.go | 2 +- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/query/config.go b/query/config.go index fe7fbc72..445a73ff 100644 --- a/query/config.go +++ b/query/config.go @@ -220,15 +220,22 @@ func (t ConfigSummaryRequest) healthClause() []clause.Expression { func (t *ConfigSummaryRequest) filterClause(q *gorm.DB) *gorm.DB { var orClause *gorm.DB for k, v := range t.Filter { - if strings.HasPrefix(v, "!") { - excludeValue := strings.TrimPrefix(v, "!") - q = q.Where("NOT (config_items.labels @> ?)", types.JSONStringMap{k: excludeValue}) - } else { + in, notIN, _, _, _ := ParseFilteringQuery(v, true) + + if len(notIN) > 0 { + for _, excludeValue := range notIN { + q = q.Where("NOT (config_items.labels @> ?)", types.JSONStringMap{k: excludeValue.(string)}) + } + } + + if len(in) > 0 { if orClause == nil { orClause = q } - orClause = orClause.Or("config_items.labels @> ?", types.JSONStringMap{k: v}) + for _, includeValue := range in { + orClause = orClause.Or("config_items.labels @> ?", types.JSONStringMap{k: includeValue.(string)}) + } } } diff --git a/tests/config_summary_test.go b/tests/config_summary_test.go index 25dccd21..6855d44f 100644 --- a/tests/config_summary_test.go +++ b/tests/config_summary_test.go @@ -105,10 +105,9 @@ var _ = ginkgo.Describe("Config Summary Search", ginkgo.Ordered, func() { ginkgo.It("should handle exclude queries", func() { request := query.ConfigSummaryRequest{ Filter: map[string]string{ - "environment": "production", - "account": "!flanksource", - "owner": "!team-1", - "database": "!logistics", + "environment": "!development,!testing", + "account": "flanksource", + "telemetry": "enabled", }, } @@ -122,7 +121,27 @@ var _ = ginkgo.Describe("Config Summary Search", ginkgo.Ordered, func() { types := lo.Map(output, func(item map[string]any, _ int) string { return item["type"].(string) }) - Expect(types).To(ConsistOf([]string{"Logistics::UI::Deployment", "Logistics::Worker::Deployment"})) + + withLabels := lo.Filter(dummy.AllDummyConfigs, func(item models.ConfigItem, _ int) bool { + env := lo.FromPtr(item.Labels)["environment"] + if env == "development" || env == "testing" { + return false + } + + if lo.FromPtr(item.Labels)["account"] == "flanksource" { + return true + } + + if lo.FromPtr(item.Labels)["telemetry"] == "enabled" { + return true + } + + return false + }) + expected := lo.Uniq(lo.Map(withLabels, func(item models.ConfigItem, _ int) string { + return lo.FromPtr(item.Type) + })) + Expect(types).To(ConsistOf(expected)) }) }) diff --git a/upstream/commands.go b/upstream/commands.go index d7af18d5..75b45908 100644 --- a/upstream/commands.go +++ b/upstream/commands.go @@ -250,7 +250,7 @@ func handleUpsertError(ctx context.Context, items []models.ExtendedDBTable, err // If foreign key error, try inserting one by one and return the ones that fail var conflicted []string for _, item := range items { - if err := ctx.DB().Debug().Clauses(clause.OnConflict{UpdateAll: true, Columns: item.PKCols()}).Omit("created_by").Create(item.Value()).Error; err != nil { + if err := ctx.DB().Clauses(clause.OnConflict{UpdateAll: true, Columns: item.PKCols()}).Omit("created_by").Create(item.Value()).Error; err != nil { if dutil.IsForeignKeyError(err) { conflicted = append(conflicted, item.PK()) } else { From fac83f3d75259a5b4c9ca3e149b63c9467ae63d4 Mon Sep 17 00:00:00 2001 From: Aditya Thebe Date: Thu, 12 Sep 2024 10:13:04 +0545 Subject: [PATCH 3/3] fix: consistent behavior in filter clause --- query/config.go | 24 +++++++++++++++++------- tests/suite_test.go | 1 + 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/query/config.go b/query/config.go index 445a73ff..bc473730 100644 --- a/query/config.go +++ b/query/config.go @@ -218,29 +218,39 @@ func (t ConfigSummaryRequest) healthClause() []clause.Expression { } func (t *ConfigSummaryRequest) filterClause(q *gorm.DB) *gorm.DB { - var orClause *gorm.DB + var includeClause *gorm.DB + var excludeClause *gorm.DB + for k, v := range t.Filter { in, notIN, _, _, _ := ParseFilteringQuery(v, true) if len(notIN) > 0 { + if excludeClause == nil { + excludeClause = q + } + for _, excludeValue := range notIN { - q = q.Where("NOT (config_items.labels @> ?)", types.JSONStringMap{k: excludeValue.(string)}) + excludeClause = excludeClause.Where("NOT (config_items.labels @> ?)", types.JSONStringMap{k: excludeValue.(string)}) } } if len(in) > 0 { - if orClause == nil { - orClause = q + if includeClause == nil { + includeClause = q } for _, includeValue := range in { - orClause = orClause.Or("config_items.labels @> ?", types.JSONStringMap{k: includeValue.(string)}) + includeClause = includeClause.Or("config_items.labels @> ?", types.JSONStringMap{k: includeValue.(string)}) } } } - if orClause != nil { - q = q.Where(orClause) + if includeClause != nil { + q = q.Where(includeClause) + } + + if excludeClause != nil { + q = q.Where(excludeClause) } return q diff --git a/tests/suite_test.go b/tests/suite_test.go index ae3503e1..a3012676 100644 --- a/tests/suite_test.go +++ b/tests/suite_test.go @@ -19,4 +19,5 @@ func TestDuty(t *testing.T) { var _ = ginkgo.BeforeSuite(func() { DefaultContext = setup.BeforeSuiteFn() }) + var _ = ginkgo.AfterSuite(setup.AfterSuiteFn)