networking: Add MTU value test for SRIOV NAD's

CATALOG.md

@@ -7,7 +7,7 @@ Depending on the workload type, not all tests are required to pass to satisfy be
 
 ## Test cases summary
 
-### Total test cases: 114
+### Total test cases: 115
 
 ### Total suites: 10
 
@@ -17,7 +17,7 @@ Depending on the workload type, not all tests are required to pass to satisfy be
 |affiliated-certification|4|
 |lifecycle|18|
 |manageability|2|
-|networking|11|
+|networking|12|
 |observability|5|
 |operator|11|
 |performance|6|
@@ -30,11 +30,11 @@ Depending on the workload type, not all tests are required to pass to satisfy be
 |---|---|
 |9|3|
 
-### Far-Edge specific tests only: 8
+### Far-Edge specific tests only: 9
 
 |Mandatory|Optional|
 |---|---|
-|7|1|
+|8|1|
 
 ### Non-Telco specific tests only: 67
 
@@ -974,6 +974,22 @@ Tags|telco,networking
 |Non-Telco|Optional|
 |Telco|Mandatory|
 
+#### networking-network-attachment-definition-sriov-mtu
+
+Property|Description
+---|---
+Unique ID|networking-network-attachment-definition-sriov-mtu
+Description|Ensures that MTU values are set correctly in NetworkAttachmentDefinitions for SRIOV network interfaces.
+Suggested Remediation|Ensure that the MTU of the SR-IOV network attachment definition is set explicitly.
+Best Practice Reference|No Doc Link - Far Edge
+Exception Process|There is no documented exception process for this.
+Tags|faredge,networking
+|**Scenario**|**Optional/Mandatory**|
+|Extended|Mandatory|
+|Far-Edge|Mandatory|
+|Non-Telco|Mandatory|
+|Telco|Mandatory|
+
 #### networking-network-policy-deny-all
 
 Property|Description

expected_results.yaml

@@ -84,6 +84,7 @@ testCases:
     - networking-dpdk-cpu-pinning-exec-probe
     - networking-icmpv6-connectivity
     - networking-restart-on-reboot-sriov-pod
+    - networking-network-attachment-definition-sriov-mtu
     - performance-exclusive-cpu-pool-rt-scheduling-policy
     - performance-isolated-cpu-pool-rt-scheduling-policy
     - performance-rt-apps-no-exec-probes

pkg/provider/pods.go

@@ -265,6 +265,61 @@ func getCNCFNetworksNamesFromPodAnnotation(networksAnnotation string) []string {
 	return networkNames
 }
 
+// isNetworkAttachmentDefinitionSRIOVConfigMTUSet is a helper function to check whether a CNI config
+// string has any config for MTU for SRIOV configs only
+
+/*
+	{
+		"cniVersion": "0.4.0",
+		"name": "vlan-100",
+		"plugins": [
+			{
+				"type": "sriov",
+				"master": "ext0",
+				"mtu": 1500,
+				"vlanId": 100,
+				"linkInContainer": true,
+				"ipam": {"type": "whereabouts", "ipRanges": [{"range": "1.1.1.0/24"}]}
+			}
+		]
+	}
+*/
+func isNetworkAttachmentDefinitionSRIOVConfigMTUSet(nadConfig string) (bool, error) {
+	const (
+		typeSriov = "sriov"
+	)
+
+	type CNIConfig struct {
+		CniVersion string  `json:"cniVersion"`
+		Name       string  `json:"name"`
+		Type       *string `json:"type,omitempty"`
+		Plugins    *[]struct {
+			Type string `json:"type"`
+			MTU  int    `json:"mtu"`
+		} `json:"plugins,omitempty"`
+	}
+
+	cniConfig := CNIConfig{}
+	if err := json.Unmarshal([]byte(nadConfig), &cniConfig); err != nil {
+		return false, fmt.Errorf("failed to unmarshal cni config %s: %v", nadConfig, err)
+	}
+
+	if cniConfig.Plugins == nil {
+		return false, fmt.Errorf("invalid multi-plugins cni config: %s", nadConfig)
+	}
+
+	log.Debug("CNI plugins: %+v", *cniConfig.Plugins)
+	for i := range *cniConfig.Plugins {
+		plugin := (*cniConfig.Plugins)[i]
+		if plugin.Type == typeSriov && plugin.MTU > 0 {
+			return true, nil
+		}
+	}
+
+	// No sriov plugin type found.
+	return false, nil
+}
+
 // isNetworkAttachmentDefinitionConfigTypeSRIOV is a helper function to check whether a CNI
 // config string has any config for sriov plugin.
 // CNI config has two modes: single CNI plugin, or multi-plugins:
