Use value ScanResult, add Summary & parseError

cli/cage/audit/aggregator.go

@@ -12,32 +12,19 @@ import (
 type aggregater struct {
 	cves            map[string]CVE
 	cveToContainers map[string]set.Set[string]
-	summaries       map[string][]*ScanResultSummary
+	summaries       map[string][]ScanResultSummary
 }
 
 func NewAggregater() *aggregater {
 	return &aggregater{
 		cves:            make(map[string]CVE),
 		cveToContainers: make(map[string]set.Set[string]),
-		summaries:       make(map[string][]*ScanResultSummary)}
+		summaries:       make(map[string][]ScanResultSummary)}
 }
 
-func (a *aggregater) Add(r *ScanResult) {
+func (a *aggregater) Add(r ScanResult) {
 	container := r.ContainerName
-	if r.Err != nil {
-		a.summaries[container] = append(a.summaries[container], &ScanResultSummary{
-			ContainerName: container,
-			Status:        "ERROR",
-		})
-		return
-	} else if len(r.Cves) == 0 {
-		a.summaries[container] = append(a.summaries[container], &ScanResultSummary{
-			ContainerName: container,
-			Status:        "N/A",
-		})
-		return
-	}
-	summary := summaryScanResult(r)
+	summary := r.Summary()
 	a.summaries[container] = append(a.summaries[container], summary)
 	for _, f := range r.Cves {
 		if _, exists := a.cves[f.Name]; !exists {

cli/cage/audit/aggregator_test.go

@@ -83,58 +83,131 @@ func TestNewAggregater(t *testing.T) {
 }
 
 func TestAggregater_Add(t *testing.T) {
-	tests := []struct {
-		name         string
-		scanResult   *ScanResult
-		wantStatus   string
-		wantCVECount int
-	}{
-		{
-			name: "add result with error",
-			scanResult: &ScanResult{
-				Err: assert.AnError,
+	t.Run("adds single CVE from scan result", func(t *testing.T) {
+		agg := NewAggregater()
+		scanResult := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves: []CVE{
+				{Name: "CVE-2021-1234", Severity: ecrtypes.FindingSeverityCritical},
 			},
-			wantStatus:   "ERROR",
-			wantCVECount: 0,
-		},
-		{
-			name:         "add result with nil findings",
-			scanResult:   &ScanResult{},
-			wantStatus:   "N/A",
-			wantCVECount: 0,
-		},
-		{
-			name: "add result with findings",
-			scanResult: &ScanResult{
-				ImageInfo: ImageInfo{
-					ContainerName: "test-container",
-				},
-				Cves: []CVE{
-					{
-						Name:     "CVE-2021-1234",
-						Severity: ecrtypes.FindingSeverityCritical,
-					},
-					{
-						Name:     "CVE-2021-5678",
-						Severity: ecrtypes.FindingSeverityHigh,
-					},
-				},
+		}
+
+		agg.Add(scanResult)
+
+		assert.Equal(t, 1, len(agg.cves))
+		assert.Equal(t, "CVE-2021-1234", agg.cves["CVE-2021-1234"].Name)
+		assert.Contains(t, agg.cveToContainers["CVE-2021-1234"].Values(), "container1")
+	})
+
+	t.Run("adds multiple CVEs from single scan result", func(t *testing.T) {
+		agg := NewAggregater()
+		scanResult := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves: []CVE{
+				{Name: "CVE-2021-1111", Severity: ecrtypes.FindingSeverityCritical},
+				{Name: "CVE-2021-2222", Severity: ecrtypes.FindingSeverityHigh},
+				{Name: "CVE-2021-3333", Severity: ecrtypes.FindingSeverityMedium},
 			},
-			wantStatus:   "VULNERABLE",
-			wantCVECount: 2,
-		},
-	}
+		}
 
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			agg := NewAggregater()
-			agg.Add(tt.scanResult)
-			assert.Equal(t, 1, len(agg.summaries))
-			assert.Equal(t, tt.wantStatus, agg.summaries[tt.scanResult.ImageInfo.ContainerName][0].Status)
-			assert.Equal(t, tt.wantCVECount, len(agg.cves))
-		})
+		agg.Add(scanResult)
 
-	}
+		assert.Equal(t, 3, len(agg.cves))
+		assert.NotNil(t, agg.cves["CVE-2021-1111"])
+		assert.NotNil(t, agg.cves["CVE-2021-2222"])
+		assert.NotNil(t, agg.cves["CVE-2021-3333"])
+	})
+
+	t.Run("adds same CVE from multiple containers", func(t *testing.T) {
+		agg := NewAggregater()
+		scanResult1 := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves: []CVE{
+				{Name: "CVE-2021-1234", Severity: ecrtypes.FindingSeverityCritical},
+			},
+		}
+		scanResult2 := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container2"},
+			Cves: []CVE{
+				{Name: "CVE-2021-1234", Severity: ecrtypes.FindingSeverityCritical},
+			},
+		}
+
+		agg.Add(scanResult1)
+		agg.Add(scanResult2)
+
+		assert.Equal(t, 1, len(agg.cves))
+		containers := agg.cveToContainers["CVE-2021-1234"].Values()
+		assert.Equal(t, 2, len(containers))
+		assert.Contains(t, containers, "container1")
+		assert.Contains(t, containers, "container2")
+	})
+
+	t.Run("stores scan result summary", func(t *testing.T) {
+		agg := NewAggregater()
+		scanResult := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves:      []CVE{},
+		}
+
+		agg.Add(scanResult)
+
+		assert.Equal(t, 1, len(agg.summaries["container1"]))
+	})
+
+	t.Run("appends multiple summaries for same container", func(t *testing.T) {
+		agg := NewAggregater()
+		scanResult1 := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves:      []CVE{},
+		}
+		scanResult2 := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves:      []CVE{},
+		}
+
+		agg.Add(scanResult1)
+		agg.Add(scanResult2)
+
+		assert.Equal(t, 2, len(agg.summaries["container1"]))
+	})
+
+	t.Run("does not duplicate CVE when added multiple times from same container", func(t *testing.T) {
+		agg := NewAggregater()
+		scanResult1 := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves: []CVE{
+				{Name: "CVE-2021-1234", Severity: ecrtypes.FindingSeverityCritical},
+			},
+		}
+		scanResult2 := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves: []CVE{
+				{Name: "CVE-2021-1234", Severity: ecrtypes.FindingSeverityCritical},
+			},
+		}
+
+		agg.Add(scanResult1)
+		agg.Add(scanResult2)
+
+		assert.Equal(t, 1, len(agg.cves))
+		containers := agg.cveToContainers["CVE-2021-1234"].Values()
+		assert.Equal(t, 1, len(containers))
+		assert.Contains(t, containers, "container1")
+	})
+
+	t.Run("handles empty CVE list", func(t *testing.T) {
+		agg := NewAggregater()
+		scanResult := ScanResult{
+			ImageInfo: ImageInfo{ContainerName: "container1"},
+			Cves:      []CVE{},
+		}
+
+		agg.Add(scanResult)
+
+		assert.Equal(t, 0, len(agg.cves))
+		assert.Equal(t, 1, len(agg.summaries["container1"]))
+	})
 }
 
 func TestAggregater_SummarizeTotal(t *testing.T) {

cli/cage/audit/command.go

@@ -44,7 +44,7 @@ func (a *command) Run(ctx context.Context) (err error) {
 	return nil
 }
 
-func (a *command) doScan(ctx context.Context) (results []*ScanResult, err error) {
+func (a *command) doScan(ctx context.Context) (results []ScanResult, err error) {
 	l := a.di.Get(key.Printer).(logger.Printer)
 	t := a.di.Get(key.Time).(types.Time)
 	defer l.PrintErrf("\r")

cli/cage/audit/command_test.go

@@ -52,7 +52,7 @@ func TestAuditCommandRun(t *testing.T) {
 			b.Set(key.Time, test.NewNeverTimer())
 		})
 
-		var results []*audit.ScanResult
+		var results []audit.ScanResult
 		gomock.InOrder(
 			mockScanner.EXPECT().Scan(t.Context(), "cluster", "service").Return(results, nil),
 		)
@@ -82,7 +82,7 @@ func TestAuditCommandRun(t *testing.T) {
 			b.Set(key.Time, test.NewNeverTimer())
 		})
 
-		var results []*audit.ScanResult
+		var results []audit.ScanResult
 		gomock.InOrder(
 			mockScanner.EXPECT().Scan(t.Context(), "cluster", "service").Return(results, nil),
 		)

cli/cage/audit/printer.go

@@ -19,8 +19,8 @@ type printer struct {
 }
 
 type Printer interface {
-	Print(result []*ScanResult)
-	PrintJSON(metadata Target, scanResults []*ScanResult)
+	Print(result []ScanResult)
+	PrintJSON(metadata Target, scanResults []ScanResult)
 }
 
 var _ Printer = (*printer)(nil)
@@ -33,7 +33,7 @@ func NewPrinter(di *di.D, noColor, logDetail bool) *printer {
 	}
 }
 
