From b7419bc940151684d19054ee3cef6b732196b50a Mon Sep 17 00:00:00 2001
From: Tim Gross <tgross@hashicorp.com>
Date: Fri, 9 Aug 2024 13:28:04 -0400
Subject: [PATCH] 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
---
 .changelog/23785.txt |  3 +++
 api/api.go           | 10 +++++++---
 api/api_test.go      | 44 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 3 deletions(-)
 create mode 100644 .changelog/23785.txt

diff --git a/.changelog/23785.txt b/.changelog/23785.txt
new file mode 100644
index 000000000..dc15e061b
--- /dev/null
+++ b/.changelog/23785.txt
@@ -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
+```
diff --git a/api/api.go b/api/api.go
index 7b42b32de..8693167ee 100644
--- a/api/api.go
+++ b/api/api.go
@@ -509,9 +509,13 @@ 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
-	if config.url, err = url.Parse(config.Address); err != nil {
-		return nil, fmt.Errorf("invalid address '%s': %v", config.Address, err)
+	// 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
diff --git a/api/api_test.go b/api/api_test.go
index 4c62cf073..879a81d57 100644
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -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)
+}