diff --git a/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md index 6372447f1ac..4129791cf73 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md +++ b/airbyte-integrations/bases/connector-acceptance-test/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 0.11.1 +Test connector image labels and make sure they are set correctly and match metadata.yaml. + ## 0.11.0 Add backward_compatibility.check_if_field_removed test to check if a field has been removed from the catalog. diff --git a/airbyte-integrations/bases/connector-acceptance-test/Dockerfile b/airbyte-integrations/bases/connector-acceptance-test/Dockerfile index ced33b85825..b90d29a5d04 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/Dockerfile +++ b/airbyte-integrations/bases/connector-acceptance-test/Dockerfile @@ -33,7 +33,7 @@ COPY pytest.ini setup.py ./ COPY connector_acceptance_test ./connector_acceptance_test RUN pip install . -LABEL io.airbyte.version=0.11.0 +LABEL io.airbyte.version=0.11.1 LABEL io.airbyte.name=airbyte/connector-acceptance-test ENTRYPOINT ["python", "-m", "pytest", "-p", "connector_acceptance_test.plugin", "-r", "fEsx"] diff --git a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/conftest.py b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/conftest.py index 0dddf960dad..d077c08539a 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/conftest.py +++ b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/conftest.py @@ -396,3 +396,8 @@ def pytest_sessionfinish(session, exitstatus): except Exception as e: logger.info(e) # debug pass + + +@pytest.fixture(name="connector_metadata") +def connector_metadata_fixture(base_path) -> dict: + return load_yaml_or_json_path(base_path / "metadata.yaml") diff --git a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py index 17a1ff606cc..54b4e75d7c0 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py +++ b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/tests/test_core.py @@ -82,7 +82,7 @@ DATE_PATTERN = "^[0-9]{2}-[0-9]{2}-[0-9]{4}$" DATETIME_PATTERN = "^[0-9]{4}-[0-9]{2}-[0-9]{2}(T[0-9]{2}:[0-9]{2}:[0-9]{2})?$" -@pytest.mark.default_timeout(10) +@pytest.mark.default_timeout(30) class TestSpec(BaseTest): spec_cache: ConnectorSpecification = None @@ -126,13 +126,6 @@ class TestSpec(BaseTest): if connector_spec: assert actual_connector_spec == connector_spec, "Spec should be equal to the one in spec.yaml or spec.json file" - def test_docker_env(self, actual_connector_spec: ConnectorSpecification, docker_runner: ConnectorRunner): - """Check that connector's docker image has required envs""" - assert docker_runner.env_variables.get("AIRBYTE_ENTRYPOINT"), "AIRBYTE_ENTRYPOINT must be set in dockerfile" - assert docker_runner.env_variables.get("AIRBYTE_ENTRYPOINT") == " ".join( - docker_runner.entry_point - ), "env should be equal to space-joined entrypoint" - def test_enum_usage(self, actual_connector_spec: ConnectorSpecification): """Check that enum lists in specs contain distinct values.""" docs_url = "https://docs.airbyte.io/connector-development/connector-specification-reference" @@ -538,6 +531,30 @@ class TestSpec(BaseTest): [additional_properties_value is True for additional_properties_value in additional_properties_values] ), "When set, additionalProperties field value must be true for backward compatibility." + # This test should not be part of TestSpec because it's testing the connector's docker image content, not the spec itself + # But it's cumbersome to declare a separate, non configurable, test class + # See https://github.com/airbytehq/airbyte/issues/15551 + def test_image_labels(self, docker_runner: ConnectorRunner, connector_metadata: dict): + """Check that connector's docker image has required labels""" + assert docker_runner.labels.get("io.airbyte.name"), "io.airbyte.name must be set in dockerfile" + assert docker_runner.labels.get("io.airbyte.version"), "io.airbyte.version must be set in dockerfile" + assert ( + docker_runner.labels.get("io.airbyte.name") == connector_metadata["data"]["dockerRepository"] + ), "io.airbyte.name must be equal to dockerRepository in metadata.yaml" + assert ( + docker_runner.labels.get("io.airbyte.version") == connector_metadata["data"]["dockerImageTag"] + ), "io.airbyte.version must be equal to dockerImageTag in metadata.yaml" + + # This test should not be part of TestSpec because it's testing the connector's docker image content, not the spec itself + # But it's cumbersome to declare a separate, non configurable, test class + # See https://github.com/airbytehq/airbyte/issues/15551 + def test_image_environment_variables(self, docker_runner: ConnectorRunner): + """Check that connector's docker image has required envs""" + assert docker_runner.env_variables.get("AIRBYTE_ENTRYPOINT"), "AIRBYTE_ENTRYPOINT must be set in dockerfile" + assert docker_runner.env_variables.get("AIRBYTE_ENTRYPOINT") == " ".join( + docker_runner.entry_point + ), "env should be equal to space-joined entrypoint" + @pytest.mark.default_timeout(30) class TestConnection(BaseTest): diff --git a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/utils/connector_runner.py b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/utils/connector_runner.py index c0f385d851d..24f3173a99b 100644 --- a/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/utils/connector_runner.py +++ b/airbyte-integrations/bases/connector-acceptance-test/connector_acceptance_test/utils/connector_runner.py @@ -184,6 +184,10 @@ class ConnectorRunner: env_vars = self._image.attrs["Config"]["Env"] return {env.split("=", 1)[0]: env.split("=", 1)[1] for env in env_vars} + @property + def labels(self): + return self._image.labels + @property def entry_point(self): return self._image.attrs["Config"]["Entrypoint"] diff --git a/tools/ci_connector_ops/ci_connector_ops/pipelines/actions/environments.py b/tools/ci_connector_ops/ci_connector_ops/pipelines/actions/environments.py index 868f7a77de7..b9dbf5ae038 100644 --- a/tools/ci_connector_ops/ci_connector_ops/pipelines/actions/environments.py +++ b/tools/ci_connector_ops/ci_connector_ops/pipelines/actions/environments.py @@ -14,6 +14,7 @@ from datetime import datetime from pathlib import Path from typing import TYPE_CHECKING, List, Optional +import yaml from ci_connector_ops.pipelines import consts from ci_connector_ops.pipelines.consts import ( CI_CONNECTOR_OPS_SOURCE_PATH, @@ -23,7 +24,7 @@ from ci_connector_ops.pipelines.consts import ( PYPROJECT_TOML_FILE_PATH, ) from ci_connector_ops.pipelines.utils import get_file_contents -from dagger import CacheSharingMode, CacheVolume, Client, Container, Directory, File, Platform, Secret +from dagger import CacheSharingMode, CacheVolume, Client, Container, DaggerError, Directory, File, Platform, Secret from dagger.engine._version import CLI_VERSION as dagger_engine_version if TYPE_CHECKING: @@ -435,19 +436,24 @@ async def with_connector_acceptance_test(context: ConnectorContext, connector_un Returns: Container: A container with connector acceptance tests installed. """ - connector_under_test_image_name = context.connector.acceptance_test_config["connector_image"] - image_sha = await load_image_to_docker_host(context, connector_under_test_image_tar, connector_under_test_image_name) + + patched_cat_config = context.connector.acceptance_test_config + patched_cat_config["connector_image"] = context.connector.acceptance_test_config["connector_image"].replace( + ":dev", f":{context.git_revision}" + ) + image_sha = await load_image_to_docker_host(context, connector_under_test_image_tar, patched_cat_config["connector_image"]) if context.connector_acceptance_test_image.endswith(":dev"): cat_container = context.connector_acceptance_test_source_dir.docker_build() else: cat_container = context.dagger_client.container().from_(context.connector_acceptance_test_image) + test_input = context.get_connector_dir().with_new_file("acceptance-test-config.yml", yaml.safe_dump(patched_cat_config)) cat_container = ( with_bound_docker_host(context, cat_container) .with_entrypoint([]) .with_exec(["pip", "install", "pytest-custom_exit_code"]) - .with_mounted_directory("/test_input", context.get_connector_dir()) + .with_mounted_directory("/test_input", test_input) ) cat_container = ( with_mounted_connector_secrets(context, cat_container, "/test_input/secrets") @@ -812,13 +818,16 @@ async def with_airbyte_python_connector(context: ConnectorContext, build_platfor context.dagger_client.container(platform=build_platform) .with_mounted_cache("/root/.cache/pip", pip_cache) .build(context.get_connector_dir()) - .with_label("io.airbyte.version", context.metadata["dockerImageTag"]) .with_label("io.airbyte.name", context.metadata["dockerRepository"]) ) cdk_version = await get_cdk_version_from_python_connector(connector_container) if cdk_version: connector_container = connector_container.with_label("io.airbyte.cdk_version", cdk_version) context.cdk_version = cdk_version + if not await connector_container.label("io.airbyte.version") == context.metadata["dockerImageTag"]: + raise DaggerError( + "Abusive caching might be happening. The connector container should have been built with the correct version as defined in metadata.yaml" + ) return await finalize_build(context, connector_container) diff --git a/tools/ci_connector_ops/ci_connector_ops/pipelines/bases.py b/tools/ci_connector_ops/ci_connector_ops/pipelines/bases.py index 8a14a6da493..920b4bcf932 100644 --- a/tools/ci_connector_ops/ci_connector_ops/pipelines/bases.py +++ b/tools/ci_connector_ops/ci_connector_ops/pipelines/bases.py @@ -22,7 +22,7 @@ from ci_connector_ops.pipelines.actions import remote_storage from ci_connector_ops.pipelines.consts import GCS_PUBLIC_DOMAIN, LOCAL_REPORTS_PATH_ROOT, PYPROJECT_TOML_FILE_PATH from ci_connector_ops.pipelines.utils import check_path_in_workdir, format_duration, slugify, with_exit_code, with_stderr, with_stdout from ci_connector_ops.utils import console -from dagger import Container, QueryError +from dagger import Container, DaggerError, QueryError from jinja2 import Environment, PackageLoader, select_autoescape from rich.console import Group from rich.panel import Panel @@ -164,9 +164,9 @@ class Step(ABC): self.stopped_at = datetime.utcnow() self.log_step_result(result) return result - except QueryError as e: + except (DaggerError, QueryError) as e: self.stopped_at = datetime.utcnow() - self.logger.error(f"QueryError on step {self.title}: {e}") + self.logger.error(f"Dagger error on step {self.title}: {e}") return StepResult(self, StepStatus.FAILURE, stderr=str(e)) def log_step_result(self, result: StepResult) -> None: