feat: add kyverno ecr pull requirement policy#605
Conversation
There was a problem hiding this comment.
I like it. My main concern here is that it doesn't act on resources that are abstractions of Pods (Deployments, StatefullSets, ...), enforcement is "soft" and that this a production exclusive feature, making it hard to catch our own mistakes before being potentially blocked in production. But you are already aware of the latter, so no changes required on that one.
This reverts commit da97484.
|
I have migrated this over to a ValidatingPolicy. I cannot get Autogen to work with ValdatingPolicy with our current version of Kyverno so I have been explicit about the resources. I also cannot recreate the functionality where it would warn if the check service is unavailable which I do not like, now it just fails silently. But we did agree to not block if the service is unavailable. The only workaround for this I can think of is to add an extra policy specifically checking for the availability of that service and warn if it cannot check. Thoughts now @wcarlsen @SEQUOIIA? |
| resources: | ||
| - deployments | ||
| - statefulsets | ||
| - daemonsets |
There was a problem hiding this comment.
Teams cannot deploy DaemonSets so do we really need this? If not I don't think we need it here.
There was a problem hiding this comment.
Yeah I agree here, I've taken it out
| ).allowed == true | ||
| ) | ||
| message: >- | ||
| One or more container images reference an ECR repository that does |
There was a problem hiding this comment.
Can messages use the info from above expression? So that the user knows which image is violating.
There was a problem hiding this comment.
Good suggestion, seems simpler here than the previous type. I have added it
wcarlsen
left a comment
wcarlsen
left a comment
There was a problem hiding this comment.
Really on the inclusion of DaemonSet needs attention. The stuff on messages is just icing on the cake and not a must.
I strongly prefer this simplified implementation, where there the status evaluation is more binary than the previous implementation.
If any exporters fetches data from Kyverno we need to validate that this works with this new resource.
Also I'm no fan of the non-blocking thingy, which I think defeats the purpose of admission. Failing silently have never been great. But that is a different discussion.
wcarlsen
left a comment
wcarlsen
left a comment
There was a problem hiding this comment.
Looking really nice. Great work
3 participants
This PR adds a kyverno policy for our ECR Pull Policy requirement
It will emit a warning if the service that ecr-pull-compliance-exporter service is not available (as will be the case in environments other than prod), but ultimately not fail even when our policies are changed from warn to enforce