From f4e341e39ca20ad2527f0999977bab1a68990e24 Mon Sep 17 00:00:00 2001
From: Michael Schurter <schmichael@hashicorp.com>
Date: Thu, 7 Dec 2017 21:58:15 -0800
Subject: [PATCH] Validate port label for host address mode

Also skip getting an address for script checks which don't use them.

Fixed a weird invalid reserved port in a TaskRunner test helper as well
as a problem with our mock Alloc/Job. Hopefully the latter doesn't cause
other tests to fail, but we were referencing an invalid PortLabel and
just not catching it before.
---
 client/task_runner_test.go        |  5 +----
 command/agent/consul/client.go    | 10 ++++++++++
 command/agent/consul/unit_test.go | 12 +++++++++++-
 nomad/mock/mock.go                |  2 +-
 4 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/client/task_runner_test.go b/client/task_runner_test.go
index 8221705071d5..d91f5a96bbc4 100644
--- a/client/task_runner_test.go
+++ b/client/task_runner_test.go
@@ -119,9 +119,6 @@ func testTaskRunnerFromAlloc(t *testing.T, restarts bool, alloc *structs.Allocat
 
 	upd := &MockTaskStateUpdater{}
 	task := alloc.Job.TaskGroups[0].Tasks[0]
-	// Initialize the port listing. This should be done by the offer process but
-	// we have a mock so that doesn't happen.
-	task.Resources.Networks[0].ReservedPorts = []structs.Port{{Label: "", Value: 80}}
 
 	allocDir := allocdir.NewAllocDir(testLogger(), filepath.Join(conf.AllocDir, alloc.ID))
 	if err := allocDir.Build(); err != nil {
@@ -347,7 +344,7 @@ func TestTaskRunner_Update(t *testing.T) {
 	task.Services[0].Checks[0] = &structs.ServiceCheck{
 		Name:      "http-check",
 		Type:      "http",
-		PortLabel: "web",
+		PortLabel: "http",
 		Path:      "${NOMAD_META_foo}",
 	}
 	task.Driver = "mock_driver"
diff --git a/command/agent/consul/client.go b/command/agent/consul/client.go
index 574e89dd94fa..78ea6429c742 100644
--- a/command/agent/consul/client.go
+++ b/command/agent/consul/client.go
@@ -632,6 +632,13 @@ func (c *ServiceClient) checkRegs(ops *operations, allocID, serviceID string, se
 			ops.scripts = append(ops.scripts, newScriptCheck(
 				allocID, task.Name, checkID, check, exec, c.client, c.logger, c.shutdownCh))
 
+			// Skip getAddress for script checks
+			checkReg, err := createCheckReg(serviceID, checkID, check, "", 0)
+			if err != nil {
+				return nil, fmt.Errorf("failed to add script check %q: %v", check.Name, err)
+			}
+			ops.regChecks = append(ops.regChecks, checkReg)
+			continue
 		}
 
 		// Default to the service's port but allow check to override
@@ -1098,6 +1105,9 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet
 	case structs.AddressModeHost:
 		// Default path: use host ip:port
 		ip, port := networks.Port(portLabel)
+		if ip == "" && port == 0 {
+			return "", 0, fmt.Errorf("invalid port %q: port label not found", portLabel)
+		}
 		return ip, port, nil
 
 	case structs.AddressModeDriver:
diff --git a/command/agent/consul/unit_test.go b/command/agent/consul/unit_test.go
index 73d82bd12056..f9cec004fc79 100644
--- a/command/agent/consul/unit_test.go
+++ b/command/agent/consul/unit_test.go
@@ -1541,6 +1541,12 @@ func TestGetAddress(t *testing.T) {
 			},
 			ErrContains: "invalid port",
 		},
+		{
+			Name:        "HostBadPort",
+			Mode:        structs.AddressModeHost,
+			PortLabel:   "bad-port-label",
+			ErrContains: "invalid port",
+		},
 		{
 			Name:        "InvalidMode",
 			Mode:        "invalid-mode",
@@ -1574,7 +1580,11 @@ func TestGetAddress(t *testing.T) {
 			if tc.ErrContains == "" {
 				assert.Nil(t, err)
 			} else {
-				assert.Contains(t, err.Error(), tc.ErrContains)
+				if err == nil {
+					t.Fatalf("expected error containing %q but err=nil", tc.ErrContains)
+				} else {
+					assert.Contains(t, err.Error(), tc.ErrContains)
+				}
 			}
 		})
 	}
diff --git a/nomad/mock/mock.go b/nomad/mock/mock.go
index 556dd0b8d05b..41813f790f33 100644
--- a/nomad/mock/mock.go
+++ b/nomad/mock/mock.go
@@ -119,7 +119,7 @@ func Job() *structs.Job {
 							},
 							{
 								Name:      "${TASK}-admin",
-								PortLabel: "admin",
+								PortLabel: "main",
 							},
 						},
 						LogConfig: structs.DefaultLogConfig(),