From da33989d63ed1e1064712055c288c080834e4813 Mon Sep 17 00:00:00 2001 From: Yash Mehrotra Date: Fri, 1 Nov 2024 12:08:17 +0530 Subject: [PATCH] chore: optimize related_change_recursive_query (#1165) * chore: optimize related_change_recursive_query * chore: update tests * chore: document soft relation problems --- query/config_changes.go | 7 +- tests/config_changes_test.go | 123 +++++++++++++++++++++++++++-------- views/006_config_views.sql | 43 +++++++++++- 3 files changed, 143 insertions(+), 30 deletions(-) diff --git a/query/config_changes.go b/query/config_changes.go index ebce9248..4da16a6c 100644 --- a/query/config_changes.go +++ b/query/config_changes.go @@ -58,6 +58,11 @@ type CatalogChangesSearchRequest struct { // upstream | downstream | both Recursive string `query:"recursive" json:"recursive"` + // FIXME: Soft toggle does not work with Recursive=both + // In that case, soft relations are always returned + // It also returns ALL soft relations throughout the tree + // not just soft related to the main config item + Soft bool `query:"soft" json:"soft"` fromParsed time.Time toParsed time.Time @@ -347,7 +352,7 @@ func FindCatalogChanges(ctx context.Context, req CatalogChangesSearchRequest) (* table := query.Table("catalog_changes") if err := uuid.Validate(req.CatalogID); err == nil { - table = query.Table("related_changes_recursive(?,?,?,?)", req.CatalogID, req.Recursive, req.IncludeDeletedConfigs, req.Depth) + table = query.Table("related_changes_recursive(?,?,?,?,?)", req.CatalogID, req.Recursive, req.IncludeDeletedConfigs, req.Depth, req.Soft) } else { clause, err := parseAndBuildFilteringQuery(req.CatalogID, "config_id", false) if err != nil { diff --git a/tests/config_changes_test.go b/tests/config_changes_test.go index 9da9c8a5..7bcb9e2c 100644 --- a/tests/config_changes_test.go +++ b/tests/config_changes_test.go @@ -1,6 +1,7 @@ package tests import ( + "strings" "time" "github.com/flanksource/duty/db" @@ -17,32 +18,36 @@ import ( var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { // Graph #1 (acyclic) // - // U + // U --- (A) + // / \ + // (B)----V W // / \ - // V W - // / \ - // X Y - // / - // Z + // (C)--X Y + // / + // Z--- (D) // Create a list of ConfigItems var ( U = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Node"), Name: lo.ToPtr("U"), ConfigClass: "Node"} - V = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Deployment"), Name: lo.ToPtr("V"), ConfigClass: "Deployment"} - W = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("W"), ConfigClass: "Pod"} - X = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ReplicaSet"), Name: lo.ToPtr("X"), ConfigClass: "ReplicaSet"} - Y = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::PersistentVolume"), Name: lo.ToPtr("Y"), ConfigClass: "PersistentVolume"} - Z = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("Z"), ConfigClass: "Pod"} + V = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Deployment"), Name: lo.ToPtr("V"), ConfigClass: "Deployment", ParentID: lo.ToPtr(U.ID), Path: U.ID.String()} + W = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("W"), ConfigClass: "Pod", ParentID: lo.ToPtr(U.ID), Path: U.ID.String()} + X = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ReplicaSet"), Name: lo.ToPtr("X"), ConfigClass: "ReplicaSet", ParentID: lo.ToPtr(V.ID), Path: strings.Join([]string{U.ID.String(), V.ID.String()}, ".")} + Y = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::PersistentVolume"), Name: lo.ToPtr("Y"), ConfigClass: "PersistentVolume", ParentID: lo.ToPtr(V.ID), Path: strings.Join([]string{U.ID.String(), V.ID.String()}, ".")} + Z = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::Pod"), Name: lo.ToPtr("Z"), ConfigClass: "Pod", ParentID: lo.ToPtr(X.ID), Path: strings.Join([]string{U.ID.String(), V.ID.String(), X.ID.String()}, ".")} + + A = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("A"), ConfigClass: "ConfigMap"} + B = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("B"), ConfigClass: "ConfigMap"} + C = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("C"), ConfigClass: "ConfigMap"} + D = models.ConfigItem{ID: uuid.New(), Tags: types.JSONStringMap{"namespace": "test-changes"}, Type: lo.ToPtr("Kubernetes::ConfigMap"), Name: lo.ToPtr("D"), ConfigClass: "ConfigMap"} ) - configItems := []models.ConfigItem{U, V, W, X, Y, Z} + configItems := []models.ConfigItem{U, V, W, X, Y, Z, A, B, C, D} // Create relationships between ConfigItems relationships := []models.ConfigRelationship{ - {ConfigID: U.ID.String(), RelatedID: V.ID.String(), Relation: "test-changes-UV"}, - {ConfigID: U.ID.String(), RelatedID: W.ID.String(), Relation: "test-changes-UW"}, - {ConfigID: V.ID.String(), RelatedID: X.ID.String(), Relation: "test-changes-VX"}, - {ConfigID: V.ID.String(), RelatedID: Y.ID.String(), Relation: "test-changes-VY"}, - {ConfigID: X.ID.String(), RelatedID: Z.ID.String(), Relation: "test-changes-XZ"}, + {ConfigID: U.ID.String(), RelatedID: A.ID.String(), Relation: "test-changes-UA"}, + {ConfigID: V.ID.String(), RelatedID: B.ID.String(), Relation: "test-changes-VB"}, + {ConfigID: X.ID.String(), RelatedID: C.ID.String(), Relation: "test-changes-XC"}, + {ConfigID: Z.ID.String(), RelatedID: D.ID.String(), Relation: "test-changes-ZD"}, } // Create changes for each config @@ -54,7 +59,12 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { YChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 4)), Severity: "warn", ConfigID: Y.ID.String(), Summary: ".name.Y", ChangeType: "diff", Source: "test-changes"} ZChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: Z.ID.String(), Summary: ".name.Z", ChangeType: "Pulled", Source: "test-changes"} - changes = []models.ConfigChange{UChange, VChange, WChange, XChange, YChange, ZChange} + AChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: A.ID.String(), Summary: ".name.A", ChangeType: "Pulled", Source: "test-changes"} + BChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: B.ID.String(), Summary: ".name.B", ChangeType: "Pulled", Source: "test-changes"} + CChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: C.ID.String(), Summary: ".name.C", ChangeType: "Pulled", Source: "test-changes"} + DChange = models.ConfigChange{ID: uuid.New().String(), CreatedAt: lo.ToPtr(time.Now().Add(-time.Hour * 5)), Severity: "info", ConfigID: D.ID.String(), Summary: ".name.D", ChangeType: "Pulled", Source: "test-changes"} + + changes = []models.ConfigChange{UChange, VChange, WChange, XChange, YChange, ZChange, AChange, BChange, CChange, DChange} ) ginkgo.BeforeAll(func() { @@ -111,13 +121,12 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { ginkgo.Context("Both ways", func() { ginkgo.It("should return changes upstream and downstream", func() { relatedChanges, err := findChanges(X.ID, "all", false) - Expect(err).To(BeNil()) - Expect(len(relatedChanges.Changes)).To(Equal(4)) + Expect(len(relatedChanges.Changes)).To(Equal(6)) relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID }) - Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, XChange.ID, ZChange.ID})) + Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, XChange.ID, ZChange.ID, CChange.ID, DChange.ID})) }) }) @@ -132,13 +141,43 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, WChange.ID, XChange.ID, YChange.ID, ZChange.ID})) }) + ginkgo.It("should return changes of a root node along with soft", func() { + relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{ + CatalogID: U.ID.String(), + Recursive: "downstream", + Soft: true, + }) + + Expect(err).To(BeNil()) + + Expect(len(relatedChanges.Changes)).To(Equal(7)) + + relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID }) + Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, WChange.ID, XChange.ID, YChange.ID, ZChange.ID, AChange.ID})) + }) + ginkgo.It("should return changes of a leaf node", func() { relatedChanges, err := findChanges(Z.ID, "all", false) Expect(err).To(BeNil()) - Expect(len(relatedChanges.Changes)).To(Equal(4)) + Expect(len(relatedChanges.Changes)).To(Equal(5)) + + relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID }) + Expect(relatedIDs).To(ConsistOf([]string{ZChange.ID, XChange.ID, VChange.ID, UChange.ID, DChange.ID})) + }) + + ginkgo.It("should return changes of a leaf node along with soft", func() { + relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{ + CatalogID: X.ID.String(), + Recursive: "downstream", + Soft: true, + }) + + Expect(err).To(BeNil()) + + Expect(len(relatedChanges.Changes)).To(Equal(3)) relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID }) - Expect(relatedIDs).To(ConsistOf([]string{ZChange.ID, XChange.ID, VChange.ID, UChange.ID})) + Expect(relatedIDs).To(ConsistOf([]string{XChange.ID, ZChange.ID, CChange.ID})) }) }) @@ -152,13 +191,43 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { Expect(relatedIDs).To(ConsistOf([]string{UChange.ID})) }) + ginkgo.It("should return changes of a leaf node along with soft", func() { + relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{ + CatalogID: U.ID.String(), + Recursive: "upstream", + Soft: true, + }) + + Expect(err).To(BeNil()) + + Expect(len(relatedChanges.Changes)).To(Equal(2)) + + relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID }) + Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, AChange.ID})) + }) + ginkgo.It("should return changes of a non-root node", func() { relatedChanges, err := findChanges(X.ID, "all", false) Expect(err).To(BeNil()) + Expect(len(relatedChanges.Changes)).To(Equal(6)) + + relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID }) + Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, ZChange.ID, XChange.ID, CChange.ID, DChange.ID})) + }) + + ginkgo.It("should return changes of a leaf node along with soft", func() { + relatedChanges, err := query.FindCatalogChanges(DefaultContext, query.CatalogChangesSearchRequest{ + CatalogID: X.ID.String(), + Recursive: "upstream", + Soft: true, + }) + + Expect(err).To(BeNil()) + Expect(len(relatedChanges.Changes)).To(Equal(4)) relatedIDs := lo.Map(relatedChanges.Changes, func(rc query.ConfigChangeRow, _ int) string { return rc.ID }) - Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, ZChange.ID, XChange.ID})) + Expect(relatedIDs).To(ConsistOf([]string{XChange.ID, UChange.ID, VChange.ID, CChange.ID})) }) }) @@ -320,10 +389,10 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { Recursive: query.CatalogChangeRecursiveAll, }) Expect(err).To(BeNil()) - Expect(len(response.Changes)).To(Equal(5)) - Expect(response.Total).To(Equal(int64(5))) + Expect(len(response.Changes)).To(Equal(8)) + Expect(response.Total).To(Equal(int64(8))) Expect(response.Summary["diff"]).To(Equal(3)) - Expect(response.Summary["Pulled"]).To(Equal(1)) + Expect(response.Summary["Pulled"]).To(Equal(4)) Expect(response.Summary["RegisterNode"]).To(Equal(1)) }) }) diff --git a/views/006_config_views.sql b/views/006_config_views.sql index a1bc277e..b22ce64b 100644 --- a/views/006_config_views.sql +++ b/views/006_config_views.sql @@ -753,7 +753,8 @@ CREATE OR REPLACE FUNCTION related_changes_recursive ( lookup_id UUID, type_filter TEXT DEFAULT 'downstream', -- 'downstream', 'upstream', 'all', 'none' or '' include_deleted_configs BOOLEAN DEFAULT FALSE, - max_depth INTEGER DEFAULT 5 + max_depth INTEGER DEFAULT 5, + soft BOOLEAN DEFAULT FALSE ) RETURNS TABLE ( id uuid, config_id uuid, @@ -784,6 +785,45 @@ BEGIN FROM config_changes cc LEFT JOIN config_items on config_items.id = cc.config_id WHERE cc.config_id = lookup_id; + + ELSIF type_filter IN ('downstream') THEN + RETURN query + SELECT + cc.id, cc.config_id, config_items.name, config_items.type, config_items.tags, cc.external_created_by, + cc.created_at, cc.severity, cc.change_type, cc.source, cc.summary, cc.created_by, cc.count, cc.first_observed, config_items.agent_id + FROM config_changes cc + LEFT JOIN config_items on config_items.id = cc.config_id + LEFT JOIN + (SELECT config_relationships.config_id, config_relationships.related_id + FROM config_relationships + WHERE relation != 'hard') AS cr + ON (cr.config_id = cc.config_id OR (soft AND cr.related_id = cc.config_id)) + WHERE starts_with(config_items.path, ( + SELECT CASE + WHEN config_items.path = '' THEN config_items.id::text + ELSE CONCAT(config_items.path, '.', config_items.id) + END + FROM config_items WHERE config_items.id = lookup_id + )) OR + (cc.config_id = lookup_id) OR + (soft AND (cr.config_id = lookup_id OR cr.related_id = lookup_id)); + + ELSIF type_filter IN ('upstream') THEN + RETURN query + SELECT + cc.id, cc.config_id, config_items.name, config_items.type, config_items.tags, cc.external_created_by, + cc.created_at, cc.severity, cc.change_type, cc.source, cc.summary, cc.created_by, cc.count, cc.first_observed, config_items.agent_id + FROM config_changes cc + LEFT JOIN config_items on config_items.id = cc.config_id + LEFT JOIN + (SELECT config_relationships.config_id, config_relationships.related_id + FROM config_relationships + WHERE relation != 'hard') AS cr + ON (cr.config_id = cc.config_id OR (soft AND cr.related_id = cc.config_id)) + WHERE cc.config_id IN (SELECT get_recursive_path.id FROM get_recursive_path(lookup_id)) OR + (cc.config_id = lookup_id) OR + (soft AND (cr.config_id = lookup_id OR cr.related_id = lookup_id)); + ELSE RETURN query SELECT @@ -798,7 +838,6 @@ BEGIN lookup_id, CASE WHEN type_filter = 'upstream' THEN 'incoming' - WHEN type_filter = 'downstream' THEN 'outgoing' ELSE type_filter END, max_depth