support multiple namespaces in one layer #193
Conversation
Signed-off-by: liang chenye <liangchenye@huawei.com>
Signed-off-by: liang chenye <liangchenye@huawei.com>
Quentin-M
added
kind/design
|
Hi, Thanks for your work here! About (1), I understand why you encrypt the namespace IDs in the API as it is preferred not to leak database IDs. However, I believe that usually, the list of Namespaces is pretty much stable as it depends mainly on the registered detectors/updaters (and not on the number of images we analyzed). I am not sure if we should make this trade-off at this level between confidentiality and readability/usability. We shall make a decision and stick to it for any later development. I'd ping @jzelinskie as he wrote the API. About (5), is there anything that led you to use the |
| @@ -220,7 +220,7 @@ The GET route for the Vulnerabilities resource displays the vulnerabilities data | |||
| ###### Example Request | |||
|
|
|||
| ```json | |||
| GET http://localhost:6060/v1/namespaces/debian%3A8/vulnerabilities?limit=2 HTTP/1.1 | |||
| GET http://localhost:6060/v1/namespaces/gAAAAABXTQKgma_TLKq0wr1D6wVB507N3fi9wsWMUypYOSXTDVxQ8OR5L5S6PqZ9Wh0IzWojnVmlspyTL4cyjytyra7U9vAHMA==/vulnerabilities?limit=2 HTTP/1.1 | |||
There was a problem hiding this comment.
I'd rather this be something along the lines of /v1/namespaces/debian/versions/9/vulnerabilities?limit=2 rather than using the encrypted token. Obviously, it'll be slower on the database if we don't already have the ID, but I'd rather make that sacrifice for a sane API.
There was a problem hiding this comment.
OK, so the other APIs could also be implement in this way.
I'll use ':nsname/version/:nsversion' in all the related APIs.
You are right. My original idea is we need to compare versions between a layer and its parent. But since parent's namespace version will be covered so a 'string' is sufficient. |
|
@liangchenye is this ready for review? Did you change the API? |
|
@jzelinskie no, I planed to rebase this after #175 been merged. |
|
@liangchenye looking forward to this getting merged! 😄 |
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>
|
This PR is now extremely out of date, but did inspire the design decisions which does implement this behavior in #432. |
Until quay/clair#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>
This implementation mainly adds a 'version' field to database.namespace, this change affects the following things:
Since namespace.Name could not represent a namespace, I use the encoded namespace.ID in Vulnerabilities API and Fixes API
Use namespaceID to add/remove/change vulnerabilities.
using sql migration file
detectors
for example change debian:8 to {Name: "debian", Version: "8"}
'unstable' is illegal as a version content, change it to '10'.
Close the previous PR since that 'head fork' became 'unknown'.