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>

build/charts/antrea/templates/webhooks/validating/crdvalidator.yaml

@@ -141,3 +141,18 @@ webhooks:
     admissionReviewVersions: ["v1", "v1beta1"]
     sideEffects: None
     timeoutSeconds: 5
+  - name: "traceflowvalidator.antrea.io"
+    clientConfig:
+      service:
+        name: "antrea"
+        namespace: {{ .Release.Namespace }}
+        path: "/validate/traceflow"
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["crd.antrea.io"]
+        apiVersions: ["v1alpha1"]
+        resources: ["traceflows"]
+        scope: "Cluster"
+    admissionReviewVersions: ["v1", "v1beta1"]
+    sideEffects: None
+    timeoutSeconds: 5

build/yamls/antrea-aks.yml

@@ -5004,3 +5004,18 @@ webhooks:
     admissionReviewVersions: ["v1", "v1beta1"]
     sideEffects: None
     timeoutSeconds: 5
+  - name: "traceflowvalidator.antrea.io"
+    clientConfig:
+      service:
+        name: "antrea"
+        namespace: kube-system
+        path: "/validate/traceflow"
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["crd.antrea.io"]
+        apiVersions: ["v1alpha1"]
+        resources: ["traceflows"]
+        scope: "Cluster"
+    admissionReviewVersions: ["v1", "v1beta1"]
+    sideEffects: None
+    timeoutSeconds: 5

build/yamls/antrea-eks.yml

@@ -5005,3 +5005,18 @@ webhooks:
     admissionReviewVersions: ["v1", "v1beta1"]
     sideEffects: None
     timeoutSeconds: 5
+  - name: "traceflowvalidator.antrea.io"
+    clientConfig:
+      service:
+        name: "antrea"
+        namespace: kube-system
+        path: "/validate/traceflow"
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["crd.antrea.io"]
+        apiVersions: ["v1alpha1"]
+        resources: ["traceflows"]
+        scope: "Cluster"
+    admissionReviewVersions: ["v1", "v1beta1"]
+    sideEffects: None
+    timeoutSeconds: 5

build/yamls/antrea-gke.yml

@@ -5002,3 +5002,18 @@ webhooks:
     admissionReviewVersions: ["v1", "v1beta1"]
     sideEffects: None
     timeoutSeconds: 5
+  - name: "traceflowvalidator.antrea.io"
+    clientConfig:
+      service:
+        name: "antrea"
+        namespace: kube-system
+        path: "/validate/traceflow"
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["crd.antrea.io"]
+        apiVersions: ["v1alpha1"]
+        resources: ["traceflows"]
+        scope: "Cluster"
+    admissionReviewVersions: ["v1", "v1beta1"]
+    sideEffects: None
+    timeoutSeconds: 5

build/yamls/antrea-ipsec.yml

@@ -5061,3 +5061,18 @@ webhooks:
     admissionReviewVersions: ["v1", "v1beta1"]
     sideEffects: None
     timeoutSeconds: 5
+  - name: "traceflowvalidator.antrea.io"
+    clientConfig:
+      service:
+        name: "antrea"
+        namespace: kube-system
+        path: "/validate/traceflow"
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["crd.antrea.io"]
+        apiVersions: ["v1alpha1"]
+        resources: ["traceflows"]
+        scope: "Cluster"
+    admissionReviewVersions: ["v1", "v1beta1"]
+    sideEffects: None
+    timeoutSeconds: 5

build/yamls/antrea.yml

@@ -5002,3 +5002,18 @@ webhooks:
     admissionReviewVersions: ["v1", "v1beta1"]
     sideEffects: None
     timeoutSeconds: 5
+  - name: "traceflowvalidator.antrea.io"
+    clientConfig:
+      service:
+        name: "antrea"
+        namespace: kube-system
+        path: "/validate/traceflow"
+    rules:
+      - operations: ["CREATE", "UPDATE"]
+        apiGroups: ["crd.antrea.io"]
+        apiVersions: ["v1alpha1"]
+        resources: ["traceflows"]
+        scope: "Cluster"
+    admissionReviewVersions: ["v1", "v1beta1"]
+    sideEffects: None
+    timeoutSeconds: 5

