-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
inputs.ping: Always SetPrivileged(true) in native mode #9072
Conversation
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.
🤝 ✅ CLA has been signed. Thank you!
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.
Looks like new artifacts were built from this PR. Get them here!
Artifact URLs
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.
Hi sspaink, I'd like to have telegraf give a more useful error message when the user doesn't have cap_net_raw set up yet. It will be hard especially for beginners to know that "ping failed: failed to run pinger: listen ip4:icmp : socket: operation not permitted" means they need to use run setcap on the telegraf binary.
Ideally the message should tell the user what they need to do to fix it, or at least point them to docs that tell them how to fix it. The more helpful the error message is, the fewer people will struggle to get this set up.
We would need to check the specific type of error on line 192 and return the helpful message, then let all other ping errors return the current generic "failed to run" message.
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.
Looks like new artifacts were built from this PR. Get them here!
Artifact URLs
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.
Thanks!
* Always SetPrivileged(true) * Improve error message (cherry picked from commit 7d66590)
* Always SetPrivileged(true) * Improve error message
* Always SetPrivileged(true) * Improve error message (cherry picked from commit 7d66590)
resolve #8919
As pointed out by the users in the above issue, the inputs.ping implementation was incomplete. The documentation made it seem you needed to set CAP_NET_RAW and set sysctl, while the CAP_NET_RAW solution wouldn't work without the call to SetPrivileged(true). As well when using FreeBSD you either need root or set CAP_NET_RAW in order to send ICMP packets, therefore requiring SetPrivileged(true).
When SetPrivileged(true) is not set the library go-ping will attempt to send a UDP ping which isn't as reliable as a ICMP ping. While a UDP ping doesn't require any permissions it will only work if the endpoint is configured to reply. This change will make using ICMP ping as the default behavior, requiring elevated permissions.
Output examples with the changes in this PR:
Example of what happens when not giving the correct permissions, it outputs an error saying the operation is not permitted:
Working example: