From baf76534033d1eb5b159aca26eb7b1eeb5969abd Mon Sep 17 00:00:00 2001 From: Brandon Palm Date: Thu, 10 Oct 2024 14:29:18 -0500 Subject: [PATCH] networking: Add MTU value test for SRIOV NAD's --- CATALOG.md | 24 ++++++-- expected_results.yaml | 1 + pkg/provider/pods.go | 102 ++++++++++++++++++++++++++++++- pkg/provider/pods_test.go | 32 ++++++++++ tests/identifiers/doclinks.go | 25 ++++---- tests/identifiers/identifiers.go | 17 ++++++ tests/identifiers/remediation.go | 2 + tests/networking/suite.go | 37 +++++++++++ 8 files changed, 222 insertions(+), 18 deletions(-) diff --git a/CATALOG.md b/CATALOG.md index 8ebcf0013..c83bf7817 100644 --- a/CATALOG.md +++ b/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 diff --git a/expected_results.yaml b/expected_results.yaml index 6b132fc19..8ab68f762 100644 --- a/expected_results.yaml +++ b/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 diff --git a/pkg/provider/pods.go b/pkg/provider/pods.go index a5e257437..bc6837653 100644 --- a/pkg/provider/pods.go +++ b/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 } diff --git a/pkg/provider/pods_test.go b/pkg/provider/pods_test.go index a997462ae..d49d55752 100644 --- a/pkg/provider/pods_test.go +++ b/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) + } +} diff --git a/tests/identifiers/doclinks.go b/tests/identifiers/doclinks.go index 11e3b87e4..6c834d8ba 100644 --- a/tests/identifiers/doclinks.go +++ b/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 diff --git a/tests/identifiers/identifiers.go b/tests/identifiers/identifiers.go index 31e11e238..764aed0f6 100644 --- a/tests/identifiers/identifiers.go +++ b/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, diff --git a/tests/identifiers/remediation.go b/tests/identifiers/remediation.go index e180da22c..815c22a7f 100644 --- a/tests/identifiers/remediation.go +++ b/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)." diff --git a/tests/networking/suite.go b/tests/networking/suite.go index 6c241503a..2be84dee0 100644 --- a/tests/networking/suite.go +++ b/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) +}