From 2aeaa8c067f0de3e2a2ef2de71e88af30c95f84d Mon Sep 17 00:00:00 2001 From: jmontesi <100689165+jmontesi@users.noreply.github.com> Date: Thu, 4 Jan 2024 18:25:24 +0100 Subject: [PATCH] networking: review logs (#1782) --- .../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 | 65 ++++++++++++------- 6 files changed, 103 insertions(+), 115 deletions(-) diff --git a/cnf-certification-test/networking/icmp/icmp.go b/cnf-certification-test/networking/icmp/icmp.go index d8ee00a6d..2b5f2a1da 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=%v", 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 e4501898d..f30b8f555 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) @@ -157,20 +156,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)) } } @@ -186,6 +187,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) @@ -197,13 +199,13 @@ 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) continue } @@ -211,13 +213,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, @@ -227,6 +229,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). @@ -251,7 +254,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) } @@ -261,7 +264,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) } @@ -278,29 +281,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())) @@ -310,8 +315,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 @@ -319,25 +324,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) + } } } } @@ -345,18 +360,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)) } } @@ -372,21 +388,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)) }