@@ -375,8 +430,50 @@ func (p *Pod) IsUsingSRIOV() (bool, error) {
 	return false, nil
 }
 
+// IsUsingSRIOVWithMTU returns true if any of the pod's interfaces is a sriov one with MTU set.
+func (p *Pod) IsUsingSRIOVWithMTU() (bool, error) {
+	const (
+		cncfNetworksAnnotation = "k8s.v1.cni.cncf.io/networks"
+	)
+
+	cncfNetworks, exist := p.Annotations[cncfNetworksAnnotation]
+	if !exist {
+		return false, nil
+	}
+
+	// Get all CNCF network names
+	cncfNetworkNames := getCNCFNetworksNamesFromPodAnnotation(cncfNetworks)
+
+	// For each CNCF network, get its network attachment definition and check
+	// whether its config's type is "sriov"
+	oc := clientsholder.GetClientsHolder()
+
+	for _, networkName := range cncfNetworkNames {
+		log.Debug("%s: Reviewing network-attachment definition %q", p, networkName)
+		nad, err := oc.CNCFNetworkingClient.NetworkAttachmentDefinitions(
+			p.Namespace).Get(context.TODO(), networkName, metav1.GetOptions{})
+		if err != nil {
+			return false, fmt.Errorf("failed to get NetworkAttachment %s: %v", networkName, err)
+		}
+
+		isSRIOV, err := isNetworkAttachmentDefinitionSRIOVConfigMTUSet(nad.Spec.Config)
+		if err != nil {
+			return false, fmt.Errorf("failed to know if network-attachment %s is sriov with MTU: %v",
+				networkName, err)
+		}
+
+		log.Debug("%s: NAD config: %s", p, nad.Spec.Config)
+		if isSRIOV {
+			return true, nil
+		}
+	}
+
+	return false, nil
+}
+
 //nolint:gocritic
