From 747195f7aaf291305681bb7d8ae070761a2aef55 Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 11:41:46 -0600 Subject: [PATCH 01/12] see if go 1.26.4 fails in ci for compatibility and upgrade tests --- .github/workflows/test-integrations.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test-integrations.yml b/.github/workflows/test-integrations.yml index 263a2e41e4ae..1042785ddd58 100644 --- a/.github/workflows/test-integrations.yml +++ b/.github/workflows/test-integrations.yml @@ -370,6 +370,7 @@ jobs: - name: Setup Git if: ${{ endsWith(github.repository, '-enterprise') }} run: git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN }}:@github.com".insteadOf "https://github.com" + # checking to see if 1.26.4 fails - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 with: go-version-file: 'go.mod' @@ -485,6 +486,7 @@ jobs: - name: Setup Git if: ${{ endsWith(github.repository, '-enterprise') }} run: git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN }}:@github.com".insteadOf "https://github.com" + # checking to see if 1.26.4 fails - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 with: go-version-file: 'go.mod' From 516492420bf43427f1cf89adce4d4e222bbb5aaa Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 12:12:32 -0600 Subject: [PATCH 02/12] pin CI for compatibility and upgrade tests to go version 1.20.5 as 1.20.6 introduces an invalid host header error --- .github/workflows/test-integrations.yml | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-integrations.yml b/.github/workflows/test-integrations.yml index 1042785ddd58..1a3876855d5e 100644 --- a/.github/workflows/test-integrations.yml +++ b/.github/workflows/test-integrations.yml @@ -370,10 +370,13 @@ jobs: - name: Setup Git if: ${{ endsWith(github.repository, '-enterprise') }} run: git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN }}:@github.com".insteadOf "https://github.com" - # checking to see if 1.26.4 fails - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 with: - go-version-file: 'go.mod' + # pinning this to 1.20.5 because this issue in go-testcontainers occurs + # in 1.20.6 with the error "http: invalid Host header, host port waiting failed" + # https://github.com/testcontainers/testcontainers-go/issues/1359 + # go-version-file: 'go.mod' + go-version: '1.20.5' - run: go env - name: docker env run: | @@ -486,10 +489,13 @@ jobs: - name: Setup Git if: ${{ endsWith(github.repository, '-enterprise') }} run: git config --global url."https://${{ secrets.ELEVATED_GITHUB_TOKEN }}:@github.com".insteadOf "https://github.com" - # checking to see if 1.26.4 fails - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 with: - go-version-file: 'go.mod' + # pinning this to 1.20.5 because this issue in go-testcontainers occurs + # in 1.20.6 with the error "http: invalid Host header, host port waiting failed" + # https://github.com/testcontainers/testcontainers-go/issues/1359 + # go-version-file: 'go.mod' + go-version: '1.20.5' - run: go env # Get go binary from workspace From f4d6ca19f8e543048e167b9c47528eeb0bdb656f Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 12:19:02 -0600 Subject: [PATCH 03/12] allow optionally specifying a specific go-version when using reusable-unit.yml. pin the 1.20 api tests to use 1.20.5 --- .github/workflows/go-tests.yml | 7 +++++++ .github/workflows/reusable-unit.yml | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 9baf90c505ed..4a166401d51d 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -376,6 +376,7 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" + go-version: "1.19" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read @@ -394,6 +395,12 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" + # pinning this to 1.20.5 because this issue in go-testcontainers occurs + # in 1.20.6 with the error "http: invalid Host header, host port waiting failed" + # https://github.com/testcontainers/testcontainers-go/issues/1359 + # remove setting this when the above issue is fixed so that the reusable + # job will just get the go version from go.mod. + go-version: "1.20.5" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read diff --git a/.github/workflows/reusable-unit.yml b/.github/workflows/reusable-unit.yml index c066cad3f48d..2c05477398f8 100644 --- a/.github/workflows/reusable-unit.yml +++ b/.github/workflows/reusable-unit.yml @@ -33,6 +33,10 @@ on: required: false type: string default: "" + go-version: + required: false + type: string + default: "" secrets: elevated-github-token: required: true @@ -59,6 +63,12 @@ jobs: if: ${{ endsWith(inputs.repository-name, '-enterprise') }} run: git config --global url."https://${{ secrets.elevated-github-token }}:@github.com".insteadOf "https://github.com" - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 + if: ${{ inputs.go-version != "" }} + with: + go-version-file: ${{ inputs.go-version }} + cache: true + - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 + if: ${{ inputs.go-version == "" }} with: go-version-file: 'go.mod' cache: true From a47407115e086bb5eff6b34a08839989534b505f Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 12:26:42 -0600 Subject: [PATCH 04/12] updating if logic on steps so they can be parsed. --- .github/workflows/reusable-unit.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/reusable-unit.yml b/.github/workflows/reusable-unit.yml index 2c05477398f8..ec8562dd70dd 100644 --- a/.github/workflows/reusable-unit.yml +++ b/.github/workflows/reusable-unit.yml @@ -63,12 +63,12 @@ jobs: if: ${{ endsWith(inputs.repository-name, '-enterprise') }} run: git config --global url."https://${{ secrets.elevated-github-token }}:@github.com".insteadOf "https://github.com" - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 - if: ${{ inputs.go-version != "" }} + if: ${{ inputs.go-version != '' }} with: go-version-file: ${{ inputs.go-version }} cache: true - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 - if: ${{ inputs.go-version == "" }} + if: ${{ inputs.go-version == '' }} with: go-version-file: 'go.mod' cache: true From 8c03b36e00719b65a87d277012dea2ac08b67442 Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 12:28:09 -0600 Subject: [PATCH 05/12] fix go-tests-1.19 to actually pass 1.19 as the version. previously was just testing 1.20 --- .github/workflows/go-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 4a166401d51d..667e7fb331a9 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -419,6 +419,7 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" + go-version: "1.19" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read From c50b17c46ec64dfea20f61d242e1998c804eb8f7 Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 12:30:27 -0600 Subject: [PATCH 06/12] fix reusable unit to specify go-version rather than go-version-file when the go-version input is supplied. --- .github/workflows/reusable-unit.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/reusable-unit.yml b/.github/workflows/reusable-unit.yml index ec8562dd70dd..7442b06c33b5 100644 --- a/.github/workflows/reusable-unit.yml +++ b/.github/workflows/reusable-unit.yml @@ -65,7 +65,7 @@ jobs: - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 if: ${{ inputs.go-version != '' }} with: - go-version-file: ${{ inputs.go-version }} + go-version: ${{ inputs.go-version }} cache: true - uses: actions/setup-go@fac708d6674e30b6ba41289acaab6d4b75aa0753 # v4.0.1 if: ${{ inputs.go-version == '' }} From cc8eaf8213471674843e4a8f6f2f05b98786af4c Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 13:48:09 -0600 Subject: [PATCH 07/12] detect slashes in hostname and replace with localhost for socket addresses./ --- api/api.go | 4 ++++ api/api_test.go | 25 ++++++++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/api/api.go b/api/api.go index 1fe0c71b61e5..d6b4a25502d7 100644 --- a/api/api.go +++ b/api/api.go @@ -1000,6 +1000,10 @@ func (r *request) toHTTP() (*http.Request, error) { return nil, err } + if strings.HasPrefix(r.url.Host, "/") { + r.url.Host = "localhost" + } + req.URL.Host = r.url.Host req.URL.Scheme = r.url.Scheme req.Host = r.url.Host diff --git a/api/api_test.go b/api/api_test.go index 4d5dd1fda830..856bf623233a 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -991,6 +991,29 @@ func TestAPI_RequestToHTTP(t *testing.T) { } } +func TestAPI_RequestToHTTP_PrefixedWithSlashes(t *testing.T) { + t.Parallel() + c, s := makeClient(t) + defer s.Stop() + + c.config.Address = "/tmp/mysocket.sock" + r := c.newRequest("DELETE", "/v1/kv/foo") + q := &QueryOptions{ + Datacenter: "foo", + } + r.setQueryOptions(q) + req, err := r.toHTTP() + require.NoError(t, err) + // validate that socket communications that do not use the host, detect + // slashes in the host name and replace it with local host. + // this is required since go started validating req.host in 1.20.6. + // prior to that they would strip out the slahes for you. They removed that + // behavior and added more strict validation as part of a CVE. + // https://github.com/golang/go/issues/11206 + require.Equal(t, "localhost", req.Host) + +} + func TestAPI_ParseQueryMeta(t *testing.T) { t.Parallel() resp := &http.Response{ @@ -1038,7 +1061,7 @@ func TestAPI_UnixSocket(t *testing.T) { socket := filepath.Join(tempDir, "test.sock") c, s := makeClientWithConfig(t, func(c *Config) { - c.Address = "unix://" + socket + c.Address = "localhost" }, func(c *testutil.TestServerConfig) { c.Addresses = &testutil.TestAddressConfig{ HTTP: "unix://" + socket, From ce10138d074ee4fc92418a5d1ae9f595cc7420da Mon Sep 17 00:00:00 2001 From: John Murret Date: Thu, 13 Jul 2023 14:16:40 -0600 Subject: [PATCH 08/12] remove pinning of 1.20.5 --- .github/workflows/go-tests.yml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 667e7fb331a9..cb1b322ae3e3 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -395,12 +395,6 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" - # pinning this to 1.20.5 because this issue in go-testcontainers occurs - # in 1.20.6 with the error "http: invalid Host header, host port waiting failed" - # https://github.com/testcontainers/testcontainers-go/issues/1359 - # remove setting this when the above issue is fixed so that the reusable - # job will just get the go version from go.mod. - go-version: "1.20.5" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read From 8f223080c027847cd4b3363fa32a66780e5cca5c Mon Sep 17 00:00:00 2001 From: John Murret Date: Fri, 14 Jul 2023 10:14:26 -0600 Subject: [PATCH 09/12] fix test --- api/api_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/api_test.go b/api/api_test.go index 856bf623233a..230d8f9fdfa5 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -1061,7 +1061,7 @@ func TestAPI_UnixSocket(t *testing.T) { socket := filepath.Join(tempDir, "test.sock") c, s := makeClientWithConfig(t, func(c *Config) { - c.Address = "localhost" + c.Address = "unix://" + socket }, func(c *testutil.TestServerConfig) { c.Addresses = &testutil.TestAddressConfig{ HTTP: "unix://" + socket, From f8578b0749fe3df10906b53f91416e9f2421bc29 Mon Sep 17 00:00:00 2001 From: John Murret Date: Fri, 14 Jul 2023 10:20:55 -0600 Subject: [PATCH 10/12] unpin the sdk 1.20 go tests --- .github/workflows/go-tests.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 667e7fb331a9..44f1950bcb1a 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -395,12 +395,7 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" - # pinning this to 1.20.5 because this issue in go-testcontainers occurs - # in 1.20.6 with the error "http: invalid Host header, host port waiting failed" - # https://github.com/testcontainers/testcontainers-go/issues/1359 - # remove setting this when the above issue is fixed so that the reusable - # job will just get the go version from go.mod. - go-version: "1.20.5" + go-version: "1.20" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read From 4452224d6ac786d1d1a723490d08aa7038088f31 Mon Sep 17 00:00:00 2001 From: John Murret Date: Fri, 14 Jul 2023 11:15:52 -0600 Subject: [PATCH 11/12] add comment. remove test --- api/api.go | 7 +++++++ api/api_test.go | 23 ----------------------- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/api/api.go b/api/api.go index d6b4a25502d7..18bb3479c9be 100644 --- a/api/api.go +++ b/api/api.go @@ -1000,6 +1000,13 @@ func (r *request) toHTTP() (*http.Request, error) { return nil, err } + // validate that socket communications that do not use the host, detect + // slashes in the host name and replace it with local host. + // this is required since go started validating req.host in 1.20.6 and 1.19.11. + // prior to that they would strip out the slashes for you. They removed that + // behavior and added more strict validation as part of a CVE. + // https://github.com/golang/go/issues/60374 + // the hope is that if strings.HasPrefix(r.url.Host, "/") { r.url.Host = "localhost" } diff --git a/api/api_test.go b/api/api_test.go index 230d8f9fdfa5..4d5dd1fda830 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -991,29 +991,6 @@ func TestAPI_RequestToHTTP(t *testing.T) { } } -func TestAPI_RequestToHTTP_PrefixedWithSlashes(t *testing.T) { - t.Parallel() - c, s := makeClient(t) - defer s.Stop() - - c.config.Address = "/tmp/mysocket.sock" - r := c.newRequest("DELETE", "/v1/kv/foo") - q := &QueryOptions{ - Datacenter: "foo", - } - r.setQueryOptions(q) - req, err := r.toHTTP() - require.NoError(t, err) - // validate that socket communications that do not use the host, detect - // slashes in the host name and replace it with local host. - // this is required since go started validating req.host in 1.20.6. - // prior to that they would strip out the slahes for you. They removed that - // behavior and added more strict validation as part of a CVE. - // https://github.com/golang/go/issues/11206 - require.Equal(t, "localhost", req.Host) - -} - func TestAPI_ParseQueryMeta(t *testing.T) { t.Parallel() resp := &http.Response{ From 19634a4b3be9a6e466acfb93df769c17eecafe43 Mon Sep 17 00:00:00 2001 From: John Murret Date: Fri, 14 Jul 2023 11:19:26 -0600 Subject: [PATCH 12/12] explicitly set version on go sdk 1.20 tests --- .github/workflows/go-tests.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 44f1950bcb1a..744a398b5aa4 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -433,6 +433,7 @@ jobs: runs-on: ${{ needs.setup.outputs.compute-xl }} repository-name: ${{ github.repository }} go-tags: "${{ github.event.repository.name == 'consul-enterprise' && 'consulent consulprem consuldev' || '' }}" + go-version: "1.20" permissions: id-token: write # NOTE: this permission is explicitly required for Vault auth. contents: read