Fix plugin applies_to scoping

.pre-commit-config.yaml

@@ -248,6 +248,17 @@ repos:
           ^pyproject\.toml$
         pass_filenames: false
         require_serial: true
+      - id: check-applies-to-roots-in-sync
+        name: Check plugin applies_to destination tables stay in sync
+        description: "The UI mirrors plugins_manager's destination -> record map; drift silently widens plugin scope"
+        entry: ./scripts/ci/prek/check_applies_to_roots_in_sync.py
+        language: python
+        files: >
+          (?x)
+          ^airflow-core/src/airflow/plugins_manager\.py$|
+          ^airflow-core/src/airflow/ui/src/utils/pluginAppliesTo\.ts$
+        pass_filenames: false
+        require_serial: true
       - id: check-ci-workflows-in-sync
         name: Check ci-arm.yml and ci-amd.yml stay in sync
         entry: ./scripts/ci/prek/check_ci_workflows_in_sync.py

airflow-core/docs/administration-and-deployment/plugins.rst

@@ -411,10 +411,11 @@ relevant instead of appearing on every Dag:
         "dag.dag_id": ["train_pipeline"],
     }
 
-Keys are **dotted field paths** into the records the page has, not a fixed set of criteria, so
-anything the REST API returns for an entity is addressable — ``state``, ``operator``, ``pool``,
-``queue``, ``try_number``, and so on. Values are matched for equality, and are compared as
-strings, so numeric and boolean fields work without quoting rules of their own
+Keys are **dotted field paths** into the records the page has, so any field the REST API
+returns for an entity is addressable. Use the names the API returns, which are not always the
+Python attribute names: a task instance's run is ``dag_run_id``, not ``run_id``, and computed
+fields such as a Dag's ``is_backfillable`` work like any other. Values are matched for equality
+and compared as strings, so numbers and booleans need no quoting rules of their own
 (``"try_number": ["2"]``, ``"is_paused": ["false"]``).
 
 An **unqualified path is rooted at the entity the destination is about** — on ``dag_run``,
@@ -423,14 +424,11 @@ name a related record as its first segment: ``dag``, ``dag_run``, ``task`` or
 ``task_instance``. Traversing a list fans out across it, so ``dag.tags.name`` collects every
 tag name and matches if any of them is listed.
 
-Paths combine like Kubernetes label selectors — **OR within a path, AND across paths**. A Dag
-matching any listed tag satisfies ``dag.tags.name``, and a view configured with both
-``dag.tags.name`` and ``state`` requires both to match.
-
-Crucially, the AND applies **only across paths the current page can evaluate**. A
-``task_instance.*`` path cannot be judged on a Dag-level page, so it is skipped there rather
-than failing the match. This lets one ``applies_to`` block be shared by a plugin's Dag- and
-task-level destinations. Which records each destination resolves:
+Paths combine like Kubernetes label selectors — **OR within a path, AND across paths** — but
+the AND applies only across paths the page can evaluate. A ``task_instance.*`` path cannot be
+judged on a Dag-level page, so it is skipped there rather than failing the match, which is what
+lets one block be shared across a plugin's destinations. Which records each destination
+resolves:
 
 .. list-table::
    :header-rows: 1
@@ -457,19 +455,29 @@ task-level destinations. Which records each destination resolves:
 If none of the configured paths can be evaluated on a given page, the view is shown. On task
 group pages the task-level records are absent, since a group is not a task.
 
-A path is also skipped when the record exists but has no such field. That means a **bad path
-widens the scope rather than narrowing it**, so paths are checked against the API response
-models when plugins load and a path naming no field is logged with a suggested correction.
-The check stops at a field whose contents the models do not describe — the dict behind
-``class_ref``, or a Dag Run's ``conf`` — so a path below one of those is still accepted and
-still widens silently. If a view appears in more places than you expect, run
-``airflow plugins list --verbose`` first, then check the path against the REST API response for
-that entity.
+A path is also skipped when the record exists but has no such field, which on its own would mean
+a **typo widens the scope rather than narrowing it**. So paths are checked against the API
+response models when plugins load, and one that names a field *no* record has is treated as a
+misconfiguration: the view is **not loaded at all**, and the reason is logged as an error with a
+suggested correction.
+
+.. code-block:: text
+
+    Plugin 'acme' has an external view 'Incidents' with an 'applies_to' path that matches no
+    field: 'dag.tags.nme' names no field 'nme' on DagTagResponse (did you mean 'name'?). It
+    could never scope the way it asks to, so the view will not be loaded.
 
