From 5ba890d6e72bdef8c329a23eddc1525bba3cd279 Mon Sep 17 00:00:00 2001 From: Parth Patel <88045217+pxp928@users.noreply.github.com> Date: Thu, 26 Sep 2024 08:10:20 -0400 Subject: [PATCH] fix error handling on certifier to fail on network error when graphQL server is not up but keep running when a service issue is encountered (#2151) Signed-off-by: pxp928 --- cmd/guaccollect/cmd/osv.go | 6 ++-- pkg/certifier/certify/certify.go | 6 ++-- .../clearlydefined/clearlydefined.go | 29 ++++++++----------- pkg/certifier/osv/osv.go | 15 +++++----- pkg/ingestor/parser/common/scanner/scanner.go | 4 +-- 5 files changed, 27 insertions(+), 33 deletions(-) diff --git a/cmd/guaccollect/cmd/osv.go b/cmd/guaccollect/cmd/osv.go index 9012443e7e..27d4875be3 100644 --- a/cmd/guaccollect/cmd/osv.go +++ b/cmd/guaccollect/cmd/osv.go @@ -222,9 +222,9 @@ func initializeNATsandCertifier(ctx context.Context, blobAddr, pubsubAddr string if err == nil { return true } - logger.Errorf("certifier ended with error: %v", err) - // exit the loop but drain the channel first - return false + logger.Errorf("certifier encountered an error: %v, continuing...", err) + // log the error but continue forward with the rest of the package processing + return true } ctx, cf := context.WithCancel(ctx) diff --git a/pkg/certifier/certify/certify.go b/pkg/certifier/certify/certify.go index 689d78fc72..7d753bbdf0 100644 --- a/pkg/certifier/certify/certify.go +++ b/pkg/certifier/certify/certify.go @@ -90,7 +90,7 @@ func Certify(ctx context.Context, query certifier.QueryComponents, emitter certi return fmt.Errorf("generate certifier documents error: %w", err) } case err := <-errChan: - if !handleErr(err) { + if err != nil { // drain channel before exiting drainComponentChannel(compChan, ctx, emitter, handleErr) return err @@ -119,14 +119,14 @@ func Certify(ctx context.Context, query certifier.QueryComponents, emitter certi select { case <-ticker.C: // add logging to determine when the certifier run is started - logger.Infof("Starting polling certifier run: %v", time.Now().UTC()) + logger.Infof("Starting certifier run: %v", time.Now().UTC()) err := runCertifier() if err != nil { return fmt.Errorf("certifier failed with an error: %w", err) } // reset the interval timer and log completion of the current certifier run ticker.Reset(interval) - logger.Infof("Certifier polling run completed: %v", time.Now().UTC()) + logger.Infof("Certifier run completed: %v", time.Now().UTC()) // if the context has been canceled return the err. case <-ctx.Done(): return ctx.Err() // nolint:wrapcheck diff --git a/pkg/certifier/clearlydefined/clearlydefined.go b/pkg/certifier/clearlydefined/clearlydefined.go index 3d7e17011f..7bfa3e4b1a 100644 --- a/pkg/certifier/clearlydefined/clearlydefined.go +++ b/pkg/certifier/clearlydefined/clearlydefined.go @@ -112,10 +112,6 @@ func getDefinitions(ctx context.Context, client *http.Client, purls []string, co return nil, fmt.Errorf("error unmarshalling JSON: %v", err) } - if len(purls) != len(definitions) { - return nil, fmt.Errorf("failed to get expected responses back! Purl count: %d, returned definition count %d", len(purls), len(definitions)) - } - for coordinate, definition := range definitions { definitionMap[coordinateToPurl[coordinate]] = definition } @@ -124,7 +120,7 @@ func getDefinitions(ctx context.Context, client *http.Client, purls []string, co } // EvaluateClearlyDefinedDefinition converts the purls into coordinates to query clearly defined -func EvaluateClearlyDefinedDefinition(ctx context.Context, client *http.Client, purls []string) ([]*processor.Document, error) { +func EvaluateClearlyDefinedDefinition(ctx context.Context, client *http.Client, purls []string, docChannel chan<- *processor.Document) ([]*processor.Document, error) { logger := logging.FromContext(ctx) var batchCoordinates []string var queryPurls []string @@ -147,7 +143,7 @@ func EvaluateClearlyDefinedDefinition(ctx context.Context, client *http.Client, batchCoordinates = append(batchCoordinates, coordinate.ToString()) } } - if genCDDocs, err := generateDefinitions(ctx, client, batchCoordinates, queryPurls); err != nil { + if genCDDocs, err := generateDefinitions(ctx, client, batchCoordinates, queryPurls, docChannel); err != nil { return nil, fmt.Errorf("generateDefinitions failed with error: %w", err) } else { generatedCDDocs = append(generatedCDDocs, genCDDocs...) @@ -158,7 +154,7 @@ func EvaluateClearlyDefinedDefinition(ctx context.Context, client *http.Client, // generateDefinitions takes in the batched coordinated to retrieve the definition. It uses the definition to check if source // information can be queried in clearly defined. -func generateDefinitions(ctx context.Context, client *http.Client, batchCoordinates, queryPurls []string) ([]*processor.Document, error) { +func generateDefinitions(ctx context.Context, client *http.Client, batchCoordinates, queryPurls []string, docChannel chan<- *processor.Document) ([]*processor.Document, error) { var generatedCDDocs []*processor.Document if len(batchCoordinates) > 0 { definitionMap, err := getDefinitions(ctx, client, queryPurls, batchCoordinates) @@ -166,13 +162,13 @@ func generateDefinitions(ctx context.Context, client *http.Client, batchCoordina return nil, fmt.Errorf("failed get package definition from clearly defined with error: %w", err) } - if genCDPkgDocs, err := generateDocument(definitionMap); err != nil { + if genCDPkgDocs, err := generateDocument(definitionMap, docChannel); err != nil { return nil, fmt.Errorf("evaluateDefinitionForSource failed with error: %w", err) } else { generatedCDDocs = append(generatedCDDocs, genCDPkgDocs...) } - if genCDSrcDocs, err := evaluateDefinitionForSource(ctx, client, definitionMap); err != nil { + if genCDSrcDocs, err := evaluateDefinitionForSource(ctx, client, definitionMap, docChannel); err != nil { return nil, fmt.Errorf("evaluateDefinitionForSource failed with error: %w", err) } else { generatedCDDocs = append(generatedCDDocs, genCDSrcDocs...) @@ -194,12 +190,8 @@ func (c *cdCertifier) CertifyComponent(ctx context.Context, rootComponent interf purls = append(purls, node.Purl) } - if genCDDocs, err := EvaluateClearlyDefinedDefinition(ctx, c.cdHTTPClient, purls); err != nil { + if _, err := EvaluateClearlyDefinedDefinition(ctx, c.cdHTTPClient, purls, docChannel); err != nil { return fmt.Errorf("could not generate document from Clearly Defined results: %w", err) - } else { - for _, doc := range genCDDocs { - docChannel <- doc - } } return nil @@ -207,7 +199,7 @@ func (c *cdCertifier) CertifyComponent(ctx context.Context, rootComponent interf // evaluateDefinitionForSource takes in the returned definitions from package coordinates to determine if // source information can be obtained to re-query clearly defined for source related license information -func evaluateDefinitionForSource(ctx context.Context, client *http.Client, definitionMap map[string]*attestation.Definition) ([]*processor.Document, error) { +func evaluateDefinitionForSource(ctx context.Context, client *http.Client, definitionMap map[string]*attestation.Definition, docChannel chan<- *processor.Document) ([]*processor.Document, error) { sourceMap := map[string]bool{} var batchCoordinates []string var sourceInputs []string @@ -238,13 +230,13 @@ func evaluateDefinitionForSource(ctx context.Context, client *http.Client, defin if err != nil { return nil, fmt.Errorf("failed get source definition from clearly defined with error: %w", err) } - return generateDocument(definitionMap) + return generateDocument(definitionMap, docChannel) } return nil, nil } // generateDocument generates the processor document for ingestion -func generateDocument(definitionMap map[string]*attestation.Definition) ([]*processor.Document, error) { +func generateDocument(definitionMap map[string]*attestation.Definition, docChannel chan<- *processor.Document) ([]*processor.Document, error) { var generatedCDDocs []*processor.Document for purl, definition := range definitionMap { if definition.Described.ReleaseDate == "" { @@ -265,6 +257,9 @@ func generateDocument(definitionMap map[string]*attestation.Definition) ([]*proc DocumentRef: events.GetDocRef(payload), }, } + if docChannel != nil { + docChannel <- doc + } generatedCDDocs = append(generatedCDDocs, doc) } return generatedCDDocs, nil diff --git a/pkg/certifier/osv/osv.go b/pkg/certifier/osv/osv.go index a2476c9def..ccca1e3b7d 100644 --- a/pkg/certifier/osv/osv.go +++ b/pkg/certifier/osv/osv.go @@ -78,18 +78,14 @@ func (o *osvCertifier) CertifyComponent(ctx context.Context, rootComponent inter purls = append(purls, node.Purl) } - if genOSVDocs, err := EvaluateOSVResponse(ctx, o.osvHTTPClient, purls); err != nil { + if _, err := EvaluateOSVResponse(ctx, o.osvHTTPClient, purls, docChannel); err != nil { return fmt.Errorf("could not generate document from OSV results: %w", err) - } else { - for _, doc := range genOSVDocs { - docChannel <- doc - } } return nil } // EvaluateOSVResponse takes a list of purls and batch queries OSV for vulnerability information -func EvaluateOSVResponse(ctx context.Context, client *http.Client, purls []string) ([]*processor.Document, error) { +func EvaluateOSVResponse(ctx context.Context, client *http.Client, purls []string, docChannel chan<- *processor.Document) ([]*processor.Document, error) { var query osv_scanner.BatchedQuery packMap := map[string]bool{} @@ -117,11 +113,11 @@ func EvaluateOSVResponse(ctx context.Context, client *http.Client, purls []strin responseMap[purl] = &response } - return generateDocument(responseMap) + return generateDocument(responseMap, docChannel) } // generateDocument generated the processor document for ingestion -func generateDocument(responseMap map[string]*osv_scanner.MinimalResponse) ([]*processor.Document, error) { +func generateDocument(responseMap map[string]*osv_scanner.MinimalResponse, docChannel chan<- *processor.Document) ([]*processor.Document, error) { var generatedOSVDocs []*processor.Document for purl, response := range responseMap { currentTime := time.Now() @@ -139,6 +135,9 @@ func generateDocument(responseMap map[string]*osv_scanner.MinimalResponse) ([]*p DocumentRef: events.GetDocRef(payload), }, } + if docChannel != nil { + docChannel <- doc + } generatedOSVDocs = append(generatedOSVDocs, doc) } return generatedOSVDocs, nil diff --git a/pkg/ingestor/parser/common/scanner/scanner.go b/pkg/ingestor/parser/common/scanner/scanner.go index 9acad69c2c..f120f5ff82 100644 --- a/pkg/ingestor/parser/common/scanner/scanner.go +++ b/pkg/ingestor/parser/common/scanner/scanner.go @@ -39,7 +39,7 @@ func PurlsVulnScan(ctx context.Context, purls []string) ([]assembler.VulnEqualIn if osvProcessorDocs, err := osv_certifier.EvaluateOSVResponse(ctx, &http.Client{ Transport: version.UATransport, - }, purls); err != nil { + }, purls, nil); err != nil { return nil, nil, fmt.Errorf("failed get response from OSV with error: %w", err) } else { for _, doc := range osvProcessorDocs { @@ -109,7 +109,7 @@ func runQueryOnBatchedPurls(ctx context.Context, cdParser common.DocumentParser, var hasSourceAtIngest []assembler.HasSourceAtIngest if cdProcessorDocs, err := cd_certifier.EvaluateClearlyDefinedDefinition(ctx, &http.Client{ Transport: version.UATransport, - }, batchPurls); err != nil { + }, batchPurls, nil); err != nil { return nil, nil, fmt.Errorf("failed get definition from clearly defined with error: %w", err) } else { for _, doc := range cdProcessorDocs {