doop-api: reduce allocations during AggregateReports()

doop-api/aggregate.go

@@ -19,7 +19,11 @@
 
 package main
 
-import "github.com/sapcc/gatekeeper-addons/internal/doop"
+import (
+	"slices"
+
+	"github.com/sapcc/gatekeeper-addons/internal/doop"
+)
 
 // AggregateReports assembles a set of individual reports into an AggregatedReport.
 func AggregateReports(reports map[string]doop.Report, f FilterSet) doop.AggregatedReport {
@@ -46,11 +50,11 @@ func visitClusterReport(target *doop.AggregatedReport, clusterReport doop.Report
 
 	target.ClusterIdentities[clusterName] = clusterReport.ClusterIdentity
 	for _, tr := range clusterReport.Templates {
-		visitTemplateReport(target, tr, clusterName, f)
+		visitTemplateReport(target, tr, f)
 	}
 }
 
-func visitTemplateReport(target *doop.AggregatedReport, tr doop.ReportForTemplate, clusterName string, f FilterSet) {
+func visitTemplateReport(target *doop.AggregatedReport, tr doop.ReportForTemplate, f FilterSet) {
 	if !f.MatchTemplateKind(tr.Kind) {
 		return
 	}
@@ -59,7 +63,7 @@ func visitTemplateReport(target *doop.AggregatedReport, tr doop.ReportForTemplat
 	for idx, candidate := range target.Templates {
 		if candidate.Kind == tr.Kind {
 			for _, cr := range tr.Constraints {
-				visitConstraintReport(&target.Templates[idx], cr, clusterName, f)
+				visitConstraintReport(&target.Templates[idx], cr, f)
 			}
 			return
 		}
@@ -70,14 +74,14 @@ func visitTemplateReport(target *doop.AggregatedReport, tr doop.ReportForTemplat
 		Kind: tr.Kind,
 	}
 	for _, cr := range tr.Constraints {
-		visitConstraintReport(&newReport, cr, clusterName, f)
+		visitConstraintReport(&newReport, cr, f)
 	}
 	if len(newReport.Constraints) > 0 {
 		target.Templates = append(target.Templates, newReport)
 	}
 }
 
-func visitConstraintReport(target *doop.ReportForTemplate, cr doop.ReportForConstraint, clusterName string, f FilterSet) {
+func visitConstraintReport(target *doop.ReportForTemplate, cr doop.ReportForConstraint, f FilterSet) {
 	if !f.MatchConstraintName(cr.Name) {
 		return
 	}
@@ -93,7 +97,7 @@ func visitConstraintReport(target *doop.ReportForTemplate, cr doop.ReportForCons
 	for idx, candidate := range target.Constraints {
 		if candidate.Name == cr.Name && candidate.Metadata == metadata {
 			for _, vg := range cr.ViolationGroups {
-				visitViolationGroup(&target.Constraints[idx], vg, clusterName, f)
+				visitViolationGroup(&target.Constraints[idx], vg, f)
 			}
 			return
 		}
@@ -105,39 +109,29 @@ func visitConstraintReport(target *doop.ReportForTemplate, cr doop.ReportForCons
 		Metadata: metadata,
 	}
 	for _, vg := range cr.ViolationGroups {
-		visitViolationGroup(&newReport, vg, clusterName, f)
+		visitViolationGroup(&newReport, vg, f)
 	}
 	if len(newReport.ViolationGroups) > 0 {
 		target.Constraints = append(target.Constraints, newReport)
 	}
 }
 
-func visitViolationGroup(target *doop.ReportForConstraint, vg doop.ViolationGroup, clusterName string, f FilterSet) {
+func visitViolationGroup(target *doop.ReportForConstraint, vg doop.ViolationGroup, f FilterSet) {
 	if !f.MatchObjectIdentity(vg.Pattern.ObjectIdentity) {
 		return
 	}
 
 	//try to merge into existing ViolationGroup
 	for idx, candidate := range target.ViolationGroups {
 		if candidate.Pattern.IsEqualTo(vg.Pattern) {
-			for _, instance := range vg.Instances {
-				clonedInstance := instance.Cloned()
-				clonedInstance.ClusterName = clusterName
-				target.ViolationGroups[idx].Instances = append(target.ViolationGroups[idx].Instances, clonedInstance)
-			}
+			target.ViolationGroups[idx].Instances = append(target.ViolationGroups[idx].Instances, vg.Instances...)
 			return
 		}
 	}
 
 	//otherwise start a new ViolationGroup
-	newGroup := doop.ViolationGroup{
+	target.ViolationGroups = append(target.ViolationGroups, doop.ViolationGroup{
 		Pattern:   vg.Pattern.Cloned(),
-		Instances: make([]doop.Violation, len(vg.Instances)),
-	}
-	for idx, instance := range vg.Instances {
-		clonedInstance := instance.Cloned()
-		clonedInstance.ClusterName = clusterName
-		newGroup.Instances[idx] = clonedInstance
-	}
-	target.ViolationGroups = append(target.ViolationGroups, newGroup)
+		Instances: slices.Clone(vg.Instances),
+	})
 }

doop-api/aggregate_test.go

@@ -32,7 +32,7 @@ import (
 
 func TestAggregateOfOneCluster(t *testing.T) {
 	inputSet := map[string]doop.Report{
-		"cluster1": mustParseJSON[doop.Report](t, "fixtures/input-cluster1.json"),
+		"cluster1": mustParseJSON[doop.Report](t, "fixtures/input-cluster1.json").SetClusterName("cluster1"),
 	}
 
 	//test that aggregating results from only one cluster barely changes the input if no filter is applied
@@ -78,13 +78,13 @@ func TestAggregateOfOneCluster(t *testing.T) {
 func TestAggregateOfTwoClusters(t *testing.T) {
 	//Each of these cluster reports has exactly one violation.
 	inputSet := map[string]doop.Report{
-		"cluster1": mustParseJSON[doop.Report](t, "fixtures/input-cluster1.json"),
+		"cluster1": mustParseJSON[doop.Report](t, "fixtures/input-cluster1.json").SetClusterName("cluster1"),
 		//this one can merge with cluster 1 on same violation group
-		"cluster2": mustParseJSON[doop.Report](t, "fixtures/input-cluster2.json"),
+		"cluster2": mustParseJSON[doop.Report](t, "fixtures/input-cluster2.json").SetClusterName("cluster2"),
 		//this one can merge with cluster 1 on different violation group, but same constraint
-		"cluster3": mustParseJSON[doop.Report](t, "fixtures/input-cluster3.json"),
+		"cluster3": mustParseJSON[doop.Report](t, "fixtures/input-cluster3.json").SetClusterName("cluster3"),
 		//this one can merge with cluster 1 on different constraint, but same template
-		"cluster4": mustParseJSON[doop.Report](t, "fixtures/input-cluster4.json"),
+		"cluster4": mustParseJSON[doop.Report](t, "fixtures/input-cluster4.json").SetClusterName("cluster4"),
 	}
 
 	//test merging of structures on all levels of the report

doop-api/downloader.go

@@ -81,6 +81,7 @@ func (d *Downloader) GetReports() (map[string]doop.Report, error) {
 			if err != nil {
 				return nil, fmt.Errorf("cannot decode report for %s: %w", name, err)
 			}
+			payload.SetClusterName(name)
 			objState.Payload = payload
 		}
 

internal/doop/report.go

@@ -30,6 +30,23 @@ type Report struct {
 	Templates       []ReportForTemplate `json:"templates"`
 }
 
+// SetClusterName sets the ClusterName field on all Violation objects in this Report.
+// This is used at report loading time to prepare the report for aggregation.
+// The self-return is used to shorten setup code in unit tests.
+func (r Report) SetClusterName(clusterName string) Report {
+	for _, t := range r.Templates {
+		for _, c := range t.Constraints {
+			for _, vg := range c.ViolationGroups {
+				for idx, v := range vg.Instances {
+					v.ClusterName = clusterName
+					vg.Instances[idx] = v
+				}
+			}
+		}
+	}
+	return r
+}
+
 // ReportForTemplate appears in type Report.
 type ReportForTemplate struct {
 	Kind        string                `json:"kind"`

internal/doop/violation.go

@@ -40,6 +40,7 @@ type Violation struct {
 	Message        string            `json:"message,omitempty"`
 	ObjectIdentity map[string]string `json:"object_identity,omitempty"`
 	// This field is only set when this Violation appears as a ViolationGroup instance inside an AggregatedReport.
+	// It is written by Report.SetClusterName() at report loading time.
 	ClusterName string `json:"cluster,omitempty"`
 }