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 so as to remove the port number. 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`, `alloc fs`, or `alloc exec` calls where we have
line-of-sight to the client, we attempt to make a HTTP API call directly to the
client node. So we create a new API 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), unless the user has set the region specifically. This fails with
an error "no path to region" when the cluster isn't non-global and requests are
sent to a non-leader.

Arguably the "right" way of fixing this would be for `ClientConfig` not to
change the API client's 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 16, 2024
1 parent 24fa743 commit ed755f5
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/24644.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
api: Fixed a bug where alloc exec/logs/fs APIs would return errors for non-global regions
```
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
1 change: 1 addition & 0 deletions e2e/terraform/provision-nomad/etc/nomad.d/base.hcl
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Copyright (c) HashiCorp, Inc.
# SPDX-License-Identifier: BUSL-1.1

region = "e2e"
bind_addr = "0.0.0.0"
data_dir = "${data_dir}"
enable_debug = true
Expand Down

0 comments on commit ed755f5

Please sign in to comment.