🐛 Fixes catalog giving access rights to everyone (group 1) to new services 🚨

services/catalog/src/simcore_service_catalog/clients/director.py

@@ -277,15 +277,13 @@ async def get_service_extras(
                     )
 
         # get org labels
-        result.update(
-            {
-                sl: labels[dl]
-                for dl, sl in _ORG_LABELS_TO_SCHEMA_LABELS.items()
-                if dl in labels
-            }
-        )
+        if service_build_details := {
+            sl: labels[dl]
+            for dl, sl in _ORG_LABELS_TO_SCHEMA_LABELS.items()
+            if dl in labels
+        }:
 
-        _logger.debug("Following service extras were compiled: %s", pformat(result))
+            result.update({"service_build_details": service_build_details})
 
         return TypeAdapter(ServiceExtras).validate_python(result)
 

services/catalog/src/simcore_service_catalog/service/access_rights.py

@@ -24,7 +24,9 @@
 
 _logger = logging.getLogger(__name__)
 
-_LEGACY_SERVICES_DATE: datetime = datetime(year=2020, month=8, day=19, tzinfo=UTC)
+_OLD_SERVICES_CUTOFF_DATETIME: datetime = datetime(
+    year=2020, month=8, day=19, tzinfo=UTC
+)
 
 
 class InheritedData(TypedDict):
@@ -37,15 +39,42 @@ def _is_frontend_service(service: ServiceMetaDataPublished) -> bool:
 
 
 async def _is_old_service(app: FastAPI, service: ServiceMetaDataPublished) -> bool:
+    #
     # NOTE: https://github.com/ITISFoundation/osparc-simcore/pull/6003#discussion_r1658200909
-    # get service build date
-    client = get_director_client(app)
+    #
+    service_extras = await get_director_client(app).get_service_extras(
+        service.key, service.version
+    )
+
+    # 1. w/o build details
+    has_no_build_data = (
+        not service_extras or service_extras.service_build_details is None
+    )
+    if has_no_build_data:
+        _logger.debug(
+            "Service %s:%s is considered legacy because it has no build details",
+            service.key,
+            service.version,
+        )
+        return True
+
+    # 2. check if built before cutoff date
+    assert service_extras.service_build_details
+    service_build_datetime = arrow.get(
+        service_extras.service_build_details.build_date
+    ).datetime
 
-    data = await client.get_service_extras(service.key, service.version)
-    if not data or data.service_build_details is None:
+    is_older_than_cutoff = service_build_datetime < _OLD_SERVICES_CUTOFF_DATETIME
+    if is_older_than_cutoff:
+        _logger.debug(
+            "Service %s:%s is considered legacy because it was built before %s",
+            service.key,
+            service.version,
+            _OLD_SERVICES_CUTOFF_DATETIME,
+        )
         return True
