From 4d065d3663b9fa680e9943d2edd2615f6076cd80 Mon Sep 17 00:00:00 2001 From: Yash Mehrotra Date: Fri, 1 Nov 2024 10:11:30 +0530 Subject: [PATCH] chore: document soft relation problems --- query/config_changes.go | 5 +- tests/config_changes_test.go | 92 +++++++++++++++++++++++++++++------- views/006_config_views.sql | 16 +++++-- 3 files changed, 92 insertions(+), 21 deletions(-) diff --git a/query/config_changes.go b/query/config_changes.go index 932caa88..4da16a6c 100644 --- a/query/config_changes.go +++ b/query/config_changes.go @@ -58,7 +58,10 @@ type CatalogChangesSearchRequest struct { // upstream | downstream | both Recursive string `query:"recursive" json:"recursive"` - // FIXME: Soft only works when Recursive=both and not with upstream/downstream + // 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 diff --git a/tests/config_changes_test.go b/tests/config_changes_test.go index c07d2929..7bcb9e2c 100644 --- a/tests/config_changes_test.go +++ b/tests/config_changes_test.go @@ -18,13 +18,13 @@ 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 ( @@ -47,7 +47,7 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { {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"}, + {ConfigID: Z.ID.String(), RelatedID: D.ID.String(), Relation: "test-changes-ZD"}, } // Create changes for each config @@ -123,10 +123,10 @@ var _ = ginkgo.Describe("Config changes recursive", ginkgo.Ordered, func() { relatedChanges, err := findChanges(X.ID, "all", false) Expect(err).To(BeNil()) - Expect(len(relatedChanges.Changes)).To(Equal(5)) + 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, CChange.ID})) + Expect(relatedIDs).To(ConsistOf([]string{UChange.ID, VChange.ID, XChange.ID, ZChange.ID, CChange.ID, DChange.ID})) }) }) @@ -141,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})) }) }) @@ -161,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(5)) + 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, CChange.ID})) + Expect(relatedIDs).To(ConsistOf([]string{XChange.ID, UChange.ID, VChange.ID, CChange.ID})) }) }) @@ -329,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(7)) - Expect(response.Total).To(Equal(int64(7))) + 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(3)) + 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 b68b7d99..b22ce64b 100644 --- a/views/006_config_views.sql +++ b/views/006_config_views.sql @@ -793,7 +793,11 @@ BEGIN 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 FROM config_relationships WHERE relation != 'hard') AS cr ON cr.config_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 @@ -802,7 +806,7 @@ BEGIN FROM config_items WHERE config_items.id = lookup_id )) OR (cc.config_id = lookup_id) OR - (soft AND cr.config_id = lookup_id ); + (soft AND (cr.config_id = lookup_id OR cr.related_id = lookup_id)); ELSIF type_filter IN ('upstream') THEN RETURN query @@ -811,10 +815,14 @@ BEGIN 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 FROM config_relationships WHERE relation != 'hard') AS cr ON cr.config_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 ); + (soft AND (cr.config_id = lookup_id OR cr.related_id = lookup_id)); ELSE RETURN query