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 <p.wieczorek2@samsung.com>

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() {