🐛 ooil can now escape quadruple $ used by OsparcVariableIdentifier (ITISFoundation#8118)

Co-authored-by: Andrei Neagu <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

Author: 3 people

packages/models-library/src/models_library/osparc_variable_identifier.py

@@ -1,24 +1,21 @@
+import os
 from copy import deepcopy
-from typing import Any, TypeVar
+from typing import Annotated, Any, Final, TypeVar
 
 from common_library.errors_classes import OsparcErrorMixin
 from models_library.basic_types import ConstrainedStr
-
-from pydantic import BaseModel
+from pydantic import BaseModel, Discriminator, PositiveInt, Tag
 
 from .utils.string_substitution import OSPARC_IDENTIFIER_PREFIX
+from .utils.types import get_types_from_annotated_union
 
 T = TypeVar("T")
 
 
-class OsparcVariableIdentifier(ConstrainedStr):
+class _BaseOsparcVariableIdentifier(ConstrainedStr):
     # NOTE: To allow parametrized value, set the type to Union[OsparcVariableIdentifier, ...]
     # NOTE: When dealing with str types, to avoid unexpected behavior, the following
     # order is suggested `OsparcVariableIdentifier | str`
-    # NOTE: in below regex `{`` and `}` are respectively escaped with `{{` and `}}`
-    pattern = (
-        rf"^\${{1,2}}(?:\{{)?{OSPARC_IDENTIFIER_PREFIX}[A-Za-z0-9_]+(?:\}})?(:-.+)?$"
-    )
 
     def _get_without_template_markers(self) -> str:
         # $VAR
@@ -42,6 +39,40 @@ def default_value(self) -> str | None:
         parts = self._get_without_template_markers().split(":-")
         return parts[1] if len(parts) > 1 else None
 
+    @staticmethod
+    def get_pattern(max_dollars: PositiveInt) -> str:
+        # NOTE: in below regex `{`` and `}` are respectively escaped with `{{` and `}}`
+        return rf"^\${{1,{max_dollars}}}(?:\{{)?{OSPARC_IDENTIFIER_PREFIX}[A-Za-z0-9_]+(?:\}})?(:-.+)?$"
+
+
+class PlatformOsparcVariableIdentifier(_BaseOsparcVariableIdentifier):
+    pattern = _BaseOsparcVariableIdentifier.get_pattern(max_dollars=2)
+
+
+class OoilOsparcVariableIdentifier(_BaseOsparcVariableIdentifier):
+    pattern = _BaseOsparcVariableIdentifier.get_pattern(max_dollars=4)
+
+
+_PLATFORM: Final[str] = "platform"
+_OOIL_VERSION: Final[str] = "ooil-version"
+
+
+def _get_discriminator_value(v: Any) -> str:
+    _ = v
+    if os.environ.get("ENABLE_OOIL_OSPARC_VARIABLE_IDENTIFIER", None):
+        return _OOIL_VERSION
+
+    return _PLATFORM
+
+
+OsparcVariableIdentifier = Annotated[
+    (
+        Annotated[PlatformOsparcVariableIdentifier, Tag(_PLATFORM)]
+        | Annotated[OoilOsparcVariableIdentifier, Tag(_OOIL_VERSION)]
+    ),
+    Discriminator(_get_discriminator_value),
+]
+
 
 class UnresolvedOsparcVariableIdentifierError(OsparcErrorMixin, TypeError):
     msg_template = "Provided argument is unresolved: value={value}"
@@ -59,9 +90,9 @@ def example_func(par: OsparcVariableIdentifier | int) -> None:
     Raises:
         TypeError: if the the OsparcVariableIdentifier was unresolved
     """
-    if isinstance(var, OsparcVariableIdentifier):
+    if isinstance(var, get_types_from_annotated_union(OsparcVariableIdentifier)):
         raise UnresolvedOsparcVariableIdentifierError(value=var)
-    return var
+    return var  # type: ignore[return-value]
 
 
 def replace_osparc_variable_identifier(  # noqa: C901
@@ -86,11 +117,11 @@ def replace_osparc_variable_identifier(  # noqa: C901
     ```
     """
 
-    if isinstance(obj, OsparcVariableIdentifier):
-        if obj.name in osparc_variables:
-            return deepcopy(osparc_variables[obj.name])  # type: ignore
-        if obj.default_value is not None:
-            return deepcopy(obj.default_value)  # type: ignore
+    if isinstance(obj, get_types_from_annotated_union(OsparcVariableIdentifier)):
+        if obj.name in osparc_variables:  # type: ignore[attr-defined]
+            return deepcopy(osparc_variables[obj.name])  # type: ignore[no-any-return,attr-defined]
+        if obj.default_value is not None:  # type: ignore[attr-defined]
+            return deepcopy(obj.default_value)  # type: ignore[no-any-return,attr-defined]
     elif isinstance(obj, dict):
         for key, value in obj.items():
             obj[key] = replace_osparc_variable_identifier(value, osparc_variables)
@@ -124,7 +155,7 @@ def raise_if_unresolved_osparc_variable_identifier_found(obj: Any) -> None:
         UnresolvedOsparcVariableIdentifierError: if not all instances of
         `OsparcVariableIdentifier` were replaced
     """
-    if isinstance(obj, OsparcVariableIdentifier):
+    if isinstance(obj, get_types_from_annotated_union(OsparcVariableIdentifier)):
         raise_if_unresolved(obj)
     elif isinstance(obj, dict):
         for key, value in obj.items():

packages/models-library/src/models_library/service_settings_nat_rule.py

@@ -1,10 +1,21 @@
 from collections.abc import Generator
 from typing import Final
 
-from pydantic import BaseModel, ConfigDict, Field, TypeAdapter, ValidationInfo, field_validator
+from pydantic import (
+    BaseModel,
+    ConfigDict,
+    Field,
+    TypeAdapter,
+    ValidationInfo,
+    field_validator,
+)
 
 from .basic_types import PortInt
-from .osparc_variable_identifier import OsparcVariableIdentifier, raise_if_unresolved
+from .osparc_variable_identifier import (
+    OsparcVariableIdentifier,
+    raise_if_unresolved,
+)
+from .utils.types import get_types_from_annotated_union
 
 # Cloudflare DNS server address
 DEFAULT_DNS_SERVER_ADDRESS: Final[str] = "1.1.1.1"  # NOSONAR
@@ -20,13 +31,15 @@ class _PortRange(BaseModel):
     @field_validator("upper")
     @classmethod
     def lower_less_than_upper(cls, v, info: ValidationInfo) -> PortInt:
-        if isinstance(v, OsparcVariableIdentifier):
+        if isinstance(v, get_types_from_annotated_union(OsparcVariableIdentifier)):
             return v  # type: ignore # bypass validation if unresolved
 
         upper = v
         lower: PortInt | OsparcVariableIdentifier | None = info.data.get("lower")
 
-        if lower and isinstance(lower, OsparcVariableIdentifier):
+        if lower and isinstance(
+            lower, get_types_from_annotated_union(OsparcVariableIdentifier)
+        ):
             return v  # type: ignore # bypass validation if unresolved
 
         if lower is None or lower >= upper:

packages/models-library/src/models_library/utils/types.py

@@ -0,0 +1,34 @@
+from functools import lru_cache
+from typing import Annotated, Any, Union, get_args, get_origin
+
+
+@lru_cache
+def get_types_from_annotated_union(annotated_alias: Any) -> tuple[type, ...]:
+    """
+    Introspects a complex Annotated alias to extract the base types from its inner Union.
+    """
+    if get_origin(annotated_alias) is not Annotated:
+        msg = "Expected an Annotated type."
+        raise TypeError(msg)
+
+    # Get the contents of Annotated, e.g., (Union[...], Discriminator(...))
+    annotated_args = get_args(annotated_alias)
+    union_type = annotated_args[0]
+
+    # The Union can be from typing.Union or the | operator
+    if get_origin(union_type) is not Union:
+        msg = "Expected a Union inside the Annotated type."
+        raise TypeError(msg)
+
+    # Get the members of the Union, e.g., (Annotated[TypeA, ...], Annotated[TypeB, ...])
+    union_members = get_args(union_type)
+
+    extracted_types = []
+    for member in union_members:
+        # Each member is also Annotated, so we extract its base type
+        if get_origin(member) is Annotated:
+            extracted_types.append(get_args(member)[0])
+        else:
+            extracted_types.append(member)  # Handle non-annotated members in the union
+
+    return tuple(extracted_types)

packages/models-library/tests/test_service_settings_nat_rule.py

@@ -9,6 +9,7 @@
     replace_osparc_variable_identifier,
 )
 from models_library.service_settings_nat_rule import NATRule
+from models_library.utils.types import get_types_from_annotated_union
 from pydantic import TypeAdapter
 
 SUPPORTED_TEMPLATES: set[str] = {
@@ -111,13 +112,13 @@ def test_______(replace_with_value: Any):
     a_var = TypeAdapter(OsparcVariableIdentifier).validate_python(
         "$OSPARC_VARIABLE_some_var"
     )
-    assert isinstance(a_var, OsparcVariableIdentifier)
+    assert isinstance(a_var, get_types_from_annotated_union(OsparcVariableIdentifier))
 
     replaced_var = replace_osparc_variable_identifier(
         a_var, {"OSPARC_VARIABLE_some_var": replace_with_value}
     )
     # NOTE: after replacement the original reference still points
-    assert isinstance(a_var, OsparcVariableIdentifier)
+    assert isinstance(a_var, get_types_from_annotated_union(OsparcVariableIdentifier))
     assert replaced_var == replace_with_value
 
 
@@ -154,15 +155,15 @@ def test_replace_an_instance_of_osparc_variable_identifier(
         formatted_template = var_template
 
     a_var = TypeAdapter(OsparcVariableIdentifier).validate_python(formatted_template)
-    assert isinstance(a_var, OsparcVariableIdentifier)
+    assert isinstance(a_var, get_types_from_annotated_union(OsparcVariableIdentifier))
 
     replace_with_identifier_default = identifier_has_default and replace_with_default
     replacement_content = (
         {} if replace_with_identifier_default else {a_var.name: replace_with_value}
     )
     replaced_var = replace_osparc_variable_identifier(a_var, replacement_content)
     # NOTE: after replacement the original reference still points
-    assert isinstance(a_var, OsparcVariableIdentifier)
+    assert isinstance(a_var, get_types_from_annotated_union(OsparcVariableIdentifier))
     if replace_with_identifier_default:
         assert replaced_var == default_value
     else:

packages/service-integration/src/service_integration/__init__.py

@@ -1,5 +1,3 @@
-""" Library to facilitate the integration of user services running in osparc-simcore
-
-"""
+"""Library to facilitate the integration of user services running in osparc-simcore"""
 
 from ._meta import __version__

packages/service-integration/tests/data/runtime.yml

@@ -1,8 +1,3 @@
-paths-mapping:
-  inputs_path: "/config/workspace/inputs"
-  outputs_path: "/config/workspace/outputs"
-  state_paths:
-    - "/config"
 settings:
   - name: resources
     type: Resources
@@ -16,7 +11,32 @@ settings:
     type: string
     value:
       - node.platform.os == linux
-  # # https://docs.docker.com/compose/compose-file/compose-file-v3/#environment
-  # - name: environment
-  #   type: string
-  #     -
+paths-mapping:
+  inputs_path: "/config/workspace/inputs"
+  outputs_path: "/config/workspace/outputs"
+  state_paths:
+    - "/config"
+callbacks-mapping:
+  inactivity:
+    service: container
+    command: ["python", "/usr/local/bin/service-monitor/activity.py"]
+    timeout: 1
+compose-spec:
+  version: "3.7"
+  services:
+    jupyter-math:
+      image: $$$${SIMCORE_REGISTRY}/simcore/services/dynamic/jupyter-math:$$$${SERVICE_VERSION}
+      environment:
+        - OSPARC_API_HOST=$$$${OSPARC_VARIABLE_API_HOST}
+        - OSPARC_API_KEY=$$$${OSPARC_VARIABLE_API_KEY}
+        - OSPARC_API_SECRET=$$$${OSPARC_VARIABLE_API_SECRET}
+container-http-entrypoint: jupyter-math
+containers-allowed-outgoing-permit-list:
+    jupyter-math:
+        - hostname: $$$${OSPARC_VARIABLE_VENDOR_SECRET_LICENSE_SERVER_HOST}
+          tcp_ports: [$$OSPARC_VARIABLE_VENDOR_SECRET_LICENSE_SERVER_PRIMARY_PORT, $$OSPARC_VARIABLE_VENDOR_SECRET_LICENSE_SERVER_SECONDARY_PORT]
+          dns_resolver:
+              address: $$$${OSPARC_VARIABLE_VENDOR_SECRET_LICENSE_DNS_RESOLVER_IP}
+              port: $$$${OSPARC_VARIABLE_VENDOR_SECRET_LICENSE_DNS_RESOLVER_PORT}
+containers-allowed-outgoing-internet:
+  - jupyter-math

packages/service-integration/tests/test_osparc_image_specs.py

@@ -8,6 +8,7 @@
 import pytest
 import yaml
 from pydantic import BaseModel
+from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
 from service_integration.compose_spec_model import BuildItem, Service
 from service_integration.osparc_config import (
     DockerComposeOverwriteConfig,
@@ -19,7 +20,13 @@
 
 
 @pytest.fixture
-def settings() -> AppSettings:
+def settings(monkeypatch: pytest.MonkeyPatch) -> AppSettings:
+    setenvs_from_dict(
+        monkeypatch,
+        {
+            "ENABLE_OOIL_OSPARC_VARIABLE_IDENTIFIER": "true",
+        },
+    )
     return AppSettings()