Remove check_http #1806

RincewindsHat · 2022-11-04T15:14:26Z

(Kinda) resolves some of the problems of check_http.
Opening this as a start for discussing the removal of check_http

andreasbaumann · 2022-11-05T07:18:15Z

There are some things to consider:

are people using check_http on machines not having a libcurl (or maybe a too old version of libcurl)?
do we have enough feedback and usage of check_curl to be sure it can replace check_http everywhere?
Alternatively we could also just enable check_curl by default and make check_http an optional build thing?

RincewindsHat · 2022-11-05T12:15:45Z

Well, #1808 goes on that list.
Apart from that: These are good questions and I don't know how to properly answer them. How can we get answers? Place a deprecation Warning in check_http and wait for a year for the bug reports?

Al2Klimov · 2023-02-08T13:53:29Z

Alternative

Re-implement check_http on top of check_curl.

RincewindsHat · 2023-02-08T13:54:35Z

@Al2Klimov what?

Al2Klimov · 2023-02-08T13:56:15Z

Like

def check_http():
  check_curl()

RincewindsHat · 2023-02-10T10:14:06Z

@Al2Klimov And what exactly should that achieve? What are you going for?

Al2Klimov · 2023-02-10T10:15:57Z

To preserve compatibility.

RincewindsHat · 2023-02-10T10:26:55Z

I would just symlink check_http -> check_curl

Al2Klimov · 2023-02-10T10:55:08Z

If they are that compatible(?), feel free to just drop it. Packagers and/or ITL can easily handle this.

waja · 2023-02-10T15:20:32Z

If they are that compatible(?), feel free to just drop it. Packagers and/or ITL can easily handle this.

I can start with this in the Debian package right after bookworm was released (without a hard drop upstream, just not including it in the package and symlinking), when it's a drop-in replacement.

andreasbaumann · 2023-02-12T07:01:58Z

I personally wouldn't do that. Rather make check_http issue a warning, that you should use check_curl if possible.
And I would not implement new features in check_http. check_curl is still marked experimental for good reason, unless
it is deployed and tested in bigger environments, just switching from check_http to check_curl could be dangerous.

bittorf · 2025-01-13T15:34:01Z

I'm willing to help with checking compat, but struggling to compile check_curl

$ cd GIT/monitoring-plugins
$ git log -1
$ commit e23325f7c3b34d8950cf27e9ab23af362d0b341b (HEAD -> master, origin/master, origin/coverity/master, origin/HEAD)
Merge: a8ac865e 558aca48
Author: Lorenz K<C3><A4>stle <[email protected]>
Date:   Tue Jan 7 11:36:08 2025 +0100

    Merge pull request #2056 from monitoring-plugins/fix_check_http_state_regex
    
    check_http: fix documentation for --state-regex

$ ./tools/setup
$ ./configure --with-libcurl --with-uriparser
$ make
$ find . -name 'check_curl'
<empty>

what is missing during ./configure ?

andreasbaumann · 2025-01-13T16:08:17Z

Do you get messages during configure like:

checking for gawk... (cached) gawk
checking for curl-config... /usr/bin/curl-config
checking for the version of libcurl... 8.11.1
checking for libcurl >= version 7.15.2... yes
checking whether libcurl is usable... yes
checking for curl_free... yes
checking for pkg-config... pkg-config
checking for the version of uriparser... 0.9.8
checking for gawk... (cached) gawk
checking for uriparser >= version 0.7.5... yes
checking whether uriparser is usable... yes
...
                    --with-libcurl: yes
                  --with-uriparser: yes

If not (for instance prerequisites are missing), the configure script just sets
"with curl", "with uriparser" to NO and compiles the rest without check_curl.

bittorf · 2025-01-14T09:42:40Z

Indeed!

[root@d579f1cbec9d monitoring-plugins]# ./configure --with-libcurl --with-uriparser >OUT 2>&1
[root@d579f1cbec9d monitoring-plugins]# grep curl OUT
...
checking for uriparser >= version 0.7.5... no
configure: WARNING: Skipping curl plugin
configure: WARNING: install the uriparser library to compile this plugin (see REQUIREMENTS).

i had to build https://github.com/uriparser/uriparser and point to the correct path:

[root@d579f1cbec9d monitoring-plugins]# ./configure --with-libcurl --with-uriparser=/usr/local/lib64 && make
[root@d579f1cbec9d monitoring-plugins]# find . -name check_curl 
./plugins/check_curl

# there it is!

But this is weird:

[root@d579f1cbec9d monitoring-plugins]# ./plugins/check_curl
./plugins/check_curl: error while loading shared libraries: liburiparser.so.1: cannot open shared object file: No such file or directory

