Issue #112 Eliminate unused AggregatorConfig.configured_oidc_providers

All usage is migrated to `OpenEoBackendConfig.oidc_providers` by now

CHANGELOG.md

@@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file.
 
 The format is roughly based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
+## [0.22.0]
+
+- Eliminate unused `AggregatorConfig.configured_oidc_providers` ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112))
+
 ## [0.21.0]
 
 - Add `AggregatorBackendConfig` to `conf/aggregator.*.py` files  ([#112](https://github.com/Open-EO/openeo-aggregator/issues/112))

src/openeo_aggregator/about.py

@@ -2,7 +2,7 @@
 import sys
 from typing import Optional
 
-__version__ = "0.21.1a1"
+__version__ = "0.22.0a1"
 
 
 def log_version_info(logger: Optional[logging.Logger] = None):

src/openeo_aggregator/backend.py

@@ -1318,9 +1318,7 @@ def __init__(self, backends: MultiBackendConnection, config: AggregatorConfig):
             user_defined_processes=user_defined_processes,
         )
         # TODO #112 once `AggregatorConfig.configured_oidc_providers` is eliminated, this `_configured_oidc_providers` is not necessary anymore.
-        self._configured_oidc_providers: List[OidcProvider] = (
-            get_backend_config().oidc_providers or config.configured_oidc_providers
-        )
+        self._configured_oidc_providers: List[OidcProvider] = get_backend_config().oidc_providers
         self._auth_entitlement_check: Union[bool, dict] = get_backend_config().auth_entitlement_check
 
         self._memoizer: Memoizer = memoizer_from_config(config=config, namespace="general")

src/openeo_aggregator/config.py

@@ -45,9 +45,6 @@ class AggregatorConfig(dict):
     # Dictionary mapping backend id to backend url
     aggregator_backends = dict_item()
 
-    # TODO #112 `configured_oidc_providers` is deprecated, use `OpenEoBackendConfig.oidc_providers` instead
-    configured_oidc_providers: List[OidcProvider] = dict_item(default=[])
-
     partitioned_job_tracking = dict_item(default=None)
     # TODO #112 Deprecated, use AggregatorBackendConfig.zookeeper_prefix instead
     zookeeper_prefix = dict_item(default="/openeo-aggregator/")

src/openeo_aggregator/connection.py

@@ -73,12 +73,13 @@ class BackendConnection(Connection):
     #       designed for single-user use case (e.g. caching, working local config files, ...)
 
     def __init__(
-            self,
-            id: str,
-            url: str,
-            configured_oidc_providers: List[OidcProvider],
-            default_timeout: int = CONNECTION_TIMEOUT_DEFAULT,
-            init_timeout: int = CONNECTION_TIMEOUT_INIT,
+        self,
+        id: str,
+        url: str,
+        *,
+        configured_oidc_providers: Optional[List[OidcProvider]] = None,
+        default_timeout: int = CONNECTION_TIMEOUT_DEFAULT,
+        init_timeout: int = CONNECTION_TIMEOUT_INIT,
     ):
         # Temporarily unlock `_auth` for `super().__init__()`
         self._auth_locked = False
@@ -93,6 +94,8 @@ def __init__(
             v=openeo_aggregator.about.__version__,
         )
         # Mapping of aggregator provider id to backend's provider id
+        if configured_oidc_providers is None:
+            configured_oidc_providers = get_backend_config().oidc_providers
         self._oidc_provider_map: Dict[str, str] = self._build_oidc_provider_map(configured_oidc_providers)
 
         self.default_timeout = default_timeout
@@ -246,7 +249,7 @@ def __init__(
     def from_config(config: AggregatorConfig) -> 'MultiBackendConnection':
         return MultiBackendConnection(
             backends=config.aggregator_backends,
-            configured_oidc_providers=get_backend_config().oidc_providers or config.configured_oidc_providers,
+            configured_oidc_providers=get_backend_config().oidc_providers,
             memoizer=memoizer_from_config(config, namespace="mbcon"),
             connections_cache_ttl=get_backend_config().connections_cache_ttl,
         )

tests/conftest.py

@@ -73,16 +73,6 @@ def backend2(requests_mock, mbldr) -> str:
     return domain
 
 
-@pytest.fixture
-def configured_oidc_providers() -> List[OidcProvider]:
-    return [
-        OidcProvider(id="egi", issuer="https://egi.test", title="EGI"),
-        OidcProvider(id="x-agg", issuer="https://x.test", title="X (agg)"),
-        OidcProvider(id="y-agg", issuer="https://y.test", title="Y (agg)"),
-        OidcProvider(id="z-agg", issuer="https://z.test", title="Z (agg)"),
-    ]
-
-
 @pytest.fixture
 def zk_client() -> DummyKazooClient:
     return DummyKazooClient()
@@ -104,14 +94,12 @@ def memoizer_config() -> dict:
 
 
 @pytest.fixture
-def base_config(configured_oidc_providers, zk_client, memoizer_config) -> AggregatorConfig:
+def base_config(zk_client, memoizer_config) -> AggregatorConfig:
     """Base config for tests (without any configured backends)."""
     conf = AggregatorConfig()
     conf.config_source = "test fixture base_config"
     # conf.flask_error_handling = False  # Temporary disable flask error handlers to simplify debugging (better stack traces).
 
-    conf.configured_oidc_providers = configured_oidc_providers
-
     conf.memoizer = memoizer_config
 
     conf.partitioned_job_tracking = {

tests/test_backend.py

@@ -52,19 +52,6 @@ def test_oidc_providers(self, multi_backend_connection, config, backend1, backen
             OidcProvider(id='z-agg', issuer='https://z.test', title='Z (agg)'),
         ]
 
-    def test_oidc_providers_legacy_config_support(self, multi_backend_connection, config, backend1, backend2):
-        """Test for legacy AggregatorConfig.configured_oidc_providers support."""
-        # TODO #112 test to remove in the future
-        with config_overrides(oidc_providers=[]):
-            implementation = AggregatorBackendImplementation(backends=multi_backend_connection, config=config)
-        providers = implementation.oidc_providers()
-        assert providers == [
-            OidcProvider(id="egi", issuer="https://egi.test", title="EGI"),
-            OidcProvider(id="x-agg", issuer="https://x.test", title="X (agg)"),
-            OidcProvider(id="y-agg", issuer="https://y.test", title="Y (agg)"),
-            OidcProvider(id="z-agg", issuer="https://z.test", title="Z (agg)"),
-        ]
-
     def test_oidc_providers_new_config_support(self, multi_backend_connection, config, backend1, backend2):
         """Test for new AggregatorBackendConfig.oidc_providers support."""
         with config_overrides(oidc_providers=[OidcProvider(id="aagg", issuer="https://aagg.test", title="Aagg)")]):

tests/test_connection.py

@@ -78,7 +78,7 @@ def test_basic_auth_from_request_failure(self, requests_mock, exception):
         # auth should be reset even with exception in `authenticated_from_request` body
         assert con.auth is None
 
-    def test_plain_oidc_auth_fails(self, requests_mock, configured_oidc_providers):
+    def test_plain_oidc_auth_fails(self, requests_mock):
         requests_mock.get("https://foo.test/", json={"api_version": "1.0.0"})
         requests_mock.get("https://foo.test/credentials/oidc", json={"providers": [
             {"id": "egi", "issuer": "https://egi.test", "title": "EGI"},
@@ -88,12 +88,12 @@ def test_plain_oidc_auth_fails(self, requests_mock, configured_oidc_providers):
             "userinfo_endpoint": "https://egi.test/userinfo",
         })
         requests_mock.post("https://egi.test/token", json={"access_token": "3nt3r"})
-        con = BackendConnection(id="foo", url="https://foo.test", configured_oidc_providers=configured_oidc_providers)
+        con = BackendConnection(id="foo", url="https://foo.test")
         with pytest.raises(LockedAuthException):
             con.authenticate_oidc_refresh_token(client_id="cl13nt", refresh_token="r3fr35")
 
     @pytest.mark.parametrize("backend_pid", ["egi", "aho"])
-    def test_oidc_auth_from_request(self, requests_mock, configured_oidc_providers, backend_pid):
+    def test_oidc_auth_from_request(self, requests_mock, backend_pid):
         requests_mock.get("https://foo.test/", json={"api_version": "1.0.0"})
         requests_mock.get("https://foo.test/credentials/oidc", json={"providers": [
             {"id": backend_pid, "issuer": "https://egi.test", "title": "EGI"},
@@ -108,7 +108,7 @@ def get_me(request: requests.Request, context):
 
         requests_mock.get("https://foo.test/me", json=get_me)
 
-        con = BackendConnection(id="foo", url="https://foo.test", configured_oidc_providers=configured_oidc_providers)
+        con = BackendConnection(id="foo", url="https://foo.test")
         request = flask.Request(environ={"HTTP_AUTHORIZATION": "Bearer oidc/egi/l3tm31n"})
         assert con.auth is None
         with con.authenticated_from_request(request=request):
@@ -127,13 +127,13 @@ def get_me(request: requests.Request, context):
             OpenEoApiError(http_status_code=500, code="Internal", message="Nope"),
         ],
     )
-    def test_oidc_auth_from_request_failure(self, requests_mock, configured_oidc_providers, exception):
+    def test_oidc_auth_from_request_failure(self, requests_mock, exception):
         requests_mock.get("https://foo.test/", json={"api_version": "1.0.0"})
         requests_mock.get("https://foo.test/credentials/oidc", json={"providers": [
             {"id": "egi", "issuer": "https://egi.test", "title": "EGI"},
         ]})
 
-        con = BackendConnection(id="foo", url="https://foo.test", configured_oidc_providers=configured_oidc_providers)
+        con = BackendConnection(id="foo", url="https://foo.test")
         request = flask.Request(environ={"HTTP_AUTHORIZATION": "Bearer oidc/egi/l3tm31n"})
         assert con.auth is None
         with pytest.raises(exception.__class__ if isinstance(exception, Exception) else exception):