From 00262a41d3d8d20dd46e1838ed59b6f499bdfc43 Mon Sep 17 00:00:00 2001 From: jmontesi Date: Thu, 4 Jan 2024 12:49:38 +0100 Subject: [PATCH 1/3] networking: review logs The log level and the information printed has been reviewed for each log or added if missing. The resources are printed between quotes for easier identification in the log line. --- .../networking/icmp/icmp.go | 50 +++++++++----- .../networking/icmp/icmp_test.go | 59 ++-------------- .../networking/netcommons/netcommons.go | 22 +++--- .../networking/policies/policies.go | 15 ++--- .../networking/policies/policies_test.go | 7 +- cnf-certification-test/networking/suite.go | 67 ++++++++++++------- 6 files changed, 105 insertions(+), 115 deletions(-) diff --git a/cnf-certification-test/networking/icmp/icmp.go b/cnf-certification-test/networking/icmp/icmp.go index d8ee00a6d..4c0b99f7f 100644 --- a/cnf-certification-test/networking/icmp/icmp.go +++ b/cnf-certification-test/networking/icmp/icmp.go @@ -50,19 +50,20 @@ func (results PingResults) String() string { func BuildNetTestContext(pods []*provider.Pod, aIPVersion netcommons.IPVersion, aType netcommons.IFType, logger *log.Logger) (netsUnderTest map[string]netcommons.NetTestContext) { netsUnderTest = make(map[string]netcommons.NetTestContext) for _, put := range pods { + logger.Info("Testing Pod %q", put) if put.SkipNetTests { - logger.Info("Skipping %s because it is excluded from all connectivity tests", put) + logger.Info("Skipping %q because it is excluded from all connectivity tests", put) continue } if aType == netcommons.MULTUS { if put.SkipMultusNetTests { - logger.Info("Skipping pod %s because it is excluded from %s connectivity tests only", put.Name, aType) + logger.Info("Skipping pod %q because it is excluded from %q connectivity tests only", put.Name, aType) continue } for netKey, multusIPAddress := range put.MultusIPs { // The first container is used to get the network namespace - ProcessContainerIpsPerNet(put.Containers[0], netKey, multusIPAddress, netsUnderTest, aIPVersion) + processContainerIpsPerNet(put.Containers[0], netKey, multusIPAddress, netsUnderTest, aIPVersion, logger) } continue } @@ -70,22 +71,23 @@ func BuildNetTestContext(pods []*provider.Pod, aIPVersion netcommons.IPVersion, const defaultNetKey = "default" defaultIPAddress := put.Status.PodIPs // The first container is used to get the network namespace - ProcessContainerIpsPerNet(put.Containers[0], defaultNetKey, netcommons.PodIPsToStringList(defaultIPAddress), netsUnderTest, aIPVersion) + processContainerIpsPerNet(put.Containers[0], defaultNetKey, netcommons.PodIPsToStringList(defaultIPAddress), netsUnderTest, aIPVersion, logger) } return netsUnderTest } // processContainerIpsPerNet takes a container ip addresses for a given network attachment's and uses it as a test target. // The first container in the loop is selected as the test initiator. the Oc context of the container is used to initiate the pings -func ProcessContainerIpsPerNet(containerID *provider.Container, +func processContainerIpsPerNet(containerID *provider.Container, netKey string, ipAddresses []string, netsUnderTest map[string]netcommons.NetTestContext, - aIPVersion netcommons.IPVersion) { + aIPVersion netcommons.IPVersion, + logger *log.Logger) { ipAddressesFiltered := netcommons.FilterIPListByIPVersion(ipAddresses, aIPVersion) if len(ipAddressesFiltered) == 0 { // if no multus addresses found, skip this container - log.Debug("Skipping %s, Network %s because no multus IPs are present", containerID, netKey) + logger.Debug("Skipping %q, Network %q because no multus IPs are present", containerID, netKey) return } // Create an entry at "key" if it is not present @@ -97,7 +99,7 @@ func ProcessContainerIpsPerNet(containerID *provider.Container, // Then modify the copy firstIPIndex := 0 if entry.TesterSource.ContainerIdentifier == nil { - log.Debug("%s selected to initiate ping tests", containerID) + logger.Debug("%q selected to initiate ping tests", containerID) entry.TesterSource.ContainerIdentifier = containerID // if multiple interfaces are present for this network on this container/pod, pick the first one as the tester source ip entry.TesterSource.IP = ipAddressesFiltered[firstIPIndex] @@ -126,7 +128,7 @@ func RunNetworkingTests( //nolint:funlen logger.Debug("%s", netcommons.PrintNetTestContextMap(netsUnderTest)) skip = false if len(netsUnderTest) == 0 { - logger.Debug("There are no %s networks to test, skipping test", aIPVersion) + logger.Debug("There are no %q networks to test, skipping test", aIPVersion) skip = true return report, skip } @@ -139,27 +141,32 @@ func RunNetworkingTests( //nolint:funlen compliantNets[netName] = 0 nonCompliantNets[netName] = 0 if len(netUnderTest.DestTargets) == 0 { - logger.Debug("There are no containers to ping for %s network %s. A minimum of 2 containers is needed to run a ping test (a source and a destination) Skipping test", aIPVersion, netName) + logger.Debug("There are no containers to ping for %q network %q. A minimum of 2 containers is needed to run a ping test (a source and a destination) Skipping test", aIPVersion, netName) continue } atLeastOneNetworkTested = true - logger.Debug("%s Ping tests on network %s. Number of target IPs: %d", aIPVersion, netName, len(netUnderTest.DestTargets)) + logger.Debug("%q Ping tests on network %q. Number of target IPs: %d", aIPVersion, netName, len(netUnderTest.DestTargets)) for _, aDestIP := range netUnderTest.DestTargets { - logger.Debug("%s ping test on network %s from ( %s srcip: %s ) to ( %s dstip: %s )", + logger.Debug("%q ping test on network %q from ( %q srcip: %q ) to ( %q dstip: %q )", aIPVersion, netName, netUnderTest.TesterSource.ContainerIdentifier, netUnderTest.TesterSource.IP, aDestIP.ContainerIdentifier, aDestIP.IP) result, err := TestPing(netUnderTest.TesterSource.ContainerIdentifier, aDestIP, count) - logger.Debug("Ping results: %s", result.String()) - logger.Info("%s ping test on network %s from ( %s srcip: %s ) to ( %s dstip: %s ) result: %s", + logger.Debug("Ping results: %q", result) + logger.Info("%q ping test on network %q from ( %q srcip: %q ) to ( %q dstip: %q ) result: %q", aIPVersion, netName, netUnderTest.TesterSource.ContainerIdentifier, netUnderTest.TesterSource.IP, - aDestIP.ContainerIdentifier, aDestIP.IP, result.String()) + aDestIP.ContainerIdentifier, aDestIP.IP, result) if err != nil { - logger.Debug("Ping failed with err:%s", err) + logger.Debug("Ping failed, err=%s", err) } if result.outcome != testhelper.SUCCESS { + logger.Error("Ping from %q (srcip: %q) to %q (dstip: %q) failed", + netUnderTest.TesterSource.ContainerIdentifier, + netUnderTest.TesterSource.IP, + aDestIP.ContainerIdentifier, + aDestIP.IP) nonCompliantNets[netName]++ nonCompliantObject := testhelper.NewContainerReportObject(netUnderTest.TesterSource.ContainerIdentifier.Namespace, netUnderTest.TesterSource.ContainerIdentifier.Podname, @@ -173,6 +180,11 @@ func RunNetworkingTests( //nolint:funlen AddField(testhelper.DestinationIP, aDestIP.IP) report.NonCompliantObjectsOut = append(report.NonCompliantObjectsOut, nonCompliantObject) } else { + logger.Info("Ping from %q (srcip: %q) to %q (dstip: %q) succeeded", + netUnderTest.TesterSource.ContainerIdentifier, + netUnderTest.TesterSource.IP, + aDestIP.ContainerIdentifier, + aDestIP.IP) compliantNets[netName]++ CompliantObject := testhelper.NewContainerReportObject(netUnderTest.TesterSource.ContainerIdentifier.Namespace, netUnderTest.TesterSource.ContainerIdentifier.Podname, @@ -188,16 +200,18 @@ func RunNetworkingTests( //nolint:funlen } } if nonCompliantNets[netName] != 0 { + logger.Error("ICMP tests failed for %d IP source/destination in this network", nonCompliantNets[netName]) report.NonCompliantObjectsOut = append(report.NonCompliantObjectsOut, testhelper.NewReportObject(fmt.Sprintf("ICMP tests failed for %d IP source/destination in this network", nonCompliantNets[netName]), testhelper.NetworkType, false). AddField(testhelper.NetworkName, netName)) } if compliantNets[netName] != 0 { - report.CompliantObjectsOut = append(report.CompliantObjectsOut, testhelper.NewReportObject(fmt.Sprintf("ICMP tests were successful for all %d IP source/destination in this network", compliantNets[netName]), testhelper.NetworkType, true). + logger.Info("ICMP tests were successful for all %d IP source/destination in this network", compliantNets[netName]) + report.CompliantObjectsOut = append(report.CompliantObjectsOut, testhelper.NewReportObject(fmt.Sprintf("ICMP tests were successful for all %d IP source/destination in this network", compliantNets[netName]), testhelper.NetworkType, true). AddField(testhelper.NetworkName, netName)) } } if !atLeastOneNetworkTested { - logger.Debug("There are no %s networks to test, skipping test", aIPVersion) + logger.Debug("There are no %q networks to test, skipping test", aIPVersion) skip = true } diff --git a/cnf-certification-test/networking/icmp/icmp_test.go b/cnf-certification-test/networking/icmp/icmp_test.go index e8945bbd3..2be16ca9f 100644 --- a/cnf-certification-test/networking/icmp/icmp_test.go +++ b/cnf-certification-test/networking/icmp/icmp_test.go @@ -298,14 +298,17 @@ func TestProcessContainerIpsPerNet(t *testing.T) { }, }, } + var logArchive strings.Builder + log.SetupLogger(&logArchive, "INFO") for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ProcessContainerIpsPerNet( + processContainerIpsPerNet( tt.args.containerID, tt.args.netKey, tt.args.ipAddresses, tt.args.netsUnderTest, tt.args.aIPVersion, + log.GetLogger(), ) if !reflect.DeepEqual(tt.args.netsUnderTest, tt.args.wantNetsUnderTest) { t.Errorf( @@ -328,7 +331,6 @@ func TestBuildNetTestContext(t *testing.T) { name string args args wantNetsUnderTest map[string]netcommons.NetTestContext - wantLogArchive []string }{ { name: "ipv4ok", @@ -394,10 +396,6 @@ func TestBuildNetTestContext(t *testing.T) { DestTargets: nil, }, }, - - wantLogArchive: []string{ - "Skipping pod: pod2 ns: ns1 because it is excluded from all connectivity tests", - }, }, { name: "ipv4ok multus", @@ -511,10 +509,6 @@ func TestBuildNetTestContext(t *testing.T) { DestTargets: nil, }, }, - - wantLogArchive: []string{ - "Skipping pod pod2 because it is excluded from Multus connectivity tests only", - }, }, } var logArchive strings.Builder @@ -548,34 +542,10 @@ func TestBuildNetTestContext(t *testing.T) { tt.wantNetsUnderTest, ) } - - logArchiveMsgs := getLogArchiveMsgs(logArchive) - if !reflect.DeepEqual(logArchiveMsgs, tt.wantLogArchive) { - t.Errorf( - "BuildNetTestContext() gotClaimsLog = %v, want %v", - logArchiveMsgs, - tt.wantLogArchive, - ) - } - logArchive.Reset() }) } } -func getLogArchiveMsgs(logArchive strings.Builder) []string { - var logArchiveMsgs []string - logLines := strings.Split(logArchive.String(), "\n") - - for _, logLine := range logLines { - logMsgs := strings.Split(logLine, "]") - if len(logMsgs) > 2 { - logArchiveMsgs = append(logArchiveMsgs, strings.TrimSpace(logMsgs[2])) - } - } - - return logArchiveMsgs -} - var ( pod1 = provider.Pod{ //nolint:dupl Pod: &corev1.Pod{ @@ -741,7 +711,6 @@ func TestRunNetworkingTests(t *testing.T) { name string args args wantReport testhelper.FailureReasonOut - wantLogArchive []string testPingSuccess bool }{ {name: "ok", @@ -810,16 +779,13 @@ func TestRunNetworkingTests(t *testing.T) { ObjectType: "Network", ObjectFieldsKeys: []string{testhelper.ReasonForCompliance, testhelper.NetworkName}, ObjectFieldsValues: []string{ - "ICMP tests were successful for all 1 IP source/destination in this network", + "ICMP tests were successful for all 1 IP source/destination in this network", "default", }, }, }, NonCompliantObjectsOut: []*testhelper.ReportObject{}, }, - wantLogArchive: []string{ - "IPv4 ping test on network default from ( container: test1 pod: test-0 ns: tnf srcip: 10.244.195.231 ) to ( container: test2 pod: test-1 ns: tnf dstip: 10.244.195.232 ) result: outcome: SUCCESS transmitted: 10 received: 10 errors: 0", - }, testPingSuccess: true, }, {name: "noNetToTest", @@ -965,10 +931,6 @@ func TestRunNetworkingTests(t *testing.T) { }, }, }, - wantLogArchive: []string{ - "IPv4 ping test on network default from ( container: test1 pod: test-0 ns: tnf srcip: 10.244.195.231 ) to ( container: test2 pod: test-1 ns: tnf dstip: 10.244.195.232 ) result: outcome: FAILURE transmitted: 10 received: 5 errors: 5", //nolint:lll - "IPv4 ping test on network default from ( container: test1 pod: test-0 ns: tnf srcip: 10.244.195.231 ) to ( container: test3 pod: test-1 ns: tnf dstip: 10.244.195.233 ) result: outcome: FAILURE transmitted: 10 received: 5 errors: 5", - }, testPingSuccess: false, }, } @@ -990,20 +952,11 @@ func TestRunNetworkingTests(t *testing.T) { ) if !gotReport.Equal(tt.wantReport) { t.Errorf( - "RunNetworkingTests() gotReport = %s, want %s", + "RunNetworkingTests() gotReport = %q, want %q", testhelper.FailureReasonOutTestString(gotReport), testhelper.FailureReasonOutTestString(tt.wantReport), ) } - logArchiveMsgs := getLogArchiveMsgs(logArchive) - if !reflect.DeepEqual(logArchiveMsgs, tt.wantLogArchive) { - t.Errorf( - "RunNetworkingTests() gotReport = %+v, want %+v", - logArchiveMsgs, - tt.wantLogArchive, - ) - } - logArchive.Reset() }) } } diff --git a/cnf-certification-test/networking/netcommons/netcommons.go b/cnf-certification-test/networking/netcommons/netcommons.go index 99c837bd5..df096a379 100644 --- a/cnf-certification-test/networking/netcommons/netcommons.go +++ b/cnf-certification-test/networking/netcommons/netcommons.go @@ -149,11 +149,12 @@ func FilterIPListByIPVersion(ipList []string, aIPVersion IPVersion) []string { return filteredIPList } -func FindRogueContainersDeclaringPorts(containers []*provider.Container, portsToTest map[int32]bool, portsOrigin string) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) { +func findRogueContainersDeclaringPorts(containers []*provider.Container, portsToTest map[int32]bool, portsOrigin string, logger *log.Logger) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) { for _, cut := range containers { + logger.Info("Testing Container %q", cut) for _, port := range cut.Ports { if portsToTest[port.ContainerPort] { - log.Debug("%s has declared a port (%d) that has been reserved", cut, port.ContainerPort) + logger.Error("%q declares %s reserved port %d (%s)", cut, portsOrigin, port.ContainerPort, port.Protocol) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewContainerReportObject(cut.Namespace, cut.Podname, cut.Name, fmt.Sprintf("Container declares %s reserved port in %v", portsOrigin, portsToTest), false). @@ -161,6 +162,7 @@ func FindRogueContainersDeclaringPorts(containers []*provider.Container, portsTo AddField(testhelper.PortNumber, strconv.Itoa(int(port.ContainerPort))). AddField(testhelper.PortProtocol, string(port.Protocol))) } else { + logger.Info("%q does not declare any %s reserved port", cut, portsOrigin) compliantObjects = append(compliantObjects, testhelper.NewContainerReportObject(cut.Namespace, cut.Podname, cut.Name, fmt.Sprintf("Container does not declare %s reserved port in %v", portsOrigin, portsToTest), true). @@ -187,16 +189,17 @@ var ReservedIstioPorts = map[int32]bool{ 15000: true, // Envoy admin port (commands/diagnostics) } -func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]bool, portsOrigin string) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) { +func findRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]bool, portsOrigin string, logger *log.Logger) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) { for _, put := range pods { - compliantObjectsEntries, nonCompliantObjectsEntries := FindRogueContainersDeclaringPorts(put.Containers, portsToTest, portsOrigin) + logger.Info("Testing Pod %q", put) + compliantObjectsEntries, nonCompliantObjectsEntries := findRogueContainersDeclaringPorts(put.Containers, portsToTest, portsOrigin, logger) nonCompliantPortFound := len(nonCompliantObjectsEntries) > 0 compliantObjects = append(compliantObjects, compliantObjectsEntries...) nonCompliantObjects = append(nonCompliantObjects, nonCompliantObjectsEntries...) cut := put.Containers[0] listeningPorts, err := netutil.GetListeningPorts(cut) if err != nil { - log.Debug("Failed to get the listening ports on %s, err: %v", cut, err) + logger.Error("Failed to get the listening ports on %q, err: %v", cut, err) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(cut.Namespace, put.Name, fmt.Sprintf("Failed to get the listening ports on pod, err: %v", err), false)) @@ -207,10 +210,10 @@ func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]b // If pod contains an "istio-proxy" container, we need to make sure that the ports returned // overlap with the known istio ports if put.ContainsIstioProxy() && ReservedIstioPorts[int32(port.PortNumber)] { - log.Debug("%s was found to be listening to port %d due to istio-proxy being present. Ignoring.", put, port.PortNumber) + logger.Info("%q was found to be listening to port %d due to istio-proxy being present. Ignoring.", put, port.PortNumber) continue } - log.Debug("%s has one container (%s) listening on port %d that has been reserved", put, cut.Name, port.PortNumber) + logger.Error("%q has one container (%q) listening on port %d (%s) that has been reserved", put, cut.Name, port.PortNumber, port.Protocol) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(cut.Namespace, put.Name, fmt.Sprintf("Pod Listens to %s reserved port in %v", portsOrigin, portsToTest), false). @@ -219,6 +222,7 @@ func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]b AddField(testhelper.PortProtocol, port.Protocol)) nonCompliantPortFound = true } else { + logger.Info("%q listens in %s unreserved port %d (%s)", put, portsOrigin, port.PortNumber, port.Protocol) compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(cut.Namespace, put.Name, fmt.Sprintf("Pod Listens to port not in %s reserved port %v", portsOrigin, portsToTest), true). @@ -240,8 +244,8 @@ func FindRoguePodsListeningToPorts(pods []*provider.Pod, portsToTest map[int32]b return compliantObjects, nonCompliantObjects } -func TestReservedPortsUsage(env *provider.TestEnvironment, reservedPorts map[int32]bool, portsOrigin string) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) { - compliantObjectsEntries, nonCompliantObjectsEntries := FindRoguePodsListeningToPorts(env.Pods, reservedPorts, portsOrigin) +func TestReservedPortsUsage(env *provider.TestEnvironment, reservedPorts map[int32]bool, portsOrigin string, logger *log.Logger) (compliantObjects, nonCompliantObjects []*testhelper.ReportObject) { + compliantObjectsEntries, nonCompliantObjectsEntries := findRoguePodsListeningToPorts(env.Pods, reservedPorts, portsOrigin, logger) compliantObjects = append(compliantObjects, compliantObjectsEntries...) nonCompliantObjects = append(nonCompliantObjects, nonCompliantObjectsEntries...) diff --git a/cnf-certification-test/networking/policies/policies.go b/cnf-certification-test/networking/policies/policies.go index 0d504583b..1c7232e87 100644 --- a/cnf-certification-test/networking/policies/policies.go +++ b/cnf-certification-test/networking/policies/policies.go @@ -17,34 +17,31 @@ package policies import ( - "github.com/test-network-function/cnf-certification-test/internal/log" networkingv1 "k8s.io/api/networking/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func IsNetworkPolicyCompliant(np *networkingv1.NetworkPolicy, policyType networkingv1.PolicyType) bool { +//nolint:gocritic // unnamed results +func IsNetworkPolicyCompliant(np *networkingv1.NetworkPolicy, policyType networkingv1.PolicyType) (bool, string) { // As long as we have decided above that there is no pod selector, // we just have to make sure that the policy type is either Ingress or Egress (or both) we can return true. // For more information about deny-all policies, there are some good examples on: // https://kubernetes.io/docs/concepts/services-networking/network-policies/ if len(np.Spec.PolicyTypes) == 0 { - log.Debug("%s: policy types found empty", np.Name) - return false + return false, "empty policy types" } // Ingress and Egress rules should be "empty" if it is a default rule. if policyType == networkingv1.PolicyTypeEgress { if np.Spec.Egress != nil || len(np.Spec.Egress) > 0 { - log.Debug("%s: egress spec found not empty", np.Name) - return false + return false, "egress spec not empty for default egress rule" } } if policyType == networkingv1.PolicyTypeIngress { if np.Spec.Ingress != nil || len(np.Spec.Ingress) > 0 { - log.Debug("%s: ingress spec found not empty", np.Name) - return false + return false, "ingress spec not empty for default ingress rule" } } @@ -57,7 +54,7 @@ func IsNetworkPolicyCompliant(np *networkingv1.NetworkPolicy, policyType network } } - return policyTypeFound + return policyTypeFound, "" } func LabelsMatch(podSelectorLabels v1.LabelSelector, podLabels map[string]string) bool { diff --git a/cnf-certification-test/networking/policies/policies_test.go b/cnf-certification-test/networking/policies/policies_test.go index d018a158b..fb0b2c5f2 100644 --- a/cnf-certification-test/networking/policies/policies_test.go +++ b/cnf-certification-test/networking/policies/policies_test.go @@ -190,8 +190,11 @@ func TestIsNetworkPolicyCompliant(t *testing.T) { } for index, tc := range testCases { - assert.Equal(t, tc.expectedEgressOutput, IsNetworkPolicyCompliant(&testCases[index].testNP, networkingv1.PolicyTypeEgress)) - assert.Equal(t, tc.expectedIngressOutput, IsNetworkPolicyCompliant(&testCases[index].testNP, networkingv1.PolicyTypeIngress)) + var isCompliant bool + isCompliant, _ = IsNetworkPolicyCompliant(&testCases[index].testNP, networkingv1.PolicyTypeEgress) + assert.Equal(t, tc.expectedEgressOutput, isCompliant) + isCompliant, _ = IsNetworkPolicyCompliant(&testCases[index].testNP, networkingv1.PolicyTypeIngress) + assert.Equal(t, tc.expectedIngressOutput, isCompliant) } } diff --git a/cnf-certification-test/networking/suite.go b/cnf-certification-test/networking/suite.go index 0c49ec7f0..30c0a12d5 100644 --- a/cnf-certification-test/networking/suite.go +++ b/cnf-certification-test/networking/suite.go @@ -49,7 +49,6 @@ var ( env provider.TestEnvironment beforeEachFn = func(check *checksdb.Check) error { - check.LogInfo("Check %s: getting test environment.", check.ID) env = provider.GetTestEnvironment() return nil } @@ -57,7 +56,7 @@ var ( //nolint:funlen func LoadChecks() { - log.Debug("Entering %s suite", common.NetworkingTestKey) + log.Debug("Loading %s suite checks", common.NetworkingTestKey) checksGroup := checksdb.NewChecksGroup(common.NetworkingTestKey). WithBeforeEachFn(beforeEachFn) @@ -168,20 +167,22 @@ func LoadChecks() { } func testExecProbDenyAtCPUPinning(check *checksdb.Check, dpdkPods []*provider.Pod) { - check.LogInfo("Check if exec probe is happening") var compliantObjects []*testhelper.ReportObject var nonCompliantObjects []*testhelper.ReportObject for _, cpuPinnedPod := range dpdkPods { execProbeFound := false for _, cut := range cpuPinnedPod.Containers { + check.LogInfo("Testing Container %q", cut) if cut.HasExecProbes() { + check.LogError("Container %q defines an exec probe", cut) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(cpuPinnedPod.Namespace, cpuPinnedPod.Name, "Exec prob is not allowed", false)) execProbeFound = true } } if !execProbeFound { + check.LogInfo("Pod %q does not define any exec probe", cpuPinnedPod) compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(cpuPinnedPod.Namespace, cpuPinnedPod.Name, "Exec prob is allowed", true)) } } @@ -197,6 +198,7 @@ func testUndeclaredContainerPortsUsage(check *checksdb.Check, env *provider.Test // First get the ports declared in the Pod's containers spec declaredPorts := make(map[netutil.PortInfo]bool) for _, cut := range put.Containers { + check.LogInfo("Testing Container %q", cut) for _, port := range cut.Ports { portInfo.PortNumber = int(port.ContainerPort) portInfo.Protocol = string(port.Protocol) @@ -208,13 +210,15 @@ func testUndeclaredContainerPortsUsage(check *checksdb.Check, env *provider.Test firstPodContainer := put.Containers[0] listeningPorts, err := netutil.GetListeningPorts(firstPodContainer) if err != nil { - check.LogDebug("Failed to get the container's listening ports, err: %v", err) + check.LogError("Failed to get container %q listening ports, err: %v", firstPodContainer, err) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, fmt.Sprintf("Failed to get the container's listening ports, err: %v", err), false)) continue } if len(listeningPorts) == 0 { - check.LogDebug("None of the containers of %s have any listening port.", put) + check.LogInfo("None of the containers of %q have any listening port.", put) + compliantObjects = append(compliantObjects, + testhelper.NewPodReportObject(put.Namespace, put.Name, "None of the containers have any listening port", true)) continue } @@ -222,13 +226,13 @@ func testUndeclaredContainerPortsUsage(check *checksdb.Check, env *provider.Test failedPod := false for listeningPort := range listeningPorts { if put.ContainsIstioProxy() && netcommons.ReservedIstioPorts[int32(listeningPort.PortNumber)] { - check.LogDebug("%s is listening on port %d protocol %s, but the pod also contains istio-proxy. Ignoring.", + check.LogInfo("%q is listening on port %d protocol %q, but the pod also contains istio-proxy. Ignoring.", put, listeningPort.PortNumber, listeningPort.Protocol) continue } if ok := declaredPorts[listeningPort]; !ok { - check.LogDebug("%s is listening on port %d protocol %s, but that port was not declared in any container spec.", + check.LogError("%q is listening on port %d protocol %q, but that port was not declared in any container spec.", put, listeningPort.PortNumber, listeningPort.Protocol) failedPod = true nonCompliantObjects = append(nonCompliantObjects, @@ -238,6 +242,7 @@ func testUndeclaredContainerPortsUsage(check *checksdb.Check, env *provider.Test AddField(testhelper.PortNumber, strconv.Itoa(listeningPort.PortNumber)). AddField(testhelper.PortProtocol, listeningPort.Protocol)) } else { + check.LogInfo("%q is listening on declared port %d protocol %q", put, listeningPort.PortNumber, listeningPort.Protocol) compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Listening port was declared in container spec", true). @@ -262,7 +267,7 @@ func testNetworkConnectivity(env *provider.TestEnvironment, aIPVersion netcommon netsUnderTest := icmp.BuildNetTestContext(env.Pods, aIPVersion, aType, check.GetLoggger()) report, skip := icmp.RunNetworkingTests(netsUnderTest, defaultNumPings, aIPVersion, check.GetLoggger()) if skip { - check.LogInfo("There are no %s networks to test with at least 2 pods, skipping test", aIPVersion) + check.LogInfo("There are no %q networks to test with at least 2 pods, skipping test", aIPVersion) } check.SetResult(report.CompliantObjectsOut, report.NonCompliantObjectsOut) } @@ -272,7 +277,7 @@ func testOCPReservedPortsUsage(check *checksdb.Check, env *provider.TestEnvironm OCPReservedPorts := map[int32]bool{ 22623: true, 22624: true} - compliantObjects, nonCompliantObjects := netcommons.TestReservedPortsUsage(env, OCPReservedPorts, "OCP") + compliantObjects, nonCompliantObjects := netcommons.TestReservedPortsUsage(env, OCPReservedPorts, "OCP", check.GetLoggger()) check.SetResult(compliantObjects, nonCompliantObjects) } @@ -289,29 +294,31 @@ func testPartnerSpecificTCPPorts(check *checksdb.Check, env *provider.TestEnviro 15001: true, 15000: true, } - compliantObjects, nonCompliantObjects := netcommons.TestReservedPortsUsage(env, ReservedPorts, "Partner") + compliantObjects, nonCompliantObjects := netcommons.TestReservedPortsUsage(env, ReservedPorts, "Partner", check.GetLoggger()) check.SetResult(compliantObjects, nonCompliantObjects) } func testDualStackServices(check *checksdb.Check, env *provider.TestEnvironment) { var compliantObjects []*testhelper.ReportObject var nonCompliantObjects []*testhelper.ReportObject - check.LogInfo("Testing services (should be either single stack ipv6 or dual-stack)") for _, s := range env.Services { + check.LogInfo("Testing Service %q", s.Name) serviceIPVersion, err := services.GetServiceIPVersion(s) if err != nil { - check.LogDebug("%s", err) + check.LogError("Could not get IP version from Service %q, err=%v", s.Name, err) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewReportObject("Could not get IP Version from service", testhelper.ServiceType, false). AddField(testhelper.Namespace, s.Namespace). AddField(testhelper.ServiceName, s.Name)) } if serviceIPVersion == netcommons.Undefined || serviceIPVersion == netcommons.IPv4 { + check.LogError("Service %q (ns: %q) only supports IPv4", s.Name, s.Namespace) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewReportObject("Service supports only IPv4", testhelper.ServiceType, false). AddField(testhelper.Namespace, s.Namespace). AddField(testhelper.ServiceName, s.Name). AddField(testhelper.ServiceIPVersion, serviceIPVersion.String())) } else { - compliantObjects = append(compliantObjects, testhelper.NewReportObject("Service support IPv6 or is dual stack", testhelper.ServiceType, false). + check.LogInfo("Service %q (ns: %q) supports IPv6 or is dual stack", s.Name, s.Namespace) + compliantObjects = append(compliantObjects, testhelper.NewReportObject("Service supports IPv6 or is dual stack", testhelper.ServiceType, true). AddField(testhelper.Namespace, s.Namespace). AddField(testhelper.ServiceName, s.Name). AddField(testhelper.ServiceIPVersion, serviceIPVersion.String())) @@ -321,8 +328,8 @@ func testDualStackServices(check *checksdb.Check, env *provider.TestEnvironment) check.SetResult(compliantObjects, nonCompliantObjects) } +//nolint:funlen func testNetworkPolicyDenyAll(check *checksdb.Check, env *provider.TestEnvironment) { - check.LogInfo("Test for Deny All in network policies") var compliantObjects []*testhelper.ReportObject var nonCompliantObjects []*testhelper.ReportObject @@ -330,25 +337,35 @@ func testNetworkPolicyDenyAll(check *checksdb.Check, env *provider.TestEnvironme // This ensures that each pod is accounted for that we are tasked with testing and excludes any pods that are not marked // for testing (via the labels). for _, put := range env.Pods { + check.LogInfo("Testing Pod %q", put) denyAllEgressFound := false denyAllIngressFound := false // Look through all of the network policies for a matching namespace. for index := range env.NetworkPolicies { - check.LogDebug("Testing network policy %s against pod %s", env.NetworkPolicies[index].Name, put.String()) + networkPolicy := env.NetworkPolicies[index] + check.LogInfo("Testing Network policy %q against pod %q", networkPolicy.Name, put) // Skip any network policies that don't match the namespace of the pod we are testing. - if env.NetworkPolicies[index].Namespace != put.Namespace { + if networkPolicy.Namespace != put.Namespace { + check.LogInfo("Skipping Network policy %q (namespace %q does not match Pod namespace %q)", networkPolicy.Name, networkPolicy.Namespace, put.Namespace) continue } // Match the pod namespace with the network policy namespace. - if policies.LabelsMatch(env.NetworkPolicies[index].Spec.PodSelector, put.Labels) { + if policies.LabelsMatch(networkPolicy.Spec.PodSelector, put.Labels) { + var reason string if !denyAllEgressFound { - denyAllEgressFound = policies.IsNetworkPolicyCompliant(&env.NetworkPolicies[index], networkingv1.PolicyTypeEgress) + denyAllEgressFound, reason = policies.IsNetworkPolicyCompliant(&networkPolicy, networkingv1.PolicyTypeEgress) + if reason != "" { + check.LogError("Network policy %q is not compliant, reason=%q", networkPolicy.Name, reason) + } } if !denyAllIngressFound { - denyAllIngressFound = policies.IsNetworkPolicyCompliant(&env.NetworkPolicies[index], networkingv1.PolicyTypeIngress) + denyAllIngressFound, reason = policies.IsNetworkPolicyCompliant(&networkPolicy, networkingv1.PolicyTypeIngress) + if reason != "" { + check.LogError("Network policy %q is not compliant, reason=%q", networkPolicy.Name, reason) + } } } } @@ -356,18 +373,19 @@ func testNetworkPolicyDenyAll(check *checksdb.Check, env *provider.TestEnvironme // Network policy has not been found that contains a deny-all rule for both ingress and egress. podIsCompliant := true if !denyAllIngressFound { - check.LogDebug("%s was found to not have a default ingress deny-all network policy.", put.Name) + check.LogError("Pod %q was found to not have a default ingress deny-all network policy.", put) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod was found to not have a default ingress deny-all network policy", false)) podIsCompliant = false } if !denyAllEgressFound { - check.LogDebug("%s was found to not have a default egress deny-all network policy.", put.Name) + check.LogError("Pod %q was found to not have a default egress deny-all network policy.", put) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod was found to not have a default egress deny-all network policy", false)) podIsCompliant = false } if podIsCompliant { + check.LogInfo("Pod %q has a default ingress/egress deny-all network policy", put) compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(put.Namespace, put.Name, "Pod has a default ingress/egress deny-all network policy", true)) } } @@ -383,21 +401,22 @@ func testRestartOnRebootLabelOnPodsUsingSriov(check *checksdb.Check, sriovPods [ var compliantObjects []*testhelper.ReportObject var nonCompliantObjects []*testhelper.ReportObject for _, pod := range sriovPods { - check.LogDebug("Pod %s uses SRIOV network/s. Checking label %s existence & value.", pod, restartOnRebootLabel) + check.LogInfo("Testing SRIOV Pod %q", pod) labelValue, exist := pod.GetLabels()[restartOnRebootLabel] if !exist { - check.LogDebug("Pod %s is using SRIOV but the label %s was not found.", pod, restartOnRebootLabel) + check.LogError("Pod %q uses SRIOV but the label %q was not found.", pod, restartOnRebootLabel) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(pod.Namespace, pod.Name, fmt.Sprintf("Pod uses SRIOV but the label %s was not found", restartOnRebootLabel), false)) continue } if labelValue != "true" { - check.LogDebug("Pod %s is using SRIOV but the %s label value is not true.", pod, restartOnRebootLabel) + check.LogError("Pod %q uses SRIOV but the %q label value is not true.", pod, restartOnRebootLabel) nonCompliantObjects = append(nonCompliantObjects, testhelper.NewPodReportObject(pod.Namespace, pod.Name, fmt.Sprintf("Pod uses SRIOV but the label %s is not set to true", restartOnRebootLabel), false)) continue } + check.LogInfo("Pod %q uses SRIOV and the %q label is set to true", pod, restartOnRebootLabel) compliantObjects = append(compliantObjects, testhelper.NewPodReportObject(pod.Namespace, pod.Name, fmt.Sprintf("Pod uses SRIOV and the label %s is set to true", restartOnRebootLabel), true)) } From a78fcf951b802e403f7306cb80a837e1870baf8d Mon Sep 17 00:00:00 2001 From: jmontesi Date: Thu, 4 Jan 2024 15:18:17 +0100 Subject: [PATCH 2/3] Fix error formatting --- cnf-certification-test/networking/icmp/icmp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cnf-certification-test/networking/icmp/icmp.go b/cnf-certification-test/networking/icmp/icmp.go index 4c0b99f7f..2b5f2a1da 100644 --- a/cnf-certification-test/networking/icmp/icmp.go +++ b/cnf-certification-test/networking/icmp/icmp.go @@ -159,7 +159,7 @@ func RunNetworkingTests( //nolint:funlen netUnderTest.TesterSource.ContainerIdentifier, netUnderTest.TesterSource.IP, aDestIP.ContainerIdentifier, aDestIP.IP, result) if err != nil { - logger.Debug("Ping failed, err=%s", err) + logger.Debug("Ping failed, err=%v", err) } if result.outcome != testhelper.SUCCESS { logger.Error("Ping from %q (srcip: %q) to %q (dstip: %q) failed", From feb334343f0aeea54f2f050d268d1c8708282d8a Mon Sep 17 00:00:00 2001 From: jmontesi Date: Thu, 4 Jan 2024 16:58:20 +0100 Subject: [PATCH 3/3] Revert setting as compliant a container that does not declare ports --- cnf-certification-test/networking/suite.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cnf-certification-test/networking/suite.go b/cnf-certification-test/networking/suite.go index efec96be8..f30b8f555 100644 --- a/cnf-certification-test/networking/suite.go +++ b/cnf-certification-test/networking/suite.go @@ -206,8 +206,6 @@ func testUndeclaredContainerPortsUsage(check *checksdb.Check, env *provider.Test } if len(listeningPorts) == 0 { check.LogInfo("None of the containers of %q have any listening port.", put) - compliantObjects = append(compliantObjects, - testhelper.NewPodReportObject(put.Namespace, put.Name, "None of the containers have any listening port", true)) continue }