Add dynamic ranges to generated communication matrix

This PR is adding the following changes:

As requested from the partenrs, the dynamic port ranges of the cluster (Linux dynamic\private ranges, kubelet node port dynamic range, host level services dynamic range), are added to the generated communication matrix in all of the supported formats. The dynamic ranges are extracted in the following ways:
Node port dynamic range: we are looking for a custom defined range (network.Spec.ServiceNodePortRange), if there is no range set, we use a default static range (30000-32767)
Linux dynamic\private range: we retrive the dynamic range by reading the host sysctl (/proc/sys/net/ipv4/ip_local_port_range).
We also have added the option of using a custom entries file that include ranges, so partners will be able to add their own ranges if needed.

The e2e tests have been modified the following:

EPS vs SS: in the comparsion between the matrices, we also check if host level open ports (from ss matrix) which don't have an EPS are in the range specified in the generated commatrix.
Doc vs EPS: in the comparison between the matrices, we also check if the ports in the generated commatrix which are not documented in the doc matrix, do appear in the doc ranges.
The custom entries samples had been modified to also include ranges.
commatrix.go file's unit tests had been modified to allow mocking of a debugpod in the tests. For that sense, we have added to the commatrixCreator struct a utils field similar to what's done in the ConnectionCheck struct.

Author: shirmoran

cmd/generate/generate.go

