From b274cad65b36d72db7afab582cc0811b2c16ea89 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Sun, 27 Oct 2024 18:17:57 +0100 Subject: [PATCH] Add tests for Sable's postgresql chathistory backend --- Makefile | 29 ++++--- irctest/basecontrollers.py | 3 + irctest/cases.py | 10 ++- irctest/controllers/sable.py | 129 +++++++++++++++++++++++++++- irctest/server_tests/chathistory.py | 120 +++++++++++++++++++++++++- irctest/specifications.py | 4 + pytest.ini | 5 ++ 7 files changed, 277 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index dc24586..cde2edc 100644 --- a/Makefile +++ b/Makefile @@ -8,71 +8,72 @@ PYTEST_ARGS ?= EXTRA_SELECTORS ?= BAHAMUT_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ and not IRCv3 \ $(EXTRA_SELECTORS) CHARYBDIS_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) ERGO_SELECTORS := \ + (Ergo or not implementation-specific) \ not deprecated \ $(EXTRA_SELECTORS) HYBRID_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ $(EXTRA_SELECTORS) INSPIRCD_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) IRCU2_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) NEFARIOUS_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) SNIRCD_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) IRC2_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) MAMMON_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) NGIRCD_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) PLEXUS4_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ $(EXTRA_SELECTORS) @@ -87,7 +88,7 @@ LIMNORIA_SELECTORS := \ # Tests marked with private_chathistory can't pass because Sable does not implement CHATHISTORY for DMs SABLE_SELECTORS := \ - not Ergo \ + (Sable or not implementation-specific) \ and not deprecated \ and not strict \ and not arbitrary_client_tags \ @@ -97,7 +98,7 @@ SABLE_SELECTORS := \ $(EXTRA_SELECTORS) SOLANUM_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ $(EXTRA_SELECTORS) @@ -119,7 +120,7 @@ THELOUNGE_SELECTORS := \ # Tests marked with private_chathistory can't pass because Unreal does not implement CHATHISTORY for DMs UNREALIRCD_SELECTORS := \ - not Ergo \ + not implementation-specific \ and not deprecated \ and not strict \ and not arbitrary_client_tags \ diff --git a/irctest/basecontrollers.py b/irctest/basecontrollers.py index 156d5c3..11126e7 100644 --- a/irctest/basecontrollers.py +++ b/irctest/basecontrollers.py @@ -67,6 +67,9 @@ class TestCaseControllerConfig: This should be used as little as possible, using the other attributes instead; as they are work with any controller.""" + sable_history_server: bool = False + """Whether to start Sable's long-term history server""" + class _BaseController: """Base class for software controllers. diff --git a/irctest/cases.py b/irctest/cases.py index 4d14b3a..7ab0e69 100644 --- a/irctest/cases.py +++ b/irctest/cases.py @@ -842,16 +842,22 @@ def mark_services(cls: TClass) -> TClass: def mark_specifications( *specifications_str: str, deprecated: bool = False, strict: bool = False ) -> Callable[[TCallable], TCallable]: - specifications = frozenset( + specifications = { Specifications.from_name(s) if isinstance(s, str) else s for s in specifications_str - ) + } if None in specifications: raise ValueError("Invalid set of specifications: {}".format(specifications)) + is_implementation_specific = all( + spec.is_implementation_specific() for spec in specifications + ) + def decorator(f: TCallable) -> TCallable: for specification in specifications: f = getattr(pytest.mark, specification.value)(f) + if is_implementation_specific: + f = getattr(pytest.mark, "implementation-specific")(f) if strict: f = pytest.mark.strict(f) if deprecated: diff --git a/irctest/controllers/sable.py b/irctest/controllers/sable.py index 200ea6f..7155dcf 100644 --- a/irctest/controllers/sable.py +++ b/irctest/controllers/sable.py @@ -5,7 +5,7 @@ import signal import subprocess import tempfile import time -from typing import Optional, Type +from typing import Optional, Sequence, Type from irctest.basecontrollers import ( BaseServerController, @@ -85,7 +85,7 @@ def certs_dir() -> Path: certs_dir = tempfile.TemporaryDirectory() (Path(certs_dir.name) / "gen_certs.sh").write_text(GEN_CERTS) subprocess.run( - ["bash", "gen_certs.sh", "My.Little.Server", "My.Little.Services"], + ["bash", "gen_certs.sh", "My.Little.Server", "My.Little.History", "My.Little.Services"], cwd=certs_dir.name, check=True, ) @@ -99,6 +99,7 @@ NETWORK_CONFIG = """ "ca_file": "%(certs_dir)s/ca_cert.pem", "peers": [ + { "name": "My.Little.History", "address": "%(history_hostname)s:%(history_port)s", "fingerprint": "%(history_cert_sha1)s" }, { "name": "My.Little.Services", "address": "%(services_hostname)s:%(services_port)s", "fingerprint": "%(services_cert_sha1)s" }, { "name": "My.Little.Server", "address": "%(server1_hostname)s:%(server1_port)s", "fingerprint": "%(server1_cert_sha1)s" } ] @@ -107,7 +108,7 @@ NETWORK_CONFIG = """ NETWORK_CONFIG_CONFIG = """ { - "object_expiry": 300, + "object_expiry": 60, // 1 minute "opers": [ { @@ -219,6 +220,58 @@ SERVER_CONFIG = """ } """ +HISTORY_SERVER_CONFIG = """ +{ + "server_id": 50, + "server_name": "My.Little.History", + + "management": { + "address": "%(history_management_hostname)s:%(history_management_port)s", + "client_ca": "%(certs_dir)s/ca_cert.pem", + "authorised_fingerprints": [ + { "name": "user1", "fingerprint": "435bc6db9f22e84ba5d9652432154617c9509370" } + ] + }, + + "server": { + "database": "%(history_db_url)s", + "auto_run_migrations": true, + }, + + "event_log": { + "event_expiry": 300, // five minutes, for local testing + }, + + "tls_config": { + "key_file": "%(certs_dir)s/My.Little.History.key", + "cert_file": "%(certs_dir)s/My.Little.History.pem" + }, + + "node_config": { + "listen_addr": "%(history_hostname)s:%(history_port)s", + "cert_file": "%(certs_dir)s/My.Little.History.pem", + "key_file": "%(certs_dir)s/My.Little.History.key" + }, + + "log": { + "dir": "log/services/", + + "module-levels": { + "": "debug", + "sable_history": "trace", + }, + + "targets": [ + { + "target": "stdout", + "level": "trace", + "modules": [ "sable_history" ] + } + ] + } +} +""" + SERVICES_CONFIG = """ { "server_id": 99, @@ -348,10 +401,11 @@ class SableController(BaseServerController, DirectoryBasedController): (server1_hostname, server1_port) = self.get_hostname_and_port() (services_hostname, services_port) = self.get_hostname_and_port() + (history_hostname, history_port) = self.get_hostname_and_port() # Sable requires inbound connections to match the configured hostname, # so we can't configure 0.0.0.0 - server1_hostname = services_hostname = "127.0.0.1" + server1_hostname = history_hostname = services_hostname = "127.0.0.1" ( server1_management_hostname, @@ -361,6 +415,10 @@ class SableController(BaseServerController, DirectoryBasedController): services_management_hostname, services_management_port, ) = self.get_hostname_and_port() + ( + history_management_hostname, + history_management_port, + ) = self.get_hostname_and_port() self.template_vars = dict( certs_dir=certs_dir(), @@ -381,6 +439,13 @@ class SableController(BaseServerController, DirectoryBasedController): services_management_hostname=services_management_hostname, services_management_port=services_management_port, services_alias_users=SERVICES_ALIAS_USERS if run_services else "", + history_hostname=history_hostname, + history_port=history_port, + history_cert_sha1=(certs_dir() / "My.Little.History.pem.sha1") + .read_text() + .strip(), + history_management_hostname=history_management_hostname, + history_management_port=history_management_port, ) with self.open_file("configs/network.conf") as fd: @@ -416,12 +481,22 @@ class SableController(BaseServerController, DirectoryBasedController): if run_services: self.services_controller = SableServicesController(self.test_config, self) + self.services_controller.faketime_cmd = faketime_cmd self.services_controller.run( protocol="sable", server_hostname=services_hostname, server_port=services_port, ) + if self.test_config.sable_history_server: + self.history_controller = SableHistoryController(self.test_config, self) + self.history_controller.faketime_cmd = faketime_cmd + self.history_controller.run( + protocol="sable", + server_hostname=history_hostname, + server_port=history_port, + ) + def kill_proc(self) -> None: os.killpg(self.pgroup_id, signal.SIGKILL) super().kill_proc() @@ -470,6 +545,8 @@ class SableServicesController(BaseServicesController): server_controller: SableController software_name = "Sable Services" + faketime_cmd: Sequence[str] + def run(self, protocol: str, server_hostname: str, server_port: int) -> None: assert protocol == "sable" assert self.server_controller.directory is not None @@ -479,6 +556,7 @@ class SableServicesController(BaseServicesController): self.proc = self.execute( [ + *self.faketime_cmd, "sable_services", "--foreground", "--server-conf", @@ -497,5 +575,48 @@ class SableServicesController(BaseServicesController): super().kill_proc() +class SableHistoryController(BaseServicesController): + server_controller: SableController + software_name = "Sable History Server" + faketime_cmd: Sequence[str] + + def run(self, protocol: str, server_hostname: str, server_port: int) -> None: + assert protocol == "sable" + assert self.server_controller.directory is not None + history_db_url=os.environ.get("PIFPAF_POSTGRESQL_URL") or os.environ.get("IRCTEST_POSTGRESQL_URL") + assert history_db_url, ( + "Cannot find a postgresql database to use as backend for sable_history. " + "Either set the IRCTEST_POSTGRESQL_URL env var to a libpq URL, or " + "run `pip3 install pifpaf` and wrap irctest in a pifpaf call (ie. " + "pifpaf run postgresql -- pytest --controller=irctest.controllers.sable ...)" + ) + + with self.server_controller.open_file("configs/history_server.conf") as fd: + fd.write(HISTORY_SERVER_CONFIG % { + **self.server_controller.template_vars, + "history_db_url": history_db_url, + }) + + self.proc = self.execute( + [ + *self.faketime_cmd, + "sable_history", + "--foreground", + "--server-conf", + self.server_controller.directory / "configs/history_server.conf", + "--network-conf", + self.server_controller.directory / "configs/network.conf", + ], + cwd=self.server_controller.directory, + preexec_fn=os.setsid, + env={"RUST_BACKTRACE": "1", **os.environ}, + ) + self.pgroup_id = os.getpgid(self.proc.pid) + + def kill_proc(self) -> None: + os.killpg(self.pgroup_id, signal.SIGKILL) + super().kill_proc() + + def get_irctest_controller_class() -> Type[SableController]: return SableController diff --git a/irctest/server_tests/chathistory.py b/irctest/server_tests/chathistory.py index 71521b6..e8b9294 100644 --- a/irctest/server_tests/chathistory.py +++ b/irctest/server_tests/chathistory.py @@ -2,6 +2,7 @@ `IRCv3 draft chathistory `_ """ +import dataclasses import functools import secrets import time @@ -31,9 +32,18 @@ def skip_ngircd(f): return newf -@cases.mark_specifications("IRCv3") -@cases.mark_services -class ChathistoryTestCase(cases.BaseServerTestCase): +class _BaseChathistoryTests(cases.BaseServerTestCase): + def _wait_before_chathistory(self): + """Hook for the Sable-specific tests that check the postgresql-based + CHATHISTORY implementation is sound. This implementation only kicks in + after the in-memory history is cleared, which happens after a 5 min timeout; + and this gives a chance to :class:``SablePostgresqlHistoryTestCase`` to + wait this timeout. + + For other tests, this does nothing. + """ + raise NotImplementedError("_BaseChathistoryTests._wait_before_chathistory") + def validate_chathistory_batch(self, msgs, target): (start, *inner_msgs, end) = msgs @@ -94,6 +104,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase): self.joinChannel(qux, real_chname) self.getMessages(qux) + self._wait_before_chathistory() + # test a nonexistent channel self.sendLine(bar, "CHATHISTORY LATEST #nonexistent_channel * 10") msgs = self.getMessages(bar) @@ -175,6 +187,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase): messages.append(echo.to_history_message()) self.assertEqual(echo.to_history_message(), delivery.to_history_message()) + self._wait_before_chathistory() + self.sendLine(bar, "CHATHISTORY LATEST %s * 10" % (bar,)) replies = [msg for msg in self.getMessages(bar) if msg.command == "PRIVMSG"] self.assertEqual([msg.to_history_message() for msg in replies], messages) @@ -228,6 +242,9 @@ class ChathistoryTestCase(cases.BaseServerTestCase): time.sleep(0.002) self.validate_echo_messages(NUM_MESSAGES, echo_messages) + + self._wait_before_chathistory() + self.validate_chathistory(subcommand, echo_messages, 1, chname) @skip_ngircd @@ -264,6 +281,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase): ) time.sleep(0.002) + self._wait_before_chathistory() + self.validate_echo_messages(NUM_MESSAGES, echo_messages) self.sendLine(1, "CHATHISTORY LATEST %s * 100" % chname) (batch_open, *messages, batch_close) = self.getMessages(1) @@ -308,6 +327,9 @@ class ChathistoryTestCase(cases.BaseServerTestCase): time.sleep(0.002) self.validate_echo_messages(NUM_MESSAGES * 2, echo_messages) + + self._wait_before_chathistory() + self.validate_chathistory(subcommand, echo_messages, 1, chname) @pytest.mark.parametrize("subcommand", SUBCOMMANDS) @@ -367,6 +389,9 @@ class ChathistoryTestCase(cases.BaseServerTestCase): self.getMessages(2) self.validate_echo_messages(NUM_MESSAGES, echo_messages) + + self._wait_before_chathistory() + self.validate_chathistory(subcommand, echo_messages, 1, c2) self.validate_chathistory(subcommand, echo_messages, 2, c1) @@ -415,6 +440,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase): ] self.assertEqual(results, new_convo) + self._wait_before_chathistory() + # additional messages with c3 should not show up in the c1-c2 history: self.validate_chathistory(subcommand, echo_messages, 1, c2) self.validate_chathistory(subcommand, echo_messages, 2, c1) @@ -718,6 +745,8 @@ class ChathistoryTestCase(cases.BaseServerTestCase): self.assertEqual(len(relay), 1) validate_tagmsg(relay[0], chname, msgid) + self._wait_before_chathistory() + self.sendLine(1, "CHATHISTORY LATEST %s * 10" % (chname,)) history_tagmsgs = [ msg for msg in self.getMessages(1) if msg.command == "TAGMSG" @@ -814,8 +843,93 @@ class ChathistoryTestCase(cases.BaseServerTestCase): validate_msg(relay) +@cases.mark_specifications("IRCv3") +@cases.mark_services +class ChathistoryTestCase(_BaseChathistoryTests): + def _wait_before_chathistory(self): + """does nothing""" + pass + + assert {f"_validate_chathistory_{cmd}" for cmd in SUBCOMMANDS} == { meth_name for meth_name in dir(ChathistoryTestCase) if meth_name.startswith("_validate_chathistory_") }, "ChathistoryTestCase.validate_chathistory and SUBCOMMANDS are out of sync" + + +@cases.mark_specifications("Sable") +@cases.mark_services +class SablePostgresqlHistoryTestCase(_BaseChathistoryTests): + faketime = "+1y x15" # for every wall clock second, 15 seconds pass for the server + + @staticmethod + def config() -> cases.TestCaseControllerConfig: + return dataclasses.replace( # type: ignore[no-any-return] + _BaseChathistoryTests.config(), + sable_history_server=True, + ) + + def _wait_before_chathistory(self): + """waits 6 seconds which appears to be a 1.5 min to Sable; which goes over + the 1 min timeout for in-memory history""" + assert self.controller.faketime_enabled, "faketime is not installed" + time.sleep(7) + + +@cases.mark_specifications("Sable") +@cases.mark_services +class SableExpiringHistoryTestCase(cases.BaseServerTestCase): + # for every wall clock second, 10 seconds pass for the server. + # At x15, sable_history does not always have time to persist messages (wtf?) + # at x30, links between nodes timeout. + faketime = "+1y x10" + + def _wait_before_chathistory(self): + """waits 6 seconds which appears to be a 1.5 min to Sable; which goes over + the 1 min timeout for in-memory history""" + assert self.controller.faketime_enabled, "faketime is not installed" + time.sleep(15) + + def testChathistoryExpired(self): + """Checks that Sable forgets about messages if the history server is not available""" + self.connectClient( + "bar", + capabilities=[ + "message-tags", + "server-time", + "echo-message", + "batch", + "labeled-response", + "sasl", + CHATHISTORY_CAP, + ], + skip_if_cap_nak=True, + ) + chname = "#chan" + secrets.token_hex(12) + self.joinChannel(1, chname) + self.getMessages(1) + self.getMessages(1) + + self.sendLine(1, f"PRIVMSG {chname} :this is a message") + self.getMessages(1) + + self._wait_before_chathistory() + + self.sendLine(1, f"CHATHISTORY LATEST {chname} * 10") + + # Sable processes CHATHISTORY asynchronously, which can be pretty slow as it + # sends cross-server requests. This means we can't just rely on a PING-PONG + # or the usual time.sleep(self.controller.sync_sleep_time) to make sure + # the ircd replied to us + time.sleep(self.controller.sync_sleep_time * 10) + + (start, *middle, end) = self.getMessages(1) + self.assertMessageMatch( + start, command="BATCH", params=[StrRe(r"\+.*"), "chathistory", chname] + ) + batch_tag = start.params[0][1:] + self.assertMessageMatch(end, command="BATCH", params=["-" + batch_tag]) + self.assertEqual( + len(middle), 0, f"Got messages that should be expired: {middle}" + ) diff --git a/irctest/specifications.py b/irctest/specifications.py index 9c4617b..41f82b4 100644 --- a/irctest/specifications.py +++ b/irctest/specifications.py @@ -9,6 +9,7 @@ class Specifications(enum.Enum): RFC2812 = "RFC2812" IRCv3 = "IRCv3" # Mark with capabilities whenever possible Ergo = "Ergo" + Sable = "Sable" Ircdocs = "ircdocs" """Any document on ircdocs.horse (especially defs.ircdocs.horse), @@ -24,6 +25,9 @@ class Specifications(enum.Enum): return spec raise ValueError(name) + def is_implementation_specific(self) -> bool: + return self in (Specifications.Ergo, Specifications.Sable) + @enum.unique class Capabilities(enum.Enum): diff --git a/pytest.ini b/pytest.ini index 375f2bb..1c13017 100644 --- a/pytest.ini +++ b/pytest.ini @@ -7,7 +7,12 @@ markers = IRCv3 modern ircdocs + + # implementations for which we have specific test get two markers: + # the implementation name and 'implementation-specific' + implementation-specific Ergo + Sable # misc marks strict