Added static check for conn-fields defined in provider.yaml

scripts/ci/prek/check_provider_conn_fields.py

@@ -0,0 +1,112 @@
+#!/usr/bin/env python
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""
+Validation helpers for the conn-fields ↔ get_connection_form_widgets() check.
+
+These functions have no third-party dependencies so they can be unit-tested
+outside of the Breeze container without any stubbing.
+
+Used by ``scripts/in_container/run_provider_yaml_files_check.py``.
+"""
+
+from __future__ import annotations
+
+from collections.abc import Callable
+
+
+def check_conn_fields_for_entry(
+    conn_type_entry: dict,
+    yaml_file_path: str,
+    get_widget_keys: Callable[[str], set[str] | None],
+) -> list[str]:
+    """
+    Validate a single connection-type entry.  Returns a (possibly empty) list of error strings.
+
+    *get_widget_keys(hook_class_name)* is a callable supplied by the caller that:
+
+    - returns the set of field keys from ``get_connection_form_widgets()`` on success,
+    - returns ``None`` to signal that the hook could not be imported, its UI
+      dependencies are absent, or it does not implement ``get_connection_form_widgets()``
+      at all — the entry is then skipped entirely (no ``conn-fields`` check), or
+    - raises any other ``Exception`` to signal an unexpected failure (converted
+      here into an error string so callers never need to catch it).
+    """
+    hook_class_name: str = conn_type_entry["hook-class-name"]
+    connection_type: str = conn_type_entry.get("connection-type", "?")
+
+    try:
+        widget_keys = get_widget_keys(hook_class_name)
+    except Exception as exc:
+        return [
+            f"Failed to call `{hook_class_name}.get_connection_form_widgets()` "
+            f"while checking {yaml_file_path}: {exc}"
+        ]
+
+    if widget_keys is None:
+        return []
+
+    conn_fields = conn_type_entry.get("conn-fields")
+    if conn_fields is None:
+        # No conn-fields declared: the new UI simply exposes no custom fields for this
+        # connection type, which is intentional.  Nothing to validate.
+        return []
+
+    error = build_mismatch_error(
+        set(conn_fields.keys()), widget_keys, connection_type, yaml_file_path, hook_class_name
+    )
+    return [error] if error else []
+
+
+def build_mismatch_error(
+    yaml_keys: set[str],
+    hook_keys: set[str],
+    connection_type: str,
+    yaml_file_path: str,
+    hook_class_name: str,
+) -> str | None:
+    """
+    Check that every key declared in ``conn-fields`` exists in
+    ``get_connection_form_widgets()``.
+
+    ``conn-fields`` is the new React UI's view of a connection type and is
+    intentionally a *subset* of the Flask form widgets — fields can be omitted
+    from ``conn-fields`` on purpose.  We therefore only flag keys that appear in
+    ``conn-fields`` but are absent from the hook's form (invalid / stale
+    declarations).  The reverse direction (hook fields not listed in
+    ``conn-fields``) is not an error.
+
+    Return an error string when stale keys are found, or ``None`` when the
+    declared keys are all valid.
+    """
+    only_in_yaml = yaml_keys - hook_keys
+
+    if not only_in_yaml:
+        return None
+
+    lines = [
+        f"Mismatch between `conn-fields` in {yaml_file_path} and "
+        f"`{hook_class_name}.get_connection_form_widgets()` "
+        f"for connection-type '{connection_type}':"
+    ]
+    lines.append(
+        "  Fields in provider.yaml conn-fields but NOT in get_connection_form_widgets(): "
+        + ", ".join(sorted(only_in_yaml))
+    )
+    lines.append("[yellow]How to fix it[/]: Remove the stale key(s) from conn-fields in provider.yaml.")
+    return "\n".join(lines)

scripts/in_container/run_provider_yaml_files_check.py

@@ -50,6 +50,11 @@
 from airflow.exceptions import AirflowOptionalProviderFeatureException, AirflowProviderDeprecationWarning
 from airflow.providers_manager import ProvidersManager
 
+# check_provider_conn_fields lives in scripts/ci/prek/ which is not on sys.path when
+# this script runs inside Breeze; resolve it relative to this file.
+sys.path.insert(0, str(pathlib.Path(__file__).parent.parent / "ci" / "prek"))
+from check_provider_conn_fields import check_conn_fields_for_entry
+
 # Those are deprecated modules that contain removed Hooks/Sensors/Operators that we left in the code
 # so that users can get a very specific error message when they try to use them.
 
@@ -473,6 +478,65 @@ def check_hook_class_name_entries_in_connection_types(yaml_files: dict[str, dict
     return num_connection_types, num_errors
 
 
+@run_check("Checking that conn-fields in provider.yaml match get_connection_form_widgets() of the hook class")
+def check_conn_fields_match_form_widgets(yaml_files: dict[str, dict]) -> tuple[int, int]:
+    """
+    For every connection-type entry whose hook declares ``conn-fields``,
+    verify that every key in ``conn-fields`` also exists in the hook's
+    ``get_connection_form_widgets()``.
+
+    ``conn-fields`` is optional and is intentionally allowed to be a *subset*
+    of the hook's form widgets (the new React UI may expose fewer fields than
+    the legacy Flask form), so extra hook widgets are not flagged — only
+    ``conn-fields`` keys absent from the hook are reported as stale.
+    """
+    num_checks = 0
+    num_errors = 0
+
+    for yaml_file_path, provider_data in yaml_files.items():
+        for conn_type_entry in provider_data.get("connection-types", []):
+            num_checks += 1
+            for error in check_conn_fields_for_entry(conn_type_entry, yaml_file_path, _get_widget_keys):
+                errors.append(error)
+                num_errors += 1
+
+    return num_checks, num_errors
+
+
+def _get_widget_keys(hook_class_name: str) -> set[str] | None:
+    """
+    Import *hook_class_name* and return the keys of ``get_connection_form_widgets()``.
+
+    Returns ``None`` when the hook or its UI dependencies cannot be imported,
+    or when the hook does not override ``get_connection_form_widgets()`` (meaning it
+    has no custom connection fields and the conn-fields check should be skipped).
+    Raises for unexpected errors so ``check_conn_fields_for_entry`` can convert them
+    to an error string.
+    """
+    try:
+        module_name, class_name = hook_class_name.rsplit(".", maxsplit=1)
+        with warnings.catch_warnings(record=True):
+            hook_class = getattr(importlib.import_module(module_name), class_name)
+    except (ImportError, AirflowOptionalProviderFeatureException, AttributeError):
+        return None
+
+    # Only validate hooks that override get_connection_form_widgets() in their own __dict__,
+    # because that method is the source-of-truth for what conn-fields should be declared.
+    # Hooks that inherit it without overriding have no provider-specific widget definition
+    # to diff against, so the check is intentionally skipped for them.  As of writing this
+    # includes HttpHook, the common/ai hooks, and AzureComputeHook — any provider whose hook
+    # falls into this category will NOT be validated here, even if it declares conn-fields.
+    if "get_connection_form_widgets" not in hook_class.__dict__:
+        return None
+
+    with warnings.catch_warnings(record=True):
+        try:
+            form_widgets: dict[str, Any] = hook_class.get_connection_form_widgets()
+            return set(form_widgets.keys())
+        except (ImportError, AirflowOptionalProviderFeatureException, AttributeError):
+            return None
+
+
 @run_check("Checking that hook classes defining conn_type are registered in connection-types")
 def check_hook_classes_with_conn_type_are_registered(yaml_files: dict[str, dict]) -> tuple[int, int]:
     """Find Hook subclasses that define conn_type but are not listed in connection-types."""
@@ -1046,6 +1110,7 @@ def check_providers_have_all_documentation_files(yaml_files: dict[str, dict]):
 
     check_completeness_of_list_of_transfers(all_parsed_yaml_files)
     check_hook_class_name_entries_in_connection_types(all_parsed_yaml_files)
+    check_conn_fields_match_form_widgets(all_parsed_yaml_files)
     check_hook_classes_with_conn_type_are_registered(all_parsed_yaml_files)
     check_executor_classes(all_parsed_yaml_files)
     check_queue_classes(all_parsed_yaml_files)

scripts/tests/ci/prek/test_check_conn_fields_match_form_widgets.py

@@ -0,0 +1,124 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import pytest
+from check_provider_conn_fields import (
+    build_mismatch_error,
+    check_conn_fields_for_entry,
+)
+
+YAML_PATH = "providers/my_provider/provider.yaml"
+HOOK_CLASS = "my_provider.hooks.my_hook.MyHook"
+CONN_TYPE = "my_conn_type"
+
+
+def _entry(conn_fields: list[str] | None) -> dict:
+    entry: dict = {"hook-class-name": HOOK_CLASS, "connection-type": CONN_TYPE}
+    if conn_fields is not None:
+        entry["conn-fields"] = {k: {} for k in conn_fields}
+    return entry
+
+
+def _get_keys(*keys: str):
+    """Return a get_widget_keys callable that always returns the given keys."""
+    return lambda _hook_class_name: set(keys)
+
+
+def _skip(_hook_class_name: str) -> None:
+    """get_widget_keys callable that signals 'skip silently'."""
+    return None
+
+
+def _raise(_hook_class_name: str) -> None:
+    raise RuntimeError("boom")
+
+
+class TestBuildMismatchError:
+    @pytest.mark.parametrize(
+        "yaml_keys, hook_keys",
+        [
+            pytest.param({"a", "b"}, {"a", "b"}, id="matching-keys"),
+            pytest.param(set(), set(), id="empty-sets"),
+            # Hook may have more keys than conn-fields — that is intentional (subset allowed).
+            pytest.param({"a"}, {"a", "extra_hook"}, id="hook-has-extra-keys-no-error"),
+        ],
+    )
+    def test_no_mismatch_returns_none(self, yaml_keys, hook_keys):
+        assert build_mismatch_error(yaml_keys, hook_keys, CONN_TYPE, YAML_PATH, HOOK_CLASS) is None
+
+    @pytest.mark.parametrize(
+        "yaml_keys, hook_keys, expected_in_error, not_expected_in_error",
+        [
+            pytest.param(
+                {"a", "extra_yaml"},
+                {"a"},
+                ["extra_yaml", "NOT in get_connection_form_widgets"],
+                ["NOT in provider.yaml conn-fields"],
+                id="extra-in-yaml",
+            ),
+            # only_in_hook no longer triggers an error — only only_in_yaml does
+            pytest.param(
+                {"a", "only_yaml"},
+                {"a", "only_hook"},
+                [
+                    "only_yaml",
+                    "NOT in get_connection_form_widgets",
+                ],
+                ["only_hook", "NOT in provider.yaml conn-fields"],
+                id="both-sides-only-yaml-reported",
+            ),
+        ],
+    )
+    def test_mismatch_error_content(self, yaml_keys, hook_keys, expected_in_error, not_expected_in_error):
+        error = build_mismatch_error(yaml_keys, hook_keys, CONN_TYPE, YAML_PATH, HOOK_CLASS)
+        assert error is not None
+        for expected in expected_in_error:
+            assert expected in error
+        for not_expected in not_expected_in_error:
+            assert not_expected not in error
+
+
+class TestCheckConnFieldsForEntry:
+    @pytest.mark.parametrize(
+        "conn_fields, get_keys",
+        [
+            pytest.param(["a", "b"], _get_keys("a", "b"), id="matching-keys"),
+            pytest.param([], _get_keys(), id="empty-on-both-sides"),
+            pytest.param(["a"], _skip, id="skip-hook-without-get-connection-form-widgets"),
+            pytest.param(None, _skip, id="skip-missing-conn-fields-when-hook-has-no-widgets"),
+            # Hook with widgets but no conn-fields is allowed: new UI intentionally omits custom fields.
+            pytest.param(None, _get_keys("field_a"), id="no-conn-fields-with-hook-widgets-is-ok"),
+            # Hook has extra keys not in conn-fields — allowed (conn-fields is a valid subset).
+            pytest.param(["a"], _get_keys("a", "extra_hook"), id="hook-extra-keys-no-error"),
+        ],
+    )
+    def test_no_errors(self, conn_fields, get_keys):
+        assert check_conn_fields_for_entry(_entry(conn_fields), YAML_PATH, get_keys) == []
+
+    @pytest.mark.parametrize(
+        "conn_fields, get_keys, expected_in_error",
+        [
+            pytest.param(["a", "extra"], _get_keys("a"), "extra", id="extra-key-in-yaml"),
+            pytest.param(["a"], _raise, "boom", id="unexpected-exception-message"),
+            pytest.param(["a"], _raise, HOOK_CLASS, id="unexpected-exception-mentions-hook-class"),
+        ],
+    )
+    def test_one_error_containing(self, conn_fields, get_keys, expected_in_error):
+        errors = check_conn_fields_for_entry(_entry(conn_fields), YAML_PATH, get_keys)
+        assert len(errors) == 1
+        assert expected_in_error in errors[0]