Skip to content

Commit 5d1f816

Browse files
Alex-Welshsd109
Alex-Welsh
and
sd109
authored
Handle missing endpoints in service catalogue (#182)
* Refactor endpoint initialisation loop This change refactors the endpoint initialisation, swapping list comprehensions and functional loops to a more traditional sequential style. This allows for improved error handling and much better readability. * Add unit tests for cloud class * Fix linter issues * Improved unit tests with comments and more cases * Fix linter issues * Apply suggestions from code review --------- Co-authored-by: Scott Davidson <[email protected]>
1 parent 4e96b1c commit 5d1f816

File tree

2 files changed

+281
-12
lines changed

2 files changed

+281
-12
lines changed

‎capi_janitor/openstack/openstack.py

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -186,18 +186,14 @@ async def __aenter__(self):
186186
return self
187187
else:
188188
raise
189-
self._endpoints = {
190-
entry["type"]: next(
191-
ep["url"]
192-
for ep in entry["endpoints"]
193-
if (
194-
ep["interface"] == self._interface
195-
and (not self._region or ep["region"] == self._region)
196-
)
197-
)
198-
for entry in response.json()["catalog"]
199-
if len(entry["endpoints"]) > 0
200-
}
189+
self._endpoints = {}
190+
for entry in response.json()["catalog"]:
191+
for ep in entry.get("endpoints", []):
192+
if ep.get("interface") == self._interface and (
193+
not self._region or ep.get("region_id") == self._region
194+
):
195+
self._endpoints[entry["type"]] = ep["url"]
196+
break
201197
return self
202198

203199
async def __aexit__(self, exc_type, exc_value, traceback):

‎capi_janitor/tests/openstack/test_openstack.py

Lines changed: 273 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,273 @@
1+
import unittest
2+
from unittest.mock import AsyncMock, MagicMock, patch
3+
4+
from capi_janitor.openstack.openstack import AuthenticationError, Cloud
5+
6+
7+
class TestCloudAsyncContext(unittest.IsolatedAsyncioTestCase):
8+
# Set up common variables for all tests
9+
async def asyncSetUp(self):
10+
# Auth is mocked to simulate authentication
11+
self.auth = MagicMock()
12+
# Transport is awaited so can be Async Mocked
13+
self.transport = AsyncMock()
14+
# Interface & Region can be fixed for the tests
15+
self.interface = "public"
16+
self.region = "region1"
17+
# Create a Cloud instance with the mocked auth and transport
18+
self.cloud = Cloud(self.auth, self.transport, self.interface, self.region)
19+
20+
# Test the __aenter__ method for auth success and general functionality
21+
@patch("capi_janitor.openstack.openstack.Client")
22+
async def test_cloud_successful_authentication(self, mock_client):
23+
# Patched client to simulate a successful authentication
24+
mock_client_instance = AsyncMock()
25+
# Return mock for the client
26+
mock_client.return_value = mock_client_instance
27+
# Mock the get method to return a simple successful response
28+
mock_client_instance.get.return_value.json = MagicMock(
29+
return_value={
30+
"catalog": [
31+
{
32+
"type": "compute",
33+
"endpoints": [
34+
{
35+
"interface": "public",
36+
"region_id": "region1",
37+
"url": "https://compute.example.com",
38+
}
39+
],
40+
}
41+
]
42+
}
43+
)
44+
# Mock the base_url for the client
45+
mock_client_instance._base_url = "https://compute.example.com"
46+
47+
# Assert return values
48+
async with self.cloud as cloud:
49+
self.assertTrue(cloud.is_authenticated)
50+
self.assertIn("compute", cloud.apis)
51+
self.assertEqual(
52+
cloud.api_client("compute")._base_url, "https://compute.example.com"
53+
)
54+
mock_client_instance.get.assert_called_once_with("/v3/auth/catalog")
55+
56+
# Test the __aenter__ method for auth failure
57+
@patch("capi_janitor.openstack.openstack.Client")
58+
async def test_cloud_authentication_failure(self, mock_client):
59+
mock_client_instance = AsyncMock()
60+
mock_client.return_value = mock_client_instance
61+
# Simulate an auth error with a named user
62+
mock_client_instance.get.side_effect = AuthenticationError("test_user")
63+
64+
with self.assertRaises(AuthenticationError) as context:
65+
async with self.cloud:
66+
pass
67+
# Assert that the AuthenticationError is raised with the correct message
68+
self.assertEqual(
69+
str(context.exception), "failed to authenticate as user: test_user"
70+
)
71+
72+
# Test the __aenter__ method for 404 error
73+
@patch("capi_janitor.openstack.openstack.Client")
74+
async def test_cloud_auth_404_error(self, mock_client):
75+
mock_client_instance = AsyncMock()
76+
mock_client.return_value = mock_client_instance
77+
# Simulate a 404 error
78+
mock_client_instance.get.side_effect = MagicMock(
79+
response=MagicMock(status_code=404)
80+
)
81+
82+
# Assert auth failed and no endpoints are returned
83+
async with self.cloud as cloud:
84+
self.assertFalse(cloud.is_authenticated)
85+
self.assertEqual(cloud.apis, [])
86+
87+
# Test the __aenter__ method for no matching interface
88+
@patch("capi_janitor.openstack.openstack.Client")
89+
async def test_cloud_no_matching_interface(self, mock_client):
90+
mock_client_instance = AsyncMock()
91+
mock_client.return_value = mock_client_instance
92+
# No matching interface in the response
93+
mock_client_instance.get.return_value.json = MagicMock(
94+
return_value={
95+
"catalog": [
96+
{
97+
"type": "compute",
98+
"endpoints": [
99+
{
100+
"interface": "internal",
101+
"region_id": "region1",
102+
"url": "https://compute.example.com",
103+
}
104+
],
105+
}
106+
]
107+
}
108+
)
109+
110+
async with self.cloud as cloud:
111+
self.assertFalse(cloud.is_authenticated)
112+
self.assertEqual(cloud.apis, [])
113+
114+
@patch("capi_janitor.openstack.openstack.Client")
115+
async def test_cloud_no_matching_region_id(self, mock_client):
116+
mock_client_instance = AsyncMock()
117+
mock_client.return_value = mock_client_instance
118+
# No matching region_id in the response
119+
mock_client_instance.get.return_value.json = MagicMock(
120+
return_value={
121+
"catalog": [
122+
{
123+
"type": "compute",
124+
"endpoints": [
125+
{
126+
"interface": "public",
127+
"region_id": "region2",
128+
"url": "https://compute.example.com",
129+
}
130+
],
131+
}
132+
]
133+
}
134+
)
135+
136+
async with self.cloud as cloud:
137+
self.assertFalse(cloud.is_authenticated)
138+
self.assertEqual(cloud.apis, [])
139+
140+
@patch("capi_janitor.openstack.openstack.Client")
141+
async def test_cloud_filter_endpoints(self, mock_client):
142+
mock_client_instance = AsyncMock()
143+
mock_client.return_value = mock_client_instance
144+
# Return multiple endpoints, one matching, one not
145+
mock_client_instance.get.return_value.json = MagicMock(
146+
return_value={
147+
"catalog": [
148+
{
149+
"type": "compute",
150+
"endpoints": [
151+
{
152+
"interface": "public",
153+
"region_id": "region1",
154+
"url": "https://compute.example.com",
155+
}
156+
],
157+
},
158+
{
159+
"type": "network",
160+
"endpoints": [
161+
{
162+
"interface": "internal",
163+
"region_id": "region1",
164+
"url": "https://network.example.com",
165+
}
166+
],
167+
},
168+
]
169+
}
170+
)
171+
172+
async with self.cloud as cloud:
173+
self.assertTrue(cloud.is_authenticated)
174+
self.assertIn("compute", cloud.apis)
175+
self.assertNotIn("network", cloud.apis)
176+
177+
@patch("capi_janitor.openstack.openstack.Client")
178+
async def test_cloud_multiple_services(self, mock_client):
179+
mock_client_instance = AsyncMock()
180+
mock_client.return_value = mock_client_instance
181+
# Return multiple services, some matching, some not
182+
mock_client_instance.get.return_value.json = MagicMock(
183+
return_value={
184+
"catalog": [
185+
{
186+
"type": "compute",
187+
"endpoints": [
188+
{
189+
"interface": "public",
190+
"region_id": "region1",
191+
"url": "https://compute.example.com",
192+
}
193+
],
194+
},
195+
{
196+
"type": "storage",
197+
"endpoints": [
198+
{
199+
"interface": "internal",
200+
"region_id": "region1",
201+
"url": "https://storage.example.com",
202+
}
203+
],
204+
},
205+
{
206+
"type": "network",
207+
"endpoints": [
208+
{
209+
"interface": "public",
210+
"region_id": "region1",
211+
"url": "https://network.example.com",
212+
}
213+
],
214+
},
215+
]
216+
}
217+
)
218+
219+
async with self.cloud as cloud:
220+
self.assertTrue(cloud.is_authenticated)
221+
self.assertIn("compute", cloud.apis)
222+
self.assertNotIn("storage", cloud.apis)
223+
self.assertIn("network", cloud.apis)
224+
225+
@patch("capi_janitor.openstack.openstack.Client")
226+
async def test_cloud_empty_endpoint_list(self, mock_client):
227+
mock_client_instance = AsyncMock()
228+
mock_client.return_value = mock_client_instance
229+
mock_client_instance.get.return_value.json = MagicMock(
230+
return_value={"catalog": [{"type": "compute", "endpoints": []}]}
231+
)
232+
233+
async with self.cloud as cloud:
234+
self.assertFalse(cloud.is_authenticated)
235+
236+
@patch("capi_janitor.openstack.openstack.Client")
237+
async def test_cloud_no_region_specified(self, mock_client):
238+
# Set up the cloud instance without a region
239+
self.cloud = Cloud(self.auth, self.transport, self.interface, region=None)
240+
mock_client_instance = AsyncMock()
241+
mock_client.return_value = mock_client_instance
242+
# Return endpoints with different region_ids
243+
mock_client_instance.get.return_value.json = MagicMock(
244+
return_value={
245+
"catalog": [
246+
{
247+
"type": "compute",
248+
"endpoints": [
249+
{
250+
"interface": "public",
251+
"region_id": "region1",
252+
"url": "https://compute.example.com",
253+
}
254+
],
255+
},
256+
{
257+
"type": "network",
258+
"endpoints": [
259+
{
260+
"interface": "public",
261+
"region_id": "region2",
262+
"url": "https://network.example.com",
263+
}
264+
],
265+
},
266+
]
267+
}
268+
)
269+
270+
async with self.cloud as cloud:
271+
self.assertTrue(cloud.is_authenticated)
272+
self.assertIn("compute", cloud.apis)
273+
self.assertIn("network", cloud.apis)

0 commit comments

Comments
 (0)