-Segments are the field names **the REST API returns**, which are not always the Python
-attribute names: a task instance's run is ``dag_run_id``, not ``run_id``, and computed fields
-such as a Dag's ``is_backfillable`` are addressable like any other. If a field appears in the
-API response for an entity, it can be named here.
+Withholding it is deliberate: the author asked to narrow by something that cannot exist, so the
+view can never appear for the reason they intended, and a view that disappears gets noticed
+while one on every page looks deliberate.
+
+A path naming a field some *other* record has is a different matter — that is the shared-block
+pattern, so it is reported as skipped here and the view loads. Neither check can see past a
+field the models do not describe — the dict behind ``class_ref``, or a Dag Run's ``conf`` — so a
+typo below one of those still widens silently.
+
+If a view appears in more places than you expect, run ``airflow plugins --verbose``, then check
+the path against the REST API response for that entity.
 
 An empty list is different from a missing field: a Dag with no tags has definitively answered
 ``dag.tags.name``, so the view is not shown. A path that stops short of a leaf is likewise a
@@ -487,14 +495,30 @@ unqualified path targets an operator on either page**:
 
     "applies_to": {"operator_name": ["KubernetesPodOperator"]}
 
-``operator_name`` is the display name shown in the UI (an operator's
-``custom_operator_name``). For a plain operator it is the class name, but the two differ for
-decorator-based tasks: a ``@task.bash`` task has the display name ``@task.bash`` and the
-private class name ``_BashDecoratedOperator``, so ``operator_name`` is the one to match on.
+``operator_name`` is the display name the UI shows (an operator's ``custom_operator_name``).
+It equals the class name for a plain operator, but not for a decorated one: a ``@task.bash``
+task is named ``@task.bash`` and classed ``_BashDecoratedOperator``.
+
+It is also the only operator field both records carry. The *class* name is spelled differently
+on each — a task instance has ``operator``, a task has ``class_ref.class_name`` — so targeting
+it takes one view per destination, each scoped on its own record:
 
-If you specifically need the operator *class* name, the two records spell it differently — a
-task instance has ``operator``, while a task carries it through ``class_ref.class_name``.
-Leave both paths **unqualified** so that each page reads its own record:
+.. code-block:: python
+
+    external_views = [
+        {
+            # ... name, href, and a url_route of its own
+            "destination": "task_instance",
+            "applies_to": {"operator": ["KubernetesPodOperator"]},
+        },
+        {
+            # ...
+            "destination": "task",
+            "applies_to": {"class_ref.class_name": ["KubernetesPodOperator"]},
+        },
+    ]
+
+One block naming both also works, since whichever field this page's record lacks is skipped:
 
 .. code-block:: python
 
@@ -505,17 +529,14 @@ Leave both paths **unqualified** so that each page reads its own record:
         "class_ref.class_name": ["KubernetesPodOperator"],
     }
 
-.. warning::
-    Do not write these as ``task_instance.operator`` and ``task.class_ref.class_name``. A task
-    instance page resolves **both** records, so both paths would be evaluated and AND-ed — and
-    the task record always carries the Dag's *current* definition. An instance that ran under
-    ``KubernetesPodOperator`` would stop matching the moment the task was changed to a
-    different operator, hiding the view on historical runs.
+Qualifying them — ``task_instance.operator`` and ``task.class_ref.class_name`` — works too, but
+says the same thing twice: a task instance page resolves both records, so both are evaluated and
+AND-ed. The task record is read at the version that instance ran, so the two agree; naming one
+of them is enough.
 
-This is the general rule where records overlap: **prefer an unqualified path**, which always
-reads the entity the page is about. Qualify a path only when you genuinely mean the related
-record — ``dag.tags.name`` from a task instance, say — and not as a way of naming the same
-concept twice.
+The general rule where records overlap: **prefer an unqualified path**, which always reads the
+entity the page is about, and qualify one only when you genuinely mean a different record —
+``dag.tags.name`` from a task instance, say.
 
 A malformed ``applies_to`` — one that is not a dictionary, has a non-string or empty path, or
 gives a path something other than a list of strings — is reported as a warning when plugins
@@ -530,7 +551,7 @@ server log, run:
 
 .. code-block:: bash
 
