-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add openSUSE/SLES support #199
Conversation
@hi, This WIP is truly exciting. Thanks for contributing!
Out of curiosity, do you know how often openSUSE/SLES base images are used? |
The official |
I really can't tell you how often it gets used. But at least for SLES it will increase for sure. I would like to hook Clair into Portus for automated vulnerability checks. |
We've got Hackweek now at SUSE and @jordimassaguerpla wants to finish this PR. |
I will refactor now by extracting common code first from sle and opensuse and then from redhat and suse. |
c5833fc
to
7e1d26f
Compare
I am going to rebase all in one commit |
9bd6acf
to
502ef61
Compare
@Quentin-M : What do you think how it looks now? I am not very good with Go, so feel free to give me any advice . I still have tomorrow and can also work on that on my spare time next week. thanks in advance |
@@ -0,0 +1,306 @@ | |||
// Copyright 2015 clair authors |
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.
That should be part of utils and not fetcher
The variable names are not really go-like |
can you be more concrete on the variable names? |
Sure, |
@jordimassaguerpla hopefully you can fix the remaining NITPICKing? Afterwards @Quentin-M should be happy for sure to merge it :) |
@guypod what are you talking about? |
Oops, my bad - too many similar looking GitHub tabs open ;) Deleted my comment to avoid future confusion. |
sure. Sorry for the delay, hackweek ended and I didn't find the time to do it.I was refactoring the rhel code to use the oval parser and that is why I had not sent it yet. I'll do it asap. |
Hi! I just refactored everything. I extracted the oval parser from rhel and use that for suse and opensuse. Closing this one. |
mm. I think can't close this one. Thomas can you? |
Why closing this one? Just rebase and squash commits? :) |
45afaf2
to
ddef422
Compare
ok :-) Branch rebased. I am closing the other one |
if err != nil { | ||
return resp, err | ||
} | ||
firstRHSA, err := strconv.Atoi(flagValue) |
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.
The tests pass but this thing of getting the firstRHSA from the flagValue ... not sure If I extracted this correctly or if I even understood what this was for .... Can someone else just take a look and see if I missed something here? Thanks and sorry I if did.
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.
This ensures that Clair doesn't try to re-parse RHSAs that have already been processed and stored - in order to make the updates faster.
var featureVersionParametersArray []database.FeatureVersion | ||
|
||
for _, fv := range featureVersionParameters { | ||
featureVersionParametersArray = append(featureVersionParametersArray, fv) |
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.
Similar to the other recommendation you can use 1 append and the splat
featureVersionParametersArray = append(featureVersionParametersArray, featureVersionParamters...)
Is there a reason you're explicitly making a copy here?
Thanks for the PR! This is a really useful refactor of the oval stuff into a shared package. I mostly commented on code that was already there, but simply moved. |
f91d1c2
to
a36f994
Compare
@jzelinskie : I've done all the changes you suggested. |
Just that last tiny thing of changing the updater flag to have an underscore and then it LGTM! Thanks a bunch for all your effort, this change is awesome! |
Cool to see it's nearby getting merged 👏 |
134e513
to
b8ceb0c
Compare
We extracted oval parser from rhel and used that for opensuse and SUSE Linux Enterpise Signed-off-by: Thomas Boerger <tboerger@suse.de> Signed-off-by: Jordi Massaguer Pla <jmassaguerpla@suse.de>
I just did the change on the updated flag and rebased everything so it looks cleaner when you merge. Thanks a lot for taking the time to review it :) ! I am very excited to be contributing :) ! |
Is this completed to integrate clair with SLES Updater. |
This change actually got reverted because I couldn't ever get an answer from SUSE's legal department about the license of the Vulnerability data. |
Hello from SUSE, I'm a member of the team that wrote this PR. What kind of legal clearance are you looking for and who have you contacted? Pinging also @msmeissn from SUSE's security team. He might be able to help with your questions about the usage of the vulnerability data. |
Hi @flavio |
I am sorry if I failed to answer, but I got the answer from my legal department. https://www.suse.com/de-de/support/security/oval/ The OVAL data is provided by SUSE under the Creative Commons License 4.0 with Attribution for Non-Commercial usage (CC-BY-NC-4.0). I hope this is satisfying. |
@jzelinskie, is this expected to be merged again at some point now? |
@Rots clair's code changed and merging the reverted code again is not possible. I'm working on that and I'll file a PR in the next weeks (I'm a bit busy ATM). |
@flavio looking forwarding to the progress on this merging. Great appreciate! |
Looking forwarding to the progress, too! |
1 similar comment
Looking forwarding to the progress, too! |
Hi, Thanks |
In order to integrate openSUSE/SLES including the available patches for CVEs I have started to work on this pull request. I already opened this pull request to get feedback early as possible. Below you can see a checklist for the remaining items.
Do I miss something? Should we abstract the shared functionality from RedHat and openSUSE/SLES?