api: only set url field in config if previously unset (#23785)

In #16872 we added support for configuring the API client with a unix domain
socket. In order to set the host correctly, we parse the address before mutating
the Address field in the configuration. But this prevents the configuration from
being reused across multiple clients, as the next time we parse the address it
will no longer be pointing to the socket. This breaks consumers like the
autoscaler, which reuse the API config between plugins.

Update the `NewClient` constructor to only override the `url` field if it hasn't
already been parsed. Include a test demonstrating safe reuse with a unix domain
socket.

Ref: https://github.com/hashicorp/nomad-autoscaler/issues/944
Ref: https://github.com/hashicorp/nomad-autoscaler/pull/945
This commit is contained in:
Tim Gross
2024-08-09 13:28:04 -04:00
committed by GitHub
parent 920f4702d6
commit b7419bc940
3 changed files with 54 additions and 3 deletions

3
.changelog/23785.txt Normal file
View File

@@ -0,0 +1,3 @@
```release-note:bug
api: Fixed a bug where an `api.Config` targeting a unix domain socket could not be reused between clients
```

View File

@@ -509,10 +509,14 @@ func NewClient(config *Config) (*Client, error) {
}
// we have to test the address that comes from DefaultConfig, because it
// could be the value of NOMAD_ADDR which is applied without testing
// could be the value of NOMAD_ADDR which is applied without testing. But
// only on the first use of this Config, otherwise we'll have mutated the
// address
if config.url == nil {
if config.url, err = url.Parse(config.Address); err != nil {
return nil, fmt.Errorf("invalid address '%s': %v", config.Address, err)
}
}
httpClient := config.HttpClient
if httpClient == nil {

View File

@@ -11,9 +11,12 @@ import (
"errors"
"fmt"
"io"
"net"
"net/http"
"net/http/httptest"
"net/url"
"os"
"path"
"strings"
"testing"
"time"
@@ -595,3 +598,44 @@ func TestClient_autoUnzip(t *testing.T) {
Body: io.NopCloser(&b),
}, nil)
}
func TestUnixSocketConfig(t *testing.T) {
td := os.TempDir() // testing.TempDir() on macOS makes paths that are too long
socketPath := path.Join(td, t.Name()+".sock")
os.Remove(socketPath) // git rid of stale ones now.
t.Cleanup(func() { os.Remove(socketPath) })
ts := httptest.NewUnstartedServer(
http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(`"10.1.1.1"`))
}))
// Create a Unix domain socket and listen for incoming connections.
socket, err := net.Listen("unix", socketPath)
must.NoError(t, err)
t.Cleanup(func() { socket.Close() })
ts.Listener = socket
ts.Start()
defer ts.Close()
cfg := &Config{Address: "unix://" + socketPath}
client1, err := NewClient(cfg)
must.NoError(t, err)
t.Cleanup(client1.Close)
resp, err := client1.Status().Leader()
must.NoError(t, err)
must.Eq(t, "10.1.1.1", resp)
client2, err := NewClient(cfg)
must.NoError(t, err)
t.Cleanup(client2.Close)
_, err = client2.Status().Leader()
must.NoError(t, err)
must.Eq(t, "10.1.1.1", resp)
}