From 54a22cef8ea360db92870cba39b32180cf37134b Mon Sep 17 00:00:00 2001
From: Nick Ethier <nethier@hashicorp.com>
Date: Wed, 6 Jan 2021 13:52:48 -0500
Subject: [PATCH] command/agent/consul: use port's to value when building
 service address under 'alloc' addr_mode

---
 CHANGELOG.md                                  |  1 +
 command/agent/consul/service_client.go        |  6 +-
 command/agent/consul/service_client_test.go   | 77 +++++++++++++++++++
 .../docs/job-specification/service.mdx        |  6 +-
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 90e396c68..f8776288d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,7 @@ IMPROVEMENTS:
 
 BUG FIXES:
  * consul: Fixed a bug where updating a task to include services would not work [[GH-9707](https://github.com/hashicorp/nomad/issues/9707)]
+ * consul: Fixed alloc address mode port advertisement to use the mapped `to` port value [[GH-9730](https://github.com/hashicorp/nomad/issues/9730)]
  * consul/connect: Fixed a bug where absent ingress envoy proxy configuration could panic client [[GH-9669](https://github.com/hashicorp/nomad/issues/9669)]
  * template: Fixed multiple issues in template src/dest and artifact dest interpolation [[GH-9671](https://github.com/hashicorp/nomad/issues/9671)]
  * template: Fixed a bug where dynamic secrets did not trigger the template `change_mode` after a client restart. [[GH-9636](https://github.com/hashicorp/nomad/issues/9636)]
diff --git a/command/agent/consul/service_client.go b/command/agent/consul/service_client.go
index 532547124..a96fd78c3 100644
--- a/command/agent/consul/service_client.go
+++ b/command/agent/consul/service_client.go
@@ -1560,7 +1560,7 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet
 
 		return driverNet.IP, port, nil
 
-	case "alloc":
+	case structs.AddressModeAlloc:
 		if netStatus == nil {
 			return "", 0, fmt.Errorf(`cannot use address_mode="alloc": no allocation network status reported`)
 		}
@@ -1572,6 +1572,10 @@ func getAddress(addrMode, portLabel string, networks structs.Networks, driverNet
 
 		// If port is a label and is found then return it
 		if port, ok := ports.Get(portLabel); ok {
+			// Use port.To value unless not set
+			if port.To > 0 {
+				return netStatus.Address, port.To, nil
+			}
 			return netStatus.Address, port.Value, nil
 		}
 
diff --git a/command/agent/consul/service_client_test.go b/command/agent/consul/service_client_test.go
index bb90313c2..0dcc5c49f 100644
--- a/command/agent/consul/service_client_test.go
+++ b/command/agent/consul/service_client_test.go
@@ -4,6 +4,9 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/hashicorp/nomad/nomad/structs"
+	"github.com/hashicorp/nomad/plugins/drivers"
+
 	"github.com/hashicorp/consul/api"
 	"github.com/stretchr/testify/require"
 )
@@ -300,3 +303,77 @@ func TestSyncLogic_maybeTweakTags_emptySC(t *testing.T) {
 		},
 	})
 }
+
+func TestServiceClient_getAddress(t *testing.T) {
+	cases := []struct {
+		name         string
+		addrMode     string
+		portLabel    string
+		networks     structs.Networks
+		driverNet    *drivers.DriverNetwork
+		ports        structs.AllocatedPorts
+		netStatus    *structs.AllocNetworkStatus
+		resultAddr   string
+		resultPort   int
+		resultHasErr bool
+	}{
+		{
+			name:         "nil case",
+			resultHasErr: true,
+		},
+		{
+			name:      "auto happy path",
+			addrMode:  "auto",
+			portLabel: "www",
+			ports: []structs.AllocatedPortMapping{
+				{
+					"www",
+					80,
+					80,
+					"10.0.0.1",
+				},
+			},
+			resultAddr: "10.0.0.1",
+			resultPort: 80,
+		},
+		{
+			name:      "alloc mode with to value",
+			addrMode:  "alloc",
+			portLabel: "www",
+			ports:     []structs.AllocatedPortMapping{{"www", 22222, 80, "10.0.0.1"}},
+			netStatus: &structs.AllocNetworkStatus{
+
+				Address: "172.16.0.1",
+			},
+			resultAddr: "172.16.0.1",
+			resultPort: 80,
+		},
+		{
+			name:      "alloc mode without to value",
+			addrMode:  "alloc",
+			portLabel: "www",
+			ports:     []structs.AllocatedPortMapping{{"www", 22222, 0, "10.0.0.1"}},
+			netStatus: &structs.AllocNetworkStatus{
+
+				Address: "172.16.0.1",
+			},
+			resultAddr: "172.16.0.1",
+			resultPort: 22222,
+		},
+	}
+	require := require.New(t)
+	for _, c := range cases {
+		tc := c
+		t.Run(c.name, func(t *testing.T) {
+
+			addr, port, err := getAddress(tc.addrMode, tc.portLabel, tc.networks, tc.driverNet, tc.ports, tc.netStatus)
+			if tc.resultHasErr {
+				require.Error(err)
+				return
+			}
+			require.Equal(addr, tc.resultAddr)
+			require.Equal(port, tc.resultPort)
+			require.NoError(err)
+		})
+	}
+}
diff --git a/website/content/docs/job-specification/service.mdx b/website/content/docs/job-specification/service.mdx
index c8af46898..070757a05 100644
--- a/website/content/docs/job-specification/service.mdx
+++ b/website/content/docs/job-specification/service.mdx
@@ -106,9 +106,13 @@ Connect][connect] integration.
   service. The value of `port` depends on which [`address_mode`](#address_mode)
   is being used:
 
+  - `alloc` - Advertises the mapped to value of the labeled port and the allocation address.
+    If a `to` value is not set, the port fallsback to using the allocated host port. The `port`
+    field may be a numeric port or a port label specified in the same group's network stanza.
+
   - `driver` - Advertise the port determined by the driver (e.g. Docker or rkt).
     The `port` may be a numeric port or a port label specified in the driver's
-    `port_map`.
+    `ports` field.
 
   - `host` - Advertise the host port for this service. `port` must match a port
     _label_ specified in the [`network`][network] stanza.