From f27962604d996442867191e116a1c7145fbbb099 Mon Sep 17 00:00:00 2001 From: Chris Baker <1675087+cgbaker@users.noreply.github.com> Date: Wed, 7 Apr 2021 07:00:26 -0500 Subject: [PATCH] Merge pull request #10302 from hashicorp/f-10301-api-header-race-condition fix potential race condition in API client --- .circleci/config.yml | 5 ++++ CHANGELOG.md | 1 + api/api.go | 4 +-- api/api_test.go | 28 +++++++++++++++++++- vendor/github.com/hashicorp/nomad/api/api.go | 4 +-- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 213a00d27f7..da97917532d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -293,11 +293,15 @@ jobs: goarch: type: string default: "amd64" + enable_race_testing: + type: boolean + default: false environment: GOTEST_PKGS_EXCLUDE: "<< parameters.exclude_packages >>" GOTEST_PKGS: "<< parameters.test_packages >>" GOTEST_MOD: "<< parameters.test_module >>" GOTESTARCH: "<< parameters.goarch >>" + ENABLE_RACE: "<<# parameters.enable_race_testing >>TRUE<>" steps: - checkout - install-golang @@ -616,6 +620,7 @@ workflows: name: "test-api" test_module: "api" filters: *backend_test_branches_filter + enable_race_testing: true - test-container: name: "test-devices" test_packages: "./devices/..." diff --git a/CHANGELOG.md b/CHANGELOG.md index 73f0f80b10b..6aecb4c8ceb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ SECURITY: BUG FIXES: * agent: Only allow querying Prometheus formatted metrics if Prometheus is enabled within the config [[GH-10140](https://github.com/hashicorp/nomad/pull/10140)] + * api: Fixed a panic that may occur on concurrent access to an SDK client [[GH-10302](https://github.com/hashicorp/nomad/issues/10302)] * api: Added missing devices block to AllocatedTaskResources [[GH-10064](https://github.com/hashicorp/nomad/pull/10064)] * api: Removed unimplemented `CSIVolumes.PluginList` API. [[GH-10158](https://github.com/hashicorp/nomad/issues/10158)] * cli: Fixed a bug where non-int proxy port would panic CLI [[GH-10072](https://github.com/hashicorp/nomad/issues/10072)] diff --git a/api/api.go b/api/api.go index bb553b64576..cd2a9250fbf 100644 --- a/api/api.go +++ b/api/api.go @@ -677,8 +677,8 @@ func (c *Client) newRequest(method, path string) (*request, error) { } } - if c.config.Headers != nil { - r.header = c.config.Headers + for key, values := range c.config.Headers { + r.header[key] = values } return r, nil diff --git a/api/api_test.go b/api/api_test.go index 0048ecdfe67..dda4a571a13 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -13,9 +13,10 @@ import ( "testing" "time" - "github.com/hashicorp/nomad/api/internal/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/hashicorp/nomad/api/internal/testutil" ) type configCallback func(c *Config) @@ -538,3 +539,28 @@ func TestCloneHttpClient(t *testing.T) { }) } + +func TestClient_HeaderRaceCondition(t *testing.T) { + require := require.New(t) + + conf := DefaultConfig() + conf.Headers = map[string][]string{ + "test-header": {"a"}, + } + client, err := NewClient(conf) + require.NoError(err) + + c := make(chan int) + + go func() { + req, _ := client.newRequest("GET", "/any/path/will/do") + r, _ := req.toHTTP() + c <- len(r.Header) + }() + req, _ := client.newRequest("GET", "/any/path/will/do") + r, _ := req.toHTTP() + + require.Len(r.Header, 2, "local request should have two headers") + require.Equal(2, <-c, "goroutine request should have two headers") + require.Len(conf.Headers, 1, "config headers should not mutate") +} diff --git a/vendor/github.com/hashicorp/nomad/api/api.go b/vendor/github.com/hashicorp/nomad/api/api.go index bb553b64576..cd2a9250fbf 100644 --- a/vendor/github.com/hashicorp/nomad/api/api.go +++ b/vendor/github.com/hashicorp/nomad/api/api.go @@ -677,8 +677,8 @@ func (c *Client) newRequest(method, path string) (*request, error) { } } - if c.config.Headers != nil { - r.header = c.config.Headers + for key, values := range c.config.Headers { + r.header[key] = values } return r, nil