fix: Reduce log spam on unknown relationship type (#1797)

Rather than log a warning for every instance of an unknown relationship type,
or similar error, log a count of how many times each of these errors is
raised.

---------

Signed-off-by: Will Murphy <will.murphy@anchore.com>

syft/formats/syftjson/to_syft_model.go

@@ -1,6 +1,7 @@
 package syftjson
 
 import (
+	"fmt"
 	"os"
 	"strconv"
 	"strings"
@@ -35,10 +36,30 @@ func toSyftModel(doc model.Document) (*sbom.SBOM, error) {
 		},
 		Source:        *toSyftSourceData(doc.Source),
 		Descriptor:    toSyftDescriptor(doc.Descriptor),
-		Relationships: toSyftRelationships(&doc, catalog, doc.ArtifactRelationships, idAliases),
+		Relationships: warnConversionErrors(toSyftRelationships(&doc, catalog, doc.ArtifactRelationships, idAliases)),
 	}, nil
 }
 
+func warnConversionErrors[T any](converted []T, errors []error) []T {
+	errorMessages := deduplicateErrors(errors)
+	for _, msg := range errorMessages {
+		log.Warn(msg)
+	}
+	return converted
+}
+
+func deduplicateErrors(errors []error) []string {
+	errorCounts := make(map[string]int)
+	var errorMessages []string
+	for _, e := range errors {
+		errorCounts[e.Error()] = errorCounts[e.Error()] + 1
+	}
+	for msg, count := range errorCounts {
+		errorMessages = append(errorMessages, fmt.Sprintf("%q occurred %d time(s)", msg, count))
+	}
+	return errorMessages
+}
+
 func toSyftFiles(files []model.File) sbom.Artifacts {
 	ret := sbom.Artifacts{
 		FileMetadata: make(map[source.Coordinates]source.FileMetadata),
@@ -131,7 +152,7 @@ func toSyftLinuxRelease(d model.LinuxRelease) *linux.Release {
 	}
 }
 
-func toSyftRelationships(doc *model.Document, catalog *pkg.Collection, relationships []model.Relationship, idAliases map[string]string) []artifact.Relationship {
+func toSyftRelationships(doc *model.Document, catalog *pkg.Collection, relationships []model.Relationship, idAliases map[string]string) ([]artifact.Relationship, []error) {
 	idMap := make(map[string]interface{})
 
 	for _, p := range catalog.Sorted() {
@@ -150,13 +171,18 @@ func toSyftRelationships(doc *model.Document, catalog *pkg.Collection, relations
 	}
 
 	var out []artifact.Relationship
+	var conversionErrors []error
 	for _, r := range relationships {
-		syftRelationship := toSyftRelationship(idMap, r, idAliases)
+		syftRelationship, err := toSyftRelationship(idMap, r, idAliases)
+		if err != nil {
+			conversionErrors = append(conversionErrors, err)
+		}
 		if syftRelationship != nil {
 			out = append(out, *syftRelationship)
 		}
 	}
-	return out
+
+	return out, conversionErrors
 }
 
 func toSyftSource(s model.Source) *source.Source {
@@ -167,7 +193,7 @@ func toSyftSource(s model.Source) *source.Source {
 	return newSrc
 }
 
-func toSyftRelationship(idMap map[string]interface{}, relationship model.Relationship, idAliases map[string]string) *artifact.Relationship {
+func toSyftRelationship(idMap map[string]interface{}, relationship model.Relationship, idAliases map[string]string) (*artifact.Relationship, error) {
 	id := func(id string) string {
 		aliased, ok := idAliases[id]
 		if ok {
@@ -178,14 +204,12 @@ func toSyftRelationship(idMap map[string]interface{}, relationship model.Relatio
 
 	from, ok := idMap[id(relationship.Parent)].(artifact.Identifiable)
 	if !ok {
-		log.Warnf("relationship mapping from key %s is not a valid artifact.Identifiable type: %+v", relationship.Parent, idMap[relationship.Parent])
+		return nil, fmt.Errorf("relationship mapping from key %s is not a valid artifact.Identifiable type: %+v", relationship.Parent, idMap[relationship.Parent])
-		return nil
 	}
 
 	to, ok := idMap[id(relationship.Child)].(artifact.Identifiable)
 	if !ok {
-		log.Warnf("relationship mapping to key %s is not a valid artifact.Identifiable type: %+v", relationship.Child, idMap[relationship.Child])
+		return nil, fmt.Errorf("relationship mapping to key %s is not a valid artifact.Identifiable type: %+v", relationship.Child, idMap[relationship.Child])
-		return nil
 	}
 
 	typ := artifact.RelationshipType(relationship.Type)
@@ -194,8 +218,7 @@ func toSyftRelationship(idMap map[string]interface{}, relationship model.Relatio
 	case artifact.OwnershipByFileOverlapRelationship, artifact.ContainsRelationship, artifact.DependencyOfRelationship, artifact.EvidentByRelationship:
 	default:
 		if !strings.Contains(string(typ), "dependency-of") {
-			log.Warnf("unknown relationship type: %s", typ)
+			return nil, fmt.Errorf("unknown relationship type: %s", string(typ))
-			return nil
 		}
 		// lets try to stay as compatible as possible with similar relationship types without dropping the relationship
 		log.Warnf("assuming %q for relationship type %q", artifact.DependencyOfRelationship, typ)
@@ -206,7 +229,7 @@ func toSyftRelationship(idMap map[string]interface{}, relationship model.Relatio
 		To:   to,
 		Type: typ,
 		Data: relationship.Metadata,
-	}
+	}, nil
 }
 
 func toSyftDescriptor(d model.Descriptor) sbom.Descriptor {

syft/formats/syftjson/to_syft_model_test.go

@@ -1,6 +1,7 @@
 package syftjson
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/scylladb/go-set/strset"
@@ -10,6 +11,7 @@ import (
 	"github.com/anchore/syft/syft/artifact"
 	"github.com/anchore/syft/syft/file"
 	"github.com/anchore/syft/syft/formats/syftjson/model"
+	"github.com/anchore/syft/syft/pkg"
 	"github.com/anchore/syft/syft/sbom"
 	"github.com/anchore/syft/syft/source"
 )
@@ -228,3 +230,120 @@ func Test_toSyftFiles(t *testing.T) {
 		})
 	}
 }
+
+func Test_toSyfRelationship(t *testing.T) {
+	packageWithId := func(id string) *pkg.Package {
+		p := &pkg.Package{}
+		p.OverrideID(artifact.ID(id))
+		return p
+	}
+	childPackage := packageWithId("some-child-id")
+	parentPackage := packageWithId("some-parent-id")
+	tests := []struct {
+		name          string
+		idMap         map[string]interface{}
+		idAliases     map[string]string
+		relationships model.Relationship
+		want          *artifact.Relationship
+		wantError     error
+	}{
+		{
+			name: "one relationship no warnings",
+			idMap: map[string]interface{}{
+				"some-child-id":  childPackage,
+				"some-parent-id": parentPackage,
+			},
+			idAliases: map[string]string{},
+			relationships: model.Relationship{
+				Parent: "some-parent-id",
+				Child:  "some-child-id",
+				Type:   string(artifact.ContainsRelationship),
+			},
+			want: &artifact.Relationship{
+				To:   childPackage,
+				From: parentPackage,
+				Type: artifact.ContainsRelationship,
+			},
+		},
+		{
+			name: "relationship unknown type one warning",
+			idMap: map[string]interface{}{
+				"some-child-id":  childPackage,
+				"some-parent-id": parentPackage,
+			},
+			idAliases: map[string]string{},
+			relationships: model.Relationship{
+				Parent: "some-parent-id",
+				Child:  "some-child-id",
+				Type:   "some-unknown-relationship-type",
+			},
+			wantError: errors.New(
+				"unknown relationship type: some-unknown-relationship-type",
+			),
+		},
+		{
+			name: "relationship missing child ID one warning",
+			idMap: map[string]interface{}{
+				"some-parent-id": parentPackage,
+			},
+			idAliases: map[string]string{},
+			relationships: model.Relationship{
+				Parent: "some-parent-id",
+				Child:  "some-child-id",
+				Type:   string(artifact.ContainsRelationship),
+			},
+			wantError: errors.New(
+				"relationship mapping to key some-child-id is not a valid artifact.Identifiable type: <nil>",
+			),
+		},
+		{
+			name: "relationship missing parent ID one warning",
+			idMap: map[string]interface{}{
+				"some-child-id": childPackage,
+			},
+			idAliases: map[string]string{},
+			relationships: model.Relationship{
+				Parent: "some-parent-id",
+				Child:  "some-child-id",
+				Type:   string(artifact.ContainsRelationship),
+			},
+			wantError: errors.New("relationship mapping from key some-parent-id is not a valid artifact.Identifiable type: <nil>"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, gotErr := toSyftRelationship(tt.idMap, tt.relationships, tt.idAliases)
+			assert.Equal(t, tt.want, got)
+			assert.Equal(t, tt.wantError, gotErr)
+		})
+	}
+}
+
+func Test_deduplicateErrors(t *testing.T) {
+	tests := []struct {
+		name   string
+		errors []error
+		want   []string
+	}{
+		{
+			name: "no errors, nil slice",
+		},
+		{
+			name: "deduplicates errors",
+			errors: []error{
+				errors.New("some error"),
+				errors.New("some error"),
+			},
+			want: []string{
+				`"some error" occurred 2 time(s)`,
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := deduplicateErrors(tt.errors)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}