Skip to content

Commit

Permalink
🐛 ensure there are no duplicates in the resolved policy notify structure
Browse files Browse the repository at this point in the history
```
      "YYBvk3VCj7E=": {
        "checksum": "0dp9+bRqUwA=",
        "qr_id": "//policy.api.mondoo.app/controls/cisc-4-4",
        "uuid": "YYBvk3VCj7E=",
        "notify": [
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "cTocL2OpWZI=",
          "jQvxGFEgdOc=",
          "XYqW5cyrngA=",
          "6GfQ1zmPQWU=",
          "zpfO4TnlynY="
        ],
```

I was seeing things like this. I believe this was a function of each
control map contributing an edge in notify
  • Loading branch information
jaym committed Aug 14, 2023
1 parent 0acaff8 commit 6f14f35
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 23 deletions.
37 changes: 21 additions & 16 deletions policy/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,25 @@ type ResolvedFrameworkNode struct {
Type ResolvedFrameworkNodeType
}

type ResolvedFrameworkReferenceSet map[string]struct{}

func (r ResolvedFrameworkReferenceSet) Add(mrn string) {
r[mrn] = struct{}{}
}

type ResolvedFramework struct {
Mrn string
GraphContentChecksum string
// ReportTargets tracks which checks/policies/controls report into which controls
// and frameworks.
// E.g. ReportTarget[check123] = [controlA, controlB]
// E.g. ReportTarget[controlA] = [frameworkX]
ReportTargets map[string][]string
ReportTargets map[string]ResolvedFrameworkReferenceSet
// ReportSources tracks all the sources that a control or framework pulls
// data from, i.e. all the checks/policies/controls that provide its data.
// E.g. ReportSources[controlA] = [check123, check45]
// E.g. ReportSources[frameworkX] = [controlA, ...]
ReportSources map[string][]string
ReportSources map[string]ResolvedFrameworkReferenceSet
Nodes map[string]ResolvedFrameworkNode
}

