From bb3c59fe5223e83e9e711daca4ab96b6b792a27d Mon Sep 17 00:00:00 2001 From: sd109 Date: Wed, 6 Nov 2024 14:00:08 +0000 Subject: [PATCH 1/4] Filter endpoint catalog by app cred region --- capi_janitor/openstack/openstack.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/capi_janitor/openstack/openstack.py b/capi_janitor/openstack/openstack.py index 034d715..f73121b 100644 --- a/capi_janitor/openstack/openstack.py +++ b/capi_janitor/openstack/openstack.py @@ -27,10 +27,11 @@ class Auth(httpx.Auth): """ Authenticator class for OpenStack connections. """ - def __init__(self, auth_url, application_credential_id, application_credential_secret): + def __init__(self, auth_url, application_credential_id, application_credential_secret, region_name): self.url = auth_url self._application_credential_id = application_credential_id self._application_credential_secret = application_credential_secret + self.region_name = region_name self._token = None self._user_id = None self._lock = asyncio.Lock() @@ -177,7 +178,14 @@ async def __aenter__(self): entry["type"]: next( ep["url"] for ep in entry["endpoints"] - if ep["interface"] == self._interface + if ( + ep["interface"] == self._interface + # NOTE(scott): Entrypoint has 'region_id' and 'region' + # fields whereas app cred has a 'region_name' field. + # This code assumes that app cred 'region_name' maps + # to catalog entry 'region' rather than 'region_id'. + and ep["region"] == self._auth.region_name + ) ) for entry in response.json()["catalog"] if len(entry["endpoints"]) > 0 @@ -230,7 +238,8 @@ def from_clouds(cls, clouds, cloud, cacert): auth = Auth( auth_url, config["auth"]["application_credential_id"], - config["auth"]["application_credential_secret"] + config["auth"]["application_credential_secret"], + config["region_name"] ) # Create a default context using the verification from the config context = httpx.create_ssl_context(verify = config.get("verify", True)) From 544ad59ef632b43c7ae30cccc9f559c0ed03b859 Mon Sep 17 00:00:00 2001 From: sd109 Date: Wed, 6 Nov 2024 16:52:55 +0000 Subject: [PATCH 2/4] Refine endpoint filtering behaviour --- capi_janitor/openstack/openstack.py | 44 +++++++++++++++++------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/capi_janitor/openstack/openstack.py b/capi_janitor/openstack/openstack.py index f73121b..689d9ed 100644 --- a/capi_janitor/openstack/openstack.py +++ b/capi_janitor/openstack/openstack.py @@ -27,11 +27,10 @@ class Auth(httpx.Auth): """ Authenticator class for OpenStack connections. """ - def __init__(self, auth_url, application_credential_id, application_credential_secret, region_name): + def __init__(self, auth_url, application_credential_id, application_credential_secret): self.url = auth_url self._application_credential_id = application_credential_id self._application_credential_secret = application_credential_secret - self.region_name = region_name self._token = None self._user_id = None self._lock = asyncio.Lock() @@ -154,11 +153,12 @@ class Cloud: """ Object for interacting with OpenStack clouds. """ - def __init__(self, auth, transport, interface): + def __init__(self, auth, transport, interface, region = None): self._auth = auth self._transport = transport self._interface = interface self._endpoints = {} + self._region = region # A map of api name to client self._clients = {} @@ -175,18 +175,7 @@ async def __aenter__(self): else: raise self._endpoints = { - entry["type"]: next( - ep["url"] - for ep in entry["endpoints"] - if ( - ep["interface"] == self._interface - # NOTE(scott): Entrypoint has 'region_id' and 'region' - # fields whereas app cred has a 'region_name' field. - # This code assumes that app cred 'region_name' maps - # to catalog entry 'region' rather than 'region_id'. - and ep["region"] == self._auth.region_name - ) - ) + entry["type"]: self._service_endpoint(entry)["url"] for entry in response.json()["catalog"] if len(entry["endpoints"]) > 0 } @@ -229,6 +218,25 @@ def api_client(self, name, prefix = None): ) return self._clients[name] + def _service_endpoint(self, endpoints): + """ + Filters the target cloud's catalog endpoints to find the relevant entry. + """ + iface_endpoints = [ep for ep in endpoints if ep["interface"] == self._interface] + # If there's no region_name field in the clouds.yaml we use the first endpoint which + # matches the interface name for consistent behaviour with the OpenStack CLI. + if not self._region: + return iface_endpoints[0] + # Otherwise, further filter by region name + region_endpoints = [ep for ep in iface_endpoints if ep["region"] == self._region] + if len(region_endpoints) != 1: + raise Exception( + "Failed to find a unique catalog endpoints for" + f" interface {region_endpoints[0]['interface']}" + f" and region {region_endpoints[0]['region']}" + ) + return region_endpoints[0] + @classmethod def from_clouds(cls, clouds, cloud, cacert): config = clouds["clouds"][cloud] @@ -238,13 +246,13 @@ def from_clouds(cls, clouds, cloud, cacert): auth = Auth( auth_url, config["auth"]["application_credential_id"], - config["auth"]["application_credential_secret"], - config["region_name"] + config["auth"]["application_credential_secret"] ) + region = config.get("region_name") # Create a default context using the verification from the config context = httpx.create_ssl_context(verify = config.get("verify", True)) # If a cacert was given, load it into the context if cacert is not None: context.load_verify_locations(cadata = cacert) transport = httpx.AsyncHTTPTransport(verify = context) - return cls(auth, transport, config.get("interface", "public")) + return cls(auth, transport, config.get("interface", "public"), region) From 1c6f929745bd359368f16f86a60c8f7611f59abf Mon Sep 17 00:00:00 2001 From: sd109 Date: Wed, 6 Nov 2024 17:25:16 +0000 Subject: [PATCH 3/4] Fix function arg --- capi_janitor/openstack/openstack.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/capi_janitor/openstack/openstack.py b/capi_janitor/openstack/openstack.py index 689d9ed..a7bff8a 100644 --- a/capi_janitor/openstack/openstack.py +++ b/capi_janitor/openstack/openstack.py @@ -218,10 +218,11 @@ def api_client(self, name, prefix = None): ) return self._clients[name] - def _service_endpoint(self, endpoints): + def _service_endpoint(self, catalog_entry): """ Filters the target cloud's catalog endpoints to find the relevant entry. """ + endpoints = catalog_entry["endpoints"] iface_endpoints = [ep for ep in endpoints if ep["interface"] == self._interface] # If there's no region_name field in the clouds.yaml we use the first endpoint which # matches the interface name for consistent behaviour with the OpenStack CLI. From e63655e6fdfbf656239a7d31e5712b4ef04285ad Mon Sep 17 00:00:00 2001 From: sd109 Date: Wed, 6 Nov 2024 17:40:47 +0000 Subject: [PATCH 4/4] Address review comments --- capi_janitor/openstack/openstack.py | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/capi_janitor/openstack/openstack.py b/capi_janitor/openstack/openstack.py index a7bff8a..f0ec848 100644 --- a/capi_janitor/openstack/openstack.py +++ b/capi_janitor/openstack/openstack.py @@ -175,7 +175,14 @@ async def __aenter__(self): else: raise self._endpoints = { - entry["type"]: self._service_endpoint(entry)["url"] + entry["type"]: next( + ep["url"] + for ep in entry["endpoints"] + if ( + ep["interface"] == self._interface and + (not self._region or ep["region"] == self._region) + ) + ) for entry in response.json()["catalog"] if len(entry["endpoints"]) > 0 } @@ -218,26 +225,6 @@ def api_client(self, name, prefix = None): ) return self._clients[name] - def _service_endpoint(self, catalog_entry): - """ - Filters the target cloud's catalog endpoints to find the relevant entry. - """ - endpoints = catalog_entry["endpoints"] - iface_endpoints = [ep for ep in endpoints if ep["interface"] == self._interface] - # If there's no region_name field in the clouds.yaml we use the first endpoint which - # matches the interface name for consistent behaviour with the OpenStack CLI. - if not self._region: - return iface_endpoints[0] - # Otherwise, further filter by region name - region_endpoints = [ep for ep in iface_endpoints if ep["region"] == self._region] - if len(region_endpoints) != 1: - raise Exception( - "Failed to find a unique catalog endpoints for" - f" interface {region_endpoints[0]['interface']}" - f" and region {region_endpoints[0]['region']}" - ) - return region_endpoints[0] - @classmethod def from_clouds(cls, clouds, cloud, cacert): config = clouds["clouds"][cloud]