monitor(hosts): add site filtering backend support

Site is modeled as a normal condition inside the existing
filter grammar rather than a separate top-level scope field, since
the product direction is toward composable filter conditions in
general and site_id shouldn't stay a permanent special case. This
also means the frontend gets component reuse for free, instead of
needing to handle site differently.

However, in the backend the site condition node _does_ need to be
handled differently. Only a single site_id condition is supported,
combined with the rest of the tree via 'and'. It can also be negated
with 'not'.

Here are some details regarding why that decision was made:

- Livestatus broadcasts one filter string to every queried site, so
  a site restriction can't be pushed down once it's combined with an
  unrelated condition via `or`, or once a subtree mixing site and
  non-site conditions is negated (De Morgan's turns the safe `and`
  into an unsupported `or`). Both are rejected with a 400.
- Combining multiple site_id conditions via and/or is technically
  reducible to one restriction, but nothing in the UI ever produces
  two to combine - one multi-select already covers that, so this
  narrower rule costs nothing today. Negation is the one exception
  worth supporting on its own: excluding a few sites out of many is
  a real capability a plain inclusion list can't express.

CMK-37572

Change-Id: Ia9a3cec325c97a674d2d07cf7eb32cf30be6f321

Author: logan-connolly

cmk/gui/monitor/hosts/_api/_filters.py

@@ -8,7 +8,9 @@
 from annotated_types import MinLen
 from pydantic import AfterValidator
 
+from cmk.ccc.site import SiteId
 from cmk.gui.openapi.framework.model import api_field, api_model
+from cmk.gui.openapi.framework.model.converter import SiteIdConverter, TypedPlainValidator
 from cmk.livestatus_client.expressions import LqSafe
 
 from .._models import HostFilter, HostState, HostStateLabel
@@ -63,6 +65,20 @@ class StateChoiceCondition:
     )
 
 
+@api_model
+class SiteChoiceCondition:
+    type: Literal["condition"] = api_field(
+        description="Node type discriminator", example="condition"
+    )
+    field: Literal["site_id"] = api_field(description="Site field", example="site_id")
+    op: Literal["one_of"] = api_field(description="Set membership operation", example="one_of")
+    value: Annotated[
+        list[Annotated[SiteId, TypedPlainValidator(str, SiteIdConverter.should_exist)]],
+        MinLen(1),
+        AfterValidator(validate_uniqueness),
+    ] = api_field(description="Site IDs to match", example=["local"])
+
+
 @api_model
 class NumericCondition:
     type: Literal["condition"] = api_field(
@@ -87,7 +103,13 @@ class BooleanCondition:
     value: bool = api_field(description="Boolean value to compare against", example=False)
 
 
-type ConditionNode = StringCondition | StateChoiceCondition | NumericCondition | BooleanCondition
+type ConditionNode = (
+    StringCondition
+    | StateChoiceCondition
+    | SiteChoiceCondition
+    | NumericCondition
+    | BooleanCondition
+)
 
 
 @api_model(slots=False)
@@ -129,6 +151,89 @@ class NotNode:
 type FilterNode = AndNode | OrNode | NotNode | ConditionNode
 
 
+def extract_site_scope(
+    node: FilterNode, all_site_ids: frozenset[SiteId]
+) -> tuple[FilterNode | None, list[SiteId] | None]:
+    """Split off the tree's 'site_id' condition, if any, as a site scope restriction.
+
+    Site ID isn't a real Livestatus column, so it can't contribute a filter line; it's pushed
+    down into which sites get queried instead. Only a single 'site_id' condition is supported,
+    optionally wrapped with a 'not' condition. If a site condition is combined with the rest of
+    the tree with anything but an 'and' condition, a `ValueError` is raised.
+
+    Additional details:
+
+    - Combining it with an unrelated condition via 'or' would need hosts from every other site
+      too (e.g. "site A's hosts OR any DOWN host"), which can't be reduced to a site restriction.
+    - Negating a mixed (site + non-site) subtree hits the same problem:
+      NOT(site_id=A AND cond) = (site_id!=A OR NOT cond), which turns the 'and' into an 'or'.
+    - The only one site condition is done to constrain the current parsing logic. This constraint
+      can be lifted when the need arises, but will come with added complexity.
+    """
+    found: list[tuple[SiteChoiceCondition, bool]] = []
+
+    def record(condition: SiteChoiceCondition, *, negated: bool, and_only: bool) -> None:
+        if not and_only:
+            raise ValueError(
+                "'site_id' conditions may only be combined via 'and'; they cannot appear "
+                "inside 'or', or inside 'not' together with other conditions."
+            )
+        if found:
+            raise ValueError("Only one 'site_id' condition is allowed per filter.")
+        found.append((condition, negated))
+
+    def walk(current: FilterNode, and_only: bool) -> FilterNode | None:
+        match current:
+            case SiteChoiceCondition():
+                record(current, negated=False, and_only=and_only)
+                return None
+
+            case NotNode(child=SiteChoiceCondition() as site_condition):
+                record(site_condition, negated=True, and_only=and_only)
+                return None
+
+            case NotNode(child=child):
+                # Passes through unchanged unless a 'site_id' condition turns up further inside,
+                # which `and_only=False` rejects (see docstring: mixed-subtree negation).
+                walk(child, and_only=False)
+                return current
+
+            case AndNode(children=children):
+                residual_children = [
+                    residual
+                    for residual in (walk(child, and_only) for child in children)
+                    if residual is not None
+                ]
+                match residual_children:
+                    case []:
+                        return None
+                    case [single]:
+                        return single
+                    case _:
+                        return AndNode(type="and", children=residual_children)
+
+            case OrNode(children=children):
+                # 'or' can never be reduced to a site restriction, so any 'site_id' condition
+                # anywhere inside is rejected by `record`; nothing is ever extracted here.
+                for child in children:
+                    walk(child, and_only=False)
+                return current
+
+            case _:
+                return current
+
+    residual = walk(node, and_only=True)
+
+    if not found:
+        return residual, None
+
+    condition, negated = found[0]
+    extracted_site_ids = {SiteId(site_id) for site_id in condition.value}
+    site_ids = list(all_site_ids - extracted_site_ids if negated else extracted_site_ids)
+
+    return residual, site_ids
+
+
 def parse_as_livestatus_filter(node: FilterNode) -> HostFilter:
     filters: list[str] = []
     _accumulate_filters(node, filters)
@@ -161,6 +266,9 @@ def _accumulate_filters(node: FilterNode, filters: list[str]) -> None:
                 case "one_of" if len(node.value) > 1:
                     filters.append(f"Or: {len(node.value)}")
 
+        case SiteChoiceCondition():
+            raise AssertionError("Site conditions are not fully supported as filter nodes.")
+
         case AndNode() | OrNode():
             for child in node.children:
                 _accumulate_filters(child, filters)

cmk/gui/monitor/hosts/_api/_list_hosts.py

@@ -9,6 +9,7 @@
 from annotated_types import Interval
 from pydantic import PlainValidator
 
+from cmk.ccc.site import SiteId
 from cmk.gui import sites
 from cmk.gui.openapi.framework import (
     ApiContext,
@@ -21,6 +22,7 @@
     VersionedEndpoint,
 )
 from cmk.gui.openapi.framework.model import api_field, api_model, ApiOmitted
+from cmk.gui.openapi.utils import RestAPIRequestGeneralException
 from cmk.gui.utils import permission_verification as permissions
 
 from .._impl import LiveStatusHostRepository
@@ -35,7 +37,7 @@
 )
 from .._repositories import HostRepository
 from ._family import MONITOR_HOSTS_FAMILY
