-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: search groupID by artifactID and version #23
refactor: search groupID by artifactID and version #23
Conversation
pkg/db/db.go
Outdated
} | ||
return indexes, nil | ||
return groupID, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we need to return index
here.
Looks like GroupID
will be enough.
pkg/db/db.go
Outdated
SELECT relevant.group_id, COUNT(relevant.group_id) as count | ||
FROM indices i | ||
JOIN (SELECT a.id, a.group_id | ||
FROM indices i | ||
JOIN artifacts a on a.id = i.artifact_id | ||
WHERE a.artifact_id = ? AND i.version = ? AND i.archive_type = ?) relevant ON relevant.id = i.artifact_id | ||
GROUP BY "group_id"`, | ||
artifactID, version, fileType) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, we need to add one more SELECT
to get maximum value from count
in sql query.
I'm not sure we need this.
I simply added logic to select the maximum quantity in rows.Next()
loop.
pkg/db/db.go
Outdated
FROM indices i | ||
JOIN artifacts a on a.id = i.artifact_id | ||
WHERE a.artifact_id = ? AND i.version = ? AND i.archive_type = ?) relevant ON relevant.id = i.artifact_id | ||
GROUP BY "group_id"`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need GROUP BY
here? I want to keep the logic in trivy-java-db as minimal as possible. That's why this function returns indexes
.
We can match the version in SQL as it is simple enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, what if the latest version is used in the project? The database probably doesn't have it yet as the database is downloaded once a week, and Trivy will miss it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need GROUP BY here? I want to keep the logic in trivy-java-db as minimal as possible. That's why this function returns indexes.
I updated query - 5c30667
take a look, please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
database is downloaded once a week
Do you mean trivy-java-db?
We update trivy-java-db every day:
- cron: "0 0 * * *" # update indexes every day in 00:00 |
Users download java-db every 3 days:
trivy-java-db/pkg/builder/builder.go
Line 20 in 15bfe3c
const updateInterval = time.Hour * 72 // 3 days |
But i understand your point.
I think it is possible. We can get wrong GroupID (if version is not added).
But i am not sure that our changes will cause many issues:
- after 1(3) days user will still see new package (perhaps we need to add info in Trivy docs about need to update trivy-java-db in this case).
- we check
pom.properties
andMANIFEST.MF
files before accessing java-db.
@knqyf263 can we merge this PR? |
Description
Change logic to search
GroupID
byArtifactID
:GroupiD
GroupID
(we need to choose onlyGroupIDs
which contain required version)See aquasecurity/trivy/issues/5627 for more info.