Skip to content

Commit

Permalink
fix: Reduce log spam on unknown relationship type (#1797)
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@willmurphyscode
willmurphyscode authored May 10, 2023
1 parent 8a3cbf2 commit 291da8c
Show file tree
Hide file tree
Showing 2 changed files with 154 additions and 12 deletions.
47 changes: 35 additions & 12 deletions syft/formats/syftjson/to_syft_model.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package syftjson

import (
"fmt"
"os"
"strconv"
"strings"
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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() {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
return nil, fmt.Errorf("relationship mapping from key %s is not a valid artifact.Identifiable type: %+v", relationship.Parent, idMap[relationship.Parent])
}

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
return nil, fmt.Errorf("relationship mapping to key %s is not a valid artifact.Identifiable type: %+v", relationship.Child, idMap[relationship.Child])
}

typ := artifact.RelationshipType(relationship.Type)
Expand All @@ -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
return nil, fmt.Errorf("unknown relationship type: %s", string(typ))
}
// 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)
Expand All @@ -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 {
Expand Down
119 changes: 119 additions & 0 deletions syft/formats/syftjson/to_syft_model_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package syftjson

import (
"errors"
"testing"

"github.com/scylladb/go-set/strset"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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)
})
}
}

0 comments on commit 291da8c

Please sign in to comment.