Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Handle Consul API URL protocol mismatch #10082

Merged
merged 4 commits into from
Feb 25, 2021
Merged

Handle Consul API URL protocol mismatch #10082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b1eccc
Update test to reproduce https failure
davemay99 Feb 24, 2021
4560ca0
Handle https protocol mismatch
davemay99 Feb 24, 2021
55446c5
Add CHANGELOG entry
davemay99 Feb 24, 2021
5caa8af
Update CHANGELOG to 1.1 release
davemay99 Feb 25, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
BUG FIXES:
* 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)]

## 1.0.4 (February 24, 2021)

Expand Down
7 changes: 6 additions & 1 deletion command/operator_debug.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,10 @@ func (e *external) addr(defaultAddr string) string {
if strings.HasPrefix(e.addrVal, "http:") {
return e.addrVal
}
if strings.HasPrefix(e.addrVal, "https:") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was going to leave a comment about this being an awfully weird addressing scheme to support, but then I looked it up the details of the caller and apparently Consul supports TLS over UDS. So... ok then! 😀

// Mismatch: e.ssl=false but addrVal is https
return strings.ReplaceAll(e.addrVal, "https://", "http://")
}
return "http://" + e.addrVal
}

Expand All @@ -1072,7 +1076,8 @@ func (e *external) addr(defaultAddr string) string {
}

if strings.HasPrefix(e.addrVal, "http:") {
return "https:" + e.addrVal[5:]
// Mismatch: e.ssl=true but addrVal is http
return strings.ReplaceAll(e.addrVal, "http://", "https://")
}

return "https://" + e.addrVal
Expand Down
32 changes: 25 additions & 7 deletions command/operator_debug_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,17 +433,35 @@ func TestDebug_Utils(t *testing.T) {
require.Empty(t, xs)

// address calculation honors CONSUL_HTTP_SSL
e := &external{addrVal: "http://127.0.0.1:8500", ssl: true}
require.Equal(t, "https://127.0.0.1:8500", e.addr("foo"))
// ssl: true - Correct alignment
e := &external{addrVal: "https://127.0.0.1:8500", ssl: true}
addr := e.addr("foo")
require.Equal(t, "https://127.0.0.1:8500", addr)

// ssl: true - protocol incorrect
e = &external{addrVal: "http://127.0.0.1:8500", ssl: true}
addr = e.addr("foo")
require.Equal(t, "https://127.0.0.1:8500", addr)

// ssl: true - protocol missing
e = &external{addrVal: "127.0.0.1:8500", ssl: true}
addr = e.addr("foo")
require.Equal(t, "https://127.0.0.1:8500", addr)

// ssl: false - correct alignment
e = &external{addrVal: "http://127.0.0.1:8500", ssl: false}
require.Equal(t, "http://127.0.0.1:8500", e.addr("foo"))
addr = e.addr("foo")
require.Equal(t, "http://127.0.0.1:8500", addr)

e = &external{addrVal: "127.0.0.1:8500", ssl: false}
require.Equal(t, "http://127.0.0.1:8500", e.addr("foo"))
// ssl: false - protocol incorrect
e = &external{addrVal: "https://127.0.0.1:8500", ssl: false}
addr = e.addr("foo")
require.Equal(t, "http://127.0.0.1:8500", addr)

e = &external{addrVal: "127.0.0.1:8500", ssl: true}
require.Equal(t, "https://127.0.0.1:8500", e.addr("foo"))
// ssl: false - protocol missing
e = &external{addrVal: "127.0.0.1:8500", ssl: false}
addr = e.addr("foo")
require.Equal(t, "http://127.0.0.1:8500", addr)
}

func TestDebug_WriteBytes_Nil(t *testing.T) {
Expand Down