diff --git a/.github/.gitignore b/.github/.gitignore new file mode 100644 index 000000000000..c7d7b18d1888 --- /dev/null +++ b/.github/.gitignore @@ -0,0 +1 @@ +connector_org_review_requirements.yaml diff --git a/.github/workflows/connector_ops_ci.yml b/.github/workflows/connector_ops_ci.yml index d5efdd9f8a61..3dbe1ff61552 100644 --- a/.github/workflows/connector_ops_ci.yml +++ b/.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 + diff --git a/tools/ci_connector_ops/ci_connector_ops/check_test_strictness_level.py b/tools/ci_connector_ops/ci_connector_ops/check_test_strictness_level.py deleted file mode 100644 index 7d6bab332364..000000000000 --- a/tools/ci_connector_ops/ci_connector_ops/check_test_strictness_level.py +++ /dev/null @@ -1,53 +0,0 @@ -# -# Copyright (c) 2022 Airbyte, Inc., all rights reserved. -# - -import logging -import sys -from typing import List - -from ci_connector_ops import utils - -RELEASE_STAGE_TO_STRICTNESS_LEVEL_MAPPING = {"generally_available": "high"} - - -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 main(): - 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) - - -if __name__ == "__main__": - main() diff --git a/tools/ci_connector_ops/ci_connector_ops/sat_config_checks.py b/tools/ci_connector_ops/ci_connector_ops/sat_config_checks.py new file mode 100644 index 000000000000..8b847d56bb2f --- /dev/null +++ b/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)}") diff --git a/tools/ci_connector_ops/ci_connector_ops/utils.py b/tools/ci_connector_ops/ci_connector_ops/utils.py index d8bff08c2372..161e20e2eaad 100644 --- a/tools/ci_connector_ops/ci_connector_ops/utils.py +++ b/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. diff --git a/tools/ci_connector_ops/setup.py b/tools/ci_connector_ops/setup.py index e34088a63c71..09928f800c24 100644 --- a/tools/ci_connector_ops/setup.py +++ b/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" ], }, ) diff --git a/tools/ci_connector_ops/tests/test_sat_config_checks.py b/tools/ci_connector_ops/tests/test_sat_config_checks.py new file mode 100644 index 000000000000..8a73fb98b1e2 --- /dev/null +++ b/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" diff --git a/tools/ci_connector_ops/tests/test_utils.py b/tools/ci_connector_ops/tests/test_utils.py new file mode 100644 index 000000000000..b3f85a13717b --- /dev/null +++ b/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" + }