Acceptance testing by crozzy · Pull Request #1822 · quay/claircore

crozzy · 2026-04-08T22:39:14Z

No description provided.

crozzy · 2026-04-08T22:42:39Z

In two minds about adding local fixtures, they do test functionality and result correctness without needing external deps, but they are sort of a collection of real and fake data. I might just clean it up to a couple of examples.

crozzy · 2026-04-08T22:43:51Z

+
+		// Detect RHEL VEX documents by publisher namespace and use the specialised parser.
+		// RHEL VEX has complex product relationships and CPE-based repository matching.
+		if c.Document.Publisher.Namespace == "https://www.redhat.com" {


I think this is strong enough, but other suggestions welcome

Feels bad that we need special Red Hat handling :/

crozzy · 2026-04-08T22:44:26Z

+		vulns = append(vulns, vs...)
+	}
+
+	// TODO(crozzy): Process known_not_affected products here


Might need to sync a little here with the not affected work

crozzy · 2026-04-08T22:47:02Z

+type Auditor interface {
+	// Audit analyses the image at ref using the provided CSAF documents
+	// and returns vulnerability findings.
+	Audit(ctx context.Context, t testing.TB, ref string, csafDocs [][]byte) ([]Result, error)


I know we had some discussion around adding testing.TB to the interface method, it was to be able to use the in-built layer caching in claircore but open to other suggestions

I think if it uses the testing package in the exported API, it should be in the test/ hierarchy. That's a simple rule that helps keep the testing package out of our "real" API surface.

crozzy · 2026-04-08T23:01:04Z

+			continue
+		}
+		switch e.Status {
+		case fixtures.StatusUnknown:


Honestly, I'm on the fence about fixtures.StatusUnknown being an actual status

Yeah, I'm not totally clear if "Unknown" and "Extra" are different concepts. Isn't an "Unknown" status just an "Extra" package?

The acceptance testing framework allows users to hook into claircore's integration testing by calling acceptance.Run() and providing an Auditor. Signed-off-by: crozzy <joseph.crosland@gmail.com>

Implement the auditor interface back by claircore. Claircore functionality is called locally. Signed-off-by: crozzy <joseph.crosland@gmail.com>

This cmd can be used to push fixtures up to a container registry. This includes a c/image, VEX files and an expected results manifest. Fixtures can then be referenced in acceptance tests. Signed-off-by: crozzy <joseph.crosland@gmail.com>

This documentation should serve as a guide for outside image maintainers (and potentially other scanning vendors) that choose to test how vulnerability ingestion, indexing and matching work. Signed-off-by: crozzy <joseph.crosland@gmail.com>

Signed-off-by: crozzy <joseph.crosland@gmail.com>

hdonnay · 2026-04-17T17:45:54Z

@@ -0,0 +1,277 @@
+//go:build integration


Why is this build-tagged instead of using integration.Skip?

hdonnay · 2026-04-17T18:28:17Z

This should probably be in a test/acceptance/cmd/fixture package so that go run and/or go tool can work.

hdonnay · 2026-04-17T20:50:27Z

+type Auditor interface {
+	// Audit analyses the image at ref using the provided CSAF documents
+	// and returns vulnerability findings.
+	Audit(ctx context.Context, t testing.TB, ref string, csafDocs [][]byte) ([]Result, error)


I think if it uses the testing package in the exported API, it should be in the test/ hierarchy. That's a simple rule that helps keep the testing package out of our "real" API surface.

hdonnay · 2026-04-17T20:58:45Z

+	if len(cmp.Extras) > 0 {
+		t.Logf("note: scanner reported %d results not in fixture", len(cmp.Extras))
+		for _, e := range cmp.Extras {
+			t.Logf("extra: %s/%s (%s) [pkg: %s]", e.TrackingID, e.ProductID, e.Status, e.Package)


Suggested change

t.Logf("extra: %s/%s (%s) [pkg: %s]", e.TrackingID, e.ProductID, e.Status, e.Package)

t.Logf("EXTRA %s/%s: %s (package: %s)", e.TrackingID, e.ProductID, e.Status, e.Package)

This message format should probably be similar to the other "reporting" messages.

hdonnay · 2026-04-17T21:01:14Z

+			return nil, fmt.Errorf("indexer db connect: %w", err)
+		}
+	}
+	indexerStore, err := postgres.InitPostgresIndexerStore(ctx, indexerPool, true)


Use the migrations package directly.

hdonnay · 2026-04-17T21:14:08Z

+			return nil, fmt.Errorf("matcher db connect: %w", err)
+		}
+	}
+	matcherStore, err := postgres.InitPostgresMatcherStore(ctx, matcherPool, true)


Use the migrations package directly.

hdonnay · 2026-04-17T21:17:27Z

+	var errs []error
+	if a.indexer != nil {
+		a.indexer.Close(ctx)
+	}
+	if a.matcher != nil {
+		a.matcher.Close(ctx)
+	}
+	if a.cachedArena != nil {
+		if err := a.cachedArena.Close(ctx); err != nil {
+			errs = append(errs, err)
+		}
+	}
+	return errors.Join(errs...)


