From 2330f3b8145dca931fb6974586c65e6de0de01d0 Mon Sep 17 00:00:00 2001 From: Bradley Jones Date: Thu, 2 Nov 2023 22:03:25 +0000 Subject: [PATCH] fix: ensure async flow waits for image analysis and handles anchore 404 Change the ordering of the async logic to ensure that scan is created and image analysis is complete before starting the build of the vuln report. Add a lock via mutex to the result store to prevent concurrent read/writes to the map. Signed-off-by: Bradley Jones --- .vscode/launch.json | 3 +- pkg/adapter/anchore/adapter.go | 60 ++++++++---- pkg/adapter/anchore/client/client.go | 41 +++++++- pkg/adapter/anchore/result_store.go | 116 +++++++++++++---------- pkg/adapter/anchore/result_store_test.go | 10 +- 5 files changed, 154 insertions(+), 76 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index dfc88b3..192b1c0 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -14,7 +14,8 @@ "from": "${workspaceFolder}", "to": "/go/src/github.com/anchore/harbor-scanner-adapter" } - ] + ], + "program": "${workspaceFolder}/cmd/harbor-scanner-adapter/main.go" } ] } diff --git a/pkg/adapter/anchore/adapter.go b/pkg/adapter/anchore/adapter.go index aa76eff..83ab0a1 100644 --- a/pkg/adapter/anchore/adapter.go +++ b/pkg/adapter/anchore/adapter.go @@ -273,12 +273,22 @@ func (s *HarborScannerAdapter) GetHarborVulnerabilityReport( Info("no result found in store checking image is analyzed in Anchore Enterprise") } + if !result.ScanCreated { + log.WithFields(log.Fields{"scanId": scanID, "scanCreated": result.ScanCreated, "resultIsComplete": result.IsComplete, "resultError": result.Error}). + Debug("scan not created yet") + if result.Error != nil { + return nil, result.Error + } + return nil, fmt.Errorf("create scan not ready") + } + if !result.AnalysisComplete { log.WithFields(log.Fields{"scanId": scanID}).Debug("checking image analysis state in Anchore Enterprise") imageAnalsisFn := func() (bool, error) { return IsImageAnalysed(imageDigest, scanID, &s.Configuration.AnchoreClientConfig) } resultStore.RequestAnalysisStatus(scanID, imageAnalsisFn) + return nil, fmt.Errorf("result not ready") } if result.ReportBuildInProgress { @@ -289,17 +299,10 @@ func (s *HarborScannerAdapter) GetHarborVulnerabilityReport( } return nil, fmt.Errorf("result not ready") } - if !result.ScanCreated { - log.WithFields(log.Fields{"scanId": scanID, "scanCreated": result.ScanCreated, "resultIsComplete": result.IsComplete, "resultError": result.Error}). - Debug("scan not created yet") - if result.Error != nil { - return nil, result.Error - } - return nil, fmt.Errorf("create scan not ready") - } fn := func() (*harbor.VulnerabilityReport, error) { rep, err := BuildHarborVulnerabilityReport( + scanID, imageRepository, imageDigest, includeDescriptions, @@ -314,7 +317,7 @@ func (s *HarborScannerAdapter) GetHarborVulnerabilityReport( return &rep, err } - log.WithField("scanId", scanID).Info("no existing result store entry found for scanId, creating a new one") + log.WithField("scanId", scanID).Info("begin building vulnerability report") requestResult := resultStore.RequestResult(scanID, fn) if requestResult.Error != nil { return nil, requestResult.Error @@ -369,7 +372,7 @@ func GetImageState(imageDigest string, clientConfig *client.Config) (ImageState, } log.WithField("imageDigest", imageDigest).Debug("no report in cache, generating") - img, err := client.GetImage(clientConfig, imageDigest) + img, err := client.GetImage(clientConfig, imageDigest, 0) if err != nil { return NotFound, err } @@ -394,6 +397,7 @@ func GetImageState(imageDigest string, clientConfig *client.Config) (ImageState, // BuildHarborVulnerabilityReport Construct the harbor-formatted vulnerability report from an analyzed image in Anchore func BuildHarborVulnerabilityReport( + scanID string, imageRepository string, imageDigest string, includeDescriptions bool, @@ -407,7 +411,7 @@ func BuildHarborVulnerabilityReport( Debug("getting harbor vulnerability report") start := time.Now() - anchoreVulnResponse, err := GetAnchoreVulnReport(imageDigest, clientConfig, filterVendorIgnoredVulns) + anchoreVulnResponse, err := GetAnchoreVulnReport(scanID, imageDigest, clientConfig, filterVendorIgnoredVulns) if err != nil { log.WithFields(log.Fields{"repository": imageRepository, "imageDigest": imageDigest}). Error("error from vulnerability report api call to Anchore") @@ -480,14 +484,17 @@ func BuildHarborVulnerabilityReport( } func GetAnchoreVulnReport( + scanID string, digest string, clientConfig *client.Config, filterVendorIgnoredVulns bool, ) (anchore.ImageVulnerabilityReport, error) { - report, err := client.GetImageVulnerabilities(clientConfig, digest, filterVendorIgnoredVulns) + report, err := client.GetImageVulnerabilities(clientConfig, digest, filterVendorIgnoredVulns, 0) if err == nil { - log.WithField("imageDigest", digest).Debug("caching result report") + log.WithFields(log.Fields{"scanID": scanID, "imageDigest": digest}).Debug("caching result report") ReportCache.Add(digest, report) + } else { + log.WithFields(log.Fields{"scanID": scanID, "imageDigest": digest, "error": err}).Debug("Error getting image vulnerabilities") } return report, err @@ -504,22 +511,34 @@ func (s *HarborScannerAdapter) GetRawVulnerabilityReport(scanID string) (harbor. return harbor.VulnerabilityReport{}, err } - rawScanID := fmt.Sprintf("%s-raw", scanID) // Used to store just the raw report results in the result store - result, _ := resultStore.PopResult(rawScanID) + rawScanID := fmt.Sprintf("%s-raw", scanID) // Used to store just the raw report results in the rawResult store + rawResult, _ := resultStore.PopResult(rawScanID) + result := resultStore.GetResult(scanID) - if !result.AnalysisComplete { + // Check Scan has been created for the non-report report. This ensures the image is in Anchore Enterprise and submitted for analysis. + if !rawResult.ScanCreated && !result.ScanCreated { + log.WithFields(log.Fields{"scanId": rawScanID, "scanCreated": result.ScanCreated, "resultIsComplete": result.IsComplete, "resultError": result.Error}). + Debug("scan not created yet") + if rawResult.Error != nil { + return nil, result.Error + } + return nil, fmt.Errorf("create scan not ready") + } + + if !rawResult.AnalysisComplete { log.WithFields(log.Fields{"scanId": scanID}).Debug("checking image analysis state in Anchore Enterprise") imageAnalsisFn := func() (bool, error) { return IsImageAnalysed(digest, scanID, &s.Configuration.AnchoreClientConfig) } resultStore.RequestAnalysisStatus(rawScanID, imageAnalsisFn) + return nil, fmt.Errorf("result not ready") } - if result.ReportBuildInProgress { - log.WithFields(log.Fields{"scanId": scanID, "resultIsComplete": result.IsComplete, "resultError": result.Error}). + if rawResult.ReportBuildInProgress { + log.WithFields(log.Fields{"scanId": scanID, "resultIsComplete": rawResult.IsComplete, "resultError": rawResult.Error}). Debug("checked result store for scan Id result") - if result.IsComplete { - return result.RawResult, result.Error + if rawResult.IsComplete { + return rawResult.RawResult, rawResult.Error } return nil, fmt.Errorf("result not ready") } @@ -528,6 +547,7 @@ func (s *HarborScannerAdapter) GetRawVulnerabilityReport(scanID string) (harbor. log.WithFields(log.Fields{"repository": repository, "imageDigest": digest, "scanId": scanID}). Info("Getting raw Anchore-formatted vulnerability report") rep, err := GetAnchoreVulnReport( + scanID, digest, &s.Configuration.AnchoreClientConfig, s.Configuration.FullVulnerabilityDescriptions, diff --git a/pkg/adapter/anchore/client/client.go b/pkg/adapter/anchore/client/client.go index 500bf5c..200877b 100644 --- a/pkg/adapter/anchore/client/client.go +++ b/pkg/adapter/anchore/client/client.go @@ -303,6 +303,7 @@ func GetImageVulnerabilities( clientConfiguration *Config, digest string, filterIgnored bool, + retryCount int, ) (anchore.ImageVulnerabilityReport, error) { log.WithFields(log.Fields{"digest": digest, "filterIgnored": filterIgnored}).Debug("retrieving scan result for image") @@ -329,6 +330,7 @@ func GetImageVulnerabilities( if err != nil { return anchore.ImageVulnerabilityReport(imageVulnerabilityReportV1), err } + log.Debug("returning v1 image vulnerability report") return anchore.ImageVulnerabilityReport(imageVulnerabilityReportV1), nil } err := json.Unmarshal(body, &imageVulnerabilityReport) @@ -337,10 +339,27 @@ func GetImageVulnerabilities( } return imageVulnerabilityReport, nil } + if resp.StatusCode == 404 { + log.WithFields(log.Fields{"digest": digest}). + Debug("Received 404 getting Image vulnerabilities from Anchore, image not found in Anchore") + + // TODO Make the retry count configurable and backoff retries + // Anchore returns 404 if the image is not found, retry up to 5 times + // This is to handle the case where the image is still being submitted for analysis + // and the image is not yet available in the Anchore DB + if retryCount < 5 { + log.WithFields(log.Fields{"digest": digest, "retryCount": retryCount}). + Debug("retrying get image vulnerabilities") + time.Sleep(5 * time.Second) + return GetImageVulnerabilities(clientConfiguration, digest, filterIgnored, retryCount+1) + } + + return imageVulnerabilityReport, fmt.Errorf("not found") + } return imageVulnerabilityReport, fmt.Errorf("error response from anchore api") } -func GetImage(clientConfiguration *Config, digest string) (anchore.Image, error) { +func GetImage(clientConfiguration *Config, digest string, retryCount int) (anchore.Image, error) { log.WithFields(log.Fields{"digest": digest}).Debug("retrieving anchore state for image") var image anchore.Image @@ -353,12 +372,30 @@ func GetImage(clientConfiguration *Config, digest string) (anchore.Image, error) log.WithFields(log.Fields{"method": "get", "url": reqURL}).Debug("sending request to anchore api") // call API get the full report until "analysis_status" = "analyzed" - _, body, errs := sendRequest(clientConfiguration, request.Get(reqURL)) + resp, body, errs := sendRequest(clientConfiguration, request.Get(reqURL)) if errs != nil { log.Errorf("could not contact anchore api") return image, errs[0] } + if resp.StatusCode == 404 { + log.WithFields(log.Fields{"digest": digest}). + Debug("Received 404 getting Image from Anchore, image not found in Anchore") + + // TODO Make the retry count configurable and backoff retries + // Anchore returns 404 if the image is not found, retry up to 5 times + // This is to handle the case where the image is still being submitted for analysis + // and the image is not yet available in the Anchore DB + if retryCount < 5 { + log.WithFields(log.Fields{"digest": digest, "retryCount": retryCount}). + Debug("retrying image status check") + time.Sleep(5 * time.Second) + return GetImage(clientConfiguration, digest, retryCount+1) + } + + return image, fmt.Errorf("not found") + } + if apiVersion == "v1" { var imageList anchore.ImageListV1 err = json.Unmarshal(body, &imageList) diff --git a/pkg/adapter/anchore/result_store.go b/pkg/adapter/anchore/result_store.go index eb7b95c..075766b 100644 --- a/pkg/adapter/anchore/result_store.go +++ b/pkg/adapter/anchore/result_store.go @@ -2,6 +2,7 @@ package anchore import ( "fmt" + "sync" log "github.com/sirupsen/logrus" @@ -32,6 +33,13 @@ type ResultStore interface { scanID string, buildFn func() (*anchore.ImageVulnerabilityReport, error), ) VulnerabilityResult + SafeUpdateResult( + scanID string, + result VulnerabilityResult, + ) // Update a result in the store + GetResult( + scanID string, + ) VulnerabilityResult // Get a result if it exists PopResult( scanID string, ) (VulnerabilityResult, bool) // Returns a result and true if found, false if not (e.g. like hash map interface) @@ -50,6 +58,7 @@ type VulnerabilityResult struct { type MemoryResultStore struct { Results map[string]VulnerabilityResult + mu sync.Mutex } var ( @@ -60,16 +69,27 @@ var ( func NewResultStore() ResultStore { newStore := MemoryResultStore{Results: make(map[string]VulnerabilityResult, 1000)} newStore.Start() - return newStore + return &newStore } -func (m MemoryResultStore) HasResult(scanID string) bool { +func (m *MemoryResultStore) HasResult(scanID string) bool { + m.mu.Lock() + defer m.mu.Unlock() found, ok := m.Results[scanID] log.Debugf("HasResult: %v", found) return ok && found.IsComplete } -func (m MemoryResultStore) PopResult(scanID string) (VulnerabilityResult, bool) { +func (m *MemoryResultStore) GetResult(scanID string) VulnerabilityResult { + m.mu.Lock() + defer m.mu.Unlock() + found := m.Results[scanID] + return found +} + +func (m *MemoryResultStore) PopResult(scanID string) (VulnerabilityResult, bool) { + m.mu.Lock() + defer m.mu.Unlock() found, ok := m.Results[scanID] if found.IsComplete { log.WithField("scanId", scanID).Debug("found completed result and removing from store to return to caller") @@ -81,7 +101,14 @@ func (m MemoryResultStore) PopResult(scanID string) (VulnerabilityResult, bool) return found, ok } -func (m MemoryResultStore) RequestCreateScan( //nolint +func (m *MemoryResultStore) SafeUpdateResult(scanID string, result VulnerabilityResult) { + log.WithField("scanId", scanID).Debug("updating result in store") + m.mu.Lock() + m.Results[scanID] = result + m.mu.Unlock() +} + +func (m *MemoryResultStore) RequestCreateScan( scanID string, buildFn func() (bool, error), ) VulnerabilityResult { @@ -116,7 +143,7 @@ func (m MemoryResultStore) RequestCreateScan( //nolint } } }() - existing = VulnerabilityResult{ + newScan := VulnerabilityResult{ ScanID: scanID, ScanCreated: false, AnalysisComplete: false, @@ -125,21 +152,21 @@ func (m MemoryResultStore) RequestCreateScan( //nolint Result: nil, Error: fmt.Errorf("create scan not ready"), } - m.Results[scanID] = existing + m.SafeUpdateResult(scanID, newScan) } return existing } -func (m MemoryResultStore) RequestAnalysisStatus( //nolint +func (m *MemoryResultStore) RequestAnalysisStatus( scanID string, buildFn func() (bool, error), ) VulnerabilityResult { existing, ok := m.PopResult(scanID) - - if !ok { + if (!ok || existing.ScanCreated) && !existing.AnalysisComplete { // Result not found so begin the async fetch go func() { complete, err := buildFn() + currentState := m.Results[scanID] if err != nil { log.Debugf("error checking analysis state for %v: %v", scanID, err) // Set IsComplete to true to remove the scan from the store if the create scan fails so that it can be retried without using the cache. @@ -147,7 +174,7 @@ func (m MemoryResultStore) RequestAnalysisStatus( //nolint ScanID: scanID, ScanCreated: true, AnalysisComplete: false, - ReportBuildInProgress: false, + ReportBuildInProgress: currentState.ReportBuildInProgress, IsComplete: true, Result: nil, Error: err, @@ -158,35 +185,28 @@ func (m MemoryResultStore) RequestAnalysisStatus( //nolint ScanID: scanID, ScanCreated: true, AnalysisComplete: complete, - ReportBuildInProgress: false, - IsComplete: false, - Result: nil, + ReportBuildInProgress: currentState.ReportBuildInProgress, + IsComplete: currentState.IsComplete, + Result: currentState.Result, Error: nil, } } }() - existing = VulnerabilityResult{ - ScanID: scanID, - ScanCreated: true, - AnalysisComplete: false, - ReportBuildInProgress: false, - IsComplete: false, - Result: nil, - Error: fmt.Errorf("image analysis not ready"), - } - m.Results[scanID] = existing + existing.ScanID = scanID + existing.ScanCreated = true + existing.Error = fmt.Errorf("image analysis not ready") + m.SafeUpdateResult(scanID, existing) } return existing } -func (m MemoryResultStore) RequestResult( +func (m *MemoryResultStore) RequestResult( scanID string, buildFn func() (*harbor.VulnerabilityReport, error), ) VulnerabilityResult { existing, _ := m.PopResult(scanID) - - if !existing.ReportBuildInProgress && existing.ScanCreated { - log.Debug("Scan created, beginning report build") + if !existing.ReportBuildInProgress && existing.ScanCreated && existing.AnalysisComplete { + log.WithField("scanID", scanID).Debug("Scan created, beginning report build") // Result not found so begin the async fetch go func() { result, err := buildFn() @@ -196,6 +216,7 @@ func (m MemoryResultStore) RequestResult( resultChannel <- VulnerabilityResult{ ScanID: scanID, ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: true, Result: nil, @@ -206,6 +227,7 @@ func (m MemoryResultStore) RequestResult( resultChannel <- VulnerabilityResult{ ScanID: scanID, ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: true, Result: result, @@ -213,46 +235,42 @@ func (m MemoryResultStore) RequestResult( } } }() - existing = VulnerabilityResult{ - ScanID: scanID, - ScanCreated: true, - ReportBuildInProgress: true, - IsComplete: false, - Result: nil, - Error: fmt.Errorf("result not ready"), - } - m.Results[scanID] = existing + existing.ReportBuildInProgress = true + existing.Error = fmt.Errorf("result not ready") + m.SafeUpdateResult(scanID, existing) } return existing } -func (m MemoryResultStore) RequestRawResult( +func (m *MemoryResultStore) RequestRawResult( scanID string, buildFn func() (*anchore.ImageVulnerabilityReport, error), ) VulnerabilityResult { existing, _ := m.PopResult(scanID) - if !existing.ReportBuildInProgress && existing.ScanCreated { + if !existing.ReportBuildInProgress && existing.ScanCreated && existing.AnalysisComplete { log.Debug("Scan created, beginning raw report build") // Result not found so begin the async fetch go func() { result, err := buildFn() if err != nil { - log.Debugf("error building result for %v: %v", scanID, err) + log.Debugf("error building raw result for %v: %v", scanID, err) // Set IsComplete to true to remove the scan from the store so that it can be retried without using the cache. resultChannel <- VulnerabilityResult{ ScanID: scanID, ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: true, Result: nil, Error: err, } } else { - log.Debugf("result built for %v", scanID) + log.Debugf("raw result built for %v", scanID) resultChannel <- VulnerabilityResult{ ScanID: scanID, ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: true, Result: nil, @@ -261,29 +279,23 @@ func (m MemoryResultStore) RequestRawResult( } } }() - existing = VulnerabilityResult{ - ScanID: scanID, - ScanCreated: true, - ReportBuildInProgress: true, - IsComplete: false, - Result: nil, - Error: fmt.Errorf("result not ready"), - } - m.Results[scanID] = existing + existing.ReportBuildInProgress = true + existing.Error = fmt.Errorf("result not ready") + m.SafeUpdateResult(scanID, existing) } return existing } -func (m MemoryResultStore) resultRetriever() { +func (m *MemoryResultStore) resultRetriever() { for { report := <-resultChannel log.WithFields(log.Fields{"scanId": report.ScanID, "imageAdded": report.ScanCreated, "isComplete": report.IsComplete, "reportError": report.Error}). Debug("scan result added to result store") - m.Results[report.ScanID] = report + m.SafeUpdateResult(report.ScanID, report) } } -func (m MemoryResultStore) Start() { +func (m *MemoryResultStore) Start() { log.Info("starting result fetch loop") go m.resultRetriever() } diff --git a/pkg/adapter/anchore/result_store_test.go b/pkg/adapter/anchore/result_store_test.go index ad98828..cbd4099 100644 --- a/pkg/adapter/anchore/result_store_test.go +++ b/pkg/adapter/anchore/result_store_test.go @@ -122,7 +122,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { scanID string buildFn func() (*harbor.VulnerabilityReport, error) } - resultStore = MemoryResultStore{Results: make(map[string]VulnerabilityResult, 1000)} + resultStore = &MemoryResultStore{Results: make(map[string]VulnerabilityResult, 1000)} resultChannel = make(chan VulnerabilityResult) testTime := time.Now() tests := []struct { @@ -140,6 +140,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { "test1": { ScanID: "test1", ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: true, Result: &harbor.VulnerabilityReport{ @@ -180,6 +181,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { ScanCreated: true, ReportBuildInProgress: true, IsComplete: true, + AnalysisComplete: true, Result: &harbor.VulnerabilityReport{ GeneratedAt: testTime, Artifact: harbor.Artifact{ @@ -236,6 +238,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { fields: fields{Results: map[string]VulnerabilityResult{ "test1": { ScanID: "test1", + AnalysisComplete: true, ScanCreated: true, ReportBuildInProgress: false, }, @@ -249,6 +252,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { want: VulnerabilityResult{ ScanID: "test1", ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: false, Result: nil, @@ -258,6 +262,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { wantChannelResult: VulnerabilityResult{ ScanID: "test1", ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: true, Result: nil, @@ -270,6 +275,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { "test1": { ScanID: "test1", ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: false, }, }}, @@ -306,6 +312,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { want: VulnerabilityResult{ ScanID: "test1", ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: false, Result: nil, @@ -316,6 +323,7 @@ func TestMemoryResultStore_RequestResult(t *testing.T) { wantChannelResult: VulnerabilityResult{ ScanID: "test1", ScanCreated: true, + AnalysisComplete: true, ReportBuildInProgress: true, IsComplete: true, Result: &harbor.VulnerabilityReport{