[root@d579f1cbec9d monitoring-plugins]# find /usr/local/lib64 -name 'liburiparser*' -ls
 59013483      4 -rw-r--r--   1  root     root          267 Jan 14 09:17 /usr/local/lib64/pkgconfig/liburiparser.pc
 59013423    140 -rwxr-xr-x   1  root     root       140304 Jan 14 09:17 /usr/local/lib64/liburiparser.so.1.0.31
 59013424      0 lrwxrwxrwx   1  root     root           22 Jan 14 09:18 /usr/local/lib64/liburiparser.so.1 -> liburiparser.so.1.0.31
 59013431      0 lrwxrwxrwx   1  root     root           17 Jan 14 09:18 /usr/local/lib64/liburiparser.so -> liburiparser.so.1

[root@d579f1cbec9d monitoring-plugins]# file /usr/local/lib64/liburiparser.so.1.0.31
/usr/local/lib64/liburiparser.so.1.0.31: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, BuildID[sha1]=81a0d7dec93d2c14c85336ddfa25463bf6acee2d, not stripped

so i tried to build a statically liburiparser.a with:

# delete shared libs:
[root@d579f1cbec9d monitoring-plugins]# rm -f /usr/local/lib64/liburiparser*

cmake -DCMAKE_BUILD_TYPE=Release -DURIPARSER_SHARED_LIBS:BOOL=OFF ../
make && make install

[root@d579f1cbec9d monitoring-plugins]# ls -l /usr/local/lib64/liburiparser.a 
-rw-r--r-- 1 root root 394574 Jan 14 09:36 /usr/local/lib64/liburiparser.a

and rebuild check_curl

[root@d579f1cbec9d monitoring-plugins]# ./configure --with-libcurl --with-uriparser=/usr/local/lib64
[root@d579f1cbec9d monitoring-plugins]# make
[root@d579f1cbec9d monitoring-plugins]# ./plugins/check_curl -H google.de
HTTP OK: HTTP/1.1 301 Moved Permanently - 790 bytes in 0.088 second response time |time=0.087831s;;;0.000000;10.000000 size=790B;;;0;

Thanks for the help, i can now start testing compatibility with check_http

andreasbaumann · 2025-01-14T09:49:25Z

/usr/local/lib64 might not be in your /etc/ld.so.conf.d or so, and you have to call ldconfig after adding it there.

bittorf · 2025-01-15T10:41:13Z

I found a case, where a drop-in replacement
of check_http with check_curl does NOT succeed.
This checks a certificate for validity time:

# i replaced the servername, but a debug dump follows below:

$ /usr/lib64/nagios/plugins/check_http \
        --sni \
        -C 21,14 \
        -H myhost.mydomain.org \
        -I 10.201.3.111 \
        -S \
        -e HTTP \
        -p 443
SSL OK - Certificate '*.mydomain.org' will expire in 46 days on 2025-03-03 00:59 +0100/CET.

$ /usr/lib64/nagios/plugins/check_curl \
        --sni \
        -C 21,14 \
        -H myhost.mydomain.org \
        -I 10.201.3.111 \
        -S \
        -e HTTP \
        -p 443
HTTP CRITICAL - Invalid HTTP response received from host on port 443: cURL returned 52 - Empty reply from server