-from ._filters import FilterNode, parse_as_livestatus_filter
+from ._filters import extract_site_scope, FilterNode, parse_as_livestatus_filter
 from ._modes import build_host_modes, ModeInfo
 from ._urls import host_view_link
 from ._validators import parse_host_search_query, parse_host_sort_options
@@ -244,8 +246,6 @@ def list_hosts(
     body: HostsRequestBody = HostsRequestBody(),
 ) -> HostsResponse:
     """List hosts to be consumed by the all host monitoring page."""
-    host_repo = LiveStatusHostRepository(connection=sites.live())
-
     # A `None` request means "remove the limit". We only honor that for users allowed to ignore
     # the hard limit; everyone else is clamped to the safety ceiling. Numeric requests are already
     # bounded to the ceiling by the request schema, so they pass through unchanged.
@@ -257,41 +257,84 @@ def list_hosts(
         case _:
             limit = body.limit
 
-    parsed_filters = (
-        HostFilter("")
-        if isinstance(body.filter, ApiOmitted)
-        else parse_as_livestatus_filter(body.filter)
-    )
+    # Validated before opening a Livestatus connection below, so a bad filter is rejected without
+    # ever needing one.
+    if isinstance(body.filter, ApiOmitted):
+        filters, site_ids = None, None
+    else:
+        try:
+            filters, site_ids = extract_site_scope(
+                node=body.filter,
+                all_site_ids=frozenset(api_context.config.sites),
+            )
+        except ValueError as exc:
+            raise RestAPIRequestGeneralException(
+                status=400, title="Invalid filter", detail=str(exc)
+            ) from exc
 
-    return _handle_list_hosts(
-        host_repo,
-        limit=limit,
-        query="" if isinstance(body.q, ApiOmitted) else body.q,
-        sorters=_DEFAULT_SORT if isinstance(body.sort, ApiOmitted) else body.sort,
-        filters=parsed_filters,
-        fields=_DEFAULT_FIELDS if isinstance(body.fields, ApiOmitted) else body.fields,
-    )
+    host_repo = LiveStatusHostRepository(connection=sites.live())
+
+    # NOTE: we never want this value scoped by the selected sites. It should always get full count.
+    # As a temporary solution, we are querying count here and passing the result to the handler.
+    # This is done to make the handler more testable without the need to be tested within a request
+    # context as `sites` triggers that side-effect.
+    total_host_count = host_repo.count_total()
+
+    fields = _DEFAULT_FIELDS if isinstance(body.fields, ApiOmitted) else body.fields
+
+    # `sites.only_sites([])` can't express "query zero sites"; an empty list is falsy to it and
+    # gets treated as "no restriction" instead, i.e. every site. This happens when the filter
+    # negates every currently configured site, so it's handled here before ever calling into
+    # Livestatus, rather than being passed through.
+    if site_ids == []:
+        return HostsResponse(
+            hosts=[],
+            meta=HostsPageMeta(limit=limit, matched=0, total=total_host_count, fields=fields),
+        )
+
+    with sites.only_sites(site_ids):
+        return _handle_list_hosts(
+            host_repo,
+            total_host_count,
+            limit=limit,
+            query="" if isinstance(body.q, ApiOmitted) else body.q,
+            sorters=_DEFAULT_SORT if isinstance(body.sort, ApiOmitted) else body.sort,
+            filters=HostFilter("") if filters is None else parse_as_livestatus_filter(filters),
+            fields=fields,
+            site_ids=site_ids,
+        )
 
 
 def _handle_list_hosts(
     host_repo: HostRepository,
+    total_host_count: int,
     *,
     limit: int | None = _DEFAULT_LIMIT,
     query: str = "",
     sorters: Sequence[HostSort] = _DEFAULT_SORT,
     filters: HostFilter = HostFilter(""),
     fields: Set[HostOptionalField] = _DEFAULT_FIELDS,
+    site_ids: Sequence[SiteId] | None = None,
 ) -> HostsResponse:
+    # Derived from the same `site_ids` the caller scoped the connection with via `only_sites`,
+    # rather than taken as a separately-passed flag, so the two can't drift apart.
+    has_site_filter = site_ids is not None
+
     hosts = host_repo.fetch(
         limit=limit,
         query=query,
         sorters=sorters,
         filters=filters,
     )
-    total_host_count = host_repo.count_total()
+    # `limit` reaches Livestatus as a per-site cap (queried in parallel across sites, then merged
+    # and sorted here), so a multi-site fetch can come back with up to `limit * len(sites)` rows.
+    # Re-applying it client-side after the merge is what makes the *global* top-`limit` hold.
+    if limit is not None:
+        hosts = hosts[:limit]
+
     if limit is None:
         matched_host_count = len(hosts)
-    elif query or filters:
+    elif query or filters or has_site_filter:
         matched_host_count = host_repo.count_matched(query=query, filters=filters)
     else:
         matched_host_count = total_host_count

cmk/gui/monitor/hosts/_pages/_monitor_all_hosts.py

@@ -28,6 +28,7 @@
 from cmk.gui.pagetypes import PagetypeTopics
 from cmk.gui.permissions import permission_registry
 from cmk.gui.type_defs import DynamicIconName, IconNames, StaticIcon, Visual
+from cmk.gui.user_sites import sorted_sites
 from cmk.gui.utils.roles import UserPermissions
 from cmk.gui.utils.urls import makeuri_contextless
 from cmk.shared_typing.monitoring.all_hosts import (
@@ -36,6 +37,7 @@
     MonitoringAllHostsApp,
     MonitoringPageLinkButton,
     RowAction,
+    Site,
 )
 from cmk.utils import paths
 
@@ -133,6 +135,10 @@ def page(self, ctx: PageContext) -> None:
                     poll_interval_ms=ctx.config.view_option_refreshes[0] * 1000,
                     user_id=str(user.id),
                     site=str(omd_site()),
+                    sites=[
+                        Site(id=str(site_id), alias=alias)
+                        for site_id, alias in sorted_sites(ctx.config.sites)
+                    ],
                     edition=Edition(edition(paths.omd_root).short),
                     actions=[
                         MonitoringAction(

packages/cmk-shared-typing/source/monitoring/all_hosts.json

@@ -11,13 +11,25 @@
     "site": {
       "type": "string"
     },
+    "sites": {
+      "type": "array",
+      "items": { "$ref": "#/$defs/site" }
+    },
     "edition": { "$ref": "#/$defs/edition" }
   },
-  "required": ["user_id", "site", "edition"],
+  "required": ["user_id", "site", "sites", "edition"],
   "$defs": {
     "edition": {
       "type": "string",
       "enum": ["community", "pro", "ultimate", "ultimatemt", "cloud"]
+    },
+    "site": {
+      "type": "object",
+      "properties": {
+        "id": { "type": "string" },
+        "alias": { "type": "string" }
+      },
+      "required": ["id", "alias"]
     }
   }
 }