-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Handle Consul API URL protocol mismatch #10082
Conversation
CHANGELOG.md
Outdated
@@ -8,6 +8,7 @@ IMPROVEMENTS: | |||
* agent: Removed leading whitespace from JSON-formatted log output. [[GH-9795](https://github.com/hashicorp/nomad/issues/9795)] | |||
* cli: Added optional `-task <task-name>` flag to `alloc logs` to match `alloc exec` [[GH-10026](https://github.com/hashicorp/nomad/issues/10026)] | |||
* cli: Improved `scaling policy` commands with -verbose, auto-completion, and prefix-matching [[GH-9964](https://github.com/hashicorp/nomad/issues/9964)] | |||
* cli: Fixed a bug where `nomad operator debug` incorrectly parsed https Consul API URLs. [[GH-9625](https://github.com/hashicorp/nomad/pull/10082)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 1.0.4 release build has already been cut; this will need to go into 1.1 unless it's a regression from 1.0.3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this has been broken from the beginning, so I understand the decision. I rebased and moved the change up to the 1.1.0 (Unreleased) section.
c952034
to
5caa8af
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -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:") { |
There was a problem hiding this comment.
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! 😀
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
This fixes a bug in nomad operator debug that occurred when an HTTPS address was specified for Consul, either through the
-consul-http-addr
CLI argument orCONSUL_HTTP_ADDR
environment variableFixes #10081