STRLCPY/syft

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 <[email protected]>

William Murphy committed with GitHub 12 months ago

291da8cd

1 parent 8a3cbf2f

Revision indexing in progress... (symbol navigation in revisions will be accurate after indexed)

Total 2 files Show one by one

■ ■ ■ ■ ■ ■

syft/formats/syftjson/to_syft_model.go

1	1		package syftjson
2	2
3	3		import (
	4	+	"fmt"
4	5		"os"
5	6		"strconv"
6	7		"strings"
		skipped 28 lines
35	36		},
36	37		Source: *toSyftSourceData(doc.Source),
37	38		Descriptor: toSyftDescriptor(doc.Descriptor),
38		-	Relationships: toSyftRelationships(&doc, catalog, doc.ArtifactRelationships, idAliases),
	39	+	Relationships: warnConversionErrors(toSyftRelationships(&doc, catalog, doc.ArtifactRelationships, idAliases)),
39	40		}, nil
	41	+	}
	42	+
	43	+	func warnConversionErrors[T any](converted []T, errors []error) []T {
	44	+	errorMessages := deduplicateErrors(errors)
	45	+	for _, msg := range errorMessages {
	46	+	log.Warn(msg)
	47	+	}
	48	+	return converted
	49	+	}
	50	+
	51	+	func deduplicateErrors(errors []error) []string {
	52	+	errorCounts := make(map[string]int)
	53	+	var errorMessages []string
	54	+	for _, e := range errors {
	55	+	errorCounts[e.Error()] = errorCounts[e.Error()] + 1
	56	+	}
	57	+	for msg, count := range errorCounts {
	58	+	errorMessages = append(errorMessages, fmt.Sprintf("%q occurred %d time(s)", msg, count))
	59	+	}
	60	+	return errorMessages
40	61		}
41	62
42	63		func toSyftFiles(files []model.File) sbom.Artifacts {
		skipped 88 lines
131	152		}
132	153		}
133	154
134		-	func toSyftRelationships(doc model.Document, catalog pkg.Collection, relationships []model.Relationship, idAliases map[string]string) []artifact.Relationship {
	155	+	func toSyftRelationships(doc model.Document, catalog pkg.Collection, relationships []model.Relationship, idAliases map[string]string) ([]artifact.Relationship, []error) {
135	156		idMap := make(map[string]interface{})
136	157
137	158		for _, p := range catalog.Sorted() {
		skipped 12 lines
150	171		}
151	172
152	173		var out []artifact.Relationship
	174	+	var conversionErrors []error
153	175		for _, r := range relationships {
154		-	syftRelationship := toSyftRelationship(idMap, r, idAliases)
	176	+	syftRelationship, err := toSyftRelationship(idMap, r, idAliases)
	177	+	if err != nil {
	178	+	conversionErrors = append(conversionErrors, err)
	179	+	}
155	180		if syftRelationship != nil {
156	181		out = append(out, *syftRelationship)
157	182		}
158	183		}
159		-	return out
	184	+
	185	+	return out, conversionErrors
160	186		}
161	187
162	188		func toSyftSource(s model.Source) *source.Source {
		skipped 4 lines
167	193		return newSrc
168	194		}
169	195
170		-	func toSyftRelationship(idMap map[string]interface{}, relationship model.Relationship, idAliases map[string]string) *artifact.Relationship {
	196	+	func toSyftRelationship(idMap map[string]interface{}, relationship model.Relationship, idAliases map[string]string) (*artifact.Relationship, error) {
171	197		id := func(id string) string {
172	198		aliased, ok := idAliases[id]
173	199		if ok {
		skipped 4 lines
178	204
179	205		from, ok := idMap[id(relationship.Parent)].(artifact.Identifiable)
180	206		if !ok {
181		-	log.Warnf("relationship mapping from key %s is not a valid artifact.Identifiable type: %+v", relationship.Parent, idMap[relationship.Parent])
182		-	return nil
	207	+	return nil, fmt.Errorf("relationship mapping from key %s is not a valid artifact.Identifiable type: %+v", relationship.Parent, idMap[relationship.Parent])
183	208		}
184	209
185	210		to, ok := idMap[id(relationship.Child)].(artifact.Identifiable)
186	211		if !ok {
187		-	log.Warnf("relationship mapping to key %s is not a valid artifact.Identifiable type: %+v", relationship.Child, idMap[relationship.Child])
188		-	return nil
	212	+	return nil, fmt.Errorf("relationship mapping to key %s is not a valid artifact.Identifiable type: %+v", relationship.Child, idMap[relationship.Child])
189	213		}
190	214
191	215		typ := artifact.RelationshipType(relationship.Type)
		skipped 2 lines
194	218		case artifact.OwnershipByFileOverlapRelationship, artifact.ContainsRelationship, artifact.DependencyOfRelationship, artifact.EvidentByRelationship:
195	219		default:
196	220		if !strings.Contains(string(typ), "dependency-of") {
197		-	log.Warnf("unknown relationship type: %s", typ)
198		-	return nil
	221	+	return nil, fmt.Errorf("unknown relationship type: %s", string(typ))
199	222		}
200	223		// lets try to stay as compatible as possible with similar relationship types without dropping the relationship
201	224		log.Warnf("assuming %q for relationship type %q", artifact.DependencyOfRelationship, typ)
		skipped 4 lines
206	229		To: to,
207	230		Type: typ,
208	231		Data: relationship.Metadata,
209		-	}
	232	+	}, nil
210	233		}
211	234
212	235		func toSyftDescriptor(d model.Descriptor) sbom.Descriptor {
		skipped 94 lines

■ ■ ■ ■ ■ ■

syft/formats/syftjson/to_syft_model_test.go

1	1		package syftjson
2	2
3	3		import (
	4	+	"errors"
4	5		"testing"
5	6
6	7		"github.com/scylladb/go-set/strset"
		skipped 3 lines
10	11		"github.com/anchore/syft/syft/artifact"
11	12		"github.com/anchore/syft/syft/file"
12	13		"github.com/anchore/syft/syft/formats/syftjson/model"
	14	+	"github.com/anchore/syft/syft/pkg"
13	15		"github.com/anchore/syft/syft/sbom"
14	16		"github.com/anchore/syft/syft/source"
15	17		)
		skipped 213 lines
229	231		}
230	232		}
231	233
	234	+	func Test_toSyfRelationship(t *testing.T) {
	235	+	packageWithId := func(id string) *pkg.Package {
	236	+	p := &pkg.Package{}
	237	+	p.OverrideID(artifact.ID(id))
	238	+	return p
	239	+	}
	240	+	childPackage := packageWithId("some-child-id")
	241	+	parentPackage := packageWithId("some-parent-id")
	242	+	tests := []struct {
	243	+	name string
	244	+	idMap map[string]interface{}
	245	+	idAliases map[string]string
	246	+	relationships model.Relationship
	247	+	want *artifact.Relationship
	248	+	wantError error
	249	+	}{
	250	+	{
	251	+	name: "one relationship no warnings",
	252	+	idMap: map[string]interface{}{
	253	+	"some-child-id": childPackage,
	254	+	"some-parent-id": parentPackage,
	255	+	},
	256	+	idAliases: map[string]string{},
	257	+	relationships: model.Relationship{
	258	+	Parent: "some-parent-id",
	259	+	Child: "some-child-id",
	260	+	Type: string(artifact.ContainsRelationship),
	261	+	},
	262	+	want: &artifact.Relationship{
	263	+	To: childPackage,
	264	+	From: parentPackage,
	265	+	Type: artifact.ContainsRelationship,
	266	+	},
	267	+	},
	268	+	{
	269	+	name: "relationship unknown type one warning",
	270	+	idMap: map[string]interface{}{
	271	+	"some-child-id": childPackage,
	272	+	"some-parent-id": parentPackage,
	273	+	},
	274	+	idAliases: map[string]string{},
	275	+	relationships: model.Relationship{
	276	+	Parent: "some-parent-id",
	277	+	Child: "some-child-id",
	278	+	Type: "some-unknown-relationship-type",
	279	+	},
	280	+	wantError: errors.New(
	281	+	"unknown relationship type: some-unknown-relationship-type",
	282	+	),
	283	+	},
	284	+	{
	285	+	name: "relationship missing child ID one warning",
	286	+	idMap: map[string]interface{}{
	287	+	"some-parent-id": parentPackage,
	288	+	},
	289	+	idAliases: map[string]string{},
	290	+	relationships: model.Relationship{
	291	+	Parent: "some-parent-id",
	292	+	Child: "some-child-id",
	293	+	Type: string(artifact.ContainsRelationship),
	294	+	},
	295	+	wantError: errors.New(
	296	+	"relationship mapping to key some-child-id is not a valid artifact.Identifiable type: <nil>",
	297	+	),
	298	+	},
	299	+	{
	300	+	name: "relationship missing parent ID one warning",
	301	+	idMap: map[string]interface{}{
	302	+	"some-child-id": childPackage,
	303	+	},
	304	+	idAliases: map[string]string{},
	305	+	relationships: model.Relationship{
	306	+	Parent: "some-parent-id",
	307	+	Child: "some-child-id",
	308	+	Type: string(artifact.ContainsRelationship),
	309	+	},
	310	+	wantError: errors.New("relationship mapping from key some-parent-id is not a valid artifact.Identifiable type: <nil>"),
	311	+	},
	312	+	}
	313	+
	314	+	for _, tt := range tests {
	315	+	t.Run(tt.name, func(t *testing.T) {
	316	+	got, gotErr := toSyftRelationship(tt.idMap, tt.relationships, tt.idAliases)
	317	+	assert.Equal(t, tt.want, got)
	318	+	assert.Equal(t, tt.wantError, gotErr)
	319	+	})
	320	+	}
	321	+	}
	322	+
	323	+	func Test_deduplicateErrors(t *testing.T) {
	324	+	tests := []struct {
	325	+	name string
	326	+	errors []error
	327	+	want []string
	328	+	}{
	329	+	{
	330	+	name: "no errors, nil slice",
	331	+	},
	332	+	{
	333	+	name: "deduplicates errors",
	334	+	errors: []error{
	335	+	errors.New("some error"),
	336	+	errors.New("some error"),
	337	+	},
	338	+	want: []string{
	339	+	`"some error" occurred 2 time(s)`,
	340	+	},
	341	+	},
	342	+	}
	343	+	for _, tt := range tests {
	344	+	t.Run(tt.name, func(t *testing.T) {
	345	+	got := deduplicateErrors(tt.errors)
	346	+	assert.Equal(t, tt.want, got)
	347	+	})
	348	+	}
	349	+	}
	350	+