diff --git a/packages/models-library/src/models_library/service_settings_labels.py b/packages/models-library/src/models_library/service_settings_labels.py index b919f6db8e9..adeedfdec7e 100644 --- a/packages/models-library/src/models_library/service_settings_labels.py +++ b/packages/models-library/src/models_library/service_settings_labels.py @@ -395,7 +395,7 @@ def _containers_allowed_outgoing_internet_in_compose_spec(cls, v, values): @validator("callbacks_mapping") @classmethod - def ensure_callbacks_mapping_container_names_defined_in_compose_spec( + def _ensure_callbacks_mapping_container_names_defined_in_compose_spec( cls, v: CallbacksMapping, values ): if v is None: @@ -423,12 +423,12 @@ def ensure_callbacks_mapping_container_names_defined_in_compose_spec( @validator("user_preferences_path", pre=True) @classmethod - def deserialize_from_json(cls, v): + def _deserialize_from_json(cls, v): return f"{v}".removeprefix('"').removesuffix('"') @validator("user_preferences_path") @classmethod - def user_preferences_path_no_included_in_other_volumes( + def _user_preferences_path_no_included_in_other_volumes( cls, v: CallbacksMapping, values ): paths_mapping: PathMappingsLabel | None = values.get("paths_mapping", None) @@ -447,7 +447,7 @@ def user_preferences_path_no_included_in_other_volumes( @root_validator @classmethod - def not_allowed_in_both_specs(cls, values): + def _not_allowed_in_both_specs(cls, values): match_keys = { "containers_allowed_outgoing_internet", "containers_allowed_outgoing_permit_list", diff --git a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py index b0a8dfc04ab..e35400e2a28 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/dynamic_sidecar/docker_compose_specs.py @@ -28,6 +28,7 @@ from ...core.dynamic_services_settings.egress_proxy import EgressProxySettings from ..osparc_variables.substitutions import ( + auto_inject_environments, resolve_and_substitute_session_variables_in_model, resolve_and_substitute_session_variables_in_specs, substitute_vendor_secrets_in_model, @@ -364,6 +365,9 @@ async def assemble_spec( # pylint: disable=too-many-arguments # noqa: PLR0913 assigned_limits=assigned_limits, ) + # resolve service-spec + service_spec = auto_inject_environments(service_spec) + service_spec = await substitute_vendor_secrets_in_specs( app=app, specs=service_spec, diff --git a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py index 0b3f87d46ea..c78ac5e04a2 100644 --- a/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py +++ b/services/director-v2/src/simcore_service_director_v2/modules/osparc_variables/substitutions.py @@ -4,7 +4,7 @@ import functools import logging from copy import deepcopy -from typing import Any +from typing import Any, Final from fastapi import FastAPI from models_library.osparc_variable_identifier import ( @@ -15,6 +15,7 @@ from models_library.products import ProductName from models_library.projects import ProjectID from models_library.projects_nodes_io import NodeID +from models_library.service_settings_labels import ComposeSpecLabelDict from models_library.services import ServiceKey, ServiceVersion from models_library.users import UserID from models_library.utils.specs_substitution import SpecsSubstitutionsResolver @@ -137,6 +138,37 @@ def create(cls, app: FastAPI): return table +_NEW_ENVIRONMENTS: Final = { + "OSPARC_API_BASE_URL": "$OSPARC_VARIABLE_API_HOST", + "OSPARC_API_KEY": "$OSPARC_VARIABLE_API_KEY", + "OSPARC_API_SECRET": "$OSPARC_VARIABLE_API_SECRET", + "OSPARC_STUDY_ID": "$OSPARC_VARIABLE_STUDY_UUID", + "OSPARC_NODE_ID": "$OSPARC_VARIABLE_NODE_ID", +} + + +def auto_inject_environments( + compose_spec: ComposeSpecLabelDict, +) -> ComposeSpecLabelDict: + # SEE https://github.com/ITISFoundation/osparc-simcore/issues/5925 + for service in compose_spec.get("services", {}).values(): + current_environment = deepcopy(service.get("environment", {})) + + # if _NEW_ENVIRONMENTS are already defined, then do not change them + if isinstance(current_environment, dict): + service["environment"] = { + **_NEW_ENVIRONMENTS, + **current_environment, + } + elif isinstance(current_environment, list): + service["environment"] += [ + f"{name}={value}" + for name, value in _NEW_ENVIRONMENTS.items() + if not any(e.startswith(name) for e in current_environment) + ] + return compose_spec + + async def resolve_and_substitute_session_variables_in_model( app: FastAPI, model: BaseModel, diff --git a/services/director-v2/tests/conftest.py b/services/director-v2/tests/conftest.py index 27f488957dc..3ea9811f961 100644 --- a/services/director-v2/tests/conftest.py +++ b/services/director-v2/tests/conftest.py @@ -9,6 +9,7 @@ import os from collections.abc import AsyncIterable, AsyncIterator from copy import deepcopy +from datetime import timedelta from pathlib import Path from typing import Any from unittest.mock import AsyncMock @@ -19,7 +20,10 @@ from asgi_lifespan import LifespanManager from faker import Faker from fastapi import FastAPI +from models_library.api_schemas_webserver.auth import ApiKeyGet +from models_library.products import ProductName from models_library.projects import Node, NodesDict +from models_library.users import UserID from pytest_mock import MockerFixture from pytest_simcore.helpers.typing_env import EnvVarsDict from pytest_simcore.helpers.utils_envs import setenvs_from_dict, setenvs_from_envfile @@ -323,3 +327,32 @@ async def wrapper(*args, **kwargs): "simcore_service_director_v2.modules.dynamic_sidecar.scheduler._core._scheduler" ) mocker.patch(f"{module_base}.exclusive", side_effect=_mock_exclusive) + + +@pytest.fixture +def mock_osparc_variables_api_auth_rpc(mocker: MockerFixture) -> None: + + fake_data = ApiKeyGet.parse_obj(ApiKeyGet.Config.schema_extra["examples"][0]) + + async def _create( + app: FastAPI, + *, + product_name: ProductName, + user_id: UserID, + name: str, + expiration: timedelta, + ): + assert app + assert product_name + assert user_id + assert expiration is None + + fake_data.display_name = name + return fake_data + + # mocks RPC interface + mocker.patch( + "simcore_service_director_v2.modules.osparc_variables._api_auth.get_or_create_api_key_and_secret", + side_effect=_create, + autospec=True, + ) diff --git a/services/director-v2/tests/integration/02/test_dynamic_services_routes.py b/services/director-v2/tests/integration/02/test_dynamic_services_routes.py index 9c4f740aaf5..0bdfb73b5c8 100644 --- a/services/director-v2/tests/integration/02/test_dynamic_services_routes.py +++ b/services/director-v2/tests/integration/02/test_dynamic_services_routes.py @@ -1,5 +1,7 @@ # pylint: disable=redefined-outer-name # pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments import asyncio import json @@ -302,6 +304,7 @@ async def test_start_status_stop( mock_projects_repository: None, mocked_service_awaits_manual_interventions: None, mock_resource_usage_tracker: None, + mock_osparc_variables_api_auth_rpc: None, ): # NOTE: this test does not like it when the catalog is not fully ready!!! diff --git a/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py b/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py index a86aeb2ff2e..0d187de43d2 100644 --- a/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py +++ b/services/director-v2/tests/integration/02/test_dynamic_sidecar_nodeports_integration.py @@ -893,6 +893,7 @@ async def test_nodeports_integration( projects_networks_db: None, mocked_service_awaits_manual_interventions: None, mock_resource_usage_tracker: None, + mock_osparc_variables_api_auth_rpc: None, initialized_app: FastAPI, update_project_workbench_with_comp_tasks: Callable, async_client: httpx.AsyncClient, diff --git a/services/director-v2/tests/integration/02/test_mixed_dynamic_sidecar_and_legacy_project.py b/services/director-v2/tests/integration/02/test_mixed_dynamic_sidecar_and_legacy_project.py index 7afa5eb5aad..759e9fd620e 100644 --- a/services/director-v2/tests/integration/02/test_mixed_dynamic_sidecar_and_legacy_project.py +++ b/services/director-v2/tests/integration/02/test_mixed_dynamic_sidecar_and_legacy_project.py @@ -227,6 +227,7 @@ async def test_legacy_and_dynamic_sidecar_run( service_resources: ServiceResourcesDict, mocked_service_awaits_manual_interventions: None, mock_resource_usage_tracker: None, + mock_osparc_variables_api_auth_rpc: None, ): """ The test will start 3 dynamic services in the same project and check diff --git a/services/director-v2/tests/unit/test_modules_osparc_variables.py b/services/director-v2/tests/unit/test_modules_osparc_variables.py index 9ed3187a25a..9ed659f00ad 100644 --- a/services/director-v2/tests/unit/test_modules_osparc_variables.py +++ b/services/director-v2/tests/unit/test_modules_osparc_variables.py @@ -9,15 +9,14 @@ import json from collections.abc import AsyncIterable from contextlib import asynccontextmanager -from datetime import timedelta +from copy import deepcopy from unittest.mock import AsyncMock, Mock import pytest from asgi_lifespan import LifespanManager from faker import Faker from fastapi import FastAPI -from models_library.api_schemas_webserver.auth import ApiKeyGet -from models_library.products import ProductName +from models_library.service_settings_labels import ComposeSpecLabelDict from models_library.services import ServiceKey, ServiceVersion from models_library.users import UserID from models_library.utils.specs_substitution import SubstitutionValue @@ -30,6 +29,9 @@ from simcore_service_director_v2.api.dependencies.database import RepoType from simcore_service_director_v2.modules.osparc_variables import substitutions from simcore_service_director_v2.modules.osparc_variables.substitutions import ( + _NEW_ENVIRONMENTS, + OsparcSessionVariablesTable, + auto_inject_environments, resolve_and_substitute_session_variables_in_specs, substitute_vendor_secrets_in_specs, ) @@ -138,35 +140,6 @@ def mock_user_repo(mocker: MockerFixture, mock_repo_db_engine: None) -> None: mocker.patch(f"{base}.UsersRepo.get_email", return_value="e@ma.il") -@pytest.fixture -def mock_api_key_manager(mocker: MockerFixture) -> None: - - fake_data = ApiKeyGet.parse_obj(ApiKeyGet.Config.schema_extra["examples"][0]) - - async def _create( - app: FastAPI, - *, - product_name: ProductName, - user_id: UserID, - name: str, - expiration: timedelta, - ): - assert app - assert product_name - assert user_id - assert expiration is None - - fake_data.display_name = name - return fake_data - - # mocks RPC interface - mocker.patch( - "simcore_service_director_v2.modules.osparc_variables._api_auth.get_or_create_api_key_and_secret", - side_effect=_create, - autospec=True, - ) - - @pytest.fixture async def fake_app(faker: Faker) -> AsyncIterable[FastAPI]: app = FastAPI() @@ -183,7 +156,10 @@ async def fake_app(faker: Faker) -> AsyncIterable[FastAPI]: async def test_resolve_and_substitute_session_variables_in_specs( - mock_user_repo: None, mock_api_key_manager: None, fake_app: FastAPI, faker: Faker + mock_user_repo: None, + mock_osparc_variables_api_auth_rpc: None, + fake_app: FastAPI, + faker: Faker, ): specs = { "product_name": "${OSPARC_VARIABLE_PRODUCT_NAME}", @@ -241,3 +217,81 @@ async def test_substitute_vendor_secrets_in_specs( print("REPLACED SPECS\n", replaced_specs) assert VENDOR_SECRET_PREFIX not in f"{replaced_specs}" + + +@pytest.fixture +def compose_spec(): + return { + "version": "3.7", + "services": { + "jupyter-math": { + "environment": [ + "OSPARC_API_KEY=$OSPARC_VARIABLE_API_KEY", + "OSPARC_API_SECRET=$OSPARC_VARIABLE_API_SECRET", + "FOO=33", + ], + "image": "${SIMCORE_REGISTRY}/simcore/services/dynamic/jupyter-math:${SERVICE_VERSION}", + "networks": {"dy-sidecar_10e1b317-de62-44ca-979e-09bf15663834": None}, + "deploy": { + "resources": { + "reservations": {"cpus": "0.1", "memory": "2147483648"}, + "limits": {"cpus": "4.0", "memory": "17179869184"}, + } + }, + "labels": [ + "io.simcore.runtime.cpu-limit=4.0", + "io.simcore.runtime.memory-limit=17179869184", + "io.simcore.runtime.node-id=10e1b317-de62-44ca-979e-09bf15663834", + "io.simcore.runtime.product-name=osparc", + "io.simcore.runtime.project-id=e341df9e-2e38-11ef-894b-0242ac140025", + "io.simcore.runtime.simcore-user-agent=undefined", + "io.simcore.runtime.swarm-stack-name=master-simcore", + "io.simcore.runtime.user-id=1", + ], + } + }, + "networks": { + "dy-sidecar_10e1b317-de62-44ca-979e-09bf15663834": { + "name": "dy-sidecar_10e1b317-de62-44ca-979e-09bf15663834", + "external": True, + "driver": "overlay", + }, + "master-simcore_interactive_services_subnet": { + "name": "master-simcore_interactive_services_subnet", + "external": True, + "driver": "overlay", + }, + }, + } + + +def test_auto_inject_environments_added_to_all_services_in_compose( + compose_spec: ComposeSpecLabelDict, +): + + before = deepcopy(compose_spec) + + after = auto_inject_environments(compose_spec) + + assert before != after + assert after == compose_spec + + auto_injected_envs = set(_NEW_ENVIRONMENTS.keys()) + for name, service in compose_spec.get("services", {}).items(): + + # all services have environment specs + assert service["environment"], f"expected in {name} service" + + # injected? + for env_name in auto_injected_envs: + assert env_name in str(service["environment"]) + + +def test_auto_inject_environments_are_registered(): + app = FastAPI() + table = OsparcSessionVariablesTable.create(app) + + registered_osparc_variables = set(table.variables_names()) + auto_injected_osparc_variables = {_.lstrip("$") for _ in _NEW_ENVIRONMENTS.values()} + + assert auto_injected_osparc_variables.issubset(registered_osparc_variables)