Simplify v6 distribution material #2277
Conversation
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
wagoodman
force-pushed
the
v6-adjustments
branch
from
6c41fd6
to
ba4a2b4
Compare
wagoodman
added
the
changelog-ignore
| } | ||
|
|
||
| // calculate the sh256sum of the archive | ||
| checksum, err := db.CalculateArchiveDigest(cfg.DBFilePath()) |
There was a problem hiding this comment.
Should the parameter here be path not cfg.DBFilePath()? It seems weird t calculate the archive digest by doing anything besides hashing the tar.
| @@ -67,6 +73,43 @@ func NewLatestFromReader(reader io.Reader) (*LatestDocument, error) { | |||
| return &l, nil | |||
| } | |||
|
|
|||
| func NewArchive(path string) (*Archive, error) { | |||
There was a problem hiding this comment.
Is this constructor still necessary if the suggestion at https://github.com/anchore/grype-db/pull/437/files#r1857293906 is implemented?
| @@ -57,6 +60,11 @@ const ( | |||
| NotAffectedFixStatus FixStatus = "not-affected" | |||
| ) | |||
|
|
|||
| const ( | |||
| // AdvisoryReferenceTag is a reference to a vulnerability advisory | |||
| AdvisoryReferenceTag string = "advisory" | |||
There was a problem hiding this comment.
Should there be more of these?
There was a problem hiding this comment.
A reference tag is a string we expect to be used often to tag a reference in https://github.com/anchore/grype/blob/main/grype/db/v6/blobs.go#L51.
What's an Advisory exactly? For example, a Red Hat Security Advisory (RHSA) is a list of patches, but a GitHub Security Advisory is a vulnerability description. Is advisory the right tag name? What kind of links or going to be tagged with this tag name?
| var persistentErr error | ||
| digest, persistentErr := db.CalculateDBDigest(c.config.DBFilePath()) | ||
|
|
||
| if validateErr := c.validate(); validateErr != nil { |
There was a problem hiding this comment.
How did the digest from line 106 get into c.validate()? Are we calculating the digest twice?
| @@ -301,13 +313,13 @@ func (c curator) activate(dbDirPath string, mon monitor) error { | |||
|
|
|||
| // overwrite the checksums file with the new checksums | |||
| dbFilePath := filepath.Join(dbDirPath, db.VulnerabilityDBFileName) | |||
| desc, err := db.CalculateDescription(dbFilePath) | |||
| digest, err := db.CalculateDBDigest(dbFilePath) | |||
There was a problem hiding this comment.
Add to method dc comment and calculates the db checksum and writes the checksum file.
| @@ -14,7 +14,7 @@ func NewDB(dbFilePath string, models []any, truncate bool) (*gorm.DB, error) { | |||
| return nil, err | |||
| } | |||
|
|
|||
| if len(models) > 0 { | |||
| if len(models) > 0 && truncate { | |||
There was a problem hiding this comment.
This line, and probably this method, could use a comment that we have only 2 modes of interacting with the DB which shouldn't mix:
- Read mode for Grype at runtime should not migrate, vacuum, etc to avoid changing checksum or other corruption
- Writing via grype-db should truncate the path to an empty db, apply all migration, migrate.
| log.Trace("fetching all provider records") | ||
|
|
||
| var providers []Provider | ||
| result := s.db.Find(&providers) |
There was a problem hiding this comment.
Does this need any sort of ordering?
| fh, err := os.OpenFile(filepath.Join(s.config.DBDirPath, ChecksumFileName), os.O_CREATE|os.O_TRUNC|os.O_WRONLY, 0644) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to open description file: %w", err) | ||
| } | ||
|
|
||
| return WriteChecksums(fh, *desc) | ||
| return WriteChecksums(fh, digest) |
There was a problem hiding this comment.
Might be worth a comment or logging like "no-op on read-only store"
| } | ||
|
|
||
| return stderrPrintLnf("Vulnerability database imported") | ||
| } | ||
|
|
||
| func isLegacy(path string) bool { | ||
| return !strings.Contains(filepath.Base(path), "vulnerability-db_v6") |
There was a problem hiding this comment.
Since a rename might make this return true when it shouldn't, it might be worth inverting to isNew for now. It might be nice to have a config or a less brittle check or something at some point.
This PR makes adjustments to the v6 schema + supporting code from previous PRs, specifically:
latest.jsonfile flat (no nested objects)This PR also introduces the ability to import both v6 and v5 schemas at once (since import now functions in both contexts) which requires disabling the static analysis check for
cross-import of schemas.