cnf:network-add-nftables-test-cases #366

gkopels · 2025-01-17T10:10:04Z

Add first of two PRs cnf network nftables custom firewall test cases

evgenLevin · 2025-01-21T04:10:51Z

tests/cnf/core/network/internal/netconfig/config.go

@@ -38,6 +38,7 @@ type NetworkConfig struct {
 	//nolint:lll
 	PrometheusOperatorNamespace string `yaml:"prometheus_operator_namespace" envconfig:"ECO_CNF_CORE_NET_PROMETHEUS_OPERATOR_NAMESPACE"`
 	MlbAddressPoolIP            string `envconfig:"ECO_CNF_CORE_NET_MLB_ADDR_LIST"`
+	SecurityIPAddList           string `envconfig:"ECO_CNF_CORE_NET_SECURITY_ADDR_LIST"`


Why do we need it? Is MlbAddressPoolIP not good for that?

We can use it. Its the same addresses

evgenLevin · 2025-01-21T04:12:25Z

tests/cnf/core/network/security/internal/tsparams/consts.go

+	//      delete table inet custom_table
+	//      table inet custom_table {
+	//      }`
+	CustomFireWallDelete = `data:;base64, ` +


Suggested change

CustomFireWallDelete = `data:;base64, ` +

CustomFirewallDelete = `data:;base64, ` +

evgenLevin · 2025-01-21T04:14:07Z

tests/cnf/core/network/security/internal/tsparams/securityvars.go

+	// ReporterCRDsToDump tells to the reporter what CRs to dump.
+	ReporterCRDsToDump = []k8sreporter.CRData{}
+	// ExternalMacVlanNADName represents default external NetworkAttachmentDefinition name.
+	ExternalMacVlanNADName = "external"


Pls move it to the tests

evgenLevin · 2025-01-21T04:15:09Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	apimachinerytype "k8s.io/apimachinery/pkg/types"
+)
+
+var (


move all var under describe or to the tsparams

evgenLevin · 2025-01-21T04:16:53Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	masterPodIPv4Prefix      = "172.16.0.1/24"
+	masterPodIPv4Address     = "172.16.0.1"


Suggested change

masterPodIPv4Prefix = "172.16.0.1/24"

masterPodIPv4Address = "172.16.0.1"

masterPodIPv4Address = "172.16.0.1"

masterPodIPv4WithPrefix = masterPodIPv4Address + "/24"

evgenLevin · 2025-01-21T04:52:13Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	return testPod
+}
+
+func validateTCPTraffic(clientPod *pod.Builder, destIPAddrs []string, interfaceName,


no need return an error

I prefer to leave the error message. There are tests that I am expecting an error

please move the func to metallb.internal.cmd

I moved it to network.internal.cmd

evgenLevin · 2025-01-21T04:52:51Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	stringgzip := "gzip"
+
+	mode := 384
+	fileContents := fileContentString


that is the base64 string

what does it mean?
you can use it
Source: &fileContentString,

I am using mc WithRawConfig to create and delete the machineConfig. This is using IgnitionConfig. So I am using the fileContents and not directly adding to Source:

Sorry, I don'y understand. This is work for me:
https://github.com/openshift-kni/eco-gotests/pull/378/files#diff-22e4ee6efd525f4fd9fbbfbc89a65635f4039a762ef79e6181e6063bbccd3712R550

Yes I already made the change. Not sure why it didnt show after I pushed. I see it now

evgenLevin · 2025-01-21T04:53:25Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+		Create()
+	Expect(err).ToNot(HaveOccurred(), "Failed to create nftables machine config")
+
+	err = WaitForMcpStable(APIClient, 4*time.Minute, 30*time.Second, "workercnf")


"workercnf" should be a var

evgenLevin · 2025-01-21T04:54:04Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+		Create()
+	Expect(err).ToNot(HaveOccurred(), "Failed to create nftables machine config")
+
+	err = WaitForMcpStable(APIClient, 4*time.Minute, 30*time.Second, "workercnf")


Please increase the timers

evgenLevin · 2025-01-21T04:55:00Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+}
+
+// WaitForMcpStable waits for the stability of the MCP with the given name.
+func WaitForMcpStable(apiClient *clients.Settings, waitingTime, stableDuration time.Duration, mcpName string) error {


I'm pretty sure we have the same func somewhere, please reuse it

evgenLevin · 2025-01-22T15:56:29Z

tests/cnf/core/network/security/internal/tsparams/consts.go

+	// This yaml is created using butane file.
+	// butane mc-update-input-rule8888.bu -o mc-custom-firewall-input-port8888.yaml.


evgenLevin · 2025-01-22T15:57:17Z

tests/cnf/core/network/security/internal/tsparams/consts.go

+`
+
+	// FRRDefaultBGPPreConfig represents FRR daemon BGP minimal config.
+	FRRDefaultBGPPreConfig = ` bgp router-id 10.10.10.11


Where do you use it?

evgenLevin · 2025-01-22T15:59:34Z

tests/cnf/core/network/security/internal/tsparams/consts.go

+		`GRXpH7IcoJX2/I3Z6/a57e/Xffuy2Gze0C2WD+2AVf/U3WGGVEW9O/uz/w0AAP//` +
+		`kU0CJQUBAAA=`
+	// FRRBaseConfig represents FRR daemon minimal configuration.
+	FRRBaseConfig = `!


I can't find where it is being used.

correct removed

evgenLevin · 2025-01-22T16:04:59Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	ipv4SecurityIPList []string,
+	hubIPv4ExternalAddresses []string,
+	ipv4NodeAddrList []string,
+	hubPodWorkerName []string,


no need.
_ = createFrrPodTest("hub-pod-worker-0", workerNodeList[0].Object.Name, hubConfigMap.Definition.Name, containerFrrName, []string{}, hub0BRstaticIPAnnotation, false)

evgenLevin · 2025-01-22T16:08:33Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+		"Failed to update the machineconfiguration cluster file")
+}
+
+func createExternalNad(name string) {


There is the same func in common

Used existing define.CreateNadWithMasterPlugin func

Still here
fixed

evgenLevin · 2025-01-22T16:29:14Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	return ipAddressListWithoutPrefix
+}
+
+func buildRoutesMapWithSpecificRoutes(podList []*pod.Builder, workerNodeList []*nodes.Builder,


reuse from frrk8-tests.go

here again. I will give you a call and we can discuss where to place it

moved to network.internal.frrconfig

evgenLevin · 2025-01-22T16:30:26Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	return routesMap
+}
+
+func setStaticRoute(frrPod *pod.Builder, action, destIP string, nextHopMap map[string]string) {


reuse frr.go

moved to network.internal.frrconfig

evgenLevin · 2025-01-22T16:33:06Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	return testPod
+}
+
+func validateTCPTraffic(clientPod *pod.Builder, destIPAddrs []string, interfaceName,


please move the func to metallb.internal.cmd

evgenLevin · 2025-01-22T16:34:34Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	stringgzip := "gzip"
+
+	mode := 384
+	fileContents := fileContentString


what does it mean?
you can use it
Source: &fileContentString,

evgenLevin · 2025-01-22T16:38:24Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	Expect(err).ToNot(HaveOccurred(), "Failed to wait for MCP to be stable")
+}
+
+func createStaticRouteOnWorkerNodes(testPodList []*pod.Builder, routeMap map[string]string, routeAction,


please rename the func, it's not only create

changed name

evgenLevin · 2025-01-27T20:35:20Z

tests/cnf/core/network/internal/cmd/cmd.go

@@ -160,3 +176,12 @@ func getNumberOfPackets(line, firstFieldSubstr string) int {

 	return numberOfPackets
 }
+
+func removePrefixFromIPList(ipAddressList []string) []string {


please reuse func from metallb common: move it and use it

I already moved it to network.internal.frrconfig

evgenLevin · 2025-01-27T20:38:29Z

tests/cnf/core/network/internal/frrconfig/frrconfig.go

+	}
+
+	if len(nextHopList) == 0 {
+		glog.Error("Nexthop IP addresses list is empty")


Please use glog.V(90).Infof instead

evgenLevin · 2025-01-27T20:41:34Z

tests/cnf/core/network/internal/frrconfig/frrconfig.go

+}
+
+// BuildRoutesMapWithSpecificRoutes creates a route map with specific routes.
+func BuildRoutesMapWithSpecificRoutes(podList []*pod.Builder, workerNodeList []*nodes.Builder,


BuildRoutesMapWithSpecificRoutes and SetStaticRoute are not related to frr. Please move them to cmd.go or netenv.go

moved to netenv

evgenLevin · 2025-01-27T20:42:28Z

tests/cnf/core/network/internal/frrconfig/frrconfig.go

+}
+
+// RemovePrefixFromIPList removes the prefix from an IP address.
+func RemovePrefixFromIPList(ipAddressList []string) []string {


not related to frr func. And you already added it to cmd.go

removed from frrconfig

evgenLevin · 2025-01-27T20:45:54Z

tests/cnf/core/network/security/internal/tsparams/securityvars.go

+
+	// ReporterNamespacesToDump tells to the reporter from where to collect logs.
+	ReporterNamespacesToDump = map[string]string{
+		"openshift-performance-addon-operator": "performance",


Why do you need to dump openshift-performance-addon-operator ns? Please add ns that you're using in the test suite

correct removed and changed to security-tests

evgenLevin · 2025-01-27T20:48:02Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+		err                      error
+	)
+	BeforeAll(func() {
+


pls remove line

evgenLevin · 2025-01-27T20:48:19Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+
+		It("Verify the creation of a new custom node firewall NFTables table with an ingress rule",
+			reportxml.ID("77412"), func() {
+


pls remove line

evgenLevin · 2025-01-27T20:49:23Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+		"Failed to update the machineconfiguration cluster file")
+}
+
+func createExternalNad(name string) {


Still here
fixed

evgenLevin · 2025-01-27T20:50:11Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+		masterConfigMap.Definition.Name, containerFrrName, []string{}, masterStaticIPAnnotation, true)
+}
+
+func createStaticIPAnnotations(internalNADName, externalNADName string, internalIPAddresses,


still here
In my code its changed. Try refreshing and look again?

evgenLevin · 2025-01-27T20:54:19Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+	stringgzip := "gzip"
+
+	mode := 384
+	fileContents := fileContentString


Sorry, I don'y understand. This is work for me:
https://github.com/openshift-kni/eco-gotests/pull/378/files#diff-22e4ee6efd525f4fd9fbbfbc89a65635f4039a762ef79e6181e6063bbccd3712R550

Yes I already made the change. Not sure why it didnt show after I pushed. I see it now

tests/cnf/core/network/internal/cmd/cmd.go

evgenLevin · 2025-01-28T16:17:05Z

tests/cnf/core/network/internal/netenv/netenv.go

+
+// CreateExternalNad creats an external network-attchment-definition using the br-ex interface.
+func CreateExternalNad(apiClient *clients.Settings, name, testNameSpace string) {
+	glog.V(2).Info("Creating external BR-EX NetworkAttachmentDefinition")


Suggested change

glog.V(2).Info("Creating external BR-EX NetworkAttachmentDefinition")

glog.V(90).Info("Creating external BR-EX NetworkAttachmentDefinition")

evgenLevin · 2025-01-28T16:18:54Z

tests/cnf/core/network/internal/netenv/netenv.go

+		return
+	}
+
+	// Create the NetworkAttachmentDefinition
+	_, err = define.CreateNadWithMasterPlugin(apiClient, name, testNameSpace, macVlanPlugin)
+	if err != nil {
+		glog.V(90).Infof("Failed to create external NetworkAttachmentDefinition: %v", err)
+
+		return


Please return the error

please return err

evgenLevin · 2025-01-28T16:19:04Z

tests/cnf/core/network/internal/netenv/netenv.go

+		return
+	}
+
+	glog.V(2).Infof("Successfully created external NetworkAttachmentDefinition: %s", name)


Suggested change

glog.V(2).Infof("Successfully created external NetworkAttachmentDefinition: %s", name)

glog.V(90).Infof("Successfully created external NetworkAttachmentDefinition: %s", name)

evgenLevin · 2025-01-28T16:19:27Z

tests/cnf/core/network/internal/netenv/netenv.go

+}
+
+// WaitForMcpStable waits for the stability of the MCP with the given name.
+func WaitForMcpStable(apiClient *clients.Settings, waitingTime, stableDuration time.Duration, mcpName string) error {


Please add glog.V(90).Infof

evgenLevin · 2025-01-28T16:21:04Z

tests/cnf/core/network/metallb/tests/frrk8-tests.go

@@ -628,14 +643,17 @@ var _ = Describe("FRR", Ordered, Label(tsparams.LabelFRRTestCases), ContinueOnFa
 					srIovInterfacesUnderTest[0], frrNodeSecIntIPv4Addresses[1], "2001:100::253", vlanID)

 				By("Adding static routes to the speakers")
-				speakerRoutesMap := buildRoutesMapWithSpecificRoutes(frrk8sPods, hubSecIntIPv4Addresses)
+				// speakerRoutesMap := buildRoutesMapWithSpecificRoutes(frrk8sPods, hubSecIntIPv4Addresses)


evgenLevin · 2025-01-28T16:21:41Z

tests/cnf/core/network/security/internal/tsparams/securityvars.go

+
+	// ReporterNamespacesToDump tells to the reporter from where to collect logs.
+	ReporterNamespacesToDump = map[string]string{
+		"security-tests": "security-tests",


Please use TestNamespaceName

evgenLevin · 2025-01-28T16:23:00Z

tests/cnf/core/network/security/tests/nftables-custom-firewall-test.go

+		masterConfigMap.Definition.Name, containerFrrName, []string{}, masterStaticIPAnnotation, true)
+}
+
+func createStaticIPAnnotations(internalNADName, externalNADName string, internalIPAddresses,


I still can see it

evgenLevin · 2025-01-29T17:33:35Z

tests/cnf/core/network/internal/netenv/netenv.go

+		return
+	}
+
+	// Create the NetworkAttachmentDefinition
+	_, err = define.CreateNadWithMasterPlugin(apiClient, name, testNameSpace, macVlanPlugin)
+	if err != nil {
+		glog.V(90).Infof("Failed to create external NetworkAttachmentDefinition: %v", err)
+
+		return


please return err

evgenLevin · 2025-01-29T17:34:13Z

tests/cnf/core/network/internal/netenv/netenv.go

+	err = mcp.WaitToBeStableFor(stableDuration, waitingTime)
+
+	if err != nil {
+		glog.V(90).Infof("Failed to create external NetworkAttachmentDefinition: %v", err)


wrong description

evgenLevin · 2025-01-29T17:34:58Z

tests/cnf/core/network/internal/netenv/netenv.go

+func WaitForMcpStable(apiClient *clients.Settings, waitingTime, stableDuration time.Duration, mcpName string) error {
+	mcp, err := mco.Pull(apiClient, mcpName)
+
+	glog.V(90).Info("Waiting for mcp to be stable after adding or deleting a machine-Config")


Suggested change

glog.V(90).Info("Waiting for mcp to be stable after adding or deleting a machine-Config")

glog.V(90).Info("Waiting for mcp to be stable")

evgenLevin · 2025-01-29T17:38:31Z

tests/cnf/core/network/security/internal/tsparams/securityvars.go

+	// TestNamespaceName security-tests namespace where all test cases are performed.
+	TestNamespaceName = "security-tests"
+	// ReporterCRDsToDump tells to the reporter what CRs to dump.
+	ReporterCRDsToDump = []k8sreporter.CRData{}


Please add CRs what you want to dump after the failure. machineconfigs or something else

Let me know when you are free for a call

gkopels force-pushed the add-custom-nftables-test-cases branch 10 times, most recently from 19b87aa to 6bbb0c1 Compare January 20, 2025 08:18

evgenLevin reviewed Jan 21, 2025

View reviewed changes

gkopels force-pushed the add-custom-nftables-test-cases branch 18 times, most recently from b336a5a to e306cc0 Compare January 22, 2025 11:11

evgenLevin reviewed Jan 22, 2025

View reviewed changes

gkopels force-pushed the add-custom-nftables-test-cases branch 14 times, most recently from ee3ba43 to 5ffcef2 Compare January 27, 2025 06:27

evgenLevin reviewed Jan 27, 2025

View reviewed changes

gkopels force-pushed the add-custom-nftables-test-cases branch 4 times, most recently from a4ab1e0 to 602b55a Compare January 28, 2025 14:23

evgenLevin reviewed Jan 28, 2025

View reviewed changes

gkopels force-pushed the add-custom-nftables-test-cases branch from 602b55a to 471b138 Compare January 29, 2025 10:57

evgenLevin reviewed Jan 29, 2025

View reviewed changes

gkopels force-pushed the add-custom-nftables-test-cases branch from 471b138 to 3686479 Compare January 30, 2025 17:31

cnf:network-add-nftables-test-cases

36d1201

gkopels force-pushed the add-custom-nftables-test-cases branch from 3686479 to 36d1201 Compare January 30, 2025 17:38

	CustomFireWallDelete = `data:;base64, ` +
	CustomFirewallDelete = `data:;base64, ` +

		masterPodIPv4Prefix = "172.16.0.1/24"
		masterPodIPv4Address = "172.16.0.1"

		// This yaml is created using butane file.
		// butane mc-update-input-rule8888.bu -o mc-custom-firewall-input-port8888.yaml.


		It("Verify the creation of a new custom node firewall NFTables table with an ingress rule",
		reportxml.ID("77412"), func() {

	glog.V(2).Info("Creating external BR-EX NetworkAttachmentDefinition")
	glog.V(90).Info("Creating external BR-EX NetworkAttachmentDefinition")

	glog.V(2).Infof("Successfully created external NetworkAttachmentDefinition: %s", name)
	glog.V(90).Infof("Successfully created external NetworkAttachmentDefinition: %s", name)

	glog.V(90).Info("Waiting for mcp to be stable after adding or deleting a machine-Config")
	glog.V(90).Info("Waiting for mcp to be stable")

cnf:network-add-nftables-test-cases #366

Are you sure you want to change the base?

cnf:network-add-nftables-test-cases #366

Conversation

gkopels commented Jan 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkopels Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgenLevin Jan 27, 2025 • edited by gkopels Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgenLevin Jan 27, 2025 • edited by gkopels Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgenLevin Jan 27, 2025 • edited by gkopels Loading

Choose a reason for hiding this comment

evgenLevin Jan 27, 2025 • edited by gkopels Loading

Choose a reason for hiding this comment

evgenLevin Jan 27, 2025 • edited by gkopels Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evgenLevin Jan 29, 2025 • edited Loading

Choose a reason for hiding this comment

gkopels Jan 21, 2025 •

edited

Loading

evgenLevin Jan 27, 2025 •

edited by gkopels

Loading

evgenLevin Jan 27, 2025 •

edited by gkopels

Loading

evgenLevin Jan 27, 2025 •

edited by gkopels

Loading

evgenLevin Jan 27, 2025 •

edited by gkopels

Loading

evgenLevin Jan 27, 2025 •

edited by gkopels

Loading

evgenLevin Jan 29, 2025 •

edited

Loading