Expand Down Expand Up @@ -503,8 +509,8 @@ func (c *ControlMap) refreshMRNs(ownerMRN string, cache *bundleCache) error {
func ResolveFramework(mrn string, frameworks map[string]*Framework) *ResolvedFramework {
res := &ResolvedFramework{
Mrn: mrn,
ReportTargets: map[string][]string{},
ReportSources: map[string][]string{},
ReportTargets: map[string]ResolvedFrameworkReferenceSet{},
ReportSources: map[string]ResolvedFrameworkReferenceSet{},
Nodes: map[string]ResolvedFrameworkNode{},
}

Expand Down Expand Up @@ -585,19 +591,16 @@ func (r *ResolvedFramework) addControl(control *ControlMap) {
func (r *ResolvedFramework) addReportLink(parent, child ResolvedFrameworkNode) {
r.Nodes[parent.Mrn] = parent
r.Nodes[child.Mrn] = child
existing, ok := r.ReportTargets[child.Mrn]
if !ok {
r.ReportTargets[child.Mrn] = []string{parent.Mrn}
} else {
r.ReportTargets[child.Mrn] = append(existing, parent.Mrn)
}

existing, ok = r.ReportSources[parent.Mrn]
if !ok {
r.ReportSources[parent.Mrn] = []string{child.Mrn}
} else {
r.ReportSources[parent.Mrn] = append(existing, child.Mrn)
if r.ReportTargets[child.Mrn] == nil {
r.ReportTargets[child.Mrn] = ResolvedFrameworkReferenceSet{}
}
if r.ReportSources[parent.Mrn] == nil {
r.ReportSources[parent.Mrn] = ResolvedFrameworkReferenceSet{}
}

r.ReportTargets[child.Mrn].Add(parent.Mrn)
r.ReportSources[parent.Mrn].Add(child.Mrn)
}

func (r *ResolvedFramework) TopologicalSort() []string {
Expand Down Expand Up @@ -631,8 +634,10 @@ func (r *ResolvedFramework) visit(node string, visited map[string]struct{}, sort
return
}
visited[node] = struct{}{}
for _, child := range r.ReportTargets[node] {
for child := range r.ReportTargets[node] {
r.visit(child, visited, sorted)

}

*sorted = append(*sorted, node)
}
4 changes: 2 additions & 2 deletions policy/frameworks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (

func TestResolvedFrameworkTopologicalSort(t *testing.T) {
framework := &ResolvedFramework{
ReportTargets: map[string][]string{},
ReportSources: map[string][]string{},
ReportTargets: map[string]ResolvedFrameworkReferenceSet{},
ReportSources: map[string]ResolvedFrameworkReferenceSet{},
Nodes: map[string]ResolvedFrameworkNode{},
}

Expand Down
9 changes: 4 additions & 5 deletions policy/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -1425,7 +1425,7 @@ func ensureControlJob(cache *frameworkResolverCache, jobs map[string]*ReportingJ
jobs[uuid] = controlJob

parents := framework.ReportTargets[controlMrn]
for _, parentMrn := range parents {
for parentMrn := range parents {
parentUuid := cache.relativeChecksum(parentMrn)

frameworkJob, ok := cache.frameworkJobsByMrn[parentMrn]
Expand Down Expand Up @@ -1455,7 +1455,7 @@ func (s *LocalServices) jobsToFrameworks(cache *frameworkResolverCache, resolved
}

func (s *LocalServices) jobsToFrameworksInner(cache *frameworkResolverCache, resolvedFramework *ResolvedFramework, job *CollectorJob, frameworkMrn string, parent *ReportingJob) error {
for _, source := range resolvedFramework.ReportSources[frameworkMrn] {
for source := range resolvedFramework.ReportSources[frameworkMrn] {
if childFramework, ok := cache.bundleMap.Frameworks[source]; ok {
var reportingJob *ReportingJob
if found, ok := cache.frameworkJobsByMrn[childFramework.Mrn]; ok {
Expand Down Expand Up @@ -1577,7 +1577,7 @@ func (s *LocalServices) jobsToControls(cache *frameworkResolverCache, framework

// Avoid adding controls which don't have any active children
shouldAdd := false
for _, child := range framework.ReportSources[mrn] {
for child := range framework.ReportSources[mrn] {
if _, ok := nuJobs[cache.relativeChecksum(child)]; ok {
shouldAdd = true
break
Expand All @@ -1589,14 +1589,13 @@ func (s *LocalServices) jobsToControls(cache *frameworkResolverCache, framework
}

controlJob := ensureControlJob(cache, nuJobs, mrn, framework, frameworkGroupByControlMrn)
// addedControlJobs[mrn] = controlJob
curJob = controlJob
case ResolvedFrameworkNodeTypeFramework:
curJob = job.ReportingJobs[cache.relativeChecksum(mrn)]
}

// Ensure that child jobs notify their parents
for _, child := range framework.ReportSources[mrn] {
for child := range framework.ReportSources[mrn] {
childJob, ok := nuJobs[cache.relativeChecksum(child)]
if !ok {
continue
Expand Down
30 changes: 30 additions & 0 deletions policy/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,8 @@ frameworks:
title: control3
- uid: control4
title: control4
- uid: control5
title: control5
- uid: framework2
name: framework2
groups:
Expand Down Expand Up @@ -394,6 +396,18 @@ framework_maps:
- uid: control4
controls:
- uid: control1
- uid: framework-map2
framework_owner:
uid: framework1
policy_dependencies:
- uid: policy1
controls:
- uid: control4
controls:
- uid: control1
- uid: control5
controls:
- uid: control1
`

t.Run("resolve with correct filters", func(t *testing.T) {
Expand Down Expand Up @@ -421,6 +435,11 @@ framework_maps:
require.NoError(t, err)
require.NotNil(t, rp)

// Check that there are no duplicates in the reporting job's notify list
for _, rj := range rp.CollectorJob.ReportingJobs {
requireUnique(t, rj.Notify)
}

require.Len(t, rp.ExecutionJob.Queries, 3)

rjTester := frameworkReportingJobTester{
Expand Down Expand Up @@ -450,6 +469,7 @@ framework_maps:
rjTester.requireReportsTo(controlMrn("control1"), controlMrn("control4"))
rjTester.requireReportsTo(controlMrn("control2"), frameworkMrn("framework1"))
rjTester.requireReportsTo(controlMrn("control4"), frameworkMrn("framework1"))
rjTester.requireReportsTo(controlMrn("control5"), frameworkMrn("framework1"))
rjTester.requireReportsTo(frameworkMrn("framework1"), frameworkMrn("parent-framework"))
rjTester.requireReportsTo(frameworkMrn("parent-framework"), "root")

Expand Down Expand Up @@ -796,3 +816,13 @@ framework_maps:
require.Len(t, frameworkJob.ChildJobs, 3)
})
}

func requireUnique(t *testing.T, items []string) {
seen := make(map[string]bool)
for _, item := range items {
if seen[item] {
t.Errorf("duplicate item found: %s", item)
}
seen[item] = true
}
}

0 comments on commit 6f14f35

Please sign in to comment.