diff --git a/build/charts/antrea/templates/webhooks/validating/crdvalidator.yaml b/build/charts/antrea/templates/webhooks/validating/crdvalidator.yaml index 67902a73ce2..8e9d4c85a4d 100644 --- a/build/charts/antrea/templates/webhooks/validating/crdvalidator.yaml +++ b/build/charts/antrea/templates/webhooks/validating/crdvalidator.yaml @@ -169,3 +169,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 diff --git a/build/yamls/antrea-aks.yml b/build/yamls/antrea-aks.yml index 9945ec9001e..271e38cab6f 100644 --- a/build/yamls/antrea-aks.yml +++ b/build/yamls/antrea-aks.yml @@ -7424,3 +7424,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 diff --git a/build/yamls/antrea-eks.yml b/build/yamls/antrea-eks.yml index 7c337384344..0e0754824b8 100644 --- a/build/yamls/antrea-eks.yml +++ b/build/yamls/antrea-eks.yml @@ -7425,3 +7425,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 diff --git a/build/yamls/antrea-gke.yml b/build/yamls/antrea-gke.yml index a3ebde5a915..e8b25e75cf8 100644 --- a/build/yamls/antrea-gke.yml +++ b/build/yamls/antrea-gke.yml @@ -7422,3 +7422,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 diff --git a/build/yamls/antrea-ipsec.yml b/build/yamls/antrea-ipsec.yml index a432cd781df..a15be30fcce 100644 --- a/build/yamls/antrea-ipsec.yml +++ b/build/yamls/antrea-ipsec.yml @@ -7481,3 +7481,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 diff --git a/build/yamls/antrea.yml b/build/yamls/antrea.yml index 228cc4c89b6..f3743b1c734 100644 --- a/build/yamls/antrea.yml +++ b/build/yamls/antrea.yml @@ -7422,3 +7422,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 diff --git a/cmd/antrea-controller/controller.go b/cmd/antrea-controller/controller.go index ee719ff13d9..7e18de4a01f 100644 --- a/cmd/antrea-controller/controller.go +++ b/cmd/antrea-controller/controller.go @@ -115,6 +115,7 @@ var allowedPaths = []string{ "/validate/group", "/validate/ippool", "/validate/supportbundlecollection", + "/validate/traceflow", "/convert/clustergroup", } @@ -298,6 +299,7 @@ func run(o *Options) error { egressController, statsAggregator, bundleCollectionController, + traceflowController, *o.config.EnablePrometheusMetrics, cipherSuites, cipher.TLSVersionMap[o.config.TLSMinVersion]) @@ -490,6 +492,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) { @@ -556,5 +559,6 @@ func createAPIServerConfig(kubeconfig string, endpointQuerier, npController, egressController, - bundleCollectionStore), nil + bundleCollectionStore, + traceflowController), nil } diff --git a/pkg/agent/controller/traceflow/traceflow_controller.go b/pkg/agent/controller/traceflow/traceflow_controller.go index 8980746571e..2ae62ef1073 100644 --- a/pkg/agent/controller/traceflow/traceflow_controller.go +++ b/pkg/agent/controller/traceflow/traceflow_controller.go @@ -300,16 +300,6 @@ func (c *Controller) startTraceflow(tf *crdv1beta1.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 *crdv1beta1.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 *crdv1beta1.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) { diff --git a/pkg/agent/controller/traceflow/traceflow_controller_test.go b/pkg/agent/controller/traceflow/traceflow_controller_test.go index e22c1310c5a..05dc39d8dee 100644 --- a/pkg/agent/controller/traceflow/traceflow_controller_test.go +++ b/pkg/agent/controller/traceflow/traceflow_controller_test.go @@ -622,49 +622,6 @@ func TestStartTraceflow(t *testing.T) { }, ofPortPod1, int32(-1)) }, }, - { - name: "empty source and destination Pod", - tf: &crdv1beta1.Traceflow{ - ObjectMeta: metav1.ObjectMeta{Name: "tf3", UID: "uid3"}, - }, - expectedErrLog: "Traceflow tf3 has neither source nor destination Pod specified", - }, - { - name: "empty source Pod", - tf: &crdv1beta1.Traceflow{ - ObjectMeta: metav1.ObjectMeta{Name: "tf4", UID: "uid4"}, - Spec: crdv1beta1.TraceflowSpec{ - Destination: crdv1beta1.Destination{ - Namespace: pod2.Namespace, - Pod: pod2.Name, - }, - }, - }, - expectedErrLog: "Traceflow tf4 does not have source Pod specified", - }, - { - name: "invalid destination IPv4", - tf: &crdv1beta1.Traceflow{ - ObjectMeta: metav1.ObjectMeta{Name: "tf5", UID: "uid5"}, - Spec: crdv1beta1.TraceflowSpec{ - Source: crdv1beta1.Source{ - Namespace: pod1.Namespace, - Pod: pod1.Name, - }, - Destination: crdv1beta1.Destination{ - IP: "192.168.1.300", - }, - }, - Status: crdv1beta1.TraceflowStatus{ - Phase: crdv1beta1.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: &crdv1beta1.Traceflow{ @@ -864,18 +821,6 @@ func TestValidateTraceflow(t *testing.T) { }, expectedErr: "using Service destination requires AntreaProxy feature enabled", }, - { - name: "invalid destination IPv4", - tf: &crdv1beta1.Traceflow{ - Spec: crdv1beta1.TraceflowSpec{ - Destination: crdv1beta1.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: &crdv1beta1.Traceflow{ diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 865a8b87b08..6e3d2045fde 100644 --- a/pkg/apiserver/apiserver.go +++ b/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, }, } } @@ -334,6 +338,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 { diff --git a/pkg/controller/traceflow/controller.go b/pkg/controller/traceflow/controller.go index 772345213ef..e6f34bf25de 100644 --- a/pkg/controller/traceflow/controller.go +++ b/pkg/controller/traceflow/controller.go @@ -37,7 +37,6 @@ import ( crdinformers "antrea.io/antrea/pkg/client/informers/externalversions/crd/v1beta1" crdlisters "antrea.io/antrea/pkg/client/listers/crd/v1beta1" "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 *crdv1beta1.Traceflow) error { - if err := c.validateTraceflow(tf); err != nil { - klog.ErrorS(err, "Invalid Traceflow request", "request", tf) - return c.updateTraceflowStatus(tf, crdv1beta1.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 *crdv1beta1.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 -} diff --git a/pkg/controller/traceflow/controller_test.go b/pkg/controller/traceflow/controller_test.go index 623eab9a6bd..661357a5c69 100644 --- a/pkg/controller/traceflow/controller_test.go +++ b/pkg/controller/traceflow/controller_test.go @@ -147,34 +147,6 @@ func TestTraceflow(t *testing.T) { assert.Equal(t, numRunningTraceflows(), 0) }) - t.Run("timeoutHostnetworkTraceflow", func(t *testing.T) { - tf2 := crdv1beta1.Traceflow{ - ObjectMeta: metav1.ObjectMeta{Name: "tf2", UID: "uid2"}, - Spec: crdv1beta1.TraceflowSpec{ - Source: crdv1beta1.Source{Namespace: "ns1", Pod: "pod2"}, - Destination: crdv1beta1.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.CrdV1beta1().Traceflows().Create(context.TODO(), &tf2, metav1.CreateOptions{}) - res, _ := tfc.waitForTraceflow("tf2", crdv1beta1.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) } diff --git a/pkg/controller/traceflow/validate.go b/pkg/controller/traceflow/validate.go new file mode 100644 index 00000000000..81da5162561 --- /dev/null +++ b/pkg/controller/traceflow/validate.go @@ -0,0 +1,90 @@ +// Copyright 2023 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package traceflow + +import ( + "encoding/json" + "fmt" + + 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" + + crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" + "antrea.io/antrea/pkg/util/k8s" +) + +func (c *Controller) Validate(review *admv1.AdmissionReview) *admv1.AdmissionResponse { + newResponse := func(allowed bool, deniedReason string) *admv1.AdmissionResponse { + resp := &admv1.AdmissionResponse{ + UID: review.Request.UID, + Allowed: allowed, + } + if !allowed { + resp.Result = &metav1.Status{ + Message: deniedReason, + } + } + return resp + } + + klog.V(2).InfoS("Validating Traceflow", "request", review.Request) + + var newObj 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 newResponse(false, err.Error()) + } + } + + switch review.Request.Operation { + case admv1.Create: + klog.V(2).InfoS("Validating CREATE request for Traceflow", "name", newObj.Name) + allowed, deniedReason := c.validate(&newObj) + return newResponse(allowed, deniedReason) + case admv1.Update: + klog.V(2).InfoS("Validating UPDATE request for Traceflow", "name", newObj.Name) + allowed, deniedReason := c.validate(&newObj) + return newResponse(allowed, deniedReason) + default: + err := fmt.Errorf("invalid request operation %s for Traceflow", review.Request.Operation) + klog.ErrorS(err, "Failed to validate Traceflow", "name", newObj.Name) + return newResponse(false, err.Error()) + } +} + +func (c *Controller) validate(tf *crdv1alpha1.Traceflow) (allowed bool, deniedReason string) { + if !tf.Spec.LiveTraffic { + if tf.Spec.Source.Namespace == "" || tf.Spec.Source.Pod == "" { + return false, "source Pod must be specified in non-live-traffic Traceflow" + } + 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, "" +} diff --git a/pkg/controller/traceflow/validate_test.go b/pkg/controller/traceflow/validate_test.go new file mode 100644 index 00000000000..575844477d0 --- /dev/null +++ b/pkg/controller/traceflow/validate_test.go @@ -0,0 +1,156 @@ +// Copyright 2023 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package traceflow + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + admv1 "k8s.io/api/admission/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + + crdv1alpha1 "antrea.io/antrea/pkg/apis/crd/v1alpha1" +) + +func TestControllerValidate(t *testing.T) { + tests := []struct { + name string + + // environment + pods []*v1.Pod + + // input + oldSpec *crdv1alpha1.TraceflowSpec + newSpec *crdv1alpha1.TraceflowSpec + + // expected output + allowed bool + deniedReason string + }{ + { + name: "Source Pod must be specified in non-live-traffic Traceflow", + newSpec: &crdv1alpha1.TraceflowSpec{ + Destination: crdv1alpha1.Destination{IP: "10.0.0.2"}, + }, + deniedReason: "source Pod must be specified in non-live-traffic Traceflow", + }, + { + 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", + }, + { + name: "Assigned source pod must exist", + newSpec: &crdv1alpha1.TraceflowSpec{ + Source: crdv1alpha1.Source{ + Namespace: "test-ns", + Pod: "test-pod", + }, + }, + deniedReason: "requested source Pod test-ns/test-pod not found", + }, + { + name: "Using hostNetwork Pod as source in non-live-traffic Traceflow is not supported", + pods: []*v1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{Namespace: "test-ns", Name: "test-pod"}, + Spec: v1.PodSpec{HostNetwork: true}, + }, + }, + newSpec: &crdv1alpha1.TraceflowSpec{ + Source: crdv1alpha1.Source{ + Namespace: "test-ns", + Pod: "test-pod", + }, + }, + deniedReason: "using hostNetwork Pod as source in non-live-traffic Traceflow is not supported", + }, + { + name: "Valid request", + pods: []*v1.Pod{ + { + ObjectMeta: metav1.ObjectMeta{Namespace: "test-ns", Name: "test-pod"}, + }, + }, + newSpec: &crdv1alpha1.TraceflowSpec{ + Source: crdv1alpha1.Source{ + Namespace: "test-ns", + Pod: "test-pod", + }, + }, + allowed: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + stopCh := make(chan struct{}) + defer close(stopCh) + pods := make([]runtime.Object, 0) + for _, p := range tc.pods { + pods = append(pods, p) + } + controller := newController(pods...) + controller.informerFactory.Start(stopCh) + controller.crdInformerFactory.Start(stopCh) + // Must wait for cache sync, otherwise resource creation events will be missing if the resources are created + // in-between list and watch call of an informer. This is because fake clientset doesn't support watching with + // resourceVersion. A watcher of fake clientset only gets events that happen after the watcher is created. + controller.informerFactory.WaitForCacheSync(stopCh) + controller.crdInformerFactory.WaitForCacheSync(stopCh) + go controller.Run(stopCh) + + 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), + } + } + review := &admv1.AdmissionReview{ + Request: request, + } + + expectedResponse := &admv1.AdmissionResponse{ + Allowed: tc.allowed, + } + if !tc.allowed { + expectedResponse.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, _ := json.Marshal(tf) + return runtime.RawExtension{Raw: raw} +} diff --git a/test/e2e/fixtures.go b/test/e2e/fixtures.go index 9cfbace71b5..03785fedb20 100644 --- a/test/e2e/fixtures.go +++ b/test/e2e/fixtures.go @@ -508,16 +508,16 @@ func deletePodWrapper(tb testing.TB, data *TestData, namespace, name string) { func createTestBusyboxPods(tb testing.TB, data *TestData, num int, ns string, nodeName string) ( podNames []string, podIPs []*PodIPs, cleanupFn func(), ) { - return createTestPods(tb, data, num, ns, nodeName, data.createBusyboxPodOnNode) + return createTestPods(tb, data, num, ns, nodeName, false, data.createBusyboxPodOnNode) } func createTestAgnhostPods(tb testing.TB, data *TestData, num int, ns string, nodeName string) ( podNames []string, podIPs []*PodIPs, cleanupFn func(), ) { - return createTestPods(tb, data, num, ns, nodeName, data.createAgnhostPodOnNode) + return createTestPods(tb, data, num, ns, nodeName, false, data.createAgnhostPodOnNode) } -func createTestPods(tb testing.TB, data *TestData, num int, ns string, nodeName string, createFunc func(string, string, string, bool) error) ( +func createTestPods(tb testing.TB, data *TestData, num int, ns string, nodeName string, hostNetwork bool, createFunc func(string, string, string, bool) error) ( podNames []string, podIPs []*PodIPs, cleanupFn func(), ) { cleanupFn = func() { @@ -541,7 +541,7 @@ func createTestPods(tb testing.TB, data *TestData, num int, ns string, nodeName createPodAndGetIP := func() (string, *PodIPs, error) { podName := randName("test-pod-") tb.Logf("Creating a test Pod '%s' and waiting for IP", podName) - if err := createFunc(podName, ns, nodeName, false); err != nil { + if err := createFunc(podName, ns, nodeName, hostNetwork); err != nil { tb.Errorf("Error when creating test Pod '%s': %v", podName, err) return "", nil, err } diff --git a/test/e2e/traceflow_test.go b/test/e2e/traceflow_test.go index 2b48057753c..9670edd668e 100644 --- a/test/e2e/traceflow_test.go +++ b/test/e2e/traceflow_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -36,7 +37,6 @@ import ( "antrea.io/antrea/pkg/apis/crd/v1alpha1" "antrea.io/antrea/pkg/apis/crd/v1beta1" "antrea.io/antrea/pkg/features" - "antrea.io/antrea/pkg/util/k8s" ) type testcase struct { @@ -87,6 +87,9 @@ func TestTraceflow(t *testing.T) { skipIfEgressDisabled(t) testTraceflowEgress(t, data) }) + t.Run("testTraceflowValidation", func(t *testing.T) { + testTraceflowValidation(t, data) + }) } func skipIfTraceflowDisabled(t *testing.T) { @@ -310,7 +313,6 @@ func testTraceflowIntraNode(t *testing.T, data *TestData) { } node1 := nodeName(nodeIdx) - agentPod, _ := data.getAntreaPodOnNode(node1) node1Pods, node1IPs, node1CleanupFn := createTestAgnhostPods(t, data, 3, data.testNamespace, node1) defer node1CleanupFn() // Give a little time for Windows containerd Nodes to setup OVS. @@ -598,48 +600,6 @@ func testTraceflowIntraNode(t *testing.T, data *TestData) { expectedPhase: v1alpha1.Failed, expectedReasons: []string{fmt.Sprintf("Node: %s, error: failed to get the destination Pod: pods \"%s\" not found", node1, "non-existing-pod")}, }, - { - name: "nonExistingSrcPodIPv4", - ipVersion: 4, - tf: &v1alpha1.Traceflow{ - ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-", data.testNamespace, "non-existing-pod", data.testNamespace, node1Pods[1])), - }, - Spec: v1alpha1.TraceflowSpec{ - Source: v1alpha1.Source{ - Namespace: data.testNamespace, - Pod: "non-existing-pod", - }, - Destination: v1alpha1.Destination{ - Namespace: data.testNamespace, - Pod: node1Pods[1], - }, - }, - }, - expectedPhase: v1alpha1.Failed, - expectedReasons: []string{fmt.Sprintf("Invalid Traceflow request, err: %+v", fmt.Errorf("requested source Pod %s not found", k8s.NamespacedName(data.testNamespace, "non-existing-pod")))}, - }, - { - name: "hostNetworkSrcPodIPv4", - ipVersion: 4, - tf: &v1alpha1.Traceflow{ - ObjectMeta: metav1.ObjectMeta{ - Name: randName(fmt.Sprintf("%s-%s-to-%s-%s-", antreaNamespace, agentPod, data.testNamespace, node1Pods[1])), - }, - Spec: v1alpha1.TraceflowSpec{ - Source: v1alpha1.Source{ - Namespace: antreaNamespace, - Pod: agentPod, - }, - Destination: v1alpha1.Destination{ - Namespace: data.testNamespace, - Pod: node1Pods[1], - }, - }, - }, - expectedPhase: v1alpha1.Failed, - expectedReasons: []string{fmt.Sprintf("Invalid Traceflow request, err: %+v", fmt.Errorf("using hostNetwork Pod as source in non-live-traffic Traceflow is not supported"))}, - }, { name: "intraNodeICMPDstIPLiveTraceflowIPv4", ipVersion: 4, @@ -2237,6 +2197,85 @@ func testTraceflowEgress(t *testing.T, data *TestData) { }) } +func testTraceflowValidation(t *testing.T, data *TestData) { + podNames, podIPs, cleanupFn := createTestPods(t, data, 1, data.testNamespace, nodeName(0), true, data.createAgnhostPodOnNode) + defer cleanupFn() + podName := podNames[0] + podIP := podIPs[0].ipv4 + + testCases := []struct { + name string + spec *v1alpha1.TraceflowSpec + allowed bool + deniedReason string + }{ + { + name: "Source Pod must be specified in non-live-traffic Traceflow", + spec: &v1alpha1.TraceflowSpec{ + Destination: v1alpha1.Destination{IP: podIP.String()}, + }, + deniedReason: "source Pod must be specified in non-live-traffic Traceflow", + }, + { + name: "Traceflow should have either source or destination Pod assigned", + spec: &v1alpha1.TraceflowSpec{ + LiveTraffic: true, + }, + deniedReason: "Traceflow {{name}} has neither source nor destination Pod specified", + }, + { + name: "Assigned source pod must exist", + spec: &v1alpha1.TraceflowSpec{ + Source: v1alpha1.Source{ + Namespace: "foo", + Pod: "bar", + }, + }, + deniedReason: "requested source Pod foo/bar not found", + }, + { + name: "Using hostNetwork Pod as source in non-live-traffic Traceflow is not supported", + spec: &v1alpha1.TraceflowSpec{ + Source: v1alpha1.Source{ + Namespace: data.testNamespace, + Pod: podName, + }, + }, + deniedReason: "using hostNetwork Pod as source in non-live-traffic Traceflow is not supported", + }, + { + name: "Valid request", + spec: &v1alpha1.TraceflowSpec{ + LiveTraffic: true, + Source: v1alpha1.Source{ + Namespace: data.testNamespace, + Pod: podName, + }, + }, + allowed: true, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + tf := &v1alpha1.Traceflow{ + Spec: *tc.spec, + } + tf.Name = randName("") + _, err := data.crdClient.CrdV1alpha1().Traceflows().Create(context.TODO(), tf, metav1.CreateOptions{}) + if tc.allowed { + assert.Nil(t, err) + } else { + tc.deniedReason = strings.Replace(tc.deniedReason, "{{name}}", tf.Name, -1) + expected := "admission webhook \"traceflowvalidator.antrea.io\" denied the request: " + tc.deniedReason + assert.Equal(t, expected, err.Error()) + } + }) + } + +} + func (data *TestData) waitForTraceflow(t *testing.T, name string, phase v1alpha1.TraceflowPhase) (*v1alpha1.Traceflow, error) { var tf *v1alpha1.Traceflow var err error