Suggested change

var errs []error

if a.indexer != nil {

a.indexer.Close(ctx)

}

if a.matcher != nil {

a.matcher.Close(ctx)

}

if a.cachedArena != nil {

if err := a.cachedArena.Close(ctx); err != nil {

errs = append(errs, err)

}

}

return errors.Join(errs...)

errs := make([]error, 3)

if a.indexer != nil {

errs[0] = a.indexer.Close(ctx)

}

if a.matcher != nil {

errs[1] = a.matcher.Close(ctx)

}

if a.cachedArena != nil {

errs[2] = a.cachedArena.Close(ctx)

}

return errors.Join(errs...)

Errors.Join filters out nil entries, so it's simpler to just catch all the possible errors.

hdonnay · 2026-04-17T21:21:41Z

+
+		// Detect RHEL VEX documents by publisher namespace and use the specialised parser.
+		// RHEL VEX has complex product relationships and CPE-based repository matching.
+		if c.Document.Publisher.Namespace == "https://www.redhat.com" {


Feels bad that we need special Red Hat handling :/

hdonnay · 2026-04-17T21:28:37Z

+}
+
+// SeverityFromCVSSScore converts a CVSS base score to claircore.Severity.
+func severityFromCVSSScore(score float64) claircore.Severity {


This seems like it's basically duplicating cvss.QualitativeScore?

hdonnay · 2026-04-17T21:32:03Z

+			continue
+		}
+		switch e.Status {
+		case fixtures.StatusUnknown:


Yeah, I'm not totally clear if "Unknown" and "Extra" are different concepts. Isn't an "Unknown" status just an "Extra" package?

hdonnay · 2026-04-17T21:33:33Z

+		want     *Comparison
+	}{
+		{
+			name: "all match affected",


The names of these subtests should be in PascalCase, so that go test's -run flag is consistent.

hdonnay · 2026-04-17T21:39:53Z

+// It maintains internal caches for repository and product lookups, so reusing
+// the same Parser instance across multiple documents is more efficient than
+// creating a new one for each document.
+type Parser struct {


I think the docs on the Parser type needs some work to explicitly call out that's it's caching the claircore representation (e.g. Vulnerabilities and Repositories) because I was briefly confused about caching CSAF objects, which are per-document.

crozzy force-pushed the acceptance-testing branch from 4fef4e2 to e80755d Compare April 8, 2026 22:40

crozzy commented Apr 8, 2026

View reviewed changes

crozzy force-pushed the acceptance-testing branch from e80755d to 2b9b4ea Compare April 8, 2026 22:58

crozzy commented Apr 8, 2026

View reviewed changes

crozzy force-pushed the acceptance-testing branch 4 times, most recently from f9f3cb6 to bb621ee Compare April 10, 2026 22:25

acceptance: add the acceptance testing framework

3ce44e3

The acceptance testing framework allows users to hook into claircore's integration testing by calling acceptance.Run() and providing an Auditor. Signed-off-by: crozzy <joseph.crosland@gmail.com>

crozzy force-pushed the acceptance-testing branch from bb621ee to 986a408 Compare April 10, 2026 22:30

crozzy marked this pull request as ready for review April 10, 2026 22:31

crozzy requested a review from a team as a code owner April 10, 2026 22:31

crozzy requested review from hdonnay and removed request for a team April 10, 2026 22:31

crozzy added 4 commits April 13, 2026 07:23

acceptance: add claircore auditor

7c77914

Implement the auditor interface back by claircore. Claircore functionality is called locally. Signed-off-by: crozzy <joseph.crosland@gmail.com>

acceptance: add go cmd for creating fixtures

993ebdd

This cmd can be used to push fixtures up to a container registry. This includes a c/image, VEX files and an expected results manifest. Fixtures can then be referenced in acceptance tests. Signed-off-by: crozzy <joseph.crosland@gmail.com>

docs: add section about acceptance testing

b81a34e

This documentation should serve as a guide for outside image maintainers (and potentially other scanning vendors) that choose to test how vulnerability ingestion, indexing and matching work. Signed-off-by: crozzy <joseph.crosland@gmail.com>

gobin: correct none-namespace bug in purl parsing

bb34ef3

Signed-off-by: crozzy <joseph.crosland@gmail.com>

crozzy force-pushed the acceptance-testing branch from 986a408 to bb34ef3 Compare April 13, 2026 14:23

hdonnay reviewed Apr 17, 2026

View reviewed changes

hdonnay requested changes Apr 17, 2026

View reviewed changes

	t.Logf("extra: %s/%s (%s) [pkg: %s]", e.TrackingID, e.ProductID, e.Status, e.Package)
	t.Logf("EXTRA %s/%s: %s (package: %s)", e.TrackingID, e.ProductID, e.Status, e.Package)

Conversation

crozzy commented Apr 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hdonnay Apr 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

hdonnay Apr 17, 2026 •

edited

Loading