-    airflow plugins list --verbose
+    airflow plugins --verbose
 
 Each invocation is a fresh process, so every warning for every plugin is reported. The
 ``--verbose`` flag is required: without it the command suppresses log output.

airflow-core/newsfragments/74278.feature.rst

@@ -1 +1 @@
-Plugin external views and React apps can be scoped with an optional ``applies_to`` block: a map of dotted field path to the values that path may take, such as ``{"state": ["failed"], "dag.tags.name": ["ml"]}``. Any field the REST API returns for the entity is addressable, so a tab or panel appears only where it is relevant instead of on every Dag. An unqualified path is rooted at the entity the ``destination`` is about; a path may instead name a related record (``dag``, ``dag_run``, ``task``, ``task_instance``) as its first segment, and traversing a list fans out across it. Values are OR-ed within a path and AND-ed across paths, and a path whose root record the current page cannot resolve is skipped rather than failing the match, so one block can be shared across a plugin's Dag- and task-level destinations. Omitting ``applies_to`` shows the item everywhere, which is the previous behaviour. This replaces the five fixed criteria (``dag_tags``, ``dag_ids``, ``task_ids``, ``operators``, ``operator_names``) added earlier in this development cycle and never released, so there is no deprecation path: ``dag_tags`` becomes ``dag.tags.name``, ``dag_ids`` becomes ``dag.dag_id``, ``task_ids`` becomes ``task_id``, and both operator criteria become ``operator_name``. The trade is that paths are resolved against the API response rather than checked against a fixed vocabulary: a path naming no field is logged with a suggested correction when plugins load, but one below a field the response models do not describe (such as ``class_ref``) cannot be checked, and widens the scope instead of narrowing it. Matching is equality-only. See :doc:`the plugins documentation </administration-and-deployment/plugins>` for details.
+Plugin external views and React apps can be scoped with an optional ``applies_to`` block, a map of dotted field path to the values that path may take, such as ``{"state": ["failed"], "dag.tags.name": ["ml"]}``. This replaces the five fixed criteria added earlier in this development cycle and never released (``dag_tags``, ``dag_ids``, ``task_ids``, ``operators``, ``operator_names``). A path naming a field no record has is treated as a misconfiguration rather than a filter that never matches, so the view is not loaded and the reason is logged. See :doc:`the plugins documentation </administration-and-deployment/plugins>` for the path syntax, the migration table and the trade-offs.

airflow-core/src/airflow/plugins_manager.py

@@ -334,21 +334,46 @@ def _describe_applies_to_error(applies_to: Any) -> str | None:
     return None
 
 
-def _validate_applies_to(plugin_name: str | None, view: ExternalViewDict | ReactAppDict, kind: str) -> None:
+def _is_applies_to_path_known_elsewhere(path: str) -> bool:
     """
-    Warn about scoping a UI plugin cannot honour, and strip it if it is malformed.
+    Whether an unqualified ``path`` names a real field on some record other than this one.
 
-    A malformed block is removed so the view still loads unscoped, matching the default for
-    a view that omits ``applies_to`` entirely. Criteria the destination cannot evaluate are
-    only warned about — they are skipped at match time by design, so that one block can be
-    shared across a plugin's Dag- and task-level destinations.
+    ``class_ref.class_name`` means nothing on a task instance but everything on a task, and an
+    author targeting both pages from one block writes exactly that. Such a path is skipped at
+    match time by design, so it must not be reported as a misspelling.
+
+    Only asked about unqualified paths: one that names its record explicitly has no other
+    reading, so a bad field there is simply wrong.
+    """
+    return any(_describe_applies_to_path_error(path, root) is None for root in _applies_to_root_models())
+
+
+def _validate_applies_to(plugin_name: str | None, view: ExternalViewDict | ReactAppDict, kind: str) -> bool:
+    """
+    Check a UI plugin's scoping, returning whether the view should be loaded at all.
+
+    Three failures, wanting three different things:
+
+    * **Malformed** block -- not a dictionary, a non-string path, a value that is not a list of
+      strings. Dropped, and the view loads unscoped, as it would without ``applies_to`` at all.
+      Nothing of the author's intent survives to act on.
+    * **Unevaluable** path -- its root record is one this destination does not have, or it names
+      a field only a sibling record has. Warned about and kept: both are skipped at match time
+      by design, which is what lets one block be shared across a plugin's destinations.
+    * **A path naming a field no record has.** The author asked to narrow by something that
+      cannot exist, so the view can never appear for the reason they intended. Returning
+      ``False`` withholds it rather than letting the browser skip the path and show it
+      everywhere. A view that disappears gets noticed; one on every page looks deliberate.
+
+    The view gets a new dict rather than an edited one, because a plugin may pass the same
+    ``applies_to`` object to several views and it is not this function's to rewrite.
     """
     if "applies_to" not in view:
