Add validating webhooks for Egress and ExternalIPPool #2358
Conversation
|
/test-all |
Codecov Report
@@ Coverage Diff @@
## main #2358 +/- ##
==========================================
+ Coverage 59.79% 64.37% +4.57%
==========================================
Files 283 284 +1
Lines 22118 22215 +97
==========================================
+ Hits 13226 14300 +1074
+ Misses 7479 6432 -1047
- Partials 1413 1483 +70
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/test-all |
| switch review.Request.Operation { | ||
| case admv1.Create: | ||
| klog.V(2).Info("Validating CREATE request for ExternalIPPool") | ||
| // Always allow CREATE request. |
There was a problem hiding this comment.
maybe the comment should say that this shouldn't happen with the webhook configuration we include the in Antrea YAML manifests?
| expectedResponse *admv1.AdmissionResponse | ||
| }{ | ||
| { | ||
| name: "CREATE: Requesting IP from non-existing ExternalIPPool should not be allowed", |
There was a problem hiding this comment.
why the weird formatting for these testcase names?
There was a problem hiding this comment.
I wanted to make names different for CREATE and UPDATE operations when both of them are requesting invalid IPs.
I have updated the UPDATE case to "Updating to ...".
| expectedResponse: &admv1.AdmissionResponse{Allowed: true}, | ||
| }, | ||
| { | ||
| name: "UPDATE: Requesting IP out of range should be allowed", |
There was a problem hiding this comment.
Done, thanks for catching it.
|
LGTM. Perhaps you want to resolve Antonin comments |
The webhook for Egress validates that the EgressIP must be within the IP ranges of the ExternalIPPool if both are set. The webhook for ExternalIPPool validates that the ExternalIPPool must not shrink. Signed-off-by: Quan Tian <qtian@vmware.com>
There was a problem hiding this comment.
@antoninbas @abhiraut thanks for the review. I have addressed all comments, PTAL.
| switch review.Request.Operation { | ||
| case admv1.Create: | ||
| klog.V(2).Info("Validating CREATE request for ExternalIPPool") | ||
| // Always allow CREATE request. |
| expectedResponse *admv1.AdmissionResponse | ||
| }{ | ||
| { | ||
| name: "CREATE: Requesting IP from non-existing ExternalIPPool should not be allowed", |
There was a problem hiding this comment.
I wanted to make names different for CREATE and UPDATE operations when both of them are requesting invalid IPs.
I have updated the UPDATE case to "Updating to ...".
| expectedResponse: &admv1.AdmissionResponse{Allowed: true}, | ||
| }, | ||
| { | ||
| name: "UPDATE: Requesting IP out of range should be allowed", |
There was a problem hiding this comment.
Done, thanks for catching it.
| expectedResponse *admv1.AdmissionResponse | ||
| }{ | ||
| { | ||
| name: "Requesting IP from non-existing ExternalIPPool should not be allowed", |
There was a problem hiding this comment.
my question was more about the large whitespace between name: and the string, which you don't have above
There was a problem hiding this comment.
I get it now but I'm not sure the reason. make fmt does it even I remove the spaces. Maybe it has a rule about in which case keeping align is desired. Perhaps keeping align is not desired when the value is a composite literal.
|
/test-all |
|
/test-networkpolicy |
The webhook for Egress validates that the EgressIP must be within the IP
ranges of the ExternalIPPool if both are set.
The webhook for ExternalIPPool validates that the ExternalIPPool must
not shrink.
For #2128
Signed-off-by: Quan Tian qtian@vmware.com