Skip to content

Commit

Permalink
Merge pull request #10302 from hashicorp/f-10301-api-header-race-cond…
Browse files Browse the repository at this point in the history
…ition

fix potential race condition in API client
  • Loading branch information
cgbaker authored Apr 7, 2021
2 parents 6caee50 + aef0e4a commit ca66719
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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<</ parameters.enable_race_testing >>"
steps:
- checkout
- install-golang
Expand Down Expand Up @@ -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/..."
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ IMPROVEMENTS:

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)]
* cli: Fixed a bug where non-int proxy port would panic CLI [[GH-10072](https://github.com/hashicorp/nomad/issues/10072)]
* cli: Fixed a bug where `nomad operator debug` incorrectly parsed https Consul API URLs. [[GH-10082](https://github.com/hashicorp/nomad/pull/10082)]
Expand Down
4 changes: 2 additions & 2 deletions api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -685,8 +685,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
Expand Down
28 changes: 27 additions & 1 deletion api/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
}
4 changes: 2 additions & 2 deletions vendor/github.com/hashicorp/nomad/api/api.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit ca66719

Please sign in to comment.