-        return
+        return True
 
     applies_to = view["applies_to"]
     if applies_to is None:
-        return
+        return True
 
     if error := _describe_applies_to_error(applies_to):
         log.warning(
@@ -359,20 +384,23 @@ def _validate_applies_to(plugin_name: str | None, view: ExternalViewDict | React
             error,
         )
         del view["applies_to"]
-        return
+        return True
 
     # A null value means "no values configured", which the matcher already ignores the same way
-    # it ignores an empty list. Leaving it in place would fail `PluginAppliesToResponse`
+    # it ignores an empty list. Carrying it through would fail `PluginAppliesToResponse`
     # serialization and drop the whole plugin -- including its other, valid views -- from the
-    # plugins API. Dropping the path keeps the rest of the block working.
-    for path in [path for path, values in applies_to.items() if values is None]:
-        del applies_to[path]
+    # plugins API.
+    #
+    # The view gets a new dict rather than an edited one, because a plugin may pass the same
+    # `applies_to` object to several views and it is not this function's to rewrite.
+    applies_to = {path: list(values) for path, values in applies_to.items() if values is not None}
+    view["applies_to"] = applies_to
 
     destination = view.get("destination", "nav")
     if destination not in _APPLIES_TO_ROOTS:
         # An unrecognised destination already fails serialization; warning here too would
         # only add noise pointing at the wrong problem.
-        return
+        return True
 
     available = _APPLIES_TO_ROOTS[destination]
     unevaluable = sorted(
@@ -398,16 +426,35 @@ def _validate_applies_to(plugin_name: str | None, view: ExternalViewDict | React
         root = _applies_to_path_root(path, destination)
         if not values or root is None or root not in available:
             continue
-        if error := _describe_applies_to_path_error(path, root):
+        error = _describe_applies_to_path_error(path, root)
+        if error is None:
+            continue
+        head, _, rest = path.partition(".")
+        if not (head in _APPLIES_TO_ROOT_NAMES and rest) and _is_applies_to_path_known_elsewhere(path):
+            # A real field, just not on this destination's record. Saying the scope widened
+            # would be untrue: the block's other paths are what scope this destination.
             log.warning(
-                "Plugin '%s' has %s '%s' with an 'applies_to' path that matches no field: %s. "
-                "That path will be ignored, so the %s will appear in more places than intended.",
+                "Plugin '%s' has %s '%s' with an 'applies_to' path '%s' that no %s has. It is "
+                "skipped here; it only scopes the destinations whose record does have it.",
                 plugin_name,
                 kind,
                 view.get("name"),
-                error,
-                kind.split()[-1],
+                path,
+                root,
             )
+            continue
+        log.error(
+            "Plugin '%s' has %s '%s' with an 'applies_to' path that matches no field: %s. "
+            "It could never scope the way it asks to, so the %s will not be loaded.",
+            plugin_name,
+            kind,
+            view.get("name"),
+            error,
+            kind.split()[-1],
+        )
+        return False
+
+    return True
 
 
 @cache
@@ -430,7 +477,9 @@ def _get_ui_plugins() -> tuple[list[ExternalViewDict], list[ReactAppDict]]:
                 )
                 external_views_to_remove.append(external_view)
                 continue
-            _validate_applies_to(plugin.name, external_view, "an external view")
+            if not _validate_applies_to(plugin.name, external_view, "an external view"):
+                external_views_to_remove.append(external_view)
+                continue
             url_route = external_view.get("url_route")
             if url_route is None:
                 continue
@@ -455,7 +504,9 @@ def _get_ui_plugins() -> tuple[list[ExternalViewDict], list[ReactAppDict]]:
                 )
                 react_apps_to_remove.append(react_app)
                 continue
-            _validate_applies_to(plugin.name, react_app, "a React App")
+            if not _validate_applies_to(plugin.name, react_app, "a React App"):
+                react_apps_to_remove.append(react_app)
+                continue
             url_route = react_app.get("url_route")
             if url_route is None:
                 continue