16144 FIX Ignore piggybacked host names starting with a period

SUP-15642

Change-Id: I8544be67055bad6ab9631c4d7411f43587c06615

.werks/16144

@@ -0,0 +1,15 @@
+Title: Ignore piggybacked host names starting with a period
+Class: fix
+Compatible: incomp
+Component: core
+Date: 1699602114
+Edition: cre
+Level: 1
+Version: 2.3.0b1
+
+We now skip piggybacked data for hosts names starting
+with a period.  Examples of such invalid names are ".",
+".hostname", and ".hostname.domain.com".
+
+Users must rename such hosts if they should remain
+in the monitoring.

cmk/checkengine/parser/_agent.py

@@ -7,6 +7,7 @@
 
 import abc
 import logging
+import re
 import time
 from collections.abc import Iterator, Mapping, MutableMapping, Sequence
 from typing import final, Final, NamedTuple
@@ -16,6 +17,7 @@
 import cmk.utils.misc
 from cmk.utils.agentdatatype import AgentRawData
 from cmk.utils.hostaddress import HostName
+from cmk.utils.regex import REGEX_HOST_NAME_CHARS
 from cmk.utils.sectionname import MutableSectionMap, SectionName
 from cmk.utils.translations import TranslationOptions
 
@@ -31,6 +33,11 @@
     SectionNameCollection,
 )
 
+# cmk.utils.regex.REGEX_HOST_NAME is too relaxed and accepts, for example, "."
+# as a valid host name.  Fixing it over there, however, could break its
+# current users.
+is_valid_hostname = re.compile(rf"^\w[{REGEX_HOST_NAME_CHARS}]*$").match
+
 
 class SectionWithHeader(NamedTuple):
     header: SectionMarker
@@ -74,6 +81,12 @@ class ParserState(abc.ABC):
         Host --> Host : ""<<<~<>>>>""
         PiggybackedHost --> PiggybackedHost : ""<<<~<HOSTNAME>>>>""
 
+        Host --> IgnoredPiggybackedHost : ""<<<~<.>>>>""
+        IgnoredPiggybackedHost --> Host : ""<<<~<>>>>""
+
+        PiggybackedHost --> IgnoredPiggybackedHost : ""<<<~<.>>>>""
+        IgnoredPiggybackedHost --> PiggybackedHost : ""<<<~<HOSTNAME>>>>""
+
     See Also:
         Gamma, Helm, Johnson, Vlissides (1995) Design Patterns "State pattern"
 
@@ -212,6 +225,16 @@ def to_piggyback_noop_parser(
             logger=self._logger,
         )
 
+    def to_piggyback_ignore_parser(self) -> PiggybackIgnoreParser:
+        return PiggybackIgnoreParser(
+            self.hostname,
+            self.sections,
+            self.piggyback_sections,
+            translation=self.translation,
+            encoding_fallback=self.encoding_fallback,
+            logger=self._logger,
+        )
+
     def to_error(self, line: bytes) -> ParserState:
         self._logger.warning(
             "%s: Ignoring invalid data %r",
@@ -256,6 +279,8 @@ def on_piggyback_header(self, line: bytes) -> ParserState:
         if piggyback_header.hostname == self.hostname:
             # Unpiggybacked "normal" host
             return self
+        if not is_valid_hostname(piggyback_header.hostname):
+            return self.to_piggyback_ignore_parser()
         return self.to_piggyback_parser(piggyback_header)
 
     def on_piggyback_footer(self, line: bytes) -> ParserState:
@@ -304,6 +329,8 @@ def on_piggyback_header(self, line: bytes) -> ParserState:
         if piggyback_header.hostname == self.hostname:
             # Unpiggybacked "normal" host
             return self.to_noop_parser()
+        if not is_valid_hostname(piggyback_header.hostname):
+            return self.to_piggyback_ignore_parser()
         return self.to_piggyback_parser(piggyback_header)
 
     def on_piggyback_footer(self, line: bytes) -> ParserState:
@@ -355,6 +382,8 @@ def on_piggyback_header(self, line: bytes) -> ParserState:
             self.translation,
             encoding_fallback=self.encoding_fallback,
         )
+        if not is_valid_hostname(piggyback_header.hostname):
+            return self.to_piggyback_ignore_parser()
         return self.to_piggyback_parser(piggyback_header)
 
     def on_piggyback_footer(self, line: bytes) -> ParserState:
@@ -405,6 +434,8 @@ def on_piggyback_header(self, line: bytes) -> ParserState:
         if piggyback_header.hostname == self.hostname:
             # Unpiggybacked "normal" host
             return self.to_noop_parser()
+        if not is_valid_hostname(piggyback_header.hostname):
+            return self.to_piggyback_ignore_parser()
         return self.to_piggyback_parser(piggyback_header)
 
     def on_piggyback_footer(self, line: bytes) -> ParserState:
@@ -421,6 +452,33 @@ def on_section_footer(self, line: bytes) -> ParserState:
         return self.to_piggyback_noop_parser(self.current_host)
 
 
