diff --git a/swh/icinga_plugins/tests/test_vault.py b/swh/icinga_plugins/tests/test_vault.py --- a/swh/icinga_plugins/tests/test_vault.py +++ b/swh/icinga_plugins/tests/test_vault.py @@ -3,6 +3,8 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information +import io +import tarfile import time from swh.icinga_plugins.tests.utils import invoke @@ -14,6 +16,20 @@ url_api = f"mock://swh-web.example.org/api/1/vault/directory/{DIR_ID}/" url_fetch = f"mock://swh-web.example.org/api/1/vault/directory/{DIR_ID}/raw/" + +def _make_tarfile(): + fd = io.BytesIO() + with tarfile.open(fileobj=fd, mode="w:gz") as tf: + tf.addfile(tarfile.TarInfo(f"swh:1:dir:{DIR_ID}/README"), b"this is a readme\n") + + tarinfo = tarfile.TarInfo(f"swh:1:dir:{DIR_ID}") + tarinfo.type = tarfile.DIRTYPE + tf.addfile(tarinfo) + return fd.getvalue() + + +TARBALL = _make_tarfile() + response_pending = { "obj_id": DIR_ID, "obj_type": "directory", @@ -66,7 +82,7 @@ scenario.add_step("post", url_api, response_pending) scenario.add_step("get", url_api, response_done) scenario.add_step( - "get", url_fetch, "xx" * 40, headers={"Content-Type": "application/gzip"} + "get", url_fetch, TARBALL, headers={"Content-Type": "application/gzip"} ) scenario.install_mock(requests_mock) @@ -101,7 +117,7 @@ scenario.add_step("get", url_api, response_pending) scenario.add_step("get", url_api, response_done) scenario.add_step( - "get", url_fetch, "xx" * 40, headers={"Content-Type": "application/gzip"} + "get", url_fetch, TARBALL, headers={"Content-Type": "application/gzip"} ) scenario.install_mock(requests_mock) @@ -237,7 +253,7 @@ scenario.add_step("post", url_api, response_pending) scenario.add_step("get", url_api, response_done) scenario.add_step( - "get", url_fetch, "xx" * 40, headers={"Content-Type": "application/gzip"} + "get", url_fetch, TARBALL, headers={"Content-Type": "application/gzip"} ) scenario.install_mock(requests_mock) @@ -328,14 +344,50 @@ assert result.exit_code == 2, result.output -def test_vault_fetch_empty(requests_mock, mocker, mocked_time): +def test_vault_fetch_missing_content_type(requests_mock, mocker, mocked_time): + scenario = WebScenario() + + scenario.add_step("get", url_api, {}, status_code=404) + scenario.add_step("post", url_api, response_pending) + scenario.add_step("get", url_api, response_done) + scenario.add_step("get", url_fetch, "") + + scenario.install_mock(requests_mock) + + get_storage_mock = mocker.patch("swh.icinga_plugins.vault.get_storage") + get_storage_mock.side_effect = FakeStorage + + result = invoke( + [ + "check-vault", + "--swh-web-url", + "mock://swh-web.example.org", + "--swh-storage-url", + "foo://example.org", + "directory", + ], + catch_exceptions=True, + ) + + assert result.output == ( + "VAULT CRITICAL - Unexpected Content-Type when downloading bundle: None\n" + "| 'total_time' = 10.00s\n" + ) + assert result.exit_code == 2, result.output + + +def test_vault_corrupt_tarball_gzip(requests_mock, mocker, mocked_time): scenario = WebScenario() scenario.add_step("get", url_api, {}, status_code=404) scenario.add_step("post", url_api, response_pending) + scenario.add_step("get", url_api, response_pending) scenario.add_step("get", url_api, response_done) scenario.add_step( - "get", url_fetch, "", headers={"Content-Type": "application/gzip"} + "get", + url_fetch, + b"this-is-not-a-tarball", + headers={"Content-Type": "application/gzip", "Content-Length": "100000"}, ) scenario.install_mock(requests_mock) @@ -356,20 +408,30 @@ ) assert result.output == ( - f"VAULT CRITICAL - cooking directory {DIR_ID} took " - f"10.00s and succeeded, but fetch was empty.\n" - f"| 'total_time' = 10.00s\n" + "VAULT CRITICAL - Error while reading tarball: not a gzip file\n" + "| 'total_time' = 20.00s\n" ) assert result.exit_code == 2, result.output -def test_vault_fetch_missing_content_type(requests_mock, mocker, mocked_time): +def test_vault_corrupt_tarball_member(requests_mock, mocker, mocked_time): + fd = io.BytesIO() + with tarfile.open(fileobj=fd, mode="w:gz") as tf: + tf.addfile(tarfile.TarInfo("wrong_dir_name/README"), b"this is a readme\n") + tarball = fd.getvalue() + scenario = WebScenario() scenario.add_step("get", url_api, {}, status_code=404) scenario.add_step("post", url_api, response_pending) + scenario.add_step("get", url_api, response_pending) scenario.add_step("get", url_api, response_done) - scenario.add_step("get", url_fetch, "") + scenario.add_step( + "get", + url_fetch, + tarball, + headers={"Content-Type": "application/gzip", "Content-Length": "100000"}, + ) scenario.install_mock(requests_mock) @@ -389,8 +451,8 @@ ) assert result.output == ( - "VAULT CRITICAL - Unexpected Content-Type when downloading bundle: None\n" - "| 'total_time' = 10.00s\n" + "VAULT CRITICAL - Unexpected member in tarball: wrong_dir_name/README\n" + "| 'total_time' = 20.00s\n" ) assert result.exit_code == 2, result.output diff --git a/swh/icinga_plugins/vault.py b/swh/icinga_plugins/vault.py --- a/swh/icinga_plugins/vault.py +++ b/swh/icinga_plugins/vault.py @@ -3,6 +3,7 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information +import tarfile import time import requests @@ -123,18 +124,30 @@ ) return 2 - response_length = 0 - for chunk in fetch_response.iter_content(decode_unicode=False): - response_length += len(chunk) - - if response_length == 0: - self.print_result( - "CRITICAL", - f"cooking directory {dir_id.hex()} took {total_time:.2f}s " - f"and succeeded, but fetch was empty.", - total_time=total_time, - ) - return 2 + try: + with tarfile.open(fileobj=fetch_response.raw, mode="r:gz") as tf: + # Note that we are streaming the tarfile from the network, + # so we are allowed at most one pass on the tf object; + # and the sooner we close it the better. + # Fortunately, checking only the first member is good enough: + tarinfo = tf.next() + swhid = f"swh:1:dir:{dir_id.hex()}" + if tarinfo.name != swhid and not tarinfo.name.startswith( + f"{swhid}/" + ): + self.print_result( + "CRITICAL", + f"Unexpected member in tarball: {tarinfo.name}", + total_time=total_time, + ) + return 2 + except tarfile.ReadError as e: + self.print_result( + "CRITICAL", + f"Error while reading tarball: {e}", + total_time=total_time, + ) + return 2 self.print_result( status,