-
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
cli: Fix sample connect check example #7389
Conversation
Update the check example to be a script check, as group level checks don't support tcp checks. Also, fix the task name.
# command = "/bin/echo" | ||
# args = ["I", "am", "alive"] |
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.
All ears for better health check.
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.
Once #7323 and #7396 are complete, we could provide example HTTP service checks on the Connect enabled service.
In the interim, I'm not sure we should make an example of script checks in Nomad - since they're created via the API they go against Consul's security recommendations
https://www.hashicorp.com/blog/protecting-consul-from-rce-risk-in-specific-configurations/
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.
If these PRs are targeting 0.11.0, we can close this one.
re RCE risk - is that an issue now, the post is from 2018 and pointed that releases then had a fix? If so, we should update our docs as we point to it in few places I believe.
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.
Yeah I think Consul considers the API configuration of script checks to be a design flaw at this point; the check documentation makes this statement
Security Warning: Enabling script checks in some configurations may introduce a remote execution vulnerability which is known to be targeted by malware. We strongly recommend enable_local_script_checks instead. See this blog post for more details.
The enable_local_script_checks
allows only for on-disk definitions of script checks.
I can take a pass at our docs, I'm about to update a bunch of our check documentation anyway.
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. |
Update the check example to be a script check, as group level checks
don't support tcp checks. Also, fix the task name.