-func (p *printer) Print(scanResults []*ScanResult) {
+func (p *printer) Print(scanResults []ScanResult) {
 	l := p.di.Get(key.Printer).(logger.Printer)
 	containerMax, imageMax := MaxHeaderWidth(scanResults)
 	// |container|status|critical|high|medium|low|info|image|
@@ -90,7 +90,7 @@ func (p *printer) Print(scanResults []*ScanResult) {
 	)
 }
 
-func (p *printer) PrintJSON(metadata Target, scanResults []*ScanResult) {
+func (p *printer) PrintJSON(metadata Target, scanResults []ScanResult) {
 	l := p.di.Get(key.Printer).(logger.Printer)
 	t := p.di.Get(key.Time).(types.Time)
 	agg := NewAggregater()
@@ -135,7 +135,7 @@ func (i *ImageInfo) formatImageLabel() string {
 	return fmt.Sprintf("%s/%s:%s", i.Registry, i.Repository, i.Tag)
 }
 
-func MaxHeaderWidth(imageInfos []*ScanResult) (int, int) {
+func MaxHeaderWidth(imageInfos []ScanResult) (int, int) {
 	containerMax := len("CONTAINER")
 	imageMax := len("IMAGE")
 	for _, info := range imageInfos {

cli/cage/audit/printer_test.go

@@ -14,8 +14,8 @@ import (
 )
 
 func makeScanResult(
-	list ...ecrtypes.FindingSeverity) []*ScanResult {
-	return []*ScanResult{
+	list ...ecrtypes.FindingSeverity) []ScanResult {
+	return []ScanResult{
 		{
 			ImageInfo: ImageInfo{
 				ContainerName: "test-container",
@@ -344,7 +344,7 @@ func TestPrinter_PrintJSON(t *testing.T) {
 			Cluster: "test-cluster",
 			Service: "test-service",
 		}
-		result := []*ScanResult{}
+		result := []ScanResult{}
 
 		printer.PrintJSON(metadata, result)
 

cli/cage/audit/scanner.go

@@ -2,8 +2,10 @@ package audit
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
+	smithy "github.com/aws/smithy-go"
 	"github.com/loilo-inc/canarycage/awsiface"
 )
 
@@ -13,7 +15,7 @@ type scanner struct {
 }
 
 type Scanner interface {
-	Scan(ctx context.Context, cluster string, service string) ([]*ScanResult, error)
+	Scan(ctx context.Context, cluster string, service string) ([]ScanResult, error)
 }
 
 var _ Scanner = (*scanner)(nil)
@@ -26,37 +28,46 @@ func (s *scanner) Scan(
 	ctx context.Context,
 	cluster string,
 	service string,
-) (results []*ScanResult, err error) {
+) (results []ScanResult, err error) {
 	ecsTool := newEcsTool(s.ecs)
 	ecrTool := newEcrTool(s.ecr)
 	var imageInfos []ImageInfo
 	if imageInfos, err = ecsTool.GetServiceImageInfos(ctx, cluster, service); err != nil {
 		return nil, err
 	}
-	findingsList := make([]*ScanResult, len(imageInfos))
+	findingsList := make([]ScanResult, len(imageInfos))
 	for i, info := range imageInfos {
 		if info.IsECRImage() {
 			findingsList[i] = scanImage(ctx, ecrTool, info)
 		} else {
-			findingsList[i] = &ScanResult{ImageInfo: info, Err: ErrNonEcrImage}
+			findingsList[i] = ScanResult{ImageInfo: info, Err: ErrNonEcrImage}
 		}
 	}
 	return findingsList, nil
 }
 
 var ErrNonEcrImage = fmt.Errorf("non-ECR image")
+var ErrScanNotFound = fmt.Errorf("scan not found")
 
-func scanImage(ctx context.Context, ecrTool EcrTool, info ImageInfo) *ScanResult {
+func scanImage(ctx context.Context, ecrTool EcrTool, info ImageInfo) ScanResult {
 	if imageID, err := ecrTool.GetActualImageIdentifier(ctx, &info); err != nil {
-		return &ScanResult{ImageInfo: info, Err: err}
+		return ScanResult{ImageInfo: info, Err: err}
 	} else if findings, err := ecrTool.GetImageScanFindings(ctx, &info, imageID); err != nil {
-		return &ScanResult{ImageInfo: info, Err: err}
+		return ScanResult{ImageInfo: info, Err: parseError(err)}
 	} else {
 		var cves []CVE
 		for _, f := range findings.Findings {
 			cve := findingToCVE(f)
 			cves = append(cves, cve)
 		}
-		return &ScanResult{ImageInfo: info, Cves: cves}
+		return ScanResult{ImageInfo: info, Cves: cves}
 	}
 }
+
+func parseError(err error) error {
+	var awserr smithy.APIError
+	if errors.As(err, &awserr) && awserr.ErrorCode() == "ScanNotFoundException" {
+		return ErrScanNotFound
+	}
+	return err
+}