Skip to content

Commit

Permalink
api: don't copy previously parsed URL when setting new address
Browse files Browse the repository at this point in the history
In #16872 we added support for unix domain sockets, but this required mutating
the `Config` when parsing the address. In #23785 we fixed a bug where if the
configuration was used across multiple clients that mutation would happen
multiple times and the address would be incorrectly parsed.

When making `alloc log` or `alloc exec` calls to a region where the region is
not "global", we create a new client from the same configuration and then set
the address. But in this case we copy the private `url` field and that causes
the URL parsing to be skipped for the new client. This results in the region
always being set to the string literal `global` (because of mTLS handling code
introduced all the way back in 4d3b75d), which fails with an error "no path
to region" when the cluster isn't non-global and requests are sent to a
non-leader.

The "right" way of fixing this would be for `ClientConfig` not to change the
region to global in the first place, but as this is a public API and extremely
longstanding behavior, it could potentially be a breaking change for some
downstream consumers. Instead, we'll avoid copying the private `url` field so
that the new address is re-parsed.

Fixes: #24635
Fixes: #24609
Ref: #16872
Ref: #23785
Ref: 4d3b75d
  • Loading branch information
tgross committed Dec 10, 2024
1 parent 3bc2c07 commit f1455cc
Showing 1 changed file with 0 additions and 1 deletion.
1 change: 0 additions & 1 deletion api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,6 @@ func (c *Config) ClientConfig(region, address string, tlsEnabled bool) *Config {
HttpAuth: c.HttpAuth,
WaitTime: c.WaitTime,
TLSConfig: c.TLSConfig.Copy(),
url: copyURL(c.url),
}

// Update the tls server name for connecting to a client
Expand Down

0 comments on commit f1455cc

Please sign in to comment.