Fix TLS support in api pkg / cli

Fixes #3013 It's a little weird that Client now has a method for returning a NewClient, but it's a convenient way to dedupe the logic to connect-directly-to-a-node which is nontrivial and had sutble differences between locations.
hashicorp · Aug 28, 2017 · 1bb8a54 · 1bb8a54
1 parent 4d97b30
commit 1bb8a54
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 112 deletions.
diff --git a/api/allocations.go b/api/allocations.go
@@ -48,43 +48,24 @@ func (a *Allocations) Info(allocID string, q *QueryOptions) (*Allocation, *Query
 }
 
 func (a *Allocations) Stats(alloc *Allocation, q *QueryOptions) (*AllocResourceUsage, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
-	if err != nil {
-		return nil, err
-	}
-	if node.Status == "down" {
-		return nil, NodeDownErr
-	}
-	if node.HTTPAddr == "" {
-		return nil, fmt.Errorf("http addr of the node where alloc %q is running is not advertised", alloc.ID)
-	}
-	client, err := NewClient(a.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled))
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return nil, err
 	}
+
 	var resp AllocResourceUsage
-	_, err = client.query("/v1/client/allocation/"+alloc.ID+"/stats", &resp, nil)
+	_, err = nodeClient.query("/v1/client/allocation/"+alloc.ID+"/stats", &resp, nil)
 	return &resp, err
 }
 
 func (a *Allocations) GC(alloc *Allocation, q *QueryOptions) error {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
-	if err != nil {
-		return err
-	}
-	if node.Status == "down" {
-		return NodeDownErr
-	}
-	if node.HTTPAddr == "" {
-		return fmt.Errorf("http addr of the node where alloc %q is running is not advertised", alloc.ID)
-	}
-	client, err := NewClient(a.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled))
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return err
 	}
 
 	var resp struct{}
-	_, err = client.query("/v1/client/allocation"+alloc.ID+"/gc", &resp, nil)
+	_, err = nodeClient.query("/v1/client/allocation/"+alloc.ID+"/gc", &resp, nil)
 	return err
 }
 

diff --git a/api/api.go b/api/api.go
@@ -285,6 +285,58 @@ func (c *Client) SetRegion(region string) {
 	c.config.Region = region
 }
 
