fix(ingestion): replace class-name allowlist with module-path check for hostPort stripping

- Replace _HOSTPORT_URL_ALLOWED frozenset (only 2 entries) with
  _DATABASE_CONNECTION_MODULE_MARKER that checks the class module path.
  Only classes under .services.connections.database. are subject to
  scheme stripping; dashboard, pipeline, metadata, and all other connector
  categories pass through unchanged because their hostPort legitimately
  holds a full URL (e.g. http://grafana:3000).

- Add isinstance(host_port, str) guard in model_post_init to prevent
  AttributeError when hostPort is Optional and left unset (None).

- Update test_connection_builders.py: replace _HOSTPORT_URL_ALLOWED import
  with _DATABASE_CONNECTION_MODULE_MARKER, rename and update the allowlist
  test to reflect the new module-path strategy, and add a regression test
  for the None hostPort crash (CassandraConnection with default hostPort).

Fixes gitar-bot Bug 1 (None crash) and Bug 2 (incomplete class-name
allowlist that failed to cover non-database URL-based connectors).

Author: RajdeepKushwaha5

ingestion/src/metadata/ingestion/connections/builders.py

@@ -12,6 +12,7 @@
 """
 Get and test connection utilities
 """
+
 from functools import partial
 from typing import Any, Callable, Dict, Optional
 from urllib.parse import quote_plus

ingestion/src/metadata/ingestion/models/custom_pydantic.py

@@ -15,6 +15,7 @@
 dependencies against any other metadata package. This class should
 be self-sufficient with only pydantic at import time.
 """
+
 import json
 import logging
 from typing import Any, Callable, Dict, Literal, Optional, Union
@@ -122,12 +123,12 @@ def strip_hostport_scheme(raw: str) -> str:
 # pinned to the original underscored symbol while the helper was private.
 _strip_hostport_scheme = strip_hostport_scheme
 
-# Connection classes whose hostPort field legitimately stores a full URL
-# (e.g. "http://airflow:8080" or "http://localhost:8585/api") and must NOT
-# have their scheme stripped by model_post_init.
-_HOSTPORT_URL_ALLOWED: frozenset = frozenset(
-    {"AirflowConnection", "OpenMetadataConnection"}
-)
+# Module sub-path that identifies database-connector classes.
+# Only *Connection classes whose module contains this sub-path have a plain
+# ``hostname[:port]`` hostPort — all other connection categories
+# (dashboard, pipeline, search, metadata, …) legitimately store a full URL
+# in hostPort and must NOT have their scheme stripped.
+_DATABASE_CONNECTION_MODULE_MARKER = ".services.connections.database."
 
 
 class BaseModel(PydanticBaseModel):
@@ -154,16 +155,21 @@ def model_post_init(self, context: Any, /):
         if not hasattr(self, "__pydantic_fields__"):
             return
 
-        # Strip accidental URL schemes from hostPort at model construction time.
-        # This protects connectors that split hostPort manually (e.g. Cassandra,
-        # Databricks, Redshift, DB2, Microsoft Fabric) without going through
-        # get_connection_url_common. ValueError for genuinely invalid inputs
-        # (e.g. non-numeric port) propagates immediately with a clear message.
+        # Strip accidental URL schemes from hostPort at model construction time,
+        # but ONLY for database-connector classes whose hostPort is a plain
+        # ``hostname[:port]``.  Dashboard, pipeline, search, and other connector
+        # categories legitimately use a full URL in hostPort (e.g.
+        # ``http://grafana:3000``), so we restrict stripping to classes whose
+        # module path contains the database-connection marker.
+        # The isinstance(str) guard prevents an AttributeError when hostPort is
+        # optional and left unset (None).
+        host_port = getattr(self, "hostPort", None)
         if (
-            hasattr(self, "hostPort")
-            and self.__class__.__name__ not in _HOSTPORT_URL_ALLOWED
+            isinstance(host_port, str)
+            and "://" in host_port
+            and _DATABASE_CONNECTION_MODULE_MARKER in (self.__class__.__module__ or "")
         ):
-            self.hostPort = strip_hostport_scheme(self.hostPort)
+            self.hostPort = strip_hostport_scheme(host_port)
 
         try:
             for field in self.__pydantic_fields__:

ingestion/tests/unit/test_connection_builders.py

@@ -11,11 +11,15 @@
 """
 Validate connection builder utilities
 """
+
 from unittest import TestCase
 
 from metadata.generated.schema.entity.services.connections.database.common.basicAuth import (
     BasicAuth,
 )
+from metadata.generated.schema.entity.services.connections.database.cassandraConnection import (
+    CassandraConnection,
+)
 from metadata.generated.schema.entity.services.connections.database.mysqlConnection import (
     MysqlConnection,
 )
@@ -27,7 +31,7 @@
     get_connection_options_dict,
     init_empty_connection_arguments,
 )
-from metadata.ingestion.models.custom_pydantic import _HOSTPORT_URL_ALLOWED
+from metadata.ingestion.models.custom_pydantic import _DATABASE_CONNECTION_MODULE_MARKER
 
 
 class ConnectionBuilderTest(TestCase):
@@ -127,20 +131,34 @@ def test_model_post_init_raises_for_invalid_port_in_hostport(self):
                 hostPort="http://localhost:abc",
             )
 
-    def test_url_allowed_connections_not_stripped(self):
+    def test_non_database_connections_not_stripped(self):
         """
-        Verify that connection classes in _HOSTPORT_URL_ALLOWED (AirflowConnection
-        and OpenMetadataConnection) are excluded from hostPort scheme stripping,
-        since their hostPort field legitimately stores a full URL.
+        Verify that non-database connection classes (metadata, dashboard, pipeline, …)
+        are NOT subject to hostPort scheme stripping because their hostPort legitimately
+        stores a full URL (e.g. OpenMetadataConnection hostPort = http://localhost:8585/api).
+
+        The guard uses the module path: only classes whose __module__ contains
+        _DATABASE_CONNECTION_MODULE_MARKER ('.services.connections.database.')
+        are stripped.  All others pass through unchanged.
         """
-        # Confirm the exclusion set contains the expected class names
-        self.assertIn("AirflowConnection", _HOSTPORT_URL_ALLOWED)
-        self.assertIn("OpenMetadataConnection", _HOSTPORT_URL_ALLOWED)
+        # Confirm the marker is correct
+        self.assertIn("database", _DATABASE_CONNECTION_MODULE_MARKER)
 
-        # OpenMetadataConnection.hostPort is a plain str field that expects a
-        # full URL like "http://localhost:8585/api" — it must NOT be stripped.
+        # OpenMetadataConnection.hostPort is a plain str that expects a full URL.
+        # Its module contains '.connections.metadata.' — NOT the database marker —
+        # so it must NOT be stripped.
         ometa = OpenMetadataConnection(
             hostPort="http://localhost:8585/api",
         )
         self.assertIn("http", ometa.hostPort)
         self.assertIn("localhost", ometa.hostPort)
+
+    def test_none_hostport_does_not_crash(self):
+        """
+        Regression test for gitar-bot bug report: constructing a database
+        connection where hostPort is Optional and left as None must NOT raise
+        AttributeError from strip_hostport_scheme(None).
+        """
+        # CassandraConnection.hostPort is Optional[str] = None by default.
+        conn = CassandraConnection()
+        self.assertIsNone(conn.hostPort)