chore: improve ignoredetails and issue implementation (#482)

Author: PeterSchafer

pkg/apiclients/testapi/findings_helper.go

@@ -0,0 +1,28 @@
+package testapi
+
+import "github.com/google/uuid"
+
+func (f *FindingData) GetIgnoreDetails() IssueIgnoreDetails {
+	if f.Attributes == nil || f.Attributes.Suppression == nil {
+		return nil
+	}
+	result := &issueIgnoreDetailsImpl{
+		finding: f,
+	}
+
+	// Extract policy information from suppression
+	if result.finding.Attributes.Suppression.Policy != nil {
+		if localRef, err := result.finding.Attributes.Suppression.Policy.AsPolicyRef0(); !(err == nil && localRef == PolicyRef0LocalPolicy) {
+			// Try as a managed policy (has ID field)
+			if managedRef, err := result.finding.Attributes.Suppression.Policy.AsManagedPolicyRef(); err == nil && managedRef.Id != uuid.Nil {
+				// It's a managed policy with a valid ID
+				idStr := managedRef.Id.String()
+				result.policyID = &idStr
+				result.ignoreData = searchFindingForIgnoreAction(result.finding, result.policyID)
+			}
+			// If both fail, policy type cannot be determined (isLocalPolicy remains nil)
+		}
+	}
+
+	return result
+}

pkg/apiclients/testapi/issues.go

@@ -334,8 +334,8 @@ type issue struct {
 	sourceLocations   []SourceLocation
 	riskScore         uint16
 	reachability      *ReachabilityEvidence
-	suppression       *Suppression
 	metadata          map[string]interface{} // case-insensitive key storage (lowercase keys)
+	ignoreDetail      IssueIgnoreDetails
 }
 
 // GetFindings returns all findings that are part of this issue.
@@ -423,10 +423,7 @@ func (i *issue) GetReachability() *ReachabilityEvidence {
 
 // GetIgnoreDetails returns the ignore/suppression details for this issue.
 func (i *issue) GetIgnoreDetails() IssueIgnoreDetails {
-	if i.suppression == nil {
-		return nil
-	}
-	return newIgnoreDetailsFromSuppression(i.suppression, i.findings)
+	return i.ignoreDetail
 }
 
 // NewIssueFromFindings creates a single Issue instance from a group of related findings.
@@ -471,8 +468,8 @@ type issueBuilder struct {
 	sourceLocations   []SourceLocation
 	riskScore         uint16
 	reachability      *ReachabilityEvidence
-	suppression       *Suppression
 	findings          []*FindingData
+	ignoreDetails     []IssueIgnoreDetails
 }
 
 // processFindings extracts data from all findings
@@ -489,18 +486,27 @@ func (b *issueBuilder) processFindings(findings []*FindingData) {
 
 // processFinding extracts data from a single finding
 func (b *issueBuilder) processFinding(finding *FindingData) {
+	// ignore all data if the given finding is ignored
+	ignoreDetails := finding.GetIgnoreDetails()
+	b.ignoreDetails = append(b.ignoreDetails, ignoreDetails)
+
+	// skipping the details of an ignored finding seems reasonable but currently breaks tests, which is why it is for now commented
+	//if ignoreDetails != nil && ignoreDetails.IsActive() {
+	//	return
+	//}
+
 	b.setBasicInfo(finding)
 	for i := range finding.Attributes.Problems {
 		b.allProblems = append(b.allProblems, &finding.Attributes.Problems[i])
 	}
+
+	b.extractPackageInfo(finding)
+	b.processProblems(finding)
 	b.extractSourceLocations(finding)
 	b.extractSeverityAndRisk(finding)
 	b.extractEffectiveSeverity(finding)
 	b.extractReachability(finding)
-	b.extractSuppression(finding)
 	b.extractDependencyPaths(finding)
-	b.extractPackageInfo(finding)
-	b.processProblems(finding)
 }
 
 // setBasicInfo sets finding type, title, and description from the first finding
@@ -569,16 +575,6 @@ func (b *issueBuilder) extractReachability(finding *FindingData) {
 	}
 }
 
-// extractSuppression extracts suppression information
-func (b *issueBuilder) extractSuppression(finding *FindingData) {
-	if b.suppression != nil {
-		return
-	}
-	if finding.Attributes.Suppression != nil {
-		b.suppression = finding.Attributes.Suppression
-	}
-}
-
 // extractDependencyPaths extracts dependency paths from evidence
 func (b *issueBuilder) extractDependencyPaths(finding *FindingData) {
 	for _, ev := range finding.Attributes.Evidence {
@@ -766,6 +762,7 @@ func (b *issueBuilder) deduplicate() {
 // build constructs the final Issue from collected data
 func (b *issueBuilder) build() *issue {
 	metadata := b.buildMetadata()
+	activeIgnoreDetail := determineActiveIgnoreDetail(b.ignoreDetails)
 
 	return &issue{
 		findings:          b.findings,
@@ -783,9 +780,27 @@ func (b *issueBuilder) build() *issue {
 		sourceLocations:   b.sourceLocations,
 		riskScore:         b.riskScore,
 		reachability:      b.reachability,
-		suppression:       b.suppression,
 		metadata:          metadata,
+		ignoreDetail:      activeIgnoreDetail,
+	}
+}
+
+func determineActiveIgnoreDetail(ignoreDetails []IssueIgnoreDetails) IssueIgnoreDetails {
+	activeIgnores := 0
+	var firstIgnoreDetail IssueIgnoreDetails
+	for _, detail := range ignoreDetails {
+		if detail != nil && detail.IsActive() {
+			firstIgnoreDetail = detail
+			activeIgnores++
+		}
 	}
+
+	// if all findings are ignored, return the first ignore details
+	if activeIgnores == len(ignoreDetails) {
+		return firstIgnoreDetail
+	}
+
+	return nil
 }
 
 // buildMetadata constructs the metadata map

pkg/apiclients/testapi/issues_ignore_details.go

@@ -2,8 +2,6 @@ package testapi
 
 import (
 	"time"
-
-	"github.com/google/uuid"
 )
 
 // IssueIgnoreDetails provides combined information about issue suppression/ignore status,
@@ -39,57 +37,59 @@ type IssueIgnoreDetails interface {
 
 	// GetIgnoredBy returns the user who created the ignore action.
 	GetIgnoredBy() *IgnoredBy
+
+	IsActive() bool
 }
 
 // issueIgnoreDetailsImpl is the concrete implementation of IssueIgnoreDetails.
 type issueIgnoreDetailsImpl struct {
-	suppression *Suppression
-	policyID    *string
-	ignoreData  *Ignore
+	finding    *FindingData
+	policyID   *string
+	ignoreData *Ignore
 }
 
 // === Suppression Status Methods ===
 
 func (id *issueIgnoreDetailsImpl) GetStatus() SuppressionStatus {
-	if id.suppression == nil {
+	if !id.isInitialized() {
 		return ""
 	}
-	return id.suppression.Status
+	return id.finding.Attributes.Suppression.Status
 }
 
 func (id *issueIgnoreDetailsImpl) GetCreatedAt() *time.Time {
-	if id.suppression == nil {
+	if !id.isInitialized() {
 		return nil
 	}
-	return id.suppression.CreatedAt
+	return id.finding.Attributes.Suppression.CreatedAt
 }
 
 func (id *issueIgnoreDetailsImpl) GetExpiresAt() *time.Time {
-	if id.suppression == nil {
+	if !id.isInitialized() {
 		return nil
 	}
-	return id.suppression.ExpiresAt
+	return id.finding.Attributes.Suppression.ExpiresAt
 }
 
 func (id *issueIgnoreDetailsImpl) GetJustification() *string {
-	if id.suppression == nil {
+	if !id.isInitialized() {
 		return nil
 	}
-	return id.suppression.Justification
+	return id.finding.Attributes.Suppression.Justification
 }
 
 func (id *issueIgnoreDetailsImpl) GetPath() *[]string {
-	if id.suppression == nil {
+	if !id.isInitialized() {
 		return nil
 	}
-	return id.suppression.Path
+	return id.finding.Attributes.Suppression.Path
 }
 
 func (id *issueIgnoreDetailsImpl) SkipIfFixable() *bool {
-	if id.suppression == nil {
+	if !id.isInitialized() {
 		return nil
 	}
-	return id.suppression.SkipIfFixable
+	return id.finding.Attributes.Suppression.SkipIfFixable
 }
 
 // === Policy Information Methods ===
@@ -128,65 +128,23 @@ func (id *issueIgnoreDetailsImpl) GetIgnoredBy() *IgnoredBy {
 	return id.ignoreData.Ignore.IgnoredBy
 }
 
-// newIgnoreDetailsFromSuppression creates an IssueIgnoreDetails from suppression data and findings.
-func newIgnoreDetailsFromSuppression(suppression *Suppression, findings []*FindingData) IssueIgnoreDetails {
-	if suppression == nil {
-		return nil
+func (id *issueIgnoreDetailsImpl) IsActive() bool {
+	if !id.isInitialized() {
+		return false
 	}
 
-	result := &issueIgnoreDetailsImpl{
-		suppression: suppression,
+	if id.finding.Attributes.Suppression.Status != SuppressionStatusIgnored {
+		return false
 	}
 
-	// Extract policy information from suppression
-	if suppression.Policy != nil {
-		// Determine policy type and ID by checking the structure
-		// PolicyRef is a union of either a string (local) or an object with ID (managed)
-
-		if localRef, err := suppression.Policy.AsPolicyRef0(); !(err == nil && localRef == PolicyRef0LocalPolicy) {
-			// Try as a managed policy (has ID field)
-			if managedRef, err := suppression.Policy.AsManagedPolicyRef(); err == nil && managedRef.Id != uuid.Nil {
-				// It's a managed policy with a valid ID
-				idStr := managedRef.Id.String()
-				result.policyID = &idStr
-
-				// Try to find the matching expanded policy data in findings
-				result.ignoreData = findMatchingIgnoreData(suppression.Policy, findings)
-			}
-			// If both fail, policy type cannot be determined (isLocalPolicy remains nil)
-		}
-	}
-
-	return result
+	return true
 }
 
-// findMatchingIgnoreData finds the Ignore action that matches the given policy reference.
-func findMatchingIgnoreData(policyRef *PolicyRef, findings []*FindingData) *Ignore {
-	if policyRef == nil {
-		return nil
+func (id *issueIgnoreDetailsImpl) isInitialized() bool {
+	if id.finding == nil || id.finding.Attributes == nil || id.finding.Attributes.Suppression == nil {
+		return false
 	}
-
-	policyIDToMatch := extractPolicyTypeFromRef(policyRef)
-
-	// Search through findings for matching policy
-	for _, finding := range findings {
-		if ignoreAction := searchFindingForIgnoreAction(finding, policyIDToMatch); ignoreAction != nil {
-			return ignoreAction
-		}
-	}
-
-	return nil
-}
-
-// extractPolicyTypeFromRef determines if a PolicyRef is local or managed and returns the policy ID.
-func extractPolicyTypeFromRef(policyRef *PolicyRef) *string {
-	// Try as a managed policy (has ID field)
-	if managedRef, err := policyRef.AsManagedPolicyRef(); err == nil && managedRef.Id != uuid.Nil {
-		idStr := managedRef.Id.String()
-		return &idStr
-	}
-
-	return nil
+	return true
 }
 
 // searchFindingForIgnoreAction searches a finding for a matching ignore action.