Factors translate out into a separate file and makes safe for in-memory RPCs.

command/agent/agent.go

@@ -1069,18 +1069,6 @@ func (a *Agent) UpdateCheck(checkID types.CheckID, status, output string) error
 	return nil
 }
 
-// TranslateAddr is used to provide the final, translated address for a node,
-// depending on how this agent and the other node are configured.
-func (a *Agent) TranslateAddr(dc string, addr string, taggedAddr map[string]string) string {
-	if a.config.TranslateWanAddrs && (a.config.Datacenter != dc) {
-		wanAddr := taggedAddr["wan"]
-		if wanAddr != "" {
-			addr = wanAddr
-		}
-	}
-	return addr
-}
-
 // persistCheckState is used to record the check status into the data dir.
 // This allows the state to be restored on a later agent start. Currently
 // only useful for TTL based checks.

command/agent/catalog_endpoint.go

@@ -2,9 +2,10 @@ package agent
 
 import (
 	"fmt"
-	"github.com/hashicorp/consul/consul/structs"
 	"net/http"
 	"strings"
+
+	"github.com/hashicorp/consul/consul/structs"
 )
 
 func (s *HTTPServer) CatalogRegister(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
@@ -72,17 +73,12 @@ func (s *HTTPServer) CatalogNodes(resp http.ResponseWriter, req *http.Request) (
 	if err := s.agent.RPC("Catalog.ListNodes", &args, &out); err != nil {
 		return nil, err
 	}
+	translateAddresses(s.agent.config, args.Datacenter, out.Nodes)
 
 	// Use empty list instead of nil
 	if out.Nodes == nil {
 		out.Nodes = make(structs.Nodes, 0)
 	}
-
-	for _, node := range out.Nodes {
-		addr := s.agent.TranslateAddr(args.Datacenter, node.Address, node.TaggedAddresses)
-		node.Address = addr
-	}
-
 	return out.Nodes, nil
 }
 
@@ -130,17 +126,12 @@ func (s *HTTPServer) CatalogServiceNodes(resp http.ResponseWriter, req *http.Req
 	if err := s.agent.RPC("Catalog.ServiceNodes", &args, &out); err != nil {
 		return nil, err
 	}
+	translateAddresses(s.agent.config, args.Datacenter, out.ServiceNodes)
 
 	// Use empty list instead of nil
 	if out.ServiceNodes == nil {
 		out.ServiceNodes = make(structs.ServiceNodes, 0)
 	}
-
-	for _, serviceNode := range out.ServiceNodes {
-		addr := s.agent.TranslateAddr(args.Datacenter, serviceNode.Address, serviceNode.TaggedAddresses)
-		serviceNode.Address = addr
-	}
-
 	return out.ServiceNodes, nil
 }
 
@@ -165,11 +156,8 @@ func (s *HTTPServer) CatalogNodeServices(resp http.ResponseWriter, req *http.Req
 	if err := s.agent.RPC("Catalog.NodeServices", &args, &out); err != nil {
 		return nil, err
 	}
-
-	if out.NodeServices != nil {
-		node := out.NodeServices.Node
-		addr := s.agent.TranslateAddr(args.Datacenter, node.Address, node.TaggedAddresses)
-		node.Address = addr
+	if out.NodeServices != nil && out.NodeServices.Node != nil {
+		translateAddresses(s.agent.config, args.Datacenter, out.NodeServices.Node)
 	}
 
 	return out.NodeServices, nil

command/agent/dns.go

@@ -411,7 +411,7 @@ RPC:
 
 	// Add the node record
 	n := out.NodeServices.Node
-	addr := d.agent.TranslateAddr(datacenter, n.Address, n.TaggedAddresses)
+	addr := translateAddress(d.agent.config, datacenter, n.Address, n.TaggedAddresses)
 	records := d.formatNodeRecord(out.NodeServices.Node, addr,
 		req.Question[0].Name, qType, d.config.NodeTTL)
 	if records != nil {
@@ -764,7 +764,7 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode
 	for _, node := range nodes {
 		// Start with the translated address but use the service address,
 		// if specified.
-		addr := d.agent.TranslateAddr(dc, node.Node.Address, node.Node.TaggedAddresses)
+		addr := translateAddress(d.agent.config, dc, node.Node.Address, node.Node.TaggedAddresses)
 		if node.Service.Address != "" {
 			addr = node.Service.Address
 		}
@@ -813,7 +813,7 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes
 
 		// Start with the translated address but use the service address,
 		// if specified.
-		addr := d.agent.TranslateAddr(dc, node.Node.Address, node.Node.TaggedAddresses)
+		addr := translateAddress(d.agent.config, dc, node.Node.Address, node.Node.TaggedAddresses)
 		if node.Service.Address != "" {
 			addr = node.Service.Address
 		}

command/agent/health_endpoint.go

@@ -131,6 +131,9 @@ func (s *HTTPServer) HealthServiceNodes(resp http.ResponseWriter, req *http.Requ
 		out.Nodes = filterNonPassing(out.Nodes)
 	}
 
+	// Translate addresses after filtering so we don't waste effort.
+	translateAddresses(s.agent.config, args.Datacenter, out.Nodes)
+
 	// Use empty list instead of nil
 	for i, _ := range out.Nodes {
 		// TODO (slackpad) It's lame that this isn't a slice of pointers
@@ -144,12 +147,6 @@ func (s *HTTPServer) HealthServiceNodes(resp http.ResponseWriter, req *http.Requ
 		out.Nodes = make(structs.CheckServiceNodes, 0)
 	}
 
-	for _, checkServiceNode := range out.Nodes {
-		node := checkServiceNode.Node
-		addr := s.agent.TranslateAddr(args.Datacenter, node.Address, node.TaggedAddresses)
-		node.Address = addr
-	}
-
 	return out.Nodes, nil
 }
 

command/agent/prepared_query_endpoint.go

@@ -122,17 +122,16 @@ func (s *HTTPServer) preparedQueryExecute(id string, resp http.ResponseWriter, r
 		return nil, err
 	}
 
+	// Note that we translate using the DC that the results came from, since
+	// a query can fail over to a different DC than where the execute request
+	// was sent to. That's why we use the reply's DC and not the one from
+	// the args.
+	translateAddresses(s.agent.config, reply.Datacenter, reply.Nodes)
+
 	// Use empty list instead of nil.
 	if reply.Nodes == nil {
 		reply.Nodes = make(structs.CheckServiceNodes, 0)
 	}
-
-	for _, checkServiceNode := range reply.Nodes {
-		node := checkServiceNode.Node
-		addr := s.agent.TranslateAddr(args.Datacenter, node.Address, node.TaggedAddresses)
-		node.Address = addr
-	}
-
 	return reply, nil
 }
 

command/agent/translate_addr.go

@@ -0,0 +1,67 @@
+package agent
+
+import (
+	"fmt"
+
+	"github.com/hashicorp/consul/consul/structs"
+)
+
+// translateAddress is used to provide the final, translated address for a node,
+// depending on how the agent and the other node are configured. The dc
+// parameter is the dc the datacenter this node is from.
+func translateAddress(config *Config, dc string, addr string, taggedAddresses map[string]string) string {
+	if config.TranslateWanAddrs && (config.Datacenter != dc) {
+		wanAddr := taggedAddresses["wan"]
+		if wanAddr != "" {
+			addr = wanAddr
+		}
+	}
+	return addr
+}
+
+// translateAddresses translates addresses in the given structure into the
+// final, translated address, depending on how the agent and the other node are
+// configured. The dc parameter is the datacenter this structure is from.
+func translateAddresses(config *Config, dc string, subj interface{}) {
+	// CAUTION - SUBTLE! An agent running on a server can, in some cases,
+	// return pointers directly into the immutable state store for
+	// performance (it's via the in-memory RPC mechanism). It's never safe
+	// to modify those values, so we short circuit here so that we never
+	// update any structures that are from our own datacenter. This works
+	// for address translation because we *never* need to translate local
+	// addresses, but this is super subtle, so we've piped all the in-place
+	// address translation into this function which makes sure this check is
+	// done. This also happens to skip looking at any of the incoming
+	// structure for the common case of not needing to translate, so it will
+	// skip a lot of work if no translation needs to be done.
+	if !config.TranslateWanAddrs || (config.Datacenter == dc) {
+		return
+	}
+
+	// Translate addresses in-place, subject to the condition checked above
+	// which ensures this is safe to do since we are operating on a local
+	// copy of the data.
+	switch v := subj.(type) {
+	case structs.CheckServiceNodes:
+		for _, entry := range v {
+			entry.Node.Address = translateAddress(config, dc,
+				entry.Node.Address, entry.Node.TaggedAddresses)
+		}
+	case *structs.Node:
+		v.Address = translateAddress(config, dc,
+			v.Address, v.TaggedAddresses)
+	case structs.Nodes:
+		for _, node := range v {
+			node.Address = translateAddress(config, dc,
+				node.Address, node.TaggedAddresses)
+		}
+	case structs.ServiceNodes:
+		for _, entry := range v {
+			entry.Address = translateAddress(config, dc,
+				entry.Address, entry.TaggedAddresses)
+		}
+	default:
+		panic(fmt.Errorf("Unhandled type passed to address translator: %#v", subj))
+
+	}
+}