(LIVE) [icinga@op24 ~]$ curl -vik --resolve qa-pbx03.novomind.com:443:10.201.3.111 https://qa-pbx03.novomind.com
* Added qa-pbx03.novomind.com:443:10.201.3.111 to DNS cache
* Rebuilt URL to: https://qa-pbx03.novomind.com/
* Hostname qa-pbx03.novomind.com was found in DNS cache
*   Trying 10.201.3.111...
* TCP_NODELAY set
* Connected to qa-pbx03.novomind.com (10.201.3.111) port 443 (#0)
* ALPN, offering h2
* ALPN, offering http/1.1
* successfully set certificate verify locations:
*   CAfile: /etc/pki/tls/certs/ca-bundle.crt
  CApath: none
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.2 (IN), TLS handshake, Certificate (11):
* TLSv1.2 (IN), TLS handshake, Server key exchange (12):
* TLSv1.2 (IN), TLS handshake, Server finished (14):
* TLSv1.2 (OUT), TLS handshake, Client key exchange (16):
* TLSv1.2 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.2 (OUT), TLS handshake, Finished (20):
* TLSv1.2 (IN), TLS handshake, Finished (20):
* SSL connection using TLSv1.2 / ECDHE-RSA-AES256-GCM-SHA384
* ALPN, server did not agree to a protocol
* Server certificate:
*  subject: CN=*.novomind.com
*  start date: Jan 31 00:00:00 2024 GMT
*  expire date: Mar  2 23:59:59 2025 GMT
*  issuer: C=GB; ST=Greater Manchester; L=Salford; O=Sectigo Limited; CN=Sectigo RSA Domain Validation Secure Server CA
*  SSL certificate verify ok.
> GET / HTTP/1.1
> Host: qa-pbx03.novomind.com
> User-Agent: curl/7.61.1
> Accept: */*
>
* TLSv1.2 (IN), TLS alert, close notify (256):
* Empty reply from server
* Connection #0 to host qa-pbx03.novomind.com left intact
curl: (52) Empty reply from server

$ echo $?
52

It seeam the "curl: (52)" message is normal but for the monitoring case not related.
Opinions?

RincewindsHat · 2025-01-15T12:32:07Z

hm, since -C ist currently documented as:

 -C, --certificate=INTEGER[,INTEGER]
    Minimum number of days a certificate has to be valid. Port defaults to 443
    (when this option is used the URL is not checked by default. You can use
     --continue-after-certificate to override this behavior)

I would call this a bug on the side of check_curl (although, by default, not to continue after the certificate tests ist a design choice I do not fully support)

waja · 2025-01-15T14:20:04Z

I didn't have a look into this, but might it be a good idea to duplicate all tests from check_http to check_curl if didn't have done it yet?

RincewindsHat · 2025-01-15T14:25:07Z

@waja afaik this is already the case.

waja · 2025-04-19T08:44:00Z

Hi ... I'm looking into make monitoring-plugins release ready for trixie and I'm asking if I should do something to prepare the removal of check_http. Beside mentioning it in the Changelog and the NEWS file, which might not read by a lot ppl, is there some recommandation?

RincewindsHat · 2025-04-20T23:23:31Z

well, ideally check_curl should behave like check_http (and likely does so in most(tm) cases), therefore the solution of just linking to check_curl seems appropriate.

The more "in your face" approach would to remove it wholesale and let people run into errors, google it, curse us and change their setup accordingly.
This could be softened by keeping check_http but adding a enforced note in the output about the deprecation which would only be noticed if somebody reviews the output.
check_http could the be removed some time later.

I am not really for or against one of these approaches, I did not yet think it through.

waja · 2025-04-21T07:10:24Z

This could be softened by keeping check_http but adding a enforced note in the output about the deprecation which would only be noticed if somebody reviews the output.
check_http could the be removed some time later.

I am not really for or against one of these approaches, I did not yet think it through.

Maybe you can have a look into #2118, which might speed up the process? :)

Remove check_http

04843ae

waja added this to the 2.4 milestone Nov 4, 2022

waja added check_curl check_http labels Nov 4, 2022

hydrapolic mentioned this pull request Feb 8, 2023

Add check_curl to ITL Icinga/icinga2#9205

Merged

juliogonzalez mentioned this pull request Sep 14, 2023

check_http: Unchunking body fails when no body is present #1872

Closed

waja mentioned this pull request Dec 11, 2023

check_http: Add "--verify-host" to verify SSL certificate hostname and trust chain #1971

Open

waja modified the milestones: 2.4, 2.5 Jul 23, 2024

waja mentioned this pull request Apr 19, 2025

check_http: Adding deprecation text #2118

Merged

s1shed mentioned this pull request Jun 1, 2025

check_http fails when an SSL/TLS handshake is rejected by default #2127

Open

RincewindsHat modified the milestones: 2.5, 4.0.0 Dec 4, 2025

Remove check_http #1806

Are you sure you want to change the base?

Remove check_http #1806

Uh oh!

Conversation

RincewindsHat commented Nov 4, 2022

Uh oh!

andreasbaumann commented Nov 5, 2022

Uh oh!

RincewindsHat commented Nov 5, 2022

Uh oh!

Al2Klimov commented Feb 8, 2023

Alternative

Uh oh!

RincewindsHat commented Feb 8, 2023

Uh oh!

Al2Klimov commented Feb 8, 2023

Uh oh!

RincewindsHat commented Feb 10, 2023

Uh oh!

Al2Klimov commented Feb 10, 2023

Uh oh!

RincewindsHat commented Feb 10, 2023

Uh oh!

Al2Klimov commented Feb 10, 2023

Uh oh!

waja commented Feb 10, 2023

Uh oh!

andreasbaumann commented Feb 12, 2023

Uh oh!

bittorf commented Jan 13, 2025

Uh oh!

andreasbaumann commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bittorf commented Jan 14, 2025

Uh oh!

andreasbaumann commented Jan 14, 2025

Uh oh!

bittorf commented Jan 15, 2025

Uh oh!

RincewindsHat commented Jan 15, 2025

Uh oh!

waja commented Jan 15, 2025

Uh oh!

RincewindsHat commented Jan 15, 2025

Uh oh!

waja commented Apr 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RincewindsHat commented Apr 20, 2025

Uh oh!

waja commented Apr 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

andreasbaumann commented Jan 13, 2025 •

edited

Loading

waja commented Apr 19, 2025 •

edited

Loading