ci-connector-ops: declare review requirements on source connectors ch…

…anges (#20303)

.github/.gitignore

@@ -0,0 +1 @@
+connector_org_review_requirements.yaml

.github/workflows/connector_ops_ci.yml

@@ -1,9 +1,11 @@
 name: Connector Ops CI
 
 on:
+  workflow_dispatch:
   pull_request:
     paths:
       - "airbyte-integrations/connectors/source-**"
+  pull_request_review:
 jobs:
   test-strictness-level:
     name: "Check test strictness level"
@@ -21,3 +23,33 @@ jobs:
         run: pip install --quiet -e ./tools/ci_connector_ops
       - name: Check test strictness level
         run: check-test-strictness-level
+  check-review-requirements:
+    name: "Check if a review is required from Connector teams"
+    runs-on: ubuntu-latest
+
+    if: ${{ github.repository == 'airbytehq/airbyte' }}
+    steps:
+      - name: Checkout Airbyte
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+      - name: Install Python
+        uses: actions/setup-python@v4
+        with:
+          python-version: "3.9"
+      - name: Install ci-connector-ops package
+        run: pip install --quiet -e ./tools/ci_connector_ops
+      - name: Write review requirements file
+        id: write-review-requirements-file
+        run: write-review-requirements-file >> $GITHUB_OUTPUT
+      - name: Get mandatory reviewers
+        id: get-mandatory-reviewers
+        run: print-mandatory-reviewers >> $GITHUB_OUTPUT
+      - name: Check if the review requirements are met
+        if: steps.write-review-requirements-file.outputs.CREATED_REQUIREMENTS_FILE == true
+        uses: Automattic/action-required-review@v3
+        with:
+          status: ${{ steps.get-mandatory-reviewers.outputs.MANDATORY_REVIEWERS  }}
+          token: ${{ secrets.OCTAVIA_4_ROOT_ACCESS }}
+          requirements-file: .github/connector_org_review_requirements.yaml
+

tools/ci_connector_ops/ci_connector_ops/sat_config_checks.py

@@ -0,0 +1,99 @@
+#
+# Copyright (c) 2022 Airbyte, Inc., all rights reserved.
+#
+
+import logging
+import sys
+from typing import List, Dict, Union
+import yaml
+
+from ci_connector_ops import utils
+
+RELEASE_STAGE_TO_STRICTNESS_LEVEL_MAPPING = {"generally_available": "high"}
+BACKWARD_COMPATIBILITY_REVIEWERS = {"connector-operations", "connector-extensibility"}
+TEST_STRICTNESS_LEVEL_REVIEWERS = {"connector-operations"}
+GA_CONNECTOR_REVIEWERS = {"gl-python"}
+REVIEW_REQUIREMENTS_FILE_PATH = ".github/connector_org_review_requirements.yaml"
+
+def find_connectors_with_bad_strictness_level() -> List[str]:
+    """Check if changed connectors have the expected SAT test strictness level according to their release stage.
+    1. Identify changed connectors
+    2. Retrieve their release stage from the catalog
+    3. Parse their acceptance test config file
+    4. Check if the test strictness level matches the strictness level expected for their release stage.
+
+    Returns:
+        List[str]: List of changed connector names that are not matching test strictness level expectations.
+    """
+    connectors_with_bad_strictness_level = []
+    changed_connector_names = utils.get_changed_connector_names()
+    for connector_name in changed_connector_names:
+        connector_release_stage = utils.get_connector_release_stage(connector_name)
+        expected_test_strictness_level = RELEASE_STAGE_TO_STRICTNESS_LEVEL_MAPPING.get(connector_release_stage)
+        _, acceptance_test_config = utils.get_acceptance_test_config(connector_name)
+        can_check_strictness_level = all(
+            [item is not None for item in [connector_release_stage, expected_test_strictness_level, acceptance_test_config]]
+        )
+        if can_check_strictness_level:
+            try:
+                assert acceptance_test_config.get("test_strictness_level") == expected_test_strictness_level
+            except AssertionError:
+                connectors_with_bad_strictness_level.append(connector_name)
+    return connectors_with_bad_strictness_level
+
+def find_changed_ga_connectors() -> List[str]:
+    """Find GA connectors modified on the current branch.
+
+    Returns:
+        List[str]: The list of GA connector that were modified on the current branch.
+    """
+    changed_connector_names = utils.get_changed_connector_names()
+    return [connector_name for connector_name in changed_connector_names if utils.get_connector_release_stage(connector_name) == "generally_available"]
+
+def find_mandatory_reviewers() -> List[Union[str, Dict[str, List]]]:
+    teams = []
+    ga_connector_changes = find_changed_ga_connectors()
+    backward_compatibility_changes = utils.get_changed_acceptance_test_config(diff_regex="disable_for_version")
+    test_strictness_level_changes = utils.get_changed_acceptance_test_config(diff_regex="test_strictness_level")
+    if ga_connector_changes:
+        teams += GA_CONNECTOR_REVIEWERS
+    if backward_compatibility_changes:
+        teams += [{"any-of": list(BACKWARD_COMPATIBILITY_REVIEWERS)}]
+    if test_strictness_level_changes:
+        teams += [{"any-of": list(TEST_STRICTNESS_LEVEL_REVIEWERS)}]
+    return teams
+
+def check_test_strictness_level():
+    connectors_with_bad_strictness_level = find_connectors_with_bad_strictness_level()
+    if connectors_with_bad_strictness_level:
+        logging.error(
+            f"The following GA connectors must enable high test strictness level: {connectors_with_bad_strictness_level}. Please check this documentation for details: https://docs.airbyte.com/connector-development/testing-connectors/source-acceptance-tests-reference/#strictness-level"
+        )
+        sys.exit(1)
+    else:
+        sys.exit(0)
+
+def write_review_requirements_file():
+    mandatory_reviewers = find_mandatory_reviewers()
+
+    if mandatory_reviewers:
+        requirements_file_content = [{
+            "name": "Required reviewers from the connector org teams",
+            "paths": "unmatched",
+            "teams": mandatory_reviewers
+        }]
+        with open(REVIEW_REQUIREMENTS_FILE_PATH, "w") as requirements_file:
+            yaml.safe_dump(requirements_file_content, requirements_file)
+        print("CREATED_REQUIREMENTS_FILE=true")
+    else:
+        print("CREATED_REQUIREMENTS_FILE=false")
+
+def print_mandatory_reviewers():
+    teams = []
+    mandatory_reviewers = find_mandatory_reviewers()
+    for mandatory_reviewer in mandatory_reviewers:
+        if isinstance(mandatory_reviewer, dict):
+            teams += mandatory_reviewer["any-of"]
+        else:
+            teams.append(mandatory_reviewer)
+    print(f"MANDATORY_REVIEWERS=A review is required from these teams: {','.join(teams)}")

tools/ci_connector_ops/ci_connector_ops/utils.py

@@ -35,6 +35,8 @@ def read_definitions(definitions_file_path: str) -> Dict:
     with open(definitions_file_path) as definitions_file:
         return yaml.safe_load(definitions_file)
 
+def get_connector_name_from_path(path):
+    return path.split("/")[2]
 
 def get_changed_connector_names() -> Set[str]:
     """Retrieve a list of connector names that were changed in the current branch (compared to master).
@@ -44,15 +46,32 @@ def get_changed_connector_names() -> Set[str]:
     """
     changed_source_connector_files = {
         file_path
-        for file_path in AIRBYTE_REPO.git.diff("--name-only", "origin/master...").split("\n")
+        for file_path in AIRBYTE_REPO.git.diff("--name-only", "origin/master").split("\n")
         if file_path.startswith(SOURCE_CONNECTOR_PATH_PREFIX)
     }
 
-    def get_connector_name_from_path(path):
-        return path.split("/")[2]
-
     return {get_connector_name_from_path(changed_file) for changed_file in changed_source_connector_files}
 
+def get_changed_acceptance_test_config(diff_regex: Optional[str]=None) -> Set[str]:
+    """Retrieve a list of connector names for which the acceptance_test_config file was changed in the current branch (compared to master).
+
+    Args:
+        diff_regex (str): Find the edited files that contain the following regex in their change.
+
+    Returns:
+        Set[str]: Set of connector names e.g {"source-pokeapi"}
+    """
+    if diff_regex is None:
+        diff_command_args = ("--name-only", "origin/master")
+    else:
+        diff_command_args = ("--name-only", f'-G{diff_regex}', "origin/master")
+
+    changed_acceptance_test_config_paths = {
+        file_path
+        for file_path in AIRBYTE_REPO.git.diff(*diff_command_args).split("\n")
+        if file_path.startswith(SOURCE_CONNECTOR_PATH_PREFIX) and file_path.endswith(ACCEPTANCE_TEST_CONFIG_FILE_NAME)
+    }
+    return {get_connector_name_from_path(changed_file) for changed_file in changed_acceptance_test_config_paths}
 
 def get_connector_definition(connector_name: str) -> Optional[Dict]:
     """Find a connector definition from the catalog.

tools/ci_connector_ops/setup.py

@@ -9,7 +9,7 @@
 
 
 setup(
-    version="0.1.0",
+    version="0.1.1",
     name="ci_connector_ops",
     description="Packaged maintained by the connector operations team to perform CI for connectors",
     author="Airbyte",
@@ -19,7 +19,9 @@
     python_requires=">=3.9",
     entry_points={
         "console_scripts": [
-            "check-test-strictness-level = ci_connector_ops.check_test_strictness_level:main",
+            "check-test-strictness-level = ci_connector_ops.sat_config_checks:check_test_strictness_level",
+            "write-review-requirements-file = ci_connector_ops.sat_config_checks:write_review_requirements_file",
+            "print-mandatory-reviewers = ci_connector_ops.sat_config_checks:print_mandatory_reviewers"
         ],
     },
 )

tools/ci_connector_ops/tests/test_sat_config_checks.py

@@ -0,0 +1,111 @@
+#
+# Copyright (c) 2022 Airbyte, Inc., all rights reserved.
+#
+
+import shutil
+from typing import List
+import yaml
+
+import pytest
+
+from ci_connector_ops import sat_config_checks
+
+@pytest.fixture
+def pokeapi_acceptance_test_config_path():
+    return "airbyte-integrations/connectors/source-pokeapi/acceptance-test-config.yml"
+
+@pytest.fixture
+def ga_connector_file():
+    return "airbyte-integrations/connectors/source-amplitude/acceptance-test-config.yml"
+
+@pytest.fixture
+def not_ga_backward_compatibility_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path) -> List:
+    expected_teams = [{"any-of": list(sat_config_checks.BACKWARD_COMPATIBILITY_REVIEWERS)}]
+    backup_path = tmp_path / "backup_poke_acceptance"
+    shutil.copyfile(pokeapi_acceptance_test_config_path, backup_path)
+    with open(pokeapi_acceptance_test_config_path, "a") as acceptance_test_config_file:
+        acceptance_test_config_file.write("disable_for_version: 0.0.0")
+    yield expected_teams
+    shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path)
+
+@pytest.fixture
+def not_ga_test_strictness_level_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path) -> List:
+    expected_teams = [{"any-of": list(sat_config_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)}]
+    backup_path = tmp_path / "non_ga_acceptance_test_config.backup"
+    shutil.copyfile(pokeapi_acceptance_test_config_path, backup_path)
+    with open(pokeapi_acceptance_test_config_path, "a") as acceptance_test_config_file:
+        acceptance_test_config_file.write("test_strictness_level: foo")
+    yield expected_teams
+    shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path)
+
+@pytest.fixture
+def not_ga_not_tracked_change_expected_team(tmp_path, pokeapi_acceptance_test_config_path):
+    expected_teams = []
+    backup_path = tmp_path / "non_ga_acceptance_test_config.backup"
+    shutil.copyfile(pokeapi_acceptance_test_config_path, backup_path)
+    with open(pokeapi_acceptance_test_config_path, "a") as acceptance_test_config_file:
+        acceptance_test_config_file.write("not_tracked")
+    yield expected_teams
+    shutil.copyfile(backup_path, pokeapi_acceptance_test_config_path)
+
+
+@pytest.fixture
+def ga_connector_file_change_expected_team(tmp_path, ga_connector_file):
+    expected_teams = list(sat_config_checks.GA_CONNECTOR_REVIEWERS)
+    backup_path = tmp_path / "ga_acceptance_test_config.backup"
+    shutil.copyfile(ga_connector_file, backup_path)
+    with open(ga_connector_file, "a") as ga_acceptance_test_config_file:
+        ga_acceptance_test_config_file.write("foobar")
+    yield expected_teams
+    shutil.copyfile(backup_path, ga_connector_file)
+
+@pytest.fixture
+def ga_connector_backward_compatibility_file_change(tmp_path, ga_connector_file):
+    expected_teams = list(sat_config_checks.GA_CONNECTOR_REVIEWERS) + [{"any-of": list(sat_config_checks.BACKWARD_COMPATIBILITY_REVIEWERS)}]
+    backup_path = tmp_path / "ga_acceptance_test_config.backup"
+    shutil.copyfile(ga_connector_file, backup_path)
+    with open(ga_connector_file, "a") as ga_acceptance_test_config_file:
+        ga_acceptance_test_config_file.write("disable_for_version: 0.0.0")
+    yield expected_teams
+    shutil.copyfile(backup_path, ga_connector_file)
+
+@pytest.fixture
+def ga_connector_test_strictness_level_file_change(tmp_path, ga_connector_file):
+    expected_teams = list(sat_config_checks.GA_CONNECTOR_REVIEWERS) + [{"any-of": list(sat_config_checks.TEST_STRICTNESS_LEVEL_REVIEWERS)}]
+    backup_path = tmp_path / "ga_acceptance_test_config.backup"
+    shutil.copyfile(ga_connector_file, backup_path)
+    with open(ga_connector_file, "a") as ga_acceptance_test_config_file:
+        ga_acceptance_test_config_file.write("test_strictness_level: 0.0.0")
+    yield expected_teams
+    shutil.copyfile(backup_path, ga_connector_file)
+
+def check_review_requirements_file_contains_expected_teams(capsys, expected_teams: List):
+    sat_config_checks.write_review_requirements_file()
+    captured = capsys.readouterr()
+    assert captured.out.split("\n")[0].split("=")[-1] == "true"
+    requirements_file_path = sat_config_checks.REVIEW_REQUIREMENTS_FILE_PATH
+    with open(requirements_file_path, "r") as requirements_file:
+        requirements = yaml.safe_load(requirements_file)
+    assert requirements[0]["teams"] == expected_teams
+
+def test_find_mandatory_reviewers_backward_compatibility(capsys, not_ga_backward_compatibility_change_expected_team):
+    check_review_requirements_file_contains_expected_teams(capsys, not_ga_backward_compatibility_change_expected_team)
+
+
+def test_find_mandatory_reviewers_test_strictness_level(capsys, not_ga_test_strictness_level_change_expected_team):
+    check_review_requirements_file_contains_expected_teams(capsys, not_ga_test_strictness_level_change_expected_team)
+
+
+def test_find_mandatory_reviewers_ga(capsys, ga_connector_file_change_expected_team):
+    check_review_requirements_file_contains_expected_teams(capsys, ga_connector_file_change_expected_team)
+
+def test_find_mandatory_reviewers_ga_backward_compatibility(capsys, ga_connector_backward_compatibility_file_change):
+    check_review_requirements_file_contains_expected_teams(capsys, ga_connector_backward_compatibility_file_change)
+
+def test_find_mandatory_reviewers_ga_test_strictness_level(capsys, ga_connector_test_strictness_level_file_change):
+    check_review_requirements_file_contains_expected_teams(capsys, ga_connector_test_strictness_level_file_change)
+
+def test_find_mandatory_reviewers_no_tracked_changed(capsys, not_ga_not_tracked_change_expected_team):
+    sat_config_checks.write_review_requirements_file()
+    captured = capsys.readouterr()
+    assert captured.out.split("\n")[0].split("=")[-1] == "false"

tools/ci_connector_ops/tests/test_utils.py

@@ -0,0 +1,16 @@
+#
+# Copyright (c) 2022 Airbyte, Inc., all rights reserved.
+#
+
+from ci_connector_ops import utils
+
+def test_get_connector_definition():
+    assert utils.get_connector_definition("source-dynamodb") == {
+        "name": "DynamoDB",
+        "sourceDefinitionId": "50401137-8871-4c5a-abb7-1f5fda35545a",
+        "dockerRepository": "airbyte/source-dynamodb",
+        "dockerImageTag": "0.1.0",
+        "documentationUrl": "https://docs.airbyte.com/integrations/sources/dynamodb",
+        "sourceType": "api",
+        "releaseStage": "alpha"
+    }