Allow images to be excluded from scanning #1659
Conversation
- add registry-exclude-image flag that accepts a list of glob expressions - exclude Kubernetes system images by default (k8s.gcr.io/*)
- add tests for image filtering
|
just wondering, when is it desirable to not scan images in a registry? |
|
@stephenmoloney see @alanjcastonguay comment on #1654 |
|
@squaremo should we exclude |
No; people routinely automate those (e.g., Weaveworks). |
|
@squaremo I've addressed your comments in the latest commits and I've also made changes to the docs. Let me know if I've missed something. Thanks |
There was a problem hiding this comment.
Cool, this looks like it'll work :-) I made some suggestions for tightening up the code; you can take or leave those (if you leave them, also leave a comment in response..)
Can you rebase out some of those unrelated documentation changes (re Helm and so on) please -- I'm happy to stamp another PR with them.
cluster/kubernetes/images_test.go
Outdated
| @@ -66,11 +67,51 @@ func TestMergeCredentials(t *testing.T) { | |||
| client := extendedClient{clientset, nil} | |||
|
|
|||
| creds := registry.ImageCreds{} | |||
| mergeCredentials(noopLog, client, ns, spec, creds, make(map[string]registry.Credentials)) | |||
| cluster := NewCluster(clientset, nil, nil, nil, log.NewNopLogger(), []string{}, []string{}) | |||
There was a problem hiding this comment.
Hrmm, it's a bit unwieldy that you have to create a cluster in order to filter the images. Maybe construct the filter independently of the cluster.
There was a problem hiding this comment.
That way you can make it a no-op if there are no exclusions, and you can build the list of included images in one step in mergeCredentials (rather than building a list of all of them, then filtering it down).
There was a problem hiding this comment.
What I meant is this: given a predicate includeImage(imageName) bool, you can step through a pod template, testing each image extracted against the predicate as you go. There's no need to build the list of images, then run through it again to filter it.
How do you get such a predicate? You can just construct it -- you have the algorithm right here -- and it need not depend on the cluster, since all that's doing is providing the list of exclusions.
stefanprodan
force-pushed
the
registry-filter
branch
2 times, most recently
from
08b4429
to
cf5c238
Compare
|
@squaremo I've removed the docs changes from this PR and I've addressed your comments in the latest commit. As for making the tests work without cluster I'm not really sure how to do that without adding the exclusion list on all functions... I would leave that for another PR if it's ok with you. |
cluster/kubernetes/images_test.go
Outdated
| @@ -66,11 +67,51 @@ func TestMergeCredentials(t *testing.T) { | |||
| client := extendedClient{clientset, nil} | |||
|
|
|||
| creds := registry.ImageCreds{} | |||
| mergeCredentials(noopLog, client, ns, spec, creds, make(map[string]registry.Credentials)) | |||
| cluster := NewCluster(clientset, nil, nil, nil, log.NewNopLogger(), []string{}, []string{}) | |||
There was a problem hiding this comment.
What I meant is this: given a predicate includeImage(imageName) bool, you can step through a pod template, testing each image extracted against the predicate as you go. There's no need to build the list of images, then run through it again to filter it.
How do you get such a predicate? You can just construct it -- you have the algorithm right here -- and it need not depend on the cluster, since all that's doing is providing the list of exclusions.
cluster/kubernetes/images.go
Outdated
| imageCreds registry.ImageCreds, | ||
| seenCreds map[string]registry.Credentials) { | ||
| // filter the images based on the exclusion list | ||
| images := filterImages(podTemplate) |
There was a problem hiding this comment.
All that happens to filterImages is that it's applied to one of the other arguments -- you may as well supply the result.
|
That last commit is a nice refinement 👍 |
k8s.gcr.io/*Fix #1654 images can be excluded by registry domain eg.:
--registry-exclude-image=gcr.io/*,quay.io/*Fix #1509 it is now possible to disable the image cache completely with:
--registry-exclude-image=*