+class PiggybackIgnoreParser(ParserState):
+    def do_action(self, line: bytes) -> PiggybackIgnoreParser:
+        return self
+
+    def on_piggyback_header(self, line: bytes) -> ParserState:
+        piggyback_header = PiggybackMarker.from_headerline(
+            line,
+            self.translation,
+            encoding_fallback=self.encoding_fallback,
+        )
+        if piggyback_header.hostname == self.hostname:
+            # Unpiggybacked "normal" host
+            return self.to_noop_parser()
+        if not is_valid_hostname(piggyback_header.hostname):
+            return self.to_piggyback_ignore_parser()
+        return self.to_piggyback_parser(piggyback_header)
+
+    def on_piggyback_footer(self, line: bytes) -> ParserState:
+        return self.to_noop_parser()
+
+    def on_section_header(self, line: bytes) -> PiggybackIgnoreParser:
+        return self
+
+    def on_section_footer(self, line: bytes) -> PiggybackIgnoreParser:
+        return self
+
+
 class HostSectionParser(ParserState):
     def __init__(
         self,
@@ -460,6 +518,8 @@ def on_piggyback_header(self, line: bytes) -> ParserState:
         if piggyback_header.hostname == self.hostname:
             # Unpiggybacked "normal" host
             return self
+        if not is_valid_hostname(piggyback_header.hostname):
+            return self.to_piggyback_ignore_parser()
         return self.to_piggyback_parser(piggyback_header)
 
     def on_piggyback_footer(self, line: bytes) -> ParserState:

tests/unit/cmk/checkengine/test_parsers.py

@@ -26,11 +26,45 @@
 from cmk.fetchers.cache import SectionStore
 
 from cmk.checkengine.parser import AgentParser, AgentRawDataSectionElem, NO_SELECTION, SNMPParser
+from cmk.checkengine.parser._agent import is_valid_hostname
 from cmk.checkengine.parser._markers import PiggybackMarker, SectionMarker
 
 StringTable = list[list[str]]
 
 
+@pytest.mark.parametrize(
+    "hostname",
+    [
+        HostName("ec2-11-111-222-333.cd-blahblah-1.compute.amazonaws.com"),
+        HostName("subdomain.domain.com"),
+        HostName("domain.com"),
+        HostName("domain"),
+        # I don't think the next two are correct but REGEX_HOST_NAME
+        # would match them as well.
+        HostName("_"),
+        HostName("dom_ain.com"),
+    ],
+)
+def test_is_valid_hostname_positive(hostname: HostName) -> None:
+    assert is_valid_hostname(hostname)
+
+
+@pytest.mark.parametrize(
+    "hostname",
+    [
+        HostName("."),
+        HostName(".."),
+        HostName(".domain"),
+        HostName(".domain.com"),
+        HostName("-subdomain.domain.com"),
+        HostName("[email protected]"),
+        HostName("@subdomain.domain.com"),
+    ],
+)
+def test_is_valid_hostname_negative(hostname: HostName) -> None:
+    assert not is_valid_hostname(hostname)
+
+
 @pytest.fixture(autouse=True)
 def enable_debug_fixture():
     debug_mode = cmk.utils.debug.debug_mode
@@ -285,7 +319,7 @@ def test_nameless_piggybacked_sections_are_skipped(
         raw_data = AgentRawData(
             b"\n".join(
                 (
-                    b"<<<<piggyback header>>>>",
+                    b"<<<<piggyback_header>>>>",
                     b"<<<a_section>>>",
                     b"a first line",
                     b"a second line",
@@ -319,6 +353,45 @@ def test_nameless_piggybacked_sections_are_skipped(
         }
         assert not store.load()
 
+    def test_invalid_piggyback_hostnames_are_skipped(
+        self,
+        parser: AgentParser,
+        store: SectionStore[Sequence[AgentRawDataSectionElem]],
+        monkeypatch: pytest.MonkeyPatch,
+    ) -> None:
+        monkeypatch.setattr(time, "time", lambda c=itertools.count(1000, 50): next(c))
+        monkeypatch.setattr(parser, "cache_piggybacked_data_for", 900)
+
+        raw_data = AgentRawData(
+            b"\n".join(
+                (
+                    b"<<<<.>>>>",  # <- invalid hostname "."
+                    b"<<<ignored_section>>>",
+                    b"a first line",
+                    b"a second line",
+                    b"<<<ignored_section_2>>>",
+                    b"a third line",
+                    b"a forth line",
+                    b"<<<<>>>>",
+                    b"<<<<piggyback>>>>",
+                    b"<<<keep>>>",
+                    b"first line kept",
+                    b"second line kept",
+                )
+            )
+        )
+        ahs = parser.parse(raw_data, selection=NO_SELECTION)
+        assert ahs.sections == {}
+        assert ahs.cache_info == {}
+        assert ahs.piggybacked_raw_data == {
+            "piggyback": [
+                b"<<<keep:cached(1000,900)>>>",
+                b"first line kept",
+                b"second line kept",
+            ]
+        }
+        assert not store.load()
+
     def test_closing_piggyback_out_of_piggyback_section_closes_section(
         self, parser: AgentParser, store: SectionStore[Sequence[AgentRawDataSectionElem]]
     ) -> None:
@@ -364,7 +437,7 @@ def test_piggyback_populates_piggyback_raw_data(
         raw_data = AgentRawData(
             b"\n".join(
                 (
-                    b"<<<<piggyback header>>>>",  # <- space is OK
+                    b"<<<<piggyback_header>>>>",
                     b"<<<section>>>",
                     b"first line",
                     b"second line",
@@ -377,9 +450,6 @@ def test_piggyback_populates_piggyback_raw_data(
                     b"third line",
                     b"forth line",
                     b"<<<<>>>>",
-                    b"<<<<../b:l*a../>>>>",
-                    b"<<<section>>>",
-                    b"first line",
                     b"<<<</b_l-u/>>>>",
                     b"<<<section>>>",
                     b"first line",
@@ -405,10 +475,6 @@ def test_piggyback_populates_piggyback_raw_data(
                 b"third line",
                 b"forth line",
             ],
-            ".._b_l_a.._": [
-                b"<<<section:cached(1000,900)>>>",
-                b"first line",
-            ],
             "_b_l-u_": [
                 b"<<<section:cached(1000,900)>>>",
                 b"first line",