Additional Pod Controller Scans #166
Conversation
- Refactored the way controllers work to be an interface - Added configurable controllers to include in scans - Added daemonsets, jobs and cronjobs in scans Explicitly did not add ReplicationControllers and ReplicaSets since they are either deprecated or controlled by deployments (generally). - Added tests to supportedController scanner
pkg/webhook/validator.go
Outdated
| if yes := v.Config.CheckIfKindIsConfiguredForValidation(req.AdmissionRequest.Kind.Kind); !yes { | ||
| logrus.Warnf("Skipping, kind (%s) isn't something we are configured to scan", req.AdmissionRequest.Kind.Kind) | ||
| // FIXME: Should we be returning an OK response as skipped? | ||
| return admission.ErrorResponse(http.StatusBadRequest, err) |
There was a problem hiding this comment.
Yes, I think for the webhook this should be an OK response for unsupported types. Otherwise (IIUC) if a new controller type is introduced, we'll reject it until Polaris gets updated to support it.
There was a problem hiding this comment.
Still seeing the FIXME here, and looks like it's still StatusBadRequest.
There was a problem hiding this comment.
I think maybe it was a bad browser cache on your side? I am not seeing it
|
You'll need to update the RBAC definitions in the chart to regenerate everything in the |
For sure, I saw that on your PR for the limit ranges. This is a WIP and I wanted to validate this isn't too over the top or out of line with the requested functionality. |
|
FYI, in #107 someone mentions that |
Codecov Report
@@ Coverage Diff @@
## master #166 +/- ##
==========================================
- Coverage 85.01% 82.84% -2.18%
==========================================
Files 9 10 +1
Lines 534 647 +113
==========================================
+ Hits 454 536 +82
- Misses 58 87 +29
- Partials 22 24 +2
Continue to review full report at Codecov.
|
- Added `ReplicationController` type controllers to the supported list - Adjusted logic for failed YAML parsing to bubble up errors - Added better logic for calculating summaries on cluster wide results - Relocated responsibilities for counting types into validators vs spreading it around more packages - Fixed bug where cronjob parsing was using wrong KIND - Added fixtures for mocking new controller types - Added example yamls to test scanning files - Added functions to NamespacedResult(s) to reduce code complexity deep set iterations - Refactored how results get added to namespacedresults so adding more later is easier - Minor signature changes for interface implementing structs for controllers
|
@bobby-brennan: I've incorporated the following (per request)
|
|
Looking great Nick! A few comments but I think this is just about ready. I really like using the interface for different controller types, makes a lot more sense than what I had. As a side note, we should start testing the webhook for correctness and feature parity. Opened #178 to track. |
pkg/webhook/validator.go
Outdated
| if yes := v.Config.CheckIfKindIsConfiguredForValidation(req.AdmissionRequest.Kind.Kind); !yes { | ||
| logrus.Warnf("Skipping, kind (%s) isn't something we are configured to scan", req.AdmissionRequest.Kind.Kind) | ||
| // FIXME: Should we be returning an OK response as skipped? | ||
| return admission.ErrorResponse(http.StatusBadRequest, err) |
There was a problem hiding this comment.
Still seeing the FIXME here, and looks like it's still StatusBadRequest.
- Added requested var name changes - Fixed Fairwinds typo - Adjusted the logic for finding key in map
Explicitly did not add ReplicationControllers and ReplicaSets since they are either deprecated or controlled by deployments (generally).