-    service_build_data = arrow.get(data.service_build_details.build_date).datetime
-    return bool(service_build_data < _LEGACY_SERVICES_DATE)
+
+    return False
 
 
 async def evaluate_default_service_ownership_and_rights(

services/catalog/tests/unit/conftest.py

@@ -21,9 +21,13 @@
 from faker import Faker
 from fastapi import FastAPI, status
 from fastapi.testclient import TestClient
-from models_library.api_schemas_directorv2.services import ServiceExtras
+from models_library.api_schemas_directorv2.services import (
+    NodeRequirements,
+    ServiceBuildDetails,
+    ServiceExtras,
+)
 from packaging.version import Version
-from pydantic import EmailStr, TypeAdapter
+from pydantic import EmailStr
 from pytest_mock import MockerFixture, MockType
 from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
 from pytest_simcore.helpers.typing_env import EnvVarsDict
@@ -407,8 +411,10 @@ def mocked_director_rest_api_base(
 
 @pytest.fixture
 def get_mocked_service_labels() -> Callable[[str, str], dict]:
-    def _(service_key: str, service_version: str) -> dict:
-        return {
+    def _(
+        service_key: str, service_version: str, *, include_org_labels: bool = True
+    ) -> dict:
+        base_labels = {
             "io.simcore.authors": '{"authors": [{"name": "John Smith", "email": "[email protected]", "affiliation": "ACME\'IS Foundation"}]}',
             "io.simcore.contact": '{"contact": "[email protected]"}',
             "io.simcore.description": '{"description": "Autonomous Nervous System Network model"}',
@@ -423,21 +429,35 @@ def _(service_key: str, service_version: str) -> dict:
                 "xxxxx", service_version
             ),
             "maintainer": "johnsmith",
-            "org.label-schema.build-date": "2023-04-17T08:04:15Z",
-            "org.label-schema.schema-version": "1.0",
-            "org.label-schema.vcs-ref": "4d79449a2e79f8a3b3b2e1dd0290af9f3d1a8792",
-            "org.label-schema.vcs-url": "https://github.com/ITISFoundation/jupyter-math.git",
             "simcore.service.restart-policy": "no-restart",
             "simcore.service.settings": '[{"name": "Resources", "type": "Resources", "value": {"Limits": {"NanoCPUs": 1000000000, "MemoryBytes": 4194304}, "Reservations": {"NanoCPUs": 4000000000, "MemoryBytes": 2147483648}}}]',
         }
 
+        if include_org_labels:
+            base_labels.update(
+                {
+                    "org.label-schema.build-date": "2023-04-17T08:04:15Z",
+                    "org.label-schema.schema-version": "1.0",
+                    "org.label-schema.vcs-ref": "4d79449a2e79f8a3b3b2e1dd0290af9f3d1a8792",
+                    "org.label-schema.vcs-url": "https://github.com/ITISFoundation/jupyter-math.git",
+                }
+            )
+
+        return base_labels
+
     return _
 
 
 @pytest.fixture
 def mock_service_extras() -> ServiceExtras:
-    return TypeAdapter(ServiceExtras).validate_python(
-        ServiceExtras.model_json_schema()["examples"][0]
+    return ServiceExtras(
+        node_requirements=NodeRequirements(CPU=1.0, GPU=None, RAM=4194304, VRAM=None),
+        service_build_details=ServiceBuildDetails(
+            build_date="2023-04-17T08:04:15Z",
+            vcs_ref="4d79449a2e79f8a3b3b2e1dd0290af9f3d1a8792",
+            vcs_url="https://github.com/ITISFoundation/jupyter-math.git",
+        ),
+        container_spec=None,
     )
 
 

services/catalog/tests/unit/test_clients_director.py

@@ -7,10 +7,12 @@
 
 
 import urllib.parse
+from collections.abc import Callable
 from typing import Any
 
+import httpx
 import pytest
-from fastapi import FastAPI
+from fastapi import FastAPI, status
 from models_library.services_metadata_published import ServiceMetaDataPublished
 from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
 from pytest_simcore.helpers.typing_env import EnvVarsDict
@@ -32,6 +34,14 @@ def app_environment(
     )
 
 
+@pytest.fixture
+def service_key_and_version(
+    expected_director_rest_api_list_services: list[dict[str, Any]],
+) -> tuple[str, str]:
+    expected_service = expected_director_rest_api_list_services[0]
+    return expected_service["key"], expected_service["version"]
+
+
 async def test_director_client_high_level_api(
     repository_lifespan_disabled: None,
     background_task_lifespan_disabled: None,
@@ -57,22 +67,19 @@ async def test_director_client_high_level_api(
         await director_api.get_service(expected_service.key, expected_service.version)
         == expected_service
     )
-    # TODO: error handling!
 
 
 async def test_director_client_low_level_api(
     repository_lifespan_disabled: None,
     background_task_lifespan_disabled: None,
     rabbitmq_and_rpc_setup_disabled: None,
     mocked_director_rest_api: MockRouter,
-    expected_director_rest_api_list_services: list[dict[str, Any]],
+    service_key_and_version: tuple[str, str],
     app: FastAPI,
 ):
     director_api = get_director_client(app)
 
-    expected_service = expected_director_rest_api_list_services[0]
-    key = expected_service["key"]
-    version = expected_service["version"]
+    key, version = service_key_and_version
 
     service_labels = await director_api.get(
         f"/services/{urllib.parse.quote_plus(key)}/{version}/labels"
@@ -84,3 +91,74 @@ async def test_director_client_low_level_api(
         f"/services/{urllib.parse.quote_plus(key)}/{version}"
     )
     assert service
+
+
+async def test_director_client_get_service_extras_with_org_labels(
+    repository_lifespan_disabled: None,
+    background_task_lifespan_disabled: None,
+    rabbitmq_and_rpc_setup_disabled: None,
+    mocked_director_rest_api: MockRouter,
+    service_key_and_version: tuple[str, str],
+    app: FastAPI,
+):
+    director_api = get_director_client(app)
+
+    key, version = service_key_and_version
+
+    service_extras = await director_api.get_service_extras(key, version)
+
+    # Check node requirements are present
+    assert service_extras.node_requirements is not None
+    assert service_extras.node_requirements.cpu > 0
+    assert service_extras.node_requirements.ram > 0
+
+    # Check service build details are present (since we have org.label-schema labels)
+    assert service_extras.service_build_details is not None
+    assert service_extras.service_build_details.build_date == "2023-04-17T08:04:15Z"
+    assert (
+        service_extras.service_build_details.vcs_ref
+        == "4d79449a2e79f8a3b3b2e1dd0290af9f3d1a8792"
+    )
+    assert (
+        service_extras.service_build_details.vcs_url
+        == "https://github.com/ITISFoundation/jupyter-math.git"
+    )
+
+
+async def test_director_client_get_service_extras_without_org_labels(
+    repository_lifespan_disabled: None,
+    background_task_lifespan_disabled: None,
+    rabbitmq_and_rpc_setup_disabled: None,
+    mocked_director_rest_api_base: MockRouter,
+    service_key_and_version: tuple[str, str],
+    get_mocked_service_labels: Callable[[str, str, bool], dict],
+    app: FastAPI,
+):
+    # Setup mock without org.label-schema labels
+    service_key, service_version = service_key_and_version
+
+    # Mock the labels endpoint without org labels
+    @mocked_director_rest_api_base.get(
+        path__regex=r"^/services/(?P<service_key>[/\w-]+)/(?P<service_version>[0-9\.]+)/labels$",
+        name="get_service_labels_no_org",
+    )
+    def _get_service_labels_no_org(request, service_key, service_version):
+        return httpx.Response(
+            status_code=status.HTTP_200_OK,
+            json={
+                "data": get_mocked_service_labels(
+                    service_key, service_version, include_org_labels=False
+                )
+            },
+        )
+
+    director_api = get_director_client(app)
+    service_extras = await director_api.get_service_extras(service_key, service_version)
+
+    # Check node requirements are present
+    assert service_extras.node_requirements is not None
+    assert service_extras.node_requirements.cpu > 0
+    assert service_extras.node_requirements.ram > 0
+
+    # Check service build details are NOT present (since we don't have org.label-schema labels)
+    assert service_extras.service_build_details is None

services/catalog/tests/unit/test_utils_service_extras.py

@@ -6,7 +6,9 @@
 import pytest
 from fastapi import FastAPI, status
 from httpx import AsyncClient
-from models_library.api_schemas_directorv2.services import ServiceExtras
+from models_library.api_schemas_directorv2.services import (
+    ServiceExtras,
+)
 from pydantic import TypeAdapter
 from respx import MockRouter