-
Notifications
You must be signed in to change notification settings - Fork 386
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Switch traceflow CRD validation to webhook validation.
Currently, the traceflow CRD validation is executed in run-time, which is less user-friendly than the webhook validation. I moved most of the validation to the webhook validation. Signed-off-by: shi0rik0 <anguuan@outlook.com>
- Loading branch information
Showing
6 changed files
with
208 additions
and
41 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| package traceflow | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "net" | ||
|
|
||
| crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" | ||
| "antrea.io/antrea/pkg/features" | ||
| "antrea.io/antrea/pkg/util/k8s" | ||
| admv1 "k8s.io/api/admission/v1" | ||
| apierrors "k8s.io/apimachinery/pkg/api/errors" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/klog/v2" | ||
| ) | ||
|
|
||
| func (c *Controller) Validate(review *admv1.AdmissionReview) (resp *admv1.AdmissionResponse) { | ||
| allowed := true | ||
| deniedReason := "" | ||
| var err error | ||
|
|
||
| defer func() { | ||
| if err == nil { | ||
| resp = &admv1.AdmissionResponse{ | ||
| UID: review.Request.UID, | ||
| Allowed: allowed, | ||
| Result: &metav1.Status{ | ||
| Message: deniedReason, | ||
| }, | ||
| } | ||
| } else { | ||
| resp = &admv1.AdmissionResponse{ | ||
| UID: review.Request.UID, | ||
| Allowed: false, | ||
| Result: &metav1.Status{ | ||
| Message: err.Error(), | ||
| }, | ||
| } | ||
| } | ||
| }() | ||
|
|
||
| klog.V(2).InfoS("Validating Traceflow", "request", review.Request) | ||
|
|
||
| var newObj, oldObj crdv1alpha1.Traceflow | ||
| if review.Request.Object.Raw != nil { | ||
| if err := json.Unmarshal(review.Request.Object.Raw, &newObj); err != nil { | ||
| klog.ErrorS(err, "Error de-serializing current Traceflow") | ||
| return | ||
| } | ||
| } | ||
| if review.Request.OldObject.Raw != nil { | ||
| if err := json.Unmarshal(review.Request.OldObject.Raw, &oldObj); err != nil { | ||
| klog.ErrorS(err, "Error de-serializing old Traceflow") | ||
| return | ||
| } | ||
| } | ||
|
|
||
| switch review.Request.Operation { | ||
| case admv1.Create: | ||
| klog.V(2).InfoS("Validating CREATE request for Traceflow") | ||
| allowed, deniedReason = c.validateCreate(&newObj) | ||
| case admv1.Update: | ||
| klog.V(2).InfoS("Validating UPDATE request for Traceflow") | ||
| allowed, deniedReason = c.validateUpdate(&oldObj, &newObj) | ||
| case admv1.Delete: | ||
| // This shouldn't happen with the webhook configuration we include in the Antrea YAML manifests. | ||
| klog.V(2).InfoS("Validating DELETE request for Traceflow") | ||
| // Always allow DELETE request. | ||
| } | ||
| return | ||
| } | ||
|
|
||
| func (c *Controller) validateCreate(tf *crdv1alpha1.Traceflow) (allowed bool, deniedReason string) { | ||
| if tf.Spec.Destination.Service != "" && !features.DefaultFeatureGate.Enabled(features.AntreaProxy) { | ||
| return false, "using Service destination requires AntreaProxy feature enabled" | ||
| } | ||
| if tf.Spec.Destination.IP != "" { | ||
| destIP := net.ParseIP(tf.Spec.Destination.IP) | ||
| if destIP == nil { | ||
| return false, fmt.Sprintf("destination IP is not valid: %s", tf.Spec.Destination.IP) | ||
| } | ||
| } | ||
| if !tf.Spec.LiveTraffic { | ||
| srcPod, err := c.podLister.Pods(tf.Spec.Source.Namespace).Get(tf.Spec.Source.Pod) | ||
| if err != nil { | ||
| if apierrors.IsNotFound(err) { | ||
| err = fmt.Errorf("requested source Pod %s not found", k8s.NamespacedName(tf.Spec.Source.Namespace, tf.Spec.Source.Pod)) | ||
| } | ||
| return false, err.Error() | ||
| } | ||
| if srcPod.Spec.HostNetwork { | ||
| return false, "using hostNetwork Pod as source in non-live-traffic Traceflow is not supported" | ||
| } | ||
| } | ||
| if tf.Spec.Source.Pod == "" && tf.Spec.Destination.Pod == "" { | ||
| return false, fmt.Sprintf("Traceflow %s has neither source nor destination Pod specified", tf.Name) | ||
| } | ||
| return true, "" | ||
| } | ||
|
|
||
| func (c *Controller) validateUpdate(oldTf, newTf *crdv1alpha1.Traceflow) (allowed bool, deniedReason string) { | ||
| return c.validateCreate(newTf) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| package traceflow | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
|
|
||
| crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| admv1 "k8s.io/api/admission/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/runtime" | ||
| ) | ||
|
|
||
| func TestControllerValidateTraceflow(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
|
|
||
| // input | ||
| oldSpec *crdv1alpha1.TraceflowSpec | ||
| newSpec *crdv1alpha1.TraceflowSpec | ||
|
|
||
| // expected output | ||
| allowed bool | ||
| deniedReason string | ||
| }{ | ||
| { | ||
| name: "Validating destination IP", | ||
| newSpec: &crdv1alpha1.TraceflowSpec{ | ||
| Destination: crdv1alpha1.Destination{IP: "foobar"}, | ||
| }, | ||
| deniedReason: "destination IP is not valid: foobar", | ||
| }, | ||
| { | ||
| name: "Traceflow should have either source or destination Pod assigned", | ||
| newSpec: &crdv1alpha1.TraceflowSpec{ | ||
| LiveTraffic: true, | ||
| }, | ||
| deniedReason: "Traceflow tf has neither source nor destination Pod specified", | ||
| }, | ||
| } | ||
| for _, tc := range tests { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| stopCh := make(chan struct{}) | ||
| defer close(stopCh) | ||
| controller := newController() | ||
|
|
||
| var request *admv1.AdmissionRequest | ||
| if tc.oldSpec != nil && tc.newSpec != nil { | ||
| request = &admv1.AdmissionRequest{ | ||
| Operation: admv1.Update, | ||
| OldObject: toRawExtension(tc.oldSpec), | ||
| Object: toRawExtension(tc.newSpec), | ||
| } | ||
| } else if tc.newSpec != nil { | ||
| request = &admv1.AdmissionRequest{ | ||
| Operation: admv1.Create, | ||
| Object: toRawExtension(tc.newSpec), | ||
| } | ||
| } else { | ||
| t.Fatal("invalid test case") | ||
| } | ||
| review := &admv1.AdmissionReview{ | ||
| Request: request, | ||
| } | ||
|
|
||
| expectedResponse := &admv1.AdmissionResponse{ | ||
| Allowed: tc.allowed, | ||
| Result: &metav1.Status{ | ||
| Message: tc.deniedReason, | ||
| }, | ||
| } | ||
|
|
||
| response := controller.Validate(review) | ||
| assert.Equal(t, expectedResponse, response) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func toRawExtension(spec *crdv1alpha1.TraceflowSpec) runtime.RawExtension { | ||
| tf := &crdv1alpha1.Traceflow{Spec: *spec} | ||
| tf.Name = "tf" | ||
| raw, err := json.Marshal(tf) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| return runtime.RawExtension{Raw: raw} | ||
| } |