From ba663ff2b445c92847f2d38afa19e6cd0d8726e0 Mon Sep 17 00:00:00 2001 From: Jay Mundrawala Date: Thu, 1 Feb 2024 10:27:36 -0600 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20resolved=20policy=20root?= =?UTF-8?q?=20pruning=20bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In #1036, code was added to prune nodes that were being used. However, this happens before the frameworks get a say in what happens. This meant it was possible to prune the root reporting job casuing the resolved policy to be broken --- policy/resolver.go | 5 +++++ policy/resolver_test.go | 43 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/policy/resolver.go b/policy/resolver.go index f5269ae0..9dfdd2d2 100644 --- a/policy/resolver.go +++ b/policy/resolver.go @@ -585,6 +585,11 @@ func (s *LocalServices) tryResolve(ctx context.Context, bundleMrn string, assetF reportingJobUUIDs := topologicalSortReportingJobs(collectorJob.ReportingJobs) for _, rjUUID := range reportingJobUUIDs { rj := collectorJob.ReportingJobs[rjUUID] + if rj.QrId == "root" { + // If we prune the root, we're going to get a broken resolved policy. + // For example, the framework code that follows wants to report to it. + continue + } if rj.Type == ReportingJob_POLICY && (len(rj.ChildJobs)+len(rj.Datapoints) == 0) { logCtx.Debug(). Str("policy", bundleMrn). diff --git a/policy/resolver_test.go b/policy/resolver_test.go index afab2680..d9d72557 100644 --- a/policy/resolver_test.go +++ b/policy/resolver_test.go @@ -265,7 +265,7 @@ policies: }) require.NoError(t, err) require.NotNil(t, rp) - require.Len(t, rp.CollectorJob.ReportingJobs, 0) + require.Len(t, rp.CollectorJob.ReportingJobs, 1) } func TestResolve_IgnoredQuery(t *testing.T) { @@ -1364,3 +1364,44 @@ queries: require.Nil(t, qrIdToRj[policyMrn("pack2")]) }) } + +func TestResolve_NeverPruneRoot(t *testing.T) { + b := parseBundle(t, ` +owner_mrn: //test.sth +policies: +- uid: policy1 + groups: + - type: chapter + filters: "false" + checks: + - uid: check1 + +queries: +- uid: check1 + title: check1 + filters: | + true + mql: | + 1 == 1 +`) + + srv := initResolver(t, []*testAsset{ + {asset: "asset1", policies: []string{policyMrn("policy1")}}, + }, []*policy.Bundle{b}) + + rp, err := srv.Resolve(context.Background(), &policy.ResolveReq{ + PolicyMrn: "asset1", + AssetFilters: []*explorer.Mquery{{Mql: "true"}}, + }) + require.NoError(t, err) + require.NotNil(t, rp) + + require.Len(t, rp.CollectorJob.ReportingJobs, 1) + + qrIdToRj := map[string]*policy.ReportingJob{} + for _, rj := range rp.CollectorJob.ReportingJobs { + qrIdToRj[rj.QrId] = rj + } + require.NotNil(t, qrIdToRj["root"]) + +}