diff --git a/devservices/commands/list_services.py b/devservices/commands/list_services.py index 888a21ae..462508ef 100644 --- a/devservices/commands/list_services.py +++ b/devservices/commands/list_services.py @@ -4,6 +4,9 @@ from argparse import ArgumentParser from argparse import Namespace +from sentry_sdk import capture_exception + +from devservices.exceptions import ConfigError from devservices.utils.console import Console from devservices.utils.devenv import get_coderoot from devservices.utils.services import get_local_services @@ -29,7 +32,12 @@ def list_services(args: Namespace) -> None: console = Console() # Get all of the services installed locally coderoot = get_coderoot() - services = get_local_services(coderoot) + try: + services = get_local_services(coderoot) + except ConfigError as e: + capture_exception(e) + console.failure(str(e)) + return state = State() starting_services = set(state.get_service_entries(StateTables.STARTING_SERVICES)) started_services = set(state.get_service_entries(StateTables.STARTED_SERVICES)) diff --git a/devservices/configs/service_config.py b/devservices/configs/service_config.py index 6b1fcf34..62e459b6 100644 --- a/devservices/configs/service_config.py +++ b/devservices/configs/service_config.py @@ -84,6 +84,8 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: ) service_config_data = config.get("x-sentry-service-config") + docker_compose_services = config.get("services", {}).keys() + valid_dependency_keys = {field.name for field in fields(Dependency)} dependencies = {} @@ -106,6 +108,16 @@ def load_service_config_from_file(repo_path: str) -> ServiceConfig: f"Error parsing service dependencies: {type_error}" ) from type_error + # Validate that all non-remote dependencies are defined in docker-compose services + for dependency_name, dependency in dependencies.items(): + if ( + dependency.remote is None + and dependency_name not in docker_compose_services + ): + raise ConfigValidationError( + f"Dependency '{dependency_name}' is not remote but is not defined in docker-compose services" + ) + service_config = ServiceConfig( version=service_config_data.get("version"), service_name=service_config_data.get("service_name"), diff --git a/devservices/utils/services.py b/devservices/utils/services.py index f8bb72a3..6c96d9a7 100644 --- a/devservices/utils/services.py +++ b/devservices/utils/services.py @@ -30,9 +30,9 @@ def get_local_services(coderoot: str) -> list[Service]: repo_path = os.path.join(coderoot, repo) try: service_config = load_service_config_from_file(repo_path) - except (ConfigParseError, ConfigValidationError) as e: - console.warning(f"{repo} was found with an invalid config: {e}") - continue + except (ConfigParseError, ConfigValidationError): + console.warning(f"{repo} was found with an invalid config") + raise except ConfigNotFoundError: # Ignore repos that don't have devservices configs continue diff --git a/tests/commands/test_list_services.py b/tests/commands/test_list_services.py index 3dc1e96f..ed0b8360 100644 --- a/tests/commands/test_list_services.py +++ b/tests/commands/test_list_services.py @@ -121,6 +121,50 @@ def test_list_running_services_started( ) +@mock.patch("devservices.utils.state.State.get_service_entries") +@mock.patch("devservices.utils.state.State.get_active_modes_for_service") +def test_list_running_services_config_error( + mock_get_active_modes_for_service: mock.Mock, + mock_get_service_entries: mock.Mock, + tmp_path: Path, + capsys: pytest.CaptureFixture[str], +) -> None: + with ( + mock.patch( + "devservices.commands.list_services.get_coderoot", + return_value=str(tmp_path / "code"), + ), + mock.patch("devservices.utils.state.STATE_DB_FILE", str(tmp_path / "state")), + ): + mock_get_service_entries.side_effect = [[], ["example-service"]] + mock_get_active_modes_for_service.side_effect = [[], ["default"]] + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "redis": {"description": "Redis"}, + "clickhouse": {"description": "Clickhouse"}, + }, + "modes": {"default": ["redis", "clickhouse"]}, + }, + "services": { + "redis": {"image": "redis:6.2.14-alpine"}, + }, + } + create_config_file(tmp_path / "code" / "example-service", config) + + args = Namespace(service_name=None, all=False) + list_services(args) + + captured = capsys.readouterr() + + assert ( + captured.out + == "\x1b[0;33mexample-service was found with an invalid config\x1b[0m\n\x1b[0;31mDependency 'clickhouse' is not remote but is not defined in docker-compose services\x1b[0m\n" + ) + + @mock.patch("devservices.utils.state.State.get_service_entries") @mock.patch("devservices.utils.state.State.get_active_modes_for_service") def test_list_all_services( diff --git a/tests/configs/test_service_config.py b/tests/configs/test_service_config.py index bcc02933..928bbcef 100644 --- a/tests/configs/test_service_config.py +++ b/tests/configs/test_service_config.py @@ -70,7 +70,13 @@ def test_load_service_config_from_file( "service_name": service_name, "dependencies": {key: value for key, value in dependencies.items()}, "modes": {key: value for key, value in modes.items()}, - } + }, + "services": { + key: { + "image": key, + } + for key in dependencies.keys() + }, } create_config_file(tmp_path, config) @@ -95,7 +101,8 @@ def test_load_service_config_from_file_no_dependencies(tmp_path: Path) -> None: "version": 0.1, "service_name": "example-service", "modes": {"default": []}, - } + }, + "services": {}, } create_config_file(tmp_path, config) @@ -126,7 +133,12 @@ def test_load_service_config_from_file_invalid_version(tmp_path: Path) -> None: "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -142,7 +154,12 @@ def test_load_service_config_from_file_missing_version(tmp_path: Path) -> None: "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -159,7 +176,12 @@ def test_load_service_config_from_file_missing_service_name(tmp_path: Path) -> N "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -177,7 +199,12 @@ def test_load_service_config_from_file_invalid_dependency(tmp_path: Path) -> Non "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency", "unknown-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -198,7 +225,12 @@ def test_load_service_config_from_file_missing_default_mode(tmp_path: Path) -> N "example-dependency": {"description": "Example dependency"} }, "modes": {"custom": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -215,7 +247,12 @@ def test_load_service_config_from_file_no_modes(tmp_path: Path) -> None: "dependencies": { "example-dependency": {"description": "Example dependency"} }, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -224,6 +261,58 @@ def test_load_service_config_from_file_no_modes(tmp_path: Path) -> None: assert str(e.value) == "Default mode is required in service config" +def test_load_service_config_from_file_remote_dependency_not_in_services( + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "example-dependency": { + "description": "Example dependency", + "remote": { + "repo_name": "example-dependency", + "repo_link": "https://github.com/example/example-dependency", + "branch": "main", + }, + }, + }, + "modes": {"default": ["example-dependency"]}, + }, + "services": {}, + } + create_config_file(tmp_path, config) + + load_service_config_from_file(str(tmp_path)) + + +def test_load_service_config_from_file_no_matching_docker_compose_service( + tmp_path: Path, +) -> None: + config = { + "x-sentry-service-config": { + "version": 0.1, + "service_name": "example-service", + "dependencies": { + "example-dependency": { + "description": "Example dependency", + }, + }, + "modes": {"default": ["example-dependency"]}, + }, + "services": {}, + } + create_config_file(tmp_path, config) + + with pytest.raises(ConfigValidationError) as e: + load_service_config_from_file(str(tmp_path)) + assert ( + str(e.value) + == "Dependency 'example-dependency' is not remote but is not defined in docker-compose services" + ) + + def test_load_service_config_from_file_invalid_dependencies(tmp_path: Path) -> None: config = { "x-sentry-service-config": { @@ -236,7 +325,12 @@ def test_load_service_config_from_file_invalid_dependencies(tmp_path: Path) -> N } }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -260,7 +354,12 @@ def test_load_service_config_from_file_invalid_modes(tmp_path: Path) -> None: "default": ["example-dependency"], "custom": "example-dependency", }, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) @@ -280,7 +379,12 @@ def test_load_service_config_from_file_no_x_sentry_service_config( "example-dependency": {"description": "Example dependency"} }, "modes": {"default": ["example-dependency"]}, - } + }, + "services": { + "example-dependency": { + "image": "example-dependency", + } + }, } create_config_file(tmp_path, config) diff --git a/tests/utils/test_services.py b/tests/utils/test_services.py index e17f6a31..1f5a136f 100644 --- a/tests/utils/test_services.py +++ b/tests/utils/test_services.py @@ -7,6 +7,7 @@ import pytest from devservices.configs.service_config import ServiceConfig +from devservices.exceptions import ConfigParseError from devservices.exceptions import ServiceNotFoundError from devservices.utils.services import find_matching_service from devservices.utils.services import get_local_services @@ -32,13 +33,11 @@ def test_get_local_services_with_invalid_config( mock_repo_path = mock_code_root / "example" create_mock_git_repo("invalid_repo", mock_repo_path) - local_services = get_local_services(str(mock_code_root)) + with pytest.raises(ConfigParseError): + local_services = get_local_services(str(mock_code_root)) + assert not local_services captured = capsys.readouterr() - assert not local_services - assert ( - "example was found with an invalid config: Error parsing config file:" - in captured.out - ) + assert "example was found with an invalid config" in captured.out def test_get_local_services_with_valid_config(tmp_path: Path) -> None: