Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix TLS support in api pkg / cli #3108

Merged
merged 3 commits into from
Aug 28, 2017
Merged

Fix TLS support in api pkg / cli #3108

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1bb8a54
Fix TLS support in api pkg / cli
schmichael Aug 28, 2017
2f30827
Refactor GetNodeClient weirdness
schmichael Aug 28, 2017
52857a6
Simplify region handling
schmichael Aug 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 5 additions & 24 deletions api/allocations.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
52 changes: 52 additions & 0 deletions api/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems odd. There may not be an agent if this is being embedded in another program and it would cause an error. It should just do nothing as the http handler would default to the global region

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this query the agent at NOMAD_ADDR for the region? That seems potentially useful even in embedded circumstances.

(This logic was cargo culted from the previous location, so I may not fully grok it)

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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand this logic. If there isn't a reason for it, can you remove and just take a pointer, not a pointer to a pointer.

*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
Expand Down
85 changes: 6 additions & 79 deletions api/fs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
12 changes: 3 additions & 9 deletions api/nodes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down