🎉 CAT: Add allowedHosts and suggestedSteams checks for `metadata.…

…yaml` for connectors `ql >= 400` (#34358)

airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+## 3.3.0
+Add `test_certified_connector_has_allowed_hosts` and `test_certified_connector_has_suggested_streams` tests to the `connector_attribute` test suite
+
 ## 3.2.0
 Add TestBasicRead.test_all_supported_file_types_present, which validates that all supported file types are present in the sandbox account for certified file-based connectors.
 

airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/config.py

@@ -142,6 +142,18 @@ class NoPrimaryKeyConfiguration(BaseConfig):
     bypass_reason: Optional[str] = Field(default=None, description="Reason why this stream does not support a primary key")
 
 
+class AllowedHostsConfiguration(BaseConfig):
+    bypass_reason: Optional[str] = Field(
+        default=None, description="Reason why the Metadata `AllowedHosts` check should be skipped for this certified connector."
+    )
+
+
+class SuggestedStreamsConfiguration(BaseConfig):
+    bypass_reason: Optional[str] = Field(
+        default=None, description="Reason why the Metadata `SuggestedStreams` check should be skipped for this certified connector."
+    )
+
+
 class UnsupportedFileTypeConfig(BaseConfig):
     extension: str
     bypass_reason: Optional[str] = Field(description="Reason why this type is considered unsupported.")
@@ -239,6 +251,12 @@ class ConnectorAttributesConfig(BaseConfig):
     streams_without_primary_key: Optional[List[NoPrimaryKeyConfiguration]] = Field(
         description="Streams that do not support a primary key such as reports streams"
     )
+    allowed_hosts: Optional[AllowedHostsConfiguration] = Field(
+        description="Used to bypass checking the `allowedHosts` field in a source's `metadata.yaml` when all external hosts should be reachable."
+    )
+    suggested_streams: Optional[SuggestedStreamsConfiguration] = Field(
+        description="Used to bypass checking the `suggestedStreams` field in a source's `metadata.yaml` when certified source doesn't have any."
+    )
 
 
 class GenericTestConfig(GenericModel, Generic[TestConfigT]):

airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py

@@ -30,6 +30,7 @@
 )
 from connector_acceptance_test.base import BaseTest
 from connector_acceptance_test.config import (
+    AllowedHostsConfiguration,
     BasicReadTestConfig,
     Config,
     ConnectionTestConfig,
@@ -1233,7 +1234,9 @@ async def test_all_supported_file_types_present(self, certified_file_based_conne
 
 @pytest.mark.default_timeout(TEN_MINUTES)
 class TestConnectorAttributes(BaseTest):
-    MANDATORY_FOR_TEST_STRICTNESS_LEVELS = []  # Used so that this is not part of the mandatory high strictness test suite yet
+    # Overide from BaseTest!
+    # Used so that this is not part of the mandatory high strictness test suite yet
+    MANDATORY_FOR_TEST_STRICTNESS_LEVELS = []
 
     @pytest.fixture(name="operational_certification_test")
     async def operational_certification_test_fixture(self, connector_metadata: dict) -> bool:
@@ -1252,7 +1255,7 @@ def streams_without_primary_key_fixture(self, inputs: ConnectorAttributesConfig)
 
     async def test_streams_define_primary_key(
         self, operational_certification_test, streams_without_primary_key, connector_config, docker_runner: ConnectorRunner
-    ):
+    ) -> None:
         output = await docker_runner.call_discover(config=connector_config)
         catalog_messages = filter_output(output, Type.CATALOG)
         streams = catalog_messages[0].catalog.streams
@@ -1261,3 +1264,72 @@ async def test_streams_define_primary_key(
 
         quoted_missing_primary_keys = {f"'{primary_key}'" for primary_key in missing_primary_keys}
         assert not missing_primary_keys, f"The following streams {', '.join(quoted_missing_primary_keys)} do not define a primary_key"
+
+    @pytest.fixture(name="allowed_hosts_test")
+    def allowed_hosts_fixture_test(self, inputs: ConnectorAttributesConfig) -> bool:
+        allowed_hosts = inputs.allowed_hosts
+        bypass_reason = allowed_hosts.bypass_reason if allowed_hosts else None
+        if bypass_reason:
+            pytest.skip(f"Skipping `metadata.allowedHosts` checks. Reason: {bypass_reason}")
+        return True
+
+    async def test_certified_connector_has_allowed_hosts(
+        self, operational_certification_test, allowed_hosts_test, connector_metadata: dict
+    ) -> None:
+        """
+        Checks whether or not the connector has `allowedHosts` and it's components defined in `metadata.yaml`.
+        Suitable for certified connectors starting `ql` >= 400.
+
+        Arguments:
+            :: operational_certification_test -- pytest.fixure defines the connector is suitable for this test or not.
+            :: connector_metadata -- `metadata.yaml` file content
+        """
+        metadata = connector_metadata.get("data", {})
+
+        has_allowed_hosts_property = "allowedHosts" in metadata.keys()
+        assert has_allowed_hosts_property, f"The `allowedHosts` property is missing in `metadata.data` for `metadata.yaml`."
+
+        allowed_hosts = metadata.get("allowedHosts", {})
+        has_hosts_property = "hosts" in allowed_hosts.keys() if allowed_hosts else False
+        assert has_hosts_property, f"The `hosts` property is missing in `metadata.data.allowedHosts` for `metadata.yaml`."
+
+        hosts = allowed_hosts.get("hosts", [])
+        has_assigned_hosts = len(hosts) > 0 if hosts else False
+        assert (
+            has_assigned_hosts
+        ), f"The `hosts` empty list is not allowed for `metadata.data.allowedHosts` for certified connectors. Please add `hosts` or define the `allowed_hosts.bypass_reason` in `acceptance-test-config.yaml`."
+
+    @pytest.fixture(name="suggested_streams_test")
+    def suggested_streams_fixture_test(self, inputs: ConnectorAttributesConfig) -> bool:
+        suggested_streams = inputs.suggested_streams
+        bypass_reason = suggested_streams.bypass_reason if suggested_streams else None
+        if bypass_reason:
+            pytest.skip(f"Skipping `metadata.suggestedStreams` checks. Reason: {bypass_reason}")
+        return True
+
+    async def test_certified_connector_has_suggested_streams(
+        self, operational_certification_test, suggested_streams_test, connector_metadata: dict
+    ) -> None:
+        """
+        Checks whether or not the connector has `suggestedStreams` and it's components defined in `metadata.yaml`.
+        Suitable for certified connectors starting `ql` >= 400.
+
+        Arguments:
+            :: operational_certification_test -- pytest.fixure defines the connector is suitable for this test or not.
+            :: connector_metadata -- `metadata.yaml` file content
+        """
+
+        metadata = connector_metadata.get("data", {})
+
+        has_suggested_streams_property = "suggestedStreams" in metadata.keys()
+        assert has_suggested_streams_property, f"The `suggestedStreams` property is missing in `metadata.data` for `metadata.yaml`."
+
+        suggested_streams = metadata.get("suggestedStreams", {})
+        has_streams_property = "streams" in suggested_streams.keys() if suggested_streams else False
+        assert has_streams_property, f"The `streams` property is missing in `metadata.data.suggestedStreams` for `metadata.yaml`."
+
+        streams = suggested_streams.get("streams", [])
+        has_assigned_suggested_streams = len(streams) > 0 if streams else False
+        assert (
+            has_assigned_suggested_streams
+        ), f"The `streams` empty list is not allowed for `metadata.data.suggestedStreams` for certified connectors."

airbyte-integrations/bases/connector-acceptance-test/unit_tests/test_connector_attributes.py

@@ -75,3 +75,103 @@ async def test_streams_define_primary_key(mocker, stream_configs, excluded_strea
             connector_config={},
             docker_runner=docker_runner_mock
         )
+
+
+@pytest.mark.parametrize(
+    "metadata_yaml, should_raise_assert_error, expected_error",
+    [
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}}},
+            True,
+            "The `allowedHosts` property is missing in `metadata.data` for `metadata.yaml`",
+        ),
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}, "allowedHosts": {}}},
+            True,
+            "The `hosts` property is missing in `metadata.data.allowedHosts` for `metadata.yaml`",
+        ),
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}, "allowedHosts": {"hosts": []}}},
+            True,
+            "'The `hosts` empty list is not allowed for `metadata.data.allowedHosts` for certified connectors",
+        ),
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}, "allowedHosts": {"hosts": ["*.test.com"]}}},
+            False,
+            None,
+        ),
+    ],
+    ids=[
+        "No `allowedHosts`",
+        "Has `allowdHosts` but no `hosts`",
+        "Has `hosts` but it's empty list",
+        "Has non-empty `hosts`",
+    ]
+)
+async def test_certified_connector_has_allowed_hosts(metadata_yaml, should_raise_assert_error, expected_error) -> None:
+    t = test_core.TestConnectorAttributes()
+
+    if should_raise_assert_error:
+        with pytest.raises(AssertionError) as e:
+            await t.test_certified_connector_has_allowed_hosts(
+                operational_certification_test=True,
+                allowed_hosts_test=True,
+                connector_metadata=metadata_yaml
+            )
+        assert expected_error in repr(e.value)
+    else:
+        await t.test_certified_connector_has_allowed_hosts(
+            operational_certification_test=True,
+            allowed_hosts_test=True,
+            connector_metadata=metadata_yaml
+        )
+
+
+@pytest.mark.parametrize(
+    "metadata_yaml, should_raise_assert_error, expected_error",
+    [
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}}},
+            True,
+            "The `suggestedStreams` property is missing in `metadata.data` for `metadata.yaml`",
+        ),
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}, "suggestedStreams": {}}},
+            True,
+            "The `streams` property is missing in `metadata.data.suggestedStreams` for `metadata.yaml`",
+        ),
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}, "suggestedStreams": {"streams": []}}},
+            True,
+            "'The `streams` empty list is not allowed for `metadata.data.suggestedStreams` for certified connectors",
+        ),
+        pytest.param(
+            {"data": {"ab_internal": {"ql": 400}, "suggestedStreams": {"streams": ["stream_1", "stream_2"]}}},
+            False,
+            None,
+        ),
+    ],
+    ids=[
+        "No `suggestedStreams`",
+        "Has `suggestedStreams` but no `streams`",
+        "Has `streams` but it's empty list",
+        "Has non-empty `streams`",
+    ]
+)
+async def test_certified_connector_has_suggested_streams(metadata_yaml, should_raise_assert_error, expected_error) -> None:
+    t = test_core.TestConnectorAttributes()
+
+    if should_raise_assert_error:
+        with pytest.raises(AssertionError) as e:
+            await t.test_certified_connector_has_suggested_streams(
+                operational_certification_test=True,
+                suggested_streams_test=True,
+                connector_metadata=metadata_yaml
+            )
+        assert expected_error in repr(e.value)
+    else:
+        await t.test_certified_connector_has_suggested_streams(
+            operational_certification_test=True,
+            suggested_streams_test=True,
+            connector_metadata=metadata_yaml
+        )

docs/connector-development/testing-connectors/connector-acceptance-tests-reference.md

@@ -298,6 +298,10 @@ Some examples of the types of tests covered are verification that streams define
 |:------------------------------------------|:-----------------|:----------------------|:-----------------------------------------------------------------------|
 | `config_path`                             | string           | `secrets/config.json` | Path to a JSON object representing a valid connector configuration     |
 | `streams_without_primary_key`             | array of objects | None                  | List of streams that do not support a primary key like reports streams |
+| `streams_without_primary_key.name`             | string | None                  | Name of the stream missing the PK |
+| `streams_without_primary_key.bypass_reason`             | string | None                  | The reason the stream doesn't have the PK |
+| `allowed_hosts.bypass_reason`             | object with `bypass_reason` | None                  | Defines the `bypass_reason` description about why the `allowedHosts` check for the certified connector should be skipped |
+| `suggested_streams.bypass_reason`             | object with `bypass_reason` | None                  | Defines the `bypass_reason` description about why the `suggestedStreams` check for the certified connector should be skipped |
 
 ## Strictness level