cmd/antrea-controller/controller.go

@@ -113,6 +113,7 @@ var allowedPaths = []string{
 	"/validate/group",
 	"/validate/ippool",
 	"/validate/supportbundlecollection",
+	"/validate/traceflow",
 	"/convert/clustergroup",
 }
 
@@ -291,6 +292,7 @@ func run(o *Options) error {
 		egressController,
 		statsAggregator,
 		bundleCollectionController,
+		traceflowController,
 		*o.config.EnablePrometheusMetrics,
 		cipherSuites,
 		cipher.TLSVersionMap[o.config.TLSMinVersion])
@@ -476,6 +478,7 @@ func createAPIServerConfig(kubeconfig string,
 	egressController *egress.EgressController,
 	statsAggregator *stats.Aggregator,
 	bundleCollectionStore *supportbundlecollection.Controller,
+	traceflowController *traceflow.Controller,
 	enableMetrics bool,
 	cipherSuites []uint16,
 	tlsMinVersion uint16) (*apiserver.Config, error) {
@@ -542,5 +545,6 @@ func createAPIServerConfig(kubeconfig string,
 		endpointQuerier,
 		npController,
 		egressController,
-		bundleCollectionStore), nil
+		bundleCollectionStore,
+		traceflowController), nil
 }

pkg/agent/controller/traceflow/traceflow_controller.go

@@ -300,16 +300,6 @@ func (c *Controller) startTraceflow(tf *crdv1alpha1.Traceflow) error {
 		return err
 	}
 