-func (p *Pod) IsUsingClusterRoleBinding(clusterRoleBindings []rbacv1.ClusterRoleBinding, logger *log.Logger) (bool, string, error) {
+func (p *Pod) IsUsingClusterRoleBinding(clusterRoleBindings []rbacv1.ClusterRoleBinding,
+	logger *log.Logger) (bool, string, error) {
 	// This function accepts a list of clusterRoleBindings and checks to see if the pod's service account is
 	// tied to any of them.  If it is, then it returns true, otherwise it returns false.
 	logger.Info("Pod %q is using service account %q", p, p.Pod.Spec.ServiceAccountName)
@@ -386,7 +483,8 @@ func (p *Pod) IsUsingClusterRoleBinding(clusterRoleBindings []rbacv1.ClusterRole
 	// role bindings.
 	for crbIndex := range clusterRoleBindings {
 		for _, subject := range clusterRoleBindings[crbIndex].Subjects {
-			if subject.Kind == rbacv1.ServiceAccountKind && subject.Name == p.Pod.Spec.ServiceAccountName && subject.Namespace == p.Pod.Namespace {
+			if subject.Kind == rbacv1.ServiceAccountKind &&
+				subject.Name == p.Pod.Spec.ServiceAccountName && subject.Namespace == p.Pod.Namespace {
 				logger.Error("Pod %q has service account %q that is tied to cluster role binding %q", p.Pod.Name, p.Pod.Spec.ServiceAccountName, clusterRoleBindings[crbIndex].Name)
 				return true, clusterRoleBindings[crbIndex].RoleRef.Name, nil
 			}

pkg/provider/pods_test.go

@@ -573,3 +573,35 @@ func TestIsRunAsNonRoot(t *testing.T) {
 func boolPtr(b bool) *bool {
 	return &b
 }
+
+func TestIsNetworkAttachmentDefinitionSRIOVConfigMTUSet(t *testing.T) {
+	testCases := []struct {
+		networkAttachmentDefinition string
+		expectedResult              bool
+		expectedErrorMsg            string
+	}{
+		// Single plugin mode:
+		{
+			networkAttachmentDefinition: "",
+			expectedErrorMsg:            "failed to unmarshal cni config : unexpected end of JSON input",
+			expectedResult:              false,
+		},
+		{
+			networkAttachmentDefinition: `{"cniVersion":"0.4.0","name":"vlan-100","plugins":[{"type":"vlan","master":"ext0","mtu":1500,"vlanId":100,"linkInContainer":true,"ipam":{"type":"whereabouts","ipRanges":[{"range":"1.1.1.0/24"}]}}]}`,
+			expectedResult:              false,
+		},
+		{
+			networkAttachmentDefinition: `{"cniVersion":"0.4.0","name":"vlan-100","plugins":[{"type":"sriov","master":"ext0","mtu":1500,"vlanId":100,"linkInContainer":true,"ipam":{"type":"whereabouts","ipRanges":[{"range":"1.1.1.0/24"}]}}]}`,
+			expectedResult:              true,
+		},
+	}
+
+	for _, testCase := range testCases {
+		isMTUSet, err := isNetworkAttachmentDefinitionSRIOVConfigMTUSet(testCase.networkAttachmentDefinition)
+		if err != nil {
+			assert.Equal(t, testCase.expectedErrorMsg, err.Error())
+		}
+
+		assert.Equal(t, testCase.expectedResult, isMTUSet)
+	}
+}

tests/identifiers/doclinks.go

@@ -8,18 +8,19 @@ const (
 	NoDocLink         = "No Doc Link"
 
 	// Networking Suite
-	TestICMPv4ConnectivityIdentifierDocLink         = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ipv4-&-ipv6"
-	TestNetworkPolicyDenyAllIdentifierDocLink       = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-vrfs-aka-routing-instances"
-	TestReservedExtendedPartnerPortsDocLink         = NoDocLinkExtended
-	TestDpdkCPUPinningExecProbeDocLink              = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-cpu-manager-pinning"
-	TestRestartOnRebootLabelOnPodsUsingSRIOVDocLink = NoDocLinkFarEdge
-	TestLimitedUseOfExecProbesIdentifierDocLink     = NoDocLinkFarEdge
-	TestICMPv6ConnectivityIdentifierDocLink         = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ipv4-&-ipv6"
-	TestICMPv4ConnectivityMultusIdentifierDocLink   = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-high-level-cnf-expectations"
-	TestICMPv6ConnectivityMultusIdentifierDocLink   = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-high-level-cnf-expectations"
-	TestServiceDualStackIdentifierDocLink           = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ipv4-&-ipv6"
-	TestUndeclaredContainerPortsUsageDocLink        = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-requirements-cnf-reqs"
-	TestOCPReservedPortsUsageDocLink                = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ports-reserved-by-openshift"
+	TestICMPv4ConnectivityIdentifierDocLink             = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ipv4-&-ipv6"
+	TestNetworkPolicyDenyAllIdentifierDocLink           = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-vrfs-aka-routing-instances"
+	TestReservedExtendedPartnerPortsDocLink             = NoDocLinkExtended
+	TestDpdkCPUPinningExecProbeDocLink                  = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-cpu-manager-pinning"
+	TestRestartOnRebootLabelOnPodsUsingSRIOVDocLink     = NoDocLinkFarEdge
+	TestNetworkAttachmentDefinitionSRIOVUsingMTUDocLink = NoDocLinkFarEdge
+	TestLimitedUseOfExecProbesIdentifierDocLink         = NoDocLinkFarEdge
+	TestICMPv6ConnectivityIdentifierDocLink             = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ipv4-&-ipv6"
+	TestICMPv4ConnectivityMultusIdentifierDocLink       = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-high-level-cnf-expectations"
+	TestICMPv6ConnectivityMultusIdentifierDocLink       = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-high-level-cnf-expectations"
+	TestServiceDualStackIdentifierDocLink               = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ipv4-&-ipv6"
+	TestUndeclaredContainerPortsUsageDocLink            = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-requirements-cnf-reqs"
+	TestOCPReservedPortsUsageDocLink                    = "https://redhat-best-practices-for-k8s.github.io/guide/#redhat-best-practices-for-k8s-ports-reserved-by-openshift"
 
 	// Access Control Suite
 	Test1337UIDIdentifierDocLink                             = NoDocLinkExtended

tests/identifiers/identifiers.go

@@ -101,6 +101,7 @@ var (
 	TestIsolatedCPUPoolSchedulingPolicy               claim.Identifier
 	TestRtAppNoExecProbes                             claim.Identifier
 	TestRestartOnRebootLabelOnPodsUsingSRIOV          claim.Identifier
+	TestNetworkAttachmentDefinitionSRIOVUsingMTU      claim.Identifier
 	TestSecConNonRootUserIdentifier                   claim.Identifier
 	TestSecContextIdentifier                          claim.Identifier
 	TestSecConPrivilegeEscalation                     claim.Identifier
@@ -579,6 +580,22 @@ func InitCatalog() map[claim.Identifier]claim.TestCaseDescription {
 		},
 		TagFarEdge)
 
+	TestNetworkAttachmentDefinitionSRIOVUsingMTU = AddCatalogEntry(
+		"network-attachment-definition-sriov-mtu",
+		common.NetworkingTestKey,
+		`Ensures that MTU values are set correctly in NetworkAttachmentDefinitions for SRIOV network interfaces.`,
+		SRIOVNetworkAttachmentDefinitionMTURemediation,
+		NoDocumentedProcess,
+		TestNetworkAttachmentDefinitionSRIOVUsingMTUDocLink,
+		false,
+		map[string]string{
+			FarEdge:  Mandatory,
+			Telco:    Mandatory,
+			NonTelco: Mandatory,
+			Extended: Mandatory,
+		},
+		TagFarEdge)
+
 	TestSecConNonRootUserIdentifier = AddCatalogEntry(
 		"security-context-non-root-user-check",
 		common.AccessControlTestKey,

tests/identifiers/remediation.go

@@ -192,6 +192,8 @@ const (
 
 	SRIOVPodsRestartOnRebootLabelRemediation = `Ensure that the label restart-on-reboot exists on pods that use SRIOV network interfaces.`
 
+	SRIOVNetworkAttachmentDefinitionMTURemediation = `Ensure that the MTU of the SR-IOV network attachment definition is set explicitly.`
+
 	HelmVersionV3Remediation = `Check Helm Chart is v3 and not v2 which is not supported due to security risks associated with Tiller.`
 
 	ContainerIsCertifiedDigestRemediation = "Ensure that your container has passed the Red Hat Container Certification Program (CCP)."

tests/networking/suite.go

@@ -153,6 +153,19 @@ func LoadChecks() {
 			testRestartOnRebootLabelOnPodsUsingSriov(c, sriovPods)
 			return nil
 		}))
+
+	// SRIOV MTU test case
+	checksGroup.Add(checksdb.NewCheck(identifiers.GetTestIDAndLabels(
+		identifiers.TestNetworkAttachmentDefinitionSRIOVUsingMTU)).
+		WithSkipCheckFn(testhelper.GetNoSRIOVPodsSkipFn(&env)).
+		WithCheckFn(func(c *checksdb.Check) error {
+			sriovPods, err := env.GetPodsUsingSRIOV()
+			if err != nil {
+				return fmt.Errorf("failure getting pods using SRIOV: %v", err)
+			}
+			testNetworkAttachmentDefinitionSRIOVUsingMTU(c, sriovPods)
+			return nil
+		}))
 }
 
 func testExecProbDenyAtCPUPinning(check *checksdb.Check, dpdkPods []*provider.Pod) {
@@ -409,3 +422,27 @@ func testRestartOnRebootLabelOnPodsUsingSriov(check *checksdb.Check, sriovPods [
 
 	check.SetResult(compliantObjects, nonCompliantObjects)
 }
+
+func testNetworkAttachmentDefinitionSRIOVUsingMTU(check *checksdb.Check, sriovPods []*provider.Pod) {
+	var compliantObjects []*testhelper.ReportObject
+	var nonCompliantObjects []*testhelper.ReportObject
+
+	for _, pod := range sriovPods {
+		result, err := pod.IsUsingSRIOVWithMTU()
+		if err != nil {
+			check.LogError("Failed to check if pod %q uses SRIOV with MTU, err: %v", pod, err)
+			nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(pod.Namespace, pod.Name, "Failed to check if pod uses SRIOV with MTU", false))
+			continue
+		}
+
+		if result {
+			check.LogInfo("Pod %q uses SRIOV with MTU", pod)
+			compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(pod.Namespace, pod.Name, "Pod uses SRIOV with MTU", true))
+		} else {
+			check.LogError("Pod %q uses SRIOV but the MTU is not set explicitly", pod)
+			nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(pod.Namespace, pod.Name, "Pod uses SRIOV but the MTU is not set explicitly", false))
+		}
+	}
+
+	check.SetResult(compliantObjects, nonCompliantObjects)
+}