From a4355569af800315d706b6876340f76c1c8fe07d Mon Sep 17 00:00:00 2001
From: Timo Reimann <ttr314@googlemail.com>
Date: Sat, 22 Apr 2017 22:07:27 +0200
Subject: [PATCH] Extract index functionality into generic helper function.

Allows to move specific test cases to dedicated tests for new function.
---
 provider/marathon/marathon.go      | 24 +++++---
 provider/marathon/marathon_test.go | 90 +++++++++++++++++++-----------
 2 files changed, 75 insertions(+), 39 deletions(-)

diff --git a/provider/marathon/marathon.go b/provider/marathon/marathon.go
index 2751f030d..643cacccb 100644
--- a/provider/marathon/marathon.go
+++ b/provider/marathon/marathon.go
@@ -482,12 +482,9 @@ func processPorts(application marathon.Application, task marathon.Task) (int, er
 	portIndexLabel, ok := (*application.Labels)[labelPortIndex]
 	if ok {
 		var err error
-		portIndex, err = strconv.Atoi(portIndexLabel)
-		switch {
-		case err != nil:
-			return 0, fmt.Errorf("failed to parse port index label: %s", err)
-		case portIndex < 0, portIndex > len(ports)-1:
-			return 0, fmt.Errorf("port index %d must be within port range (0, %d)", portIndex, len(ports)-1)
+		portIndex, err = parseIndex(portIndexLabel, len(ports))
+		if err != nil {
+			return 0, fmt.Errorf("cannot use port index to select from %d ports: %s", len(ports), err)
 		}
 	}
 	return ports[portIndex], nil
@@ -543,7 +540,8 @@ func (p *Provider) getBackendServer(task marathon.Task, applications []marathon.
 			log.Errorf("Found %d task IP addresses but missing IP address index for Marathon application %s on task %s", numTaskIPAddresses, application.ID, task.ID)
 			return ""
 		}
-		ipAddressIdx, err := strconv.Atoi(ipAddressIdxStr)
+
+		ipAddressIdx, err := parseIndex(ipAddressIdxStr, numTaskIPAddresses)
 		if err != nil {
 			log.Errorf("Cannot use IP address index to select from %d task IP addresses for Marathon application %s on task %s: %s", numTaskIPAddresses, application.ID, task.ID, err)
 			return ""
@@ -552,3 +550,15 @@ func (p *Provider) getBackendServer(task marathon.Task, applications []marathon.
 		return task.IPAddresses[ipAddressIdx].IPAddress
 	}
 }
+
+func parseIndex(index string, length int) (int, error) {
+	parsed, err := strconv.Atoi(index)
+	switch {
+	case err != nil:
+		return 0, fmt.Errorf("failed to parse index '%s': %s", index, err)
+	case parsed < 0, parsed > length-1:
+		return 0, fmt.Errorf("index %d must be within range (0, %d)", parsed, length-1)
+	}
+
+	return parsed, nil
+}
diff --git a/provider/marathon/marathon_test.go b/provider/marathon/marathon_test.go
index ab9b0ba09..cef215510 100644
--- a/provider/marathon/marathon_test.go
+++ b/provider/marathon/marathon_test.go
@@ -1067,38 +1067,6 @@ func TestMarathonGetPort(t *testing.T) {
 			},
 			expected: "",
 		},
-		{
-			desc: "sub-zero port index specified",
-			applications: []marathon.Application{
-				{
-					ID: "app",
-					Labels: &map[string]string{
-						"traefik.portIndex": "-1",
-					},
-				},
-			},
-			task: marathon.Task{
-				AppID: "app",
-				Ports: []int{80},
-			},
-			expected: "",
-		},
-		{
-			desc: "too high port index specified",
-			applications: []marathon.Application{
-				{
-					ID: "app",
-					Labels: &map[string]string{
-						"traefik.portIndex": "42",
-					},
-				},
-			},
-			task: marathon.Task{
-				AppID: "app",
-				Ports: []int{80, 443},
-			},
-			expected: "",
-		},
 		{
 			desc: "task port preferred over application port",
 			applications: []marathon.Application{
@@ -1534,3 +1502,61 @@ func TestGetBackendServer(t *testing.T) {
 		})
 	}
 }
+
+func TestParseIndex(t *testing.T) {
+	tests := []struct {
+		idxStr        string
+		length        int
+		shouldSucceed bool
+		parsed        int
+	}{
+		{
+			idxStr:        "illegal",
+			length:        42,
+			shouldSucceed: false,
+		},
+		{
+			idxStr:        "-1",
+			length:        42,
+			shouldSucceed: false,
+		},
+		{
+			idxStr:        "10",
+			length:        1,
+			shouldSucceed: false,
+		},
+		{
+			idxStr:        "10",
+			length:        10,
+			shouldSucceed: false,
+		},
+		{
+			idxStr:        "0",
+			length:        1,
+			shouldSucceed: true,
+			parsed:        0,
+		},
+		{
+			idxStr:        "10",
+			length:        11,
+			shouldSucceed: true,
+			parsed:        10,
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(fmt.Sprintf("parseIndex(%s, %d)", test.idxStr, test.length), func(t *testing.T) {
+			t.Parallel()
+			parsed, err := parseIndex(test.idxStr, test.length)
+
+			if test.shouldSucceed != (err == nil) {
+				t.Fatalf("got error '%s', want error: %t", err, !test.shouldSucceed)
+			}
+
+			if test.shouldSucceed && parsed != test.parsed {
+				t.Errorf("got parsed index %d, want %d", parsed, test.parsed)
+			}
+		})
+	}
+}