Repository: allegro/marathon-consul (go)

Issue:
Health check invalid port number
Health check are always created in consul with port number from $PORT0 marathon variable. This is only valid for health checks with `Port Type: Port Index`.
For health checks with `Port Type: Port Number` port number should be taken from key "port" from marathon health check definition.

Eg.: marathon health check definition with `Port Type: Port Number` with static port `8123`:

```
"healthChecks": [
    {
      "path": "/v1/version",
      "protocol": "HTTP",
      "gracePeriodSeconds": 60,
      "intervalSeconds": 10,
      "timeoutSeconds": 5,
      "maxConsecutiveFailures": 3,
      "ignoreHttp1xx": false,
      "port": 8123
    }
  ]
```

So there is `"port": 8123` instead of `"portIndex": 0,`.

This check in consul looks like:
```
  {
    "ModifyIndex": 5488518,
    "CreateIndex": 5488430,
    "Node": "dev-mesos-p2",
    "CheckID": "service:ops_mesos-dns.cd314424-6b12-11e6-8d24-0242f3fd597e",
    "Name": "Service 'mesos-dns' check",
    "Status": "critical",
    "Notes": "",
    "Output": "Get http://10.14.30.74:31313/v1/version: dial tcp 10.14.30.74:31313: getsockopt: connection refused",
    "ServiceID": "ops_mesos-dns.cd314424-6b12-11e6-8d24-0242f3fd597e",
    "ServiceName": "mesos-dns"
  }
```

Port number should be `8123` instead of `31313`.
Port for check was taken from $PORT0 instead of static port key from check definition.

Pull request description:
Fixes #108 | Bounds checking for port index

Patch:
diff --git a/consul/consul.go b/consul/consul.go
index 53539ad..11c48bf 100644
--- a/consul/consul.go
+++ b/consul/consul.go
@@ -308,11 +308,12 @@ func (c *Consul) marathonToConsulChecks(task *apps.Task, healthChecks []apps.Hea
 				Info(fmt.Sprintf("Ignoring health check of type %s", check.Protocol))
 			continue
 		}
-		var port int
-		if check.Port != 0 {
-			port = check.Port
-		} else {
-			port = task.Ports[check.PortIndex]
+
+		port, err := getHealthCheckPort(check, *task)
+		if err != nil {
+			log.WithField("Id", task.AppID.String()).WithField("Address", serviceAddress).WithError(err).
+				Warn(fmt.Sprintf("Ignoring health check of type %s", check.Protocol))
+			continue
 		}
 
 		consulCheck := consulapi.AgentServiceCheck{
@@ -348,6 +349,22 @@ func (c *Consul) marathonToConsulChecks(task *apps.Task, healthChecks []apps.Hea
 	return checks
 }
 
+func getHealthCheckPort(check apps.HealthCheck, task apps.Task) (int, error) {
+	port := 0
+	if check.Port != 0 {
+		port = check.Port
+	} else if check.PortIndex >= 0 && check.PortIndex < len(task.Ports) {
+		port = task.Ports[check.PortIndex]
+	} else {
+		return 0, fmt.Errorf("Port index (%d) out of bounds should from range [0,%d)", check.PortIndex, len(task.Ports))
+	}
+
+	if port < 1 || port > 65535 {
+		return 0, fmt.Errorf("Port %d is invalid", port)
+	}
+	return port, nil
+}
+
 func (c *Consul) getIgnoredHealthCheckTypes() []string {
 	ignoredTypes := make([]string, 0)
 	for _, ignoredType := range strings.Split(strings.ToUpper(c.config.IgnoredHealthChecks), ",") {


Test patch:
diff --git a/consul/consul_test.go b/consul/consul_test.go
index c62be6f..16bfd8b 100644
--- a/consul/consul_test.go
+++ b/consul/consul_test.go
@@ -858,6 +858,30 @@ func TestMarathonTaskToConsulServiceMapping_IgnoredHealthcheckTypes(t *testing.T
 				TimeoutSeconds:         20,
 				MaxConsecutiveFailures: 3,
 			},
+			{
+				Path:                   "/invalid/port/index",
+				Protocol:               "HTTPS",
+				PortIndex:              2,
+				IntervalSeconds:        50,
+				TimeoutSeconds:         20,
+				MaxConsecutiveFailures: 3,
+			},
+			{
+				Path:                   "/negative/port/index",
+				Protocol:               "HTTPS",
+				PortIndex:              -1,
+				IntervalSeconds:        50,
+				TimeoutSeconds:         20,
+				MaxConsecutiveFailures: 3,
+			},
+			{
+				Path:                   "/port/out/of/range",
+				Protocol:               "HTTPS",
+				Port:                   65536,
+				IntervalSeconds:        50,
+				TimeoutSeconds:         20,
+				MaxConsecutiveFailures: 3,
+			},
 			{
 				Protocol:               "TCP",
 				PortIndex:              1,