-	liveTraffic := tf.Spec.LiveTraffic
-	if tf.Spec.Source.Pod == "" && tf.Spec.Destination.Pod == "" {
-		klog.Errorf("Traceflow %s has neither source nor destination Pod specified", tf.Name)
-		return nil
-	}
-	if tf.Spec.Source.Pod == "" && !liveTraffic {
-		klog.Errorf("Traceflow %s does not have source Pod specified", tf.Name)
-		return nil
-	}
-
 	receiverOnly := false
 	var pod, ns string
 	if tf.Spec.Source.Pod != "" {
@@ -327,6 +317,7 @@ func (c *Controller) startTraceflow(tf *crdv1alpha1.Traceflow) error {
 	podInterfaces := c.interfaceStore.GetContainerInterfacesByPod(pod, ns)
 	isSender := len(podInterfaces) > 0 && !receiverOnly
 
+	liveTraffic := tf.Spec.LiveTraffic
 	var packet, matchPacket *binding.Packet
 	var ofPort uint32
 	if len(podInterfaces) > 0 {
@@ -388,9 +379,6 @@ func (c *Controller) validateTraceflow(tf *crdv1alpha1.Traceflow) error {
 	}
 	if tf.Spec.Destination.IP != "" {
 		destIP := net.ParseIP(tf.Spec.Destination.IP)
-		if destIP == nil {
-			return fmt.Errorf("destination IP is not valid: %s", tf.Spec.Destination.IP)
-		}
 		// When AntreaProxy is enabled, serviceCIDR is not required and may be set to a
 		// default value which does not match the cluster configuration.
 		if !features.DefaultFeatureGate.Enabled(features.AntreaProxy) && c.serviceCIDR.Contains(destIP) {

pkg/agent/controller/traceflow/traceflow_controller_test.go

@@ -620,49 +620,6 @@ func TestStartTraceflow(t *testing.T) {
 				}, ofPortPod1, int32(-1))
 			},
 		},
-		{
-			name: "empty source and destination Pod",
-			tf: &crdv1alpha1.Traceflow{
-				ObjectMeta: metav1.ObjectMeta{Name: "tf3", UID: "uid3"},
-			},
-			expectedErrLog: "Traceflow tf3 has neither source nor destination Pod specified",
-		},
-		{
-			name: "empty source Pod",
-			tf: &crdv1alpha1.Traceflow{
-				ObjectMeta: metav1.ObjectMeta{Name: "tf4", UID: "uid4"},
-				Spec: crdv1alpha1.TraceflowSpec{
-					Destination: crdv1alpha1.Destination{
-						Namespace: pod2.Namespace,
-						Pod:       pod2.Name,
-					},
-				},
-			},
-			expectedErrLog: "Traceflow tf4 does not have source Pod specified",
-		},
-		{
-			name: "invalid destination IPv4",
-			tf: &crdv1alpha1.Traceflow{
-				ObjectMeta: metav1.ObjectMeta{Name: "tf5", UID: "uid5"},
-				Spec: crdv1alpha1.TraceflowSpec{
-					Source: crdv1alpha1.Source{
-						Namespace: pod1.Namespace,
-						Pod:       pod1.Name,
-					},
-					Destination: crdv1alpha1.Destination{
-						IP: "192.168.1.300",
-					},
-				},
-				Status: crdv1alpha1.TraceflowStatus{
-					Phase:        crdv1alpha1.Running,
-					DataplaneTag: 1,
-				},
-			},
-			nodeConfig: &config.NodeConfig{
-				Name: "node-1",
-			},
-			expectedErr: "destination IP is not valid: 192.168.1.300",
-		},
 		{
 			name: "live traceflow receive only",
 			tf: &crdv1alpha1.Traceflow{
@@ -862,18 +819,6 @@ func TestValidateTraceflow(t *testing.T) {
 			},
 			expectedErr: "using Service destination requires AntreaProxy feature enabled",
 		},
-		{
-			name: "invalid destination IPv4",
-			tf: &crdv1alpha1.Traceflow{
-				Spec: crdv1alpha1.TraceflowSpec{
-					Destination: crdv1alpha1.Destination{
-						IP: "192.168.1.300",
-					},
-				},
-			},
-			antreaProxyEnabled: true,
-			expectedErr:        "destination IP is not valid: 192.168.1.300",
-		},
 		{
 			name: "AntreaProxy feature disabled with ClusterIP destination",
 			tf: &crdv1alpha1.Traceflow{

pkg/apiserver/apiserver.go

@@ -66,6 +66,7 @@ import (
 	"antrea.io/antrea/pkg/controller/querier"
 	"antrea.io/antrea/pkg/controller/stats"
 	controllerbundlecollection "antrea.io/antrea/pkg/controller/supportbundlecollection"
+	"antrea.io/antrea/pkg/controller/traceflow"
 	"antrea.io/antrea/pkg/features"
 )
 
@@ -116,6 +117,7 @@ type ExtraConfig struct {
 	statsAggregator               *stats.Aggregator
 	networkPolicyStatusController *controllernetworkpolicy.StatusController
 	bundleCollectionController    *controllerbundlecollection.Controller
+	traceflowController           *traceflow.Controller
 }
 
 // Config defines the config for Antrea apiserver.
@@ -158,7 +160,8 @@ func NewConfig(
 	endpointQuerier controllernetworkpolicy.EndpointQuerier,
 	npController *controllernetworkpolicy.NetworkPolicyController,
 	egressController *egress.EgressController,
-	bundleCollectionController *controllerbundlecollection.Controller) *Config {
+	bundleCollectionController *controllerbundlecollection.Controller,
+	traceflowController *traceflow.Controller) *Config {
 	return &Config{
 		genericConfig: genericConfig,
 		extraConfig: ExtraConfig{
@@ -178,6 +181,7 @@ func NewConfig(
 			networkPolicyStatusController: networkPolicyStatusController,
 			egressController:              egressController,
 			bundleCollectionController:    bundleCollectionController,
+			traceflowController:           traceflowController,
 		},
 	}
 }
@@ -333,6 +337,10 @@ func installHandlers(c *ExtraConfig, s *genericapiserver.GenericAPIServer) {
 	if features.DefaultFeatureGate.Enabled(features.SupportBundleCollection) {
 		s.Handler.NonGoRestfulMux.HandleFunc("/validate/supportbundlecollection", webhook.HandlerForValidateFunc(c.bundleCollectionController.Validate))
 	}
+
+	if features.DefaultFeatureGate.Enabled(features.Traceflow) {
+		s.Handler.NonGoRestfulMux.HandleFunc("/validate/traceflow", webhook.HandlerForValidateFunc(c.traceflowController.Validate))
+	}
 }
 
 func DefaultCAConfig() *certificate.CAConfig {

pkg/controller/traceflow/controller.go

@@ -37,7 +37,6 @@ import (
 	crdinformers "antrea.io/antrea/pkg/client/informers/externalversions/crd/v1alpha1"
 	crdlisters "antrea.io/antrea/pkg/client/listers/crd/v1alpha1"
 	"antrea.io/antrea/pkg/controller/grouping"
-	"antrea.io/antrea/pkg/util/k8s"
 )
 
 const (
@@ -249,10 +248,6 @@ func (c *Controller) syncTraceflow(traceflowName string) error {
 }
 
 func (c *Controller) startTraceflow(tf *crdv1alpha1.Traceflow) error {
-	if err := c.validateTraceflow(tf); err != nil {
-		klog.ErrorS(err, "Invalid Traceflow request", "request", tf)
-		return c.updateTraceflowStatus(tf, crdv1alpha1.Failed, fmt.Sprintf("Invalid Traceflow request, err: %+v", err), 0)
-	}
 	// Allocate data plane tag.
 	tag, err := c.allocateTag(tf.Name)
 	if err != nil {
@@ -413,19 +408,3 @@ func (c *Controller) deallocateTag(name string, tag uint8) {
 		}
 	}
 }
-
-func (c *Controller) validateTraceflow(tf *crdv1alpha1.Traceflow) error {
-	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 err
-		}
-		if srcPod.Spec.HostNetwork {
-			return fmt.Errorf("using hostNetwork Pod as source in non-live-traffic Traceflow is not supported")
-		}
-	}
-	return nil
-}

pkg/controller/traceflow/controller_test.go

@@ -149,34 +149,6 @@ func TestTraceflow(t *testing.T) {
 		assert.Equal(t, numRunningTraceflows(), 0)
 	})
 
-	t.Run("timeoutHostnetworkTraceflow", func(t *testing.T) {
-		tf2 := crdv1alpha1.Traceflow{
-			ObjectMeta: metav1.ObjectMeta{Name: "tf2", UID: "uid2"},
-			Spec: crdv1alpha1.TraceflowSpec{
-				Source:      crdv1alpha1.Source{Namespace: "ns1", Pod: "pod2"},
-				Destination: crdv1alpha1.Destination{Namespace: "ns2", Pod: "pod2"},
-				Timeout:     2, // 2 seconds timeout
-			},
-		}
-		pod2 := corev1.Pod{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "pod2",
-				Namespace: "ns1",
-			},
-			Spec: corev1.PodSpec{HostNetwork: true},
-		}
-
-		tfc.kubeClient.CoreV1().Pods("ns1").Create(context.TODO(), &pod2, metav1.CreateOptions{})
-		createdPod, _ := tfc.waitForPodInNamespace("ns1", "pod2", time.Second)
-		require.NotNil(t, createdPod)
-		tfc.client.CrdV1alpha1().Traceflows().Create(context.TODO(), &tf2, metav1.CreateOptions{})
-		res, _ := tfc.waitForTraceflow("tf2", crdv1alpha1.Failed, time.Second)
-		require.NotNil(t, res)
-		// DataplaneTag should not be allocated by Controller.
-		assert.True(t, res.Status.DataplaneTag == 0)
-		assert.Equal(t, numRunningTraceflows(), 0)
-	})
-
 	close(stopCh)
 }