close
Skip to content

feat: add ICMP ping check support to item#2180

Merged
lissy93 merged 11 commits into
lissy93:masterfrom
bgillet:features/ping-check
Jun 6, 2026
Merged

feat: add ICMP ping check support to item#2180
lissy93 merged 11 commits into
lissy93:masterfrom
bgillet:features/ping-check

Conversation

@bgillet
Copy link
Copy Markdown
Contributor

@bgillet bgillet commented Jun 2, 2026

Category

Feature

Overview

Items only had a host status check using a HTTP(S) endpoint and checking the returned HTTP code.
Now, it is also possible to make a real ICMP PING to the same host defined by the item's url or using a different IPV4 or hostname. The number of ICMP packets sent as well as the timeout and the period between new pings can be customized globally or per item. See the documentation for details.

image image

@bgillet bgillet requested a review from lissy93 as a code owner June 2, 2026 21:04
@bgillet bgillet force-pushed the features/ping-check branch from fd3768e to 03b713a Compare June 4, 2026 23:00
Copy link
Copy Markdown
Owner

@lissy93 lissy93 left a comment

Choose a reason for hiding this comment

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

This looks awesome!!

My only thought, is that I don't think this would work in Docker. Since the container doesn't run as root, and opening an ICMP socket requires root. And the ping binary isn't present (that could be fixed, by installing it in the Dockerfile).

Few really small things:

  • Revert the conf.yml
  • Small lint warning in Item.vue (variable imported not used)
  • Vaidate host within the server endpoint too

Comment thread src/components/LinkItems/Item.vue Outdated
Comment thread user-data/conf.yml Outdated
Comment thread services/ping-check.js Outdated
Comment thread services/ping-check.js
@bgillet bgillet force-pushed the features/ping-check branch from 689bbae to 4af65d0 Compare June 5, 2026 20:24
@bgillet
Copy link
Copy Markdown
Contributor Author

bgillet commented Jun 5, 2026

My only thought, is that I don't think this would work in Docker. Since the container doesn't run as root, and opening an ICMP socket requires root. And the ping binary isn't present (that could be fixed, by installing it in the Dockerfile).

Hello Lissy,

I was wondering. Do you want me to update the Dockerfile to add the ping binary and maybe giving rights to the node user to call it or do you want to make it yourself to insure the security of the container ?

Regards

Benjamin

@lissy93
Copy link
Copy Markdown
Owner

lissy93 commented Jun 6, 2026

I was wondering. Do you want me to update the Dockerfile to add the ping binary and maybe giving rights to the node user to call it or do you want to make it yourself to insure the security of the container ?

The Dockerfile must not be changed to root-only. Changing UID will break all existing installs. Running as root will drastically reduce security of all installs.

Seems that the Busybox ping output differs from the iputils (which pingman's parser expects), so it's going to need the iputils-ping and a setcap of CAP_NET_RAW.

I've not tested yet, but it should be:

RUN apk add --no-cache tzdata tini iputils-ping \
 && apk add --no-cache --virtual .setcap libcap-setcap \
 && setcap cap_net_raw+ep "$(readlink -f "$(command -v ping)")" \
 && apk del .setcap

Comment thread src/utils/Validator.js Outdated
Comment thread src/components/MinimalView/MinimalSection.vue Outdated
Comment thread src/components/LinkItems/StatusIndicator.vue
Comment thread user-data/conf.yml
Comment thread README.md Outdated
bgillet added 7 commits June 6, 2026 09:30
… CI NodeJS version (20).

Fixed a behavior bug in status indicator component which didn't changed icon to yellow when checking.
Made otherStatusText a computed field to make it reflecting dynamically a new check request.
Added more examples to default conf.yml
Corrected tests using localhost which is not a valid host for pingman.
Added a Validator.js file in src/utils with function to check validity of hosts.
Reverted conf.yml file to the original version.
@bgillet bgillet force-pushed the features/ping-check branch from e6bd9fa to 35e08f7 Compare June 6, 2026 07:38
Copy link
Copy Markdown
Owner

@lissy93 lissy93 left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks @bgillet 🙌
Are you ready for it to be merged?

@bgillet
Copy link
Copy Markdown
Contributor Author

bgillet commented Jun 6, 2026

No. I'm not ready. Sorry. I still have to remove Validator redudant functions and I need to test the modifications of the Dockerfile to make sure the ping works in the container.

Maybe I'm not doing it the right way in GitHub. I make a commit for each fix and mark the related conversation as resolved. This way, I ensure the CI is still OK after each modification.

@bgillet
Copy link
Copy Markdown
Contributor Author

bgillet commented Jun 6, 2026

Now, I think it ready to be merged if you validate it.

@lissy93 lissy93 merged commit daa576d into lissy93:master Jun 6, 2026
12 checks passed
@lissy93
Copy link
Copy Markdown
Owner

lissy93 commented Jun 6, 2026

Thanks @bgillet - merged!
And the CI/CD just finished, so this is live in 4.2.7 and on DockerHub + GHCR 🎉

@bgillet bgillet deleted the features/ping-check branch June 6, 2026 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants