From d688c4c4e37526e276690b5b51d1044b7e220aff Mon Sep 17 00:00:00 2001 From: Pawel Wieczorek Date: Wed, 18 Mar 2020 16:32:45 +0100 Subject: Reduce cyclomatic complexity Moving CSV data conversion and "expected failure" filtering away from main function made testing these features easier. Utility behaviour remained unchanged. Issue-ID: SECCOM-261 Change-Id: I4cabfc7b352434c84a613c02f44af3c9630be970 Signed-off-by: Pawel Wieczorek --- test/security/sslendpoints/main.go | 27 +++----- test/security/sslendpoints/ports/ports.go | 31 +++++++++ test/security/sslendpoints/ports/ports_test.go | 91 ++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 17 deletions(-) diff --git a/test/security/sslendpoints/main.go b/test/security/sslendpoints/main.go index 8c136d5c4..38950c4f4 100644 --- a/test/security/sslendpoints/main.go +++ b/test/security/sslendpoints/main.go @@ -39,7 +39,7 @@ func main() { xfailName = flag.String("xfail", "", "(optional) absolute path to the expected failures file") flag.Parse() - var xfails [][]string + xfails := make(map[uint16]string) if *xfailName != "" { xfailFile, err := os.Open(*xfailName) if err != nil { @@ -53,11 +53,18 @@ func main() { r.Comment = xfailComment r.FieldsPerRecord = xfailFields - xfails, err = r.ReadAll() + xfailData, err := r.ReadAll() if err != nil { log.Printf("Unable to read expected failures file: %v", err) log.Println("All non-SSL NodePorts will be reported") } + + var ok bool + xfails, ok = ports.ConvertNodePorts(xfailData) + if !ok { + log.Println("No usable data in expected failures file") + log.Println("All non-SSL NodePorts will be reported") + } } // use the current context in kubeconfig @@ -99,21 +106,7 @@ func main() { } // filter out expected failures here before running the scan - for _, xfail := range xfails { - port, err := strconv.Atoi(xfail[1]) - if err != nil { - log.Printf("Unable to parse port expected to fail: %v", err) - continue - } - service, ok := nodeports[uint16(port)] - if !ok { - continue - } - if service != xfail[0] { - continue - } - delete(nodeports, uint16(port)) - } + ports.FilterXFailNodePorts(xfails, nodeports) // extract ports for running the scan var ports []string diff --git a/test/security/sslendpoints/ports/ports.go b/test/security/sslendpoints/ports/ports.go index a80fb782c..dae7dbec7 100644 --- a/test/security/sslendpoints/ports/ports.go +++ b/test/security/sslendpoints/ports/ports.go @@ -1,9 +1,40 @@ package ports import ( + "log" + "strconv" + v1 "k8s.io/api/core/v1" ) +// ConvertNodePorts converts CSV data to NodePorts map. +func ConvertNodePorts(data [][]string) (map[uint16]string, bool) { + result := make(map[uint16]string) + for _, record := range data { + port, err := strconv.Atoi(record[1]) + if err != nil { + log.Printf("Unable to parse port field: %v", err) + continue + } + result[uint16(port)] = record[0] + } + return result, len(result) > 0 +} + +// FilterXFailNodePorts removes NodePorts expected to fail from map. +func FilterXFailNodePorts(xfails, nodeports map[uint16]string) { + for port, xfailService := range xfails { + service, ok := nodeports[port] + if !ok { + continue + } + if service != xfailService { + continue + } + delete(nodeports, port) + } +} + // FilterNodePorts extracts NodePorts from ServiceList. func FilterNodePorts(services *v1.ServiceList) (map[uint16]string, bool) { nodeports := make(map[uint16]string) diff --git a/test/security/sslendpoints/ports/ports_test.go b/test/security/sslendpoints/ports/ports_test.go index 0480b71af..10cf14b63 100644 --- a/test/security/sslendpoints/ports/ports_test.go +++ b/test/security/sslendpoints/ports/ports_test.go @@ -1,6 +1,8 @@ package ports_test import ( + "strconv" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -21,6 +23,10 @@ var _ = Describe("Ports", func() { serviceL = "serviceL" serviceZ = "serviceZ" + notParsablePort1 = "3p1c" + notParsablePort2 = "0n4p" + notParsablePort3 = "5GxD" + externalIpControl = "1.2.3.4" internalIpControl = "192.168.121.100" internalIpWorker = "192.168.121.200" @@ -29,6 +35,9 @@ var _ = Describe("Ports", func() { ) var ( + csvSomeUnparsable [][]string + csvAllUnparsable [][]string + servicesEmpty *v1.ServiceList servicesSingleWithNodePort *v1.ServiceList servicesSingleWithMultipleNodePorts *v1.ServiceList @@ -45,6 +54,17 @@ var _ = Describe("Ports", func() { ) BeforeEach(func() { + csvSomeUnparsable = [][]string{ + []string{serviceR, strconv.Itoa(nodePortO)}, + []string{serviceL, strconv.Itoa(nodePortN)}, + []string{serviceZ, notParsablePort1}, + } + csvAllUnparsable = [][]string{ + []string{serviceR, notParsablePort1}, + []string{serviceL, notParsablePort2}, + []string{serviceZ, notParsablePort3}, + } + servicesEmpty = &v1.ServiceList{} servicesSingleWithNodePort = &v1.ServiceList{ Items: []v1.Service{ @@ -228,6 +248,77 @@ var _ = Describe("Ports", func() { } }) + Describe("CSV data to NodePorts conversion", func() { + Context("With no data", func() { + It("should return an empty map", func() { + cnp, ok := ConvertNodePorts([][]string{}) + Expect(ok).To(BeFalse()) + Expect(cnp).To(BeEmpty()) + }) + }) + Context("With some ports unparsable", func() { + It("should return only parsable records", func() { + expected := map[uint16]string{nodePortO: serviceR, nodePortN: serviceL} + cnp, ok := ConvertNodePorts(csvSomeUnparsable) + Expect(ok).To(BeTrue()) + Expect(cnp).To(Equal(expected)) + }) + }) + Context("With all ports unparsable", func() { + It("should return an empty map", func() { + cnp, ok := ConvertNodePorts(csvAllUnparsable) + Expect(ok).To(BeFalse()) + Expect(cnp).To(BeEmpty()) + }) + }) + }) + + Describe("NodePorts expected to fail filtering", func() { + Context("With no data", func() { + It("should leave nodeports unchanged", func() { + nodeports := map[uint16]string{nodePortO: serviceR} + expected := make(map[uint16]string) + for k, v := range nodeports { + expected[k] = v + } + FilterXFailNodePorts(map[uint16]string{}, nodeports) + Expect(nodeports).To(Equal(expected)) + }) + }) + Context("With port absent in NodePorts", func() { + It("should leave nodeports unchanged", func() { + xfail := map[uint16]string{nodePortP: serviceZ} + nodeports := map[uint16]string{nodePortO: serviceR} + expected := make(map[uint16]string) + for k, v := range nodeports { + expected[k] = v + } + FilterXFailNodePorts(xfail, nodeports) + Expect(nodeports).To(Equal(expected)) + }) + }) + Context("With other service than in NodePorts", func() { + It("should leave nodeports unchanged", func() { + xfail := map[uint16]string{nodePortO: serviceZ} + nodeports := map[uint16]string{nodePortO: serviceR} + expected := make(map[uint16]string) + for k, v := range nodeports { + expected[k] = v + } + FilterXFailNodePorts(xfail, nodeports) + Expect(nodeports).To(Equal(expected)) + }) + }) + Context("With all NodePorts expected to fail", func() { + It("should leave no nodeports", func() { + xfail := map[uint16]string{nodePortO: serviceR, nodePortN: serviceL} + nodeports := map[uint16]string{nodePortO: serviceR, nodePortN: serviceL} + FilterXFailNodePorts(xfail, nodeports) + Expect(nodeports).To(BeEmpty()) + }) + }) + }) + Describe("NodePorts extraction", func() { Context("With empty service list", func() { It("should report no NodePorts", func() { -- cgit 1.2.3-korg