From 12e148f25e7826965ad7b67a046e3ebc48ea4687 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Mon, 16 Dec 2024 11:05:29 -0500 Subject: [PATCH] api: don't copy previously parsed URL when setting new address (#24644) 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 4d3b75d867da), 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: https://github.com/hashicorp/nomad/issues/24635 Fixes: https://github.com/hashicorp/nomad/issues/24609 Ref: https://github.com/hashicorp/nomad/pull/16872 Ref: https://github.com/hashicorp/nomad/pull/23785 Ref: https://github.com/hashicorp/nomad/commit/4d3b75d867dae508011c198c84318f54b4aa6684 --- .changelog/24644.txt | 3 +++ api/api.go | 1 - e2e/terraform/etc/nomad.d/base.hcl | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 .changelog/24644.txt diff --git a/.changelog/24644.txt b/.changelog/24644.txt new file mode 100644 index 00000000000..bd586d5da81 --- /dev/null +++ b/.changelog/24644.txt @@ -0,0 +1,3 @@ +```release-note:bug +api: Fixed a bug where alloc exec/logs/fs APIs would return errors for non-global regions +``` diff --git a/api/api.go b/api/api.go index 8693167ee4c..07cc4d9f23e 100644 --- a/api/api.go +++ b/api/api.go @@ -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 diff --git a/e2e/terraform/etc/nomad.d/base.hcl b/e2e/terraform/etc/nomad.d/base.hcl index 2208e62ab32..4532140661d 100644 --- a/e2e/terraform/etc/nomad.d/base.hcl +++ b/e2e/terraform/etc/nomad.d/base.hcl @@ -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