+// GetNodeClient returns a new Client that will dial the specified node. If the
+// QueryOptions is set, the function will ensure that it is initialized and
+// that the Params field is valid.
+func (c *Client) GetNodeClient(nodeID string, q **QueryOptions) (*Client, error) {
+	node, _, err := c.Nodes().Info(nodeID, &QueryOptions{})
+	if err != nil {
+		return nil, err
+	}
+	if node.Status == "down" {
+		return nil, NodeDownErr
+	}
+	if node.HTTPAddr == "" {
+		return nil, fmt.Errorf("http addr of node %q (%s) is not advertised", node.Name, nodeID)
+	}
+
+	region := ""
+	if q != nil && *q != nil && (*q).Region != "" {
+		region = (*q).Region
+	} else if c.config.Region != "" {
+		// Use the region from the client
+		region = c.config.Region
+	} else {
+		// Use the region from the agent
+		agentRegion, err := c.Agent().Region()
+		if err != nil {
+			return nil, err
+		}
+		region = agentRegion
+	}
+
+	// Get an API client for the node
+	conf := c.config.CopyConfig(node.HTTPAddr, node.TLSEnabled)
+	conf.TLSConfig.TLSServerName = fmt.Sprintf("client.%s.nomad", region)
+	nodeClient, err := NewClient(conf)
+	if err != nil {
+		return nil, err
+	}
+
+	// Set the query params
+	if q == nil {
+		return nodeClient, nil
+	}
+
+	if *q == nil {
+		*q = &QueryOptions{}
+	}
+	if actQ := *q; actQ.Params == nil {
+		actQ.Params = make(map[string]string)
+	}
+	return nodeClient, nil
+}
+
 // request is used to help build up a request
 type request struct {
 	config *Config

diff --git a/api/fs.go b/api/fs.go
@@ -49,58 +49,9 @@ func (c *Client) AllocFS() *AllocFS {
 	return &AllocFS{client: c}
 }
 
-// getNodeClient returns a Client that will dial the node. If the QueryOptions
-// is set, the function will ensure that it is initialized and that the Params
-// field is valid.
-func (a *AllocFS) getNodeClient(node *Node, allocID string, q **QueryOptions) (*Client, error) {
-	if node.HTTPAddr == "" {
-		return nil, fmt.Errorf("http addr of the node where alloc %q is running is not advertised", allocID)
-	}
-
-	region := ""
-	if q != nil && *q != nil && (*q).Region != "" {
-		region = (*q).Region
-	} else if a.client.config.Region != "" {
-		// Use the region from the client
-		region = a.client.config.Region
-	} else {
-		// Use the region from the agent
-		agentRegion, err := a.client.Agent().Region()
-		if err != nil {
-			return nil, err
-		}
-		region = agentRegion
-	}
-
-	// Get an API client for the node
-	conf := a.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled)
-	conf.TLSConfig.TLSServerName = fmt.Sprintf("client.%s.nomad", region)
-	nodeClient, err := NewClient(conf)
-	if err != nil {
-		return nil, err
-	}
-
-	// Set the query params
-	if q == nil {
-		return nodeClient, nil
-	}
-
-	if *q == nil {
-		*q = &QueryOptions{}
-	}
-	if actQ := *q; actQ.Params == nil {
-		actQ.Params = make(map[string]string)
-	}
-	return nodeClient, nil
-}
-
 // List is used to list the files at a given path of an allocation directory
 func (a *AllocFS) List(alloc *Allocation, path string, q *QueryOptions) ([]*AllocFileInfo, *QueryMeta, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, nil, err
-	}
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -117,11 +68,7 @@ func (a *AllocFS) List(alloc *Allocation, path string, q *QueryOptions) ([]*Allo
 
 // Stat is used to stat a file at a given path of an allocation directory
 func (a *AllocFS) Stat(alloc *Allocation, path string, q *QueryOptions) (*AllocFileInfo, *QueryMeta, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, nil, err
-	}
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -138,12 +85,7 @@ func (a *AllocFS) Stat(alloc *Allocation, path string, q *QueryOptions) (*AllocF
 // ReadAt is used to read bytes at a given offset until limit at the given path
 // in an allocation directory. If limit is <= 0, there is no limit.
 func (a *AllocFS) ReadAt(alloc *Allocation, path string, offset int64, limit int64, q *QueryOptions) (io.ReadCloser, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, err
-	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return nil, err
 	}
@@ -161,12 +103,7 @@ func (a *AllocFS) ReadAt(alloc *Allocation, path string, offset int64, limit int
 // Cat is used to read contents of a file at the given path in an allocation
 // directory
 func (a *AllocFS) Cat(alloc *Allocation, path string, q *QueryOptions) (io.ReadCloser, error) {
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, &QueryOptions{})
-	if err != nil {
-		return nil, err
-	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return nil, err
 	}
@@ -190,12 +127,7 @@ func (a *AllocFS) Cat(alloc *Allocation, path string, q *QueryOptions) (io.ReadC
 func (a *AllocFS) Stream(alloc *Allocation, path, origin string, offset int64,
 	cancel <-chan struct{}, q *QueryOptions) (<-chan *StreamFrame, error) {
 
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
-	if err != nil {
-		return nil, err
-	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return nil, err
 	}
@@ -259,12 +191,7 @@ func (a *AllocFS) Stream(alloc *Allocation, path, origin string, offset int64,
 func (a *AllocFS) Logs(alloc *Allocation, follow bool, task, logType, origin string,
 	offset int64, cancel <-chan struct{}, q *QueryOptions) (<-chan *StreamFrame, error) {
 
-	node, _, err := a.client.Nodes().Info(alloc.NodeID, q)
-	if err != nil {
-		return nil, err
-	}
-
-	nodeClient, err := a.getNodeClient(node, alloc.ID, &q)
+	nodeClient, err := a.client.GetNodeClient(alloc.NodeID, &q)
 	if err != nil {
 		return nil, err
 	}

diff --git a/api/nodes.go b/api/nodes.go
@@ -92,19 +92,13 @@ func (n *Nodes) Stats(nodeID string, q *QueryOptions) (*HostStats, error) {
 }
 
 func (n *Nodes) GC(nodeID string, q *QueryOptions) error {
-	node, _, err := n.client.Nodes().Info(nodeID, q)
-	if err != nil {
-		return err
-	}
-	if node.HTTPAddr == "" {
-		return fmt.Errorf("http addr of the node %q is running is not advertised", nodeID)
-	}
-	client, err := NewClient(n.client.config.CopyConfig(node.HTTPAddr, node.TLSEnabled))
+	nodeClient, err := n.client.GetNodeClient(nodeID, &q)
 	if err != nil {
 		return err
 	}
+
 	var resp struct{}
-	_, err = client.query("/v1/client/gc", &resp, nil)
+	_, err = nodeClient.query("/v1/client/gc", &resp, nil)
 	return err
 }