Module: check_mk
Branch: master
Commit: 0c171e0e780f5a9ab15711f2d1531d2dc7fecb0e
URL:
http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=0c171e0e780f5a…
Author: Moritz Kiemer <mo(a)mathias-kettner.de>
Date: Mon Apr 1 09:38:08 2019 +0200
7385 FIX check_http: Fix port mixup in URL-mode
The check_http check in URL mode mixed up the servers port and the proxy port if a proxy
was used.
If users were using a proxy to check a URL, the configured port was used as the proxys
port, while
it was not possible to configure a port for the server. Users who have been relying on
this wrong
behaviour need to edit their rules.
The rule has been reworked to clearly show which settings apply the server, and which to
the proxy.
Change-Id: I2eef53f196032f8b718b3a7aad4c49832ba27191
---
.werks/7385 | 14 +++++++++++
checks/check_http | 46 +++++++++++++++---------------------
tests/unit/checks/test_check_http.py | 35 +++++++++++++++++++--------
3 files changed, 58 insertions(+), 37 deletions(-)
diff --git a/.werks/7385 b/.werks/7385
new file mode 100644
index 0000000..f110d77
--- /dev/null
+++ b/.werks/7385
@@ -0,0 +1,14 @@
+Title: check_http: Fix port mixup in URL-mode
+Level: 1
+Component: checks
+Compatible: incomp
+Edition: cre
+Version: 1.6.0i1
+Date: 1554103908
+Class: fix
+
+The check_http check in URL mode mixed up the servers port and the proxy port if a proxy
was used.
+If users were using a proxy to check a URL, the configured port was used as the proxys
port, while
+it was not possible to configure a port for the server. Users who have been relying on
this wrong
+behaviour need to edit their rules.
+The rule has been reworked to clearly show which settings apply the server, and which to
the proxy.
diff --git a/checks/check_http b/checks/check_http
index 5e63dd1..7f9b75c 100644
--- a/checks/check_http
+++ b/checks/check_http
@@ -105,41 +105,19 @@ def _certificate_args(host, proxy, settings, sni_flag):
warn, crit = settings["cert_days"]
args += ['-C', '%d,%d' % (warn, crit)]
- specify_port = proxy.port if proxy else host.port
- if specify_port:
- args += ['-p', specify_port]
-
if proxy:
args += ['--ssl', '-j', 'CONNECT']
- args.append(proxy.address)
-
elif sni_flag:
args += ['-H', host.address]
- address = host.address
- if proxy and host.port:
- address += ':%s' % host.port
- args += [address]
-
return args
def _url_args(host, proxy, settings):
args = []
- if proxy:
- args += ["-I", proxy.address]
- args += ["-H", host.address]
- else:
- args += ["-I", host.address]
- # Proxy *and* virthost is not allowed
- if "virthost" in settings:
- args += ["-H", settings["virthost"]]
-
- # TODO: I think this should be overridden by the proxy port
- # in the same way as in the cert check. (mo)
- if host.port:
- args += ['-p', host.port]
+ if "virthost" in settings:
+ args += ["-H", settings["virthost"]]
if "uri" in settings:
args += ['-u', settings["uri"]]
@@ -230,6 +208,18 @@ def _common_args(host, proxy, sni_flag):
if proxy and proxy.auth:
args += ["-b", proxy.auth]
+ specify_port = proxy.port if proxy else host.port
+ if specify_port:
+ args += ['-p', specify_port]
+
+ # last two arguments correspond to -I/-H, respectively
+ if proxy:
+ args += [proxy.address]
+ address = host.address
+ if proxy and host.port:
+ address += ':%s' % host.port
+ args += [address]
+
return args
@@ -242,10 +232,12 @@ def check_http_arguments(params):
proxy = _get_proxy(params)
sni_flag = bool(params.get("sni"))
- args = _common_args(host, proxy, sni_flag)
if mode_name == 'cert':
- return args + _certificate_args(host, proxy, settings, sni_flag)
- return args + _url_args(host, proxy, settings)
+ args = _certificate_args(host, proxy, settings, sni_flag)
+ else:
+ args = _url_args(host, proxy, settings)
+
+ return args + _common_args(host, proxy, sni_flag)
def check_http_description(params):
diff --git a/tests/unit/checks/test_check_http.py b/tests/unit/checks/test_check_http.py
index 037ee8a..5426dc3 100644
--- a/tests/unit/checks/test_check_http.py
+++ b/tests/unit/checks/test_check_http.py
@@ -13,8 +13,15 @@ pytestmark = pytest.mark.checks
'virthost': ('www.test123.de', True)
}),
[
- '-I', 'www.test123.de', '-H',
'www.test123.de', '-p', 80, '-u', '/images',
- '--onredirect=follow', '-L'
+ '-H',
+ 'www.test123.de',
+ '-u',
+ '/images',
+ '--onredirect=follow',
+ '-L',
+ '-p',
+ 80,
+ 'www.test123.de',
],
),
(
@@ -28,8 +35,16 @@ pytestmark = pytest.mark.checks
'virthost': ('www.test123.de', True)
}),
[
- '-I', '163.172.86.64', '-H',
'www.test123.de', '-p', 3128, '-u', '/images',
'--ssl',
- '--extended-perfdata', '-j', 'CONNECT'
+ '-H',
+ 'www.test123.de',
+ '-u',
+ '/images',
+ '--ssl',
+ '--extended-perfdata',
+ '-j',
+ 'CONNECT',
+ '163.172.86.64',
+ 'www.test123.de:3128',
],
),
(
@@ -56,7 +71,7 @@ pytestmark = pytest.mark.checks
'port': '42',
'proxy': 'p.roxy:23',
}),
- ['-C', '10,20', '-p', '23', '--ssl',
'-j', 'CONNECT', 'p.roxy', 'www.test123.com:42'],
+ ['-C', '10,20', '--ssl', '-j', 'CONNECT',
'-p', '23', 'p.roxy', 'www.test123.com:42'],
),
(
(None, {
@@ -66,7 +81,7 @@ pytestmark = pytest.mark.checks
'proxy': '[dead:beef::face]:23',
}),
[
- '-C', '10,20', '-p', '23', '--ssl',
'-j', 'CONNECT', '[dead:beef::face]',
+ '-C', '10,20', '--ssl', '-j',
'CONNECT', '-p', '23', '[dead:beef::face]',
'www.test123.com:42'
],
),
@@ -75,26 +90,26 @@ pytestmark = pytest.mark.checks
'virthost': ("virtual.host", True),
'proxy': "foo.bar",
}),
- ['-I', 'foo.bar', '-H', 'virtual.host'],
+ ['-H', 'virtual.host', 'foo.bar',
'virtual.host'],
),
(
(None, {
'virthost': ("virtual.host", False),
'proxy': "foo.bar",
}),
- ['-I', 'foo.bar', '-H', 'virtual.host'],
+ ['-H', 'virtual.host', 'foo.bar',
'virtual.host'],
),
(
(None, {
'virthost': ("virtual.host", True),
}),
- ['-I', 'virtual.host', '-H', 'virtual.host'],
+ ['-H', 'virtual.host', 'virtual.host'],
),
(
(None, {
'virthost': ("virtual.host", False),
}),
- ['-I', '$_HOSTADDRESS_4$', '-H',
'virtual.host'],
+ ['-H', 'virtual.host', '$_HOSTADDRESS_4$'],
),
])
def test_check_http_argument_parsing(check_manager, params, expected_args):