Fix panic when create spidermultusconfig in validate webhook

Signed-off-by: cyclinder <qifeng.guo@daocloud.io>
spidernet-io · Sep 14, 2024 · f8271d2 · f8271d2
1 parent b64851c
commit f8271d2
Show file tree

Hide file tree

Showing 4 changed files with 91 additions and 2 deletions.
diff --git a/pkg/coordinatormanager/coordinator_validate.go b/pkg/coordinatormanager/coordinator_validate.go
@@ -97,7 +97,7 @@ func ValidateCoordinatorSpec(spec *spiderpoolv2beta1.CoordinatorSpec, requireOpt
 		)
 	}
 	if spec.HostRPFilter != nil {
-		if err := validateCoordinatorHostRPFilter(spec.PodRPFilter); err != nil {
+		if err := validateCoordinatorHostRPFilter(spec.HostRPFilter); err != nil {
 			return err
 		}
 	}

diff --git a/pkg/multuscniconfig/multusconfig_mutate.go b/pkg/multuscniconfig/multusconfig_mutate.go
@@ -194,6 +194,8 @@ func setCoordinatorDefaultConfig(coordinator *spiderpoolv2beta1.CoordinatorSpec)
 			DetectIPConflict:   ptr.To(false),
 			PodMACPrefix:       ptr.To(""),
 			PodDefaultRouteNIC: ptr.To(""),
+			HostRPFilter:       ptr.To(0),
+			PodRPFilter:        ptr.To(0),
 			TunePodRoutes:      ptr.To(true),
 		}
 	}
@@ -230,5 +232,13 @@ func setCoordinatorDefaultConfig(coordinator *spiderpoolv2beta1.CoordinatorSpec)
 		coordinator.TxQueueLen = ptr.To(0)
 	}
 
+	if coordinator.PodRPFilter == nil {
+		coordinator.PodRPFilter = ptr.To(0)
+	}
+
+	if coordinator.HostRPFilter == nil {
+		coordinator.HostRPFilter = ptr.To(0)
+	}
+
 	return coordinator
 }
diff --git a/test/doc/spidermultus.md b/test/doc/spidermultus.md
@@ -23,4 +23,5 @@
 | M00019  | set spidermultusconfig.spec to empty and see if works                                                                                                   | p3       |       | done   |       |
 | M00020  | annotating custom names that are too long or empty should fail                                                                                          | p3       |       | done   |       |
 | M00021  |    create a spidermultusconfig and pod to verify chainCNI json config if works                                                                                     | p3       |       | done   |       |
-
+| M00022 | test podRPFilter and hostRPFilter in spidermultusconfig | p3 | | done |
+| M00023 | set hostRPFilter and podRPFilter to a invalid value | p3 | | done |
diff --git a/test/e2e/spidermultus/spidermultus_test.go b/test/e2e/spidermultus/spidermultus_test.go
@@ -597,6 +597,84 @@ var _ = Describe("test spidermultus", Label("SpiderMultusConfig"), func() {
 		}).WithTimeout(time.Minute * 3).WithPolling(time.Second * 5).Should(BeNil())
 	})
 
+	It("test hostRPFilter and podRPFilter in spiderMultusConfig", Label("M00022"), func() {
+		var smcName string = "rpfilter-multus-" + common.GenerateString(10, true)
+
+		smc := &spiderpoolv2beta1.SpiderMultusConfig{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      smcName,
+				Namespace: namespace,
+			},
+			Spec: spiderpoolv2beta1.MultusCNIConfigSpec{
+				CniType: ptr.To(constant.MacvlanCNI),
+				MacvlanConfig: &spiderpoolv2beta1.SpiderMacvlanCniConfig{
+					Master: []string{common.NIC1},
+				},
+				EnableCoordinator: ptr.To(true),
+				CoordinatorConfig: &spiderpoolv2beta1.CoordinatorSpec{
+					HostRPFilter: nil,
+					PodRPFilter:  nil,
+				},
+			},
+		}
+		GinkgoWriter.Printf("spidermultus cr: %+v \n", smc)
+		err := frame.CreateSpiderMultusInstance(smc)
+		Expect(err).NotTo(HaveOccurred())
+
+		Eventually(func() error {
+			netAttachDef, err := frame.GetMultusInstance(smcName, namespace)
+			if nil != err {
+				return err
+			}
+			if netAttachDef.Spec.Config == "" {
+				return fmt.Errorf("SpiderMultusConfig %s/%s corresponding net-attach-def resource doesn't have CNI configuration", smcName, namespace)
+			}
+
+			configByte, err := netutils.GetCNIConfigFromSpec(netAttachDef.Spec.Config, netAttachDef.Name)
+			if nil != err {
+				return fmt.Errorf("GetCNIConfig: err in getCNIConfigFromSpec: %v", err)
+			}
+
+			networkConfigList, err := libcni.ConfListFromBytes(configByte)
+			if nil != err {
+				return err
+			}
+
+			if len(networkConfigList.Plugins) != 2 {
+				return fmt.Errorf("unexpected CNI configuration: %s", netAttachDef.Spec.Config)
+			}
+
+			GinkgoWriter.Printf("SpiderMultusConfig with disableIPAM: %s\n", netAttachDef.Spec.Config)
+
+			return nil
+		}).WithTimeout(time.Minute * 3).WithPolling(time.Second * 5).Should(BeNil())
+	})
+
+	It("set hostRPFilter and podRPFilter to a invalid value", Label("M00023"), func() {
+		var smcName string = "invalid-rpfilter-multus-" + common.GenerateString(10, true)
+
+		smc := &spiderpoolv2beta1.SpiderMultusConfig{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      smcName,
+				Namespace: namespace,
+			},
+			Spec: spiderpoolv2beta1.MultusCNIConfigSpec{
+				CniType: ptr.To(constant.MacvlanCNI),
+				MacvlanConfig: &spiderpoolv2beta1.SpiderMacvlanCniConfig{
+					Master: []string{common.NIC1},
+				},
+				EnableCoordinator: ptr.To(true),
+				CoordinatorConfig: &spiderpoolv2beta1.CoordinatorSpec{
+					HostRPFilter: ptr.To(14),
+					PodRPFilter:  nil,
+				},
+			},
+		}
+		GinkgoWriter.Printf("spidermultus cr: %+v \n", smc)
+		err := frame.CreateSpiderMultusInstance(smc)
+		Expect(err).To(HaveOccurred(), "create spiderMultus instance failed: %v\n", err)
+	})
+
 	It("set disableIPAM to true and see if multus's nad has ipam config", Label("M00017"), func() {
 		var smcName string = "disable-ipam-multus-" + common.GenerateString(10, true)