@@ -6,9 +6,6 @@ import (
 	"path/filepath"
 	"slices"
 	"strings"
-	"time"
-
-	"context"
 
 	"github.com/openshift-kni/commatrix/pkg/client"
 	commatrixcreator "github.com/openshift-kni/commatrix/pkg/commatrix-creator"
@@ -20,13 +17,9 @@ import (
 	configv1 "github.com/openshift/api/config/v1"
 	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
-	corev1 "k8s.io/api/core/v1"
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/cli-runtime/pkg/genericclioptions"
 	"k8s.io/cli-runtime/pkg/genericiooptions"
 	"k8s.io/kubectl/pkg/util/templates"
-	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/openshift-kni/commatrix/pkg/types"
 )
@@ -261,7 +254,7 @@ func generateMatrix(o *GenerateOptions, controlPlaneTopology configv1.TopologyMo
 	}
 
 	log.Debug("Creating communication matrix")
-	commMatrix, err := commatrixcreator.New(epExporter, o.customEntriesPath, o.customEntriesFormat, platformType, controlPlaneTopology, ipv6Enabled)
+	commMatrix, err := commatrixcreator.New(epExporter, o.customEntriesPath, o.customEntriesFormat, platformType, controlPlaneTopology, ipv6Enabled, o.utilsHelpers)
 	if err != nil {
 		return nil, err
 	}
@@ -300,21 +293,6 @@ func generateSS(o *GenerateOptions) (*types.ComMatrix, error) {
 	defer func() {
 		if delErr := o.utilsHelpers.DeleteNamespace(consts.DefaultDebugNamespace); delErr != nil {
 			log.Warnf("failed to delete namespace %s: %v", consts.DefaultDebugNamespace, delErr)
-			return
-		}
-		if pollErr := wait.PollUntilContextTimeout(context.TODO(), time.Second, 2*time.Minute, true, func(ctx context.Context) (bool, error) {
-			ns := &corev1.Namespace{}
-			err := o.cs.Get(context.TODO(), ctrlclient.ObjectKey{Name: consts.DefaultDebugNamespace}, ns)
-			if apierrors.IsNotFound(err) {
-				return true, nil
-			}
-			if err != nil {
-				log.Warningf("retrying due to error: %v", err)
-				return false, nil // keep retrying
-			}
-			return false, nil
-		}); pollErr != nil {
-			log.Errorf("error while waiting for namespace %s deletion: %v", consts.DefaultDebugNamespace, pollErr)
 		}
 	}()
 

pkg/commatrix-creator/commatrix.go

@@ -9,9 +9,11 @@ import (
 
 	log "github.com/sirupsen/logrus"
 
+	dynamicranges "github.com/openshift-kni/commatrix/pkg/dynamic-ranges"
 	"github.com/openshift-kni/commatrix/pkg/endpointslices"
 	"github.com/openshift-kni/commatrix/pkg/mcp"
 	"github.com/openshift-kni/commatrix/pkg/types"
+	"github.com/openshift-kni/commatrix/pkg/utils"
 	configv1 "github.com/openshift/api/config/v1"
 )
 
@@ -22,16 +24,18 @@ type CommunicationMatrixCreator struct {
 	platformType         configv1.PlatformType
 	controlPlaneTopology configv1.TopologyMode
 	ipv6Enabled          bool
+	utilsHelpers         utils.UtilsInterface
 }
 
-func New(exporter *endpointslices.EndpointSlicesExporter, customEntriesPath string, customEntriesFormat string, platformType configv1.PlatformType, controlPlaneTopology configv1.TopologyMode, ipv6Enabled bool) (*CommunicationMatrixCreator, error) {
+func New(exporter *endpointslices.EndpointSlicesExporter, customEntriesPath string, customEntriesFormat string, platformType configv1.PlatformType, controlPlaneTopology configv1.TopologyMode, ipv6Enabled bool, utilsHelpers utils.UtilsInterface) (*CommunicationMatrixCreator, error) {
 	return &CommunicationMatrixCreator{
 		exporter:             exporter,
 		customEntriesPath:    customEntriesPath,
 		customEntriesFormat:  customEntriesFormat,
 		platformType:         platformType,
 		controlPlaneTopology: controlPlaneTopology,
 		ipv6Enabled:          ipv6Enabled,
+		utilsHelpers:         utilsHelpers,
 	}, nil
 }
 
@@ -74,23 +78,33 @@ func (cm *CommunicationMatrixCreator) CreateEndpointMatrix() (*types.ComMatrix,
 	staticEntries = expandStaticEntriesByPool(staticEntries, PoolRolesForStaticEntriesExpansion)
 	epSliceComDetails = append(epSliceComDetails, staticEntries...)
 
+	var customMatrix *types.ComMatrix
 	if cm.customEntriesPath != "" {
 		log.Debug("Loading custom entries from file")
-		customComDetails, err := cm.GetComDetailsListFromFile()
+		customMatrix, err = cm.GetComMatrixFromFile()
 		if err != nil {
 			log.Errorf("Failed adding custom entries: %s", err)
 			return nil, fmt.Errorf("failed adding custom entries: %s", err)
 		}
-		epSliceComDetails = append(epSliceComDetails, customComDetails...)
+		epSliceComDetails = append(epSliceComDetails, customMatrix.Ports...)
 	}
 
-	commMatrix := &types.ComMatrix{Ports: epSliceComDetails}
+	dynamicRanges, err := dynamicranges.GetDynamicRanges(cm.exporter, cm.utilsHelpers)
+	if err != nil {
+		log.Errorf("Failed to get dynamic ranges: %v", err)
+		return nil, fmt.Errorf("failed to get dynamic ranges: %w", err)
+	}
+	if customMatrix != nil && len(customMatrix.DynamicRanges) > 0 {
+		dynamicRanges = append(dynamicRanges, customMatrix.DynamicRanges...)
+	}
+
+	commMatrix := &types.ComMatrix{Ports: epSliceComDetails, DynamicRanges: dynamicRanges}
 	log.Debug("Sorting ComMatrix and removing duplicates")
 	commMatrix.SortAndRemoveDuplicates()
 	return commMatrix, nil
 }
 
-func (cm *CommunicationMatrixCreator) GetComDetailsListFromFile() ([]types.ComDetails, error) {
+func (cm *CommunicationMatrixCreator) GetComMatrixFromFile() (*types.ComMatrix, error) {
 	log.Debugf("Opening file %s", cm.customEntriesPath)
 	f, err := os.Open(filepath.Clean(cm.customEntriesPath))
 	if err != nil {
@@ -107,7 +121,7 @@ func (cm *CommunicationMatrixCreator) GetComDetailsListFromFile() ([]types.ComDe
 	}
 
 	log.Debugf("Unmarshalling file content with format %s", cm.customEntriesFormat)
-	res, err := types.ParseToComDetailsList(raw, cm.customEntriesFormat)
+	res, err := types.ParseToComMatrix(raw, cm.customEntriesFormat)
 	if err != nil {
 		log.Errorf("Failed to unmarshal %s file: %v", cm.customEntriesFormat, err)
 		return nil, fmt.Errorf("failed to unmarshal custom entries file: %v", err)

pkg/commatrix-creator/commatrix_test.go

@@ -6,15 +6,18 @@ import (
 
 	g "github.com/onsi/ginkgo/v2"
 	o "github.com/onsi/gomega"
+	gomock "go.uber.org/mock/gomock"
 	corev1 "k8s.io/api/core/v1"
 	discoveryv1 "k8s.io/api/discovery/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"github.com/openshift-kni/commatrix/pkg/client"
+	"github.com/openshift-kni/commatrix/pkg/consts"
 	"github.com/openshift-kni/commatrix/pkg/endpointslices"
 	matrixdiff "github.com/openshift-kni/commatrix/pkg/matrix-diff"
 	"github.com/openshift-kni/commatrix/pkg/types"
+	mock_utils "github.com/openshift-kni/commatrix/pkg/utils/mock"
 	configv1 "github.com/openshift/api/config/v1"
 	machineconfigurationv1 "github.com/openshift/api/machineconfiguration/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -67,6 +70,16 @@ var (
 			Optional:  false,
 		},
 	}
+	exampleDynamicRanges = []types.DynamicRange{
+		{
+			Direction:   "ingress",
+			Protocol:    "TCP",
+			MinPort:     9000,
+			MaxPort:     9999,
+			Description: "example dynamic range",
+			Optional:    false,
+		},
+	}
 )
 
 // Test resources.
@@ -289,56 +302,66 @@ var (
 			},
 		},
 	}
+
+	testNetwork = &configv1.Network{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "cluster",
+		},
+		Spec: configv1.NetworkSpec{
+			ServiceNodePortRange: "30000-32767",
+		},
+	}
 )
 
 var _ = g.Describe("Commatrix creator pkg tests", func() {
 	g.Context("Get Costume entries List From File", func() {
 		for _, format := range []string{types.FormatCSV, types.FormatJSON, types.FormatYAML} {
 			g.It(fmt.Sprintf("Should successfully extract ComDetails from a %s file", format), func() {
 				g.By(fmt.Sprintf("Creating new communication matrix with %s static entries format", format))
-				cm, err := New(nil, fmt.Sprintf("../../samples/custom-entries/example-custom-entries.%s", format), format, configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false)
+				cm, err := New(nil, fmt.Sprintf("../../samples/custom-entries/example-custom-entries.%s", format), format, configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false, nil)
 				o.Expect(err).ToNot(o.HaveOccurred())
 
-				g.By("Getting ComDetails List From File")
-				gotComDetails, err := cm.GetComDetailsListFromFile()
+				g.By("Getting ComMatrix From File")
+				gotComMatrix, err := cm.GetComMatrixFromFile()
 				o.Expect(err).ToNot(o.HaveOccurred())
 
-				g.By("Comparing gotten ComDetails to wanted ComDetials")
-				o.Expect(gotComDetails).To(o.Equal(exampleComDetailsList))
+				g.By("Comparing gotten ComMatrix.Ports to wanted ComDetials")
+				o.Expect(gotComMatrix.Ports).To(o.Equal(exampleComDetailsList))
+				o.Expect(gotComMatrix.DynamicRanges).To(o.Equal(exampleDynamicRanges))
 			})
 		}
 
 		g.It("Should return an error due to non-matched customEntriesPath and customEntriesFormat types", func() {
 			g.By("Creating new communication matrix with non-matched customEntriesPath and customEntriesFormat")
-			cm, err := New(nil, "../../samples/custom-entries/example-custom-entries.csv", types.FormatJSON, configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false)
+			cm, err := New(nil, "../../samples/custom-entries/example-custom-entries.csv", types.FormatJSON, configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false, nil)
 			o.Expect(err).ToNot(o.HaveOccurred())
 
-			g.By("Getting ComDetails List From File")
-			gotComDetails, err := cm.GetComDetailsListFromFile()
+			g.By("Getting ComMatrix From File")
+			gotComMatrix, err := cm.GetComMatrixFromFile()
 			o.Expect(err).To(o.HaveOccurred())
 
-			g.By("Comparing gotten ComDetails to empty ComDetials")
-			o.Expect(gotComDetails).To(o.Equal(nilComDetailsList))
+			g.By("Expecting nil ComMatrix on error")
+			o.Expect(gotComMatrix).To(o.BeNil())
 		})
 
 		g.It("Should return an error due to an invalid customEntriesFormat", func() {
 			g.By("Creating new communication matrix with invalid customEntriesFormat")
-			cm, err := New(nil, "../../samples/custom-entries/example-custom-entries.csv", types.FormatNFT, configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false)
+			cm, err := New(nil, "../../samples/custom-entries/example-custom-entries.csv", types.FormatNFT, configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false, nil)
 			o.Expect(err).ToNot(o.HaveOccurred())
 
-			g.By("Getting ComDetails List From File")
-			gotComDetails, err := cm.GetComDetailsListFromFile()
+			g.By("Getting ComMatrix From File")
+			gotComMatrix, err := cm.GetComMatrixFromFile()
 			o.Expect(err).To(o.HaveOccurred())
 
-			g.By("Comparing gotten ComDetails to empty ComDetials")
-			o.Expect(gotComDetails).To(o.Equal(nilComDetailsList))
+			g.By("Expecting nil ComMatrix on error")
+			o.Expect(gotComMatrix).To(o.BeNil())
 		})
 	})
 
 	g.Context("Get static entries from file", func() {
 		g.It("Should successfully get static entries suitable to baremetal standard cluster", func() {
 			g.By("Creating new communication matrix suitable to baremetal standard cluster")
-			cm, err := New(nil, "", "", configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false)
+			cm, err := New(nil, "", "", configv1.BareMetalPlatformType, configv1.HighlyAvailableTopologyMode, false, nil)
 			o.Expect(err).ToNot(o.HaveOccurred())
 
 			g.By("Getting static entries comDetails of the created communication matrix")
@@ -353,7 +376,7 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 
 		g.It("Should successfully get static entries suitable to baremetal SNO cluster", func() {
 			g.By("Creating new communication matrix suitable to baremetal SNO cluster")
-			cm, err := New(nil, "", "", configv1.BareMetalPlatformType, configv1.SingleReplicaTopologyMode, false)
+			cm, err := New(nil, "", "", configv1.BareMetalPlatformType, configv1.SingleReplicaTopologyMode, false, nil)
 			o.Expect(err).ToNot(o.HaveOccurred())
 
 			g.By("Getting static entries comDetails of the created communication matrix")
@@ -367,7 +390,7 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 
 		g.It("Should return an error due to an invalid value for cluster environment", func() {
 			g.By("Creating new communication matrix with an invalid value for cluster environment")
-			cm, err := New(nil, "", "", "invalid", configv1.SingleReplicaTopologyMode, false)
+			cm, err := New(nil, "", "", "invalid", configv1.SingleReplicaTopologyMode, false, nil)
 			o.Expect(err).ToNot(o.HaveOccurred())
 
 			g.By("Getting static entries comDetails of the created communication matrix")
@@ -381,6 +404,9 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 	})
 
 	g.Context("Create EndpointSlice Matrix", func() {
+		var ctrl *gomock.Controller
+		var mockUtils *mock_utils.MockUtilsInterface
+
 		g.BeforeEach(func() {
 			sch := runtime.NewScheme()
 
@@ -390,9 +416,11 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 			o.Expect(err).NotTo(o.HaveOccurred())
 			err = machineconfigurationv1.AddToScheme(sch)
 			o.Expect(err).NotTo(o.HaveOccurred())
+			err = configv1.AddToScheme(sch)
+			o.Expect(err).NotTo(o.HaveOccurred())
 
-			fakeClient := fake.NewClientBuilder().WithScheme(sch).WithObjects(testNode, testNodeWorker, testPod, testService, testEndpointSlice, mcpWorker, mcpMaster).Build()
-			fakeClientset := fakek.NewSimpleClientset()
+			fakeClient := fake.NewClientBuilder().WithScheme(sch).WithObjects(testNode, testNodeWorker, testPod, testService, testEndpointSlice, mcpWorker, mcpMaster, testNetwork).Build()
+			fakeClientset := fakek.NewSimpleClientset(testNode, testNodeWorker)
 
 			clientset := &client.ClientSet{
 				Client:          fakeClient,
@@ -401,11 +429,43 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 
 			endpointSlices, err = endpointslices.New(clientset)
 			o.Expect(err).ToNot(o.HaveOccurred())
+
+			// Set up mock utils to avoid pod creation in tests
+			ctrl = gomock.NewController(g.GinkgoT())
+			mockUtils = mock_utils.NewMockUtilsInterface(ctrl)
+
+			// Mock all the utils calls needed for getLinuxDynamicPrivateRange
+			mockUtils.EXPECT().CreateNamespace(consts.DefaultDebugNamespace).Return(nil).AnyTimes()
+			mockUtils.EXPECT().DeleteNamespace(consts.DefaultDebugNamespace).Return(nil).AnyTimes()
+
+			// Create a mock pod
+			mockPod := &corev1.Pod{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "debug-pod",
+					Namespace: consts.DefaultDebugNamespace,
+				},
+				Status: corev1.PodStatus{
+					Phase: corev1.PodRunning,
+				},
+			}
+			mockUtils.EXPECT().CreatePodOnNode(gomock.Any(), consts.DefaultDebugNamespace, gomock.Any(), gomock.Any()).Return(mockPod, nil).AnyTimes()
+			mockUtils.EXPECT().DeletePod(mockPod).Return(nil).AnyTimes()
+			mockUtils.EXPECT().WaitForPodStatus(consts.DefaultDebugNamespace, mockPod, corev1.PodRunning).Return(nil).AnyTimes()
+
+			// Mock the command output to return a default port range
+			mockUtils.EXPECT().RunCommandOnPod(mockPod, gomock.Any()).Return([]byte("32768 60999\n"), nil).AnyTimes()
+		})
+
+		g.AfterEach(func() {
+			// Finish the controller
+			if ctrl != nil {
+				ctrl.Finish()
+			}
 		})
 
 		g.It("Should successfully create an endpoint matrix with custom entries", func() {
 			g.By("Creating new communication matrix with static entries")
-			commatrixCreator, err := New(endpointSlices, "../../samples/custom-entries/example-custom-entries.csv", types.FormatCSV, configv1.AWSPlatformType, configv1.SingleReplicaTopologyMode, false)
+			commatrixCreator, err := New(endpointSlices, "../../samples/custom-entries/example-custom-entries.csv", types.FormatCSV, configv1.AWSPlatformType, configv1.SingleReplicaTopologyMode, false, mockUtils)
 			o.Expect(err).ToNot(o.HaveOccurred())
 			commatrix, err := commatrixCreator.CreateEndpointMatrix()
 			o.Expect(err).ToNot(o.HaveOccurred())
@@ -429,7 +489,7 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 
 		g.It("Should successfully create an endpoint matrix without custom entries", func() {
 			g.By("Creating new communication matrix without static entries")
-			commatrixCreator, err := New(endpointSlices, "", "", configv1.AWSPlatformType, configv1.SingleReplicaTopologyMode, false)
+			commatrixCreator, err := New(endpointSlices, "", "", configv1.AWSPlatformType, configv1.SingleReplicaTopologyMode, false, mockUtils)
 			o.Expect(err).ToNot(o.HaveOccurred())
 			commatrix, err := commatrixCreator.CreateEndpointMatrix()
 			o.Expect(err).ToNot(o.HaveOccurred())
@@ -449,7 +509,7 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 
 		g.It("Should include IPv6 static entries when ipv6Enabled is true on Standard", func() {
 			g.By("Creating communication matrix with ipv6Enabled=true for Standard")
-			commatrixCreator, err := New(endpointSlices, "", "", configv1.AWSPlatformType, configv1.HighlyAvailableTopologyMode, true)
+			commatrixCreator, err := New(endpointSlices, "", "", configv1.AWSPlatformType, configv1.HighlyAvailableTopologyMode, true, mockUtils)
 			o.Expect(err).ToNot(o.HaveOccurred())
 			commatrix, err := commatrixCreator.CreateEndpointMatrix()
 			o.Expect(err).ToNot(o.HaveOccurred())
@@ -496,7 +556,7 @@ var _ = g.Describe("Commatrix creator pkg tests", func() {
 			o.Expect(err).ToNot(o.HaveOccurred())
 
 			g.By("Creating endpoint matrix")
-			commatrixCreator, err := New(localhostEndpointSlices, "", "", configv1.AWSPlatformType, configv1.SingleReplicaTopologyMode, false)
+			commatrixCreator, err := New(localhostEndpointSlices, "", "", configv1.AWSPlatformType, configv1.SingleReplicaTopologyMode, false, nil)
 			o.Expect(err).ToNot(o.HaveOccurred())
 			commatrix, err := commatrixCreator.CreateEndpointMatrix()
 			o.Expect(err).ToNot(o.HaveOccurred())