connectors-ci/cat: validate image under test is the right one (#27919)

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.
 

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"]

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")

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):

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"]

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)
 
 

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: