diff --git a/.werks/16144 b/.werks/16144 new file mode 100644 index 00000000000..33d47eb5867 --- /dev/null +++ b/.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. diff --git a/cmk/checkengine/parser/_agent.py b/cmk/checkengine/parser/_agent.py index f5a187f9f5a..31255b58bb9 100644 --- a/cmk/checkengine/parser/_agent.py +++ b/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 : ""<<<~>>>"" + Host --> IgnoredPiggybackedHost : ""<<<~<.>>>>"" + IgnoredPiggybackedHost --> Host : ""<<<~<>>>>"" + + PiggybackedHost --> IgnoredPiggybackedHost : ""<<<~<.>>>>"" + IgnoredPiggybackedHost --> PiggybackedHost : ""<<<~>>>"" + 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: diff --git a/tests/unit/cmk/checkengine/test_parsers.py b/tests/unit/cmk/checkengine/test_parsers.py index 11368cdcc43..576e04a6b1b 100644 --- a/tests/unit/cmk/checkengine/test_parsers.py +++ b/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@domain.com"), + 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"<<<>>>", + b"<<<>>>", b"<<>>", 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"<<>>", + b"a first line", + b"a second line", + b"<<>>", + b"a third line", + b"a forth line", + b"<<<<>>>>", + b"<<<>>>", + b"<<>>", + 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"<<>>", + 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"<<<>>>", # <- space is OK + b"<<<>>>", b"<<
>>", 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"<<
>>", - b"first line", b"<<<>>>", b"<<
>>", b"first line", @@ -405,10 +475,6 @@ def test_piggyback_populates_piggyback_raw_data( b"third line", b"forth line", ], - ".._b_l_a.._": [ - b"<<>>", - b"first line", - ], "_b_l-u_": [ b"<<>>", b"first line",