Fix TLS support in api pkg / cli #3108
Conversation
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.
api/api.go
Outdated
| region = c.config.Region | ||
| } else { | ||
| // Use the region from the agent | ||
| agentRegion, err := c.Agent().Region() |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)
api/api.go
Outdated
| return nodeClient, nil | ||
| } | ||
|
|
||
| if *q == nil { |
There was a problem hiding this comment.
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.
- No need to for a pointer to a pointer - Properly set and use QueryOptions.Region
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
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.