From dfcdd57bd1241845ddea221590a670734426529b Mon Sep 17 00:00:00 2001 From: Gregory Thiemonge Date: Tue, 23 Feb 2021 08:29:41 +0100 Subject: [PATCH] Improve error messages in list/show calls Some list/show calls didn't use the @correct_return_codes decorator, users may have received obscure error messages when they perfomed unauthorized/invalid requests. Story 2008647 Task 41910 Change-Id: I9ad1f709403e8a33ef07ccf859c4267412fc3186 --- octaviaclient/api/v2/octavia.py | 23 ++++ octaviaclient/tests/unit/api/test_octavia.py | 117 ++++++++++++++++++ ...es-for-invalid-calls-bc9eb3d18d3e4fdf.yaml | 5 + 3 files changed, 145 insertions(+) create mode 100644 releasenotes/notes/improve-error-messages-for-invalid-calls-bc9eb3d18d3e4fdf.yaml diff --git a/octaviaclient/api/v2/octavia.py b/octaviaclient/api/v2/octavia.py index 164375f..33594a3 100644 --- a/octaviaclient/api/v2/octavia.py +++ b/octaviaclient/api/v2/octavia.py @@ -82,6 +82,7 @@ class OctaviaAPI(api.BaseAPI): if not self.endpoint.endswith(self._endpoint_suffix): self.endpoint += self._endpoint_suffix + @correct_return_codes def load_balancer_list(self, **params): """List all load balancers @@ -154,6 +155,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def load_balancer_stats_show(self, lb_id, **kwargs): """Shows the current statistics for a load balancer. @@ -167,6 +169,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def load_balancer_status_show(self, lb_id, **kwargs): """Display load balancer status tree in json format. @@ -194,6 +197,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def listener_list(self, **kwargs): """List all listeners @@ -264,6 +268,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def listener_stats_show(self, listener_id, **kwargs): """Shows the current statistics for a listener @@ -277,6 +282,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def pool_list(self, **kwargs): """List all pools @@ -347,6 +353,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def member_list(self, pool_id, **kwargs): """Lists the member from a given pool id @@ -432,6 +439,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def l7policy_list(self, **kwargs): """List all l7policies @@ -502,6 +510,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def l7rule_list(self, l7policy_id, **kwargs): """List all l7rules for a l7policy @@ -583,6 +592,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def health_monitor_list(self, **kwargs): """List all health monitors @@ -656,6 +666,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def quota_list(self, **params): """List all quotas @@ -712,6 +723,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def quota_defaults_show(self): """Show quota defaults @@ -737,6 +749,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def amphora_list(self, **kwargs): """List all amphorae @@ -792,6 +805,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def amphora_stats_show(self, amphora_id, **kwargs): """Show the current statistics for an amphora @@ -805,6 +819,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def provider_list(self): """List all providers @@ -816,6 +831,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def provider_flavor_capability_list(self, provider): """Show the flavor capabilities of the specified provider. @@ -830,6 +846,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def provider_availability_zone_capability_list(self, provider): """Show the availability zone capabilities of the specified provider. @@ -844,6 +861,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def flavor_list(self, **kwargs): """List all flavors @@ -928,6 +946,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def flavorprofile_list(self, **kwargs): """List all flavor profiles @@ -941,6 +960,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def flavorprofile_show(self, flavorprofile_id): """Show a flavor profile @@ -984,6 +1004,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def availabilityzone_list(self, **kwargs): """List all availabilityzones @@ -1072,6 +1093,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def availabilityzoneprofile_list(self, **kwargs): """List all availabilityzone profiles @@ -1085,6 +1107,7 @@ class OctaviaAPI(api.BaseAPI): return response + @correct_return_codes def availabilityzoneprofile_show(self, availabilityzoneprofile_id): """Show a availabilityzone profile diff --git a/octaviaclient/tests/unit/api/test_octavia.py b/octaviaclient/tests/unit/api/test_octavia.py index 632dc52..536cc8c 100644 --- a/octaviaclient/tests/unit/api/test_octavia.py +++ b/octaviaclient/tests/unit/api/test_octavia.py @@ -127,6 +127,15 @@ LIST_AZPF_RESP = { {'name': 'azpf2'}] } +POLICY_ERROR_STRING = ( + "Policy does not allow this request to be performed.") + +LIST_POLICY_ERR_RESP = { + 'faultcode': "Client", + 'faultstring': POLICY_ERROR_STRING, + 'debuginfo': None +} + SINGLE_LB_RESP = {'loadbalancer': {'id': FAKE_LB, 'name': 'lb1'}} SINGLE_LB_UPDATE = {"loadbalancer": {"admin_state_up": False}} SINGLE_LB_STATS_RESP = {'bytes_in': '0'} @@ -205,6 +214,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.load_balancer_list() self.assertEqual(LIST_LB_RESP, ret) + def test_list_load_balancer_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'loadbalancers', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.load_balancer_list) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_load_balancer(self): self.requests_mock.register_uri( 'GET', @@ -343,6 +364,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.listener_list() self.assertEqual(LIST_LI_RESP, ret) + def test_list_listener_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'listeners', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.listener_list) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_listener(self): self.requests_mock.register_uri( 'GET', @@ -438,6 +471,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.pool_list() self.assertEqual(LIST_PO_RESP, ret) + def test_list_pool_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'pools', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.pool_list) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_pool(self): self.requests_mock.register_uri( 'GET', @@ -523,6 +568,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.member_list(FAKE_PO) self.assertEqual(LIST_ME_RESP, ret) + def test_list_member_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'pools/' + FAKE_PO + '/members', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.member_list, FAKE_PO) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_member(self): self.requests_mock.register_uri( 'GET', @@ -610,6 +667,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.l7policy_list() self.assertEqual(LIST_L7PO_RESP, ret) + def test_list_l7policy_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'l7policies', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.l7policy_list) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_l7policy(self): self.requests_mock.register_uri( 'GET', @@ -695,6 +764,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.l7rule_list(FAKE_L7PO) self.assertEqual(LIST_L7RU_RESP, ret) + def test_list_l7rule_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'l7policies/' + FAKE_L7PO + '/rules', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.l7rule_list, FAKE_L7PO) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_l7rule(self): self.requests_mock.register_uri( 'GET', @@ -790,6 +871,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.health_monitor_list() self.assertEqual(LIST_HM_RESP, ret) + def test_list_health_monitor_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'healthmonitors', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.health_monitor_list) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_health_monitor(self): self.requests_mock.register_uri( 'GET', @@ -875,6 +968,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.quota_list() self.assertEqual(LIST_QT_RESP, ret) + def test_list_quota_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_LBAAS_URL + 'quotas', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.quota_list) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_quota(self): self.requests_mock.register_uri( 'GET', @@ -938,6 +1043,18 @@ class TestLoadBalancer(TestOctaviaClient): ret = self.api.amphora_list() self.assertEqual(LIST_AMP_RESP, ret) + def test_list_amphora_not_allowed(self): + self.requests_mock.register_uri( + 'GET', + FAKE_OCTAVIA_URL + 'amphorae', + json=LIST_POLICY_ERR_RESP, + status_code=403, + ) + ret = self.assertRaises( + octavia.OctaviaClientException, + self.api.amphora_list) + self.assertEqual(POLICY_ERROR_STRING, ret.message) + def test_show_amphora(self): self.requests_mock.register_uri( 'GET', diff --git a/releasenotes/notes/improve-error-messages-for-invalid-calls-bc9eb3d18d3e4fdf.yaml b/releasenotes/notes/improve-error-messages-for-invalid-calls-bc9eb3d18d3e4fdf.yaml new file mode 100644 index 0000000..efe997b --- /dev/null +++ b/releasenotes/notes/improve-error-messages-for-invalid-calls-bc9eb3d18d3e4fdf.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Improve CLI error messages when a user performs an unauthorized/invalid + request.