Skip to content
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

Redhatrelease detector #229

Merged
merged 3 commits into from
Sep 6, 2016
Merged

Conversation

vbatts
Copy link
Contributor

@vbatts vbatts commented Aug 12, 2016

Due to the detector registration and fact that their in a non-ordered
map, it is random whether the osrelease or redhatrelease detector would
hit. And likely resulted in alternately formatted namespace strings.

This change causes the osrelease to not detect when data has
centos-release or redhat-release, which is not great because if the
redhatrelease detector is not compiled in, then that would not be a
fallback that the osrelease detector could rely on. :-\

Until #193 is merged, having
vulnerabilities that are tagged both rhel and centos would duplicate in
the database or use a change that requires a migration.

But presently due to the fetcher logic, the rhel provided
vulnerabilities are labelled for centos, and then the namespace does not
match and therefore not tested against.

So until such a day that a vulnerability could have both rhel and centos
label, then hack this in. It'll accomplish the same during this interim.

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
Due to the detector registration and fact that their in a non-ordered
map, it is random whether the osrelease or redhatrelease detector would
hit. And likely resulted in alternately formatted namespace strings.

This change causes the osrelease to not detect when data has
centos-release or redhat-release, which is not _great_ because if the
redhatrelease detector is not compiled in, then that would not be a
fallback that the osrelease detector could rely on. :-\

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
Until quay#193 is merged, having
vulnerabilities that are tagged both rhel and centos would duplicate in
the database or use a change that requires a migration.

But presently due to the fetcher logic, the rhel provided
vulnerabilities are labelled for centos, and then the namespace does not
match and therefore not tested against.

So until such a day that a vulnerability could have both rhel and centos
label, then hack this in. It'll accomplish the same during this interim.

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
@vbatts
Copy link
Contributor Author

vbatts commented Aug 31, 2016

bump

@jzelinskie
Copy link
Contributor

LGTM. Is this causing enough of an issue that it should be backported into stable?

@vbatts
Copy link
Contributor Author

vbatts commented Aug 31, 2016

Oh that is an idea. Let me check

On Wed, Aug 31, 2016, 18:44 Jimmy Zelinskie notifications@github.com
wrote:

LGTM. Is this causing enough of an issue that it should be backported into
stable?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#229 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEF6QkGNFwSK-Wp9Hwd6OAk9pM8IaPnks5qlgOzgaJpZM4JjZYs
.

@brianwcook
Copy link

@jzelinskie Any idea when the next release will occcur that would include this if it isn't backported?

@jzelinskie
Copy link
Contributor

jzelinskie commented Sep 1, 2016

@brianwcook realistically on the order of months. We need to spend some time to making sure we get the multiple-namespace support in properly and make a few other breaking changes to clean things up. Doing so requires a large amount of thought around the future of project's architecture and our maintainers currently have a lot on their plates with other projects.

Backporting should be a simple git cherry-pick, so I'm totally fine with getting useful changes like this into our current stable version.

@brianwcook
Copy link

@jzelinskie in that case, then yes it would be great to have it backported to stable!

@jzelinskie jzelinskie merged commit 80870bf into quay:master Sep 6, 2016
jzelinskie added a commit that referenced this pull request Sep 6, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lacking/review needs to be reviewed by a maintainer
Development

Successfully merging this pull request may close these issues.

3 participants