From 789a3fc8edd67367629e7dfd93262461d0acb810 Mon Sep 17 00:00:00 2001 From: Jiri Hnidek Date: Thu, 8 Aug 2024 03:07:28 +0200 Subject: [PATCH] fix: insights-client failed, when --group was used (#4070) * Card ID: CCT-186 * When system was registered against Satellite server, and "insights-client --register" was triggered --group="tag" CLI option, then registration failed * Satellite also contains some bug, and it always returns status code 200 and empty list regardless existence of given group. * Updated code according documentation: https://console.redhat.com/docs/api/inventory/v1#operations-tag-groups But I'm not able to test it, because it seems that there is no server providing this API now. * Added 4 unit test Signed-off-by: Jiri Hnidek --- insights/client/connection.py | 84 ++++++-- .../connection/test_LEGACY_reg_check.py | 203 ++++++++++++++++++ 2 files changed, 269 insertions(+), 18 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index 98e0f8175f..eaffa2112d 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -574,35 +574,83 @@ def create_system(self, new_machine_id=False): # -LEGACY- def group_systems(self, group_name, systems): """ - Adds an array of systems to specified group + Adds an array of systems to specified group. When given group_name + does not exist, then client tries to create a new group and adds + given system to the new group. Args: group_name: Display name of group systems: Array of {'machine_id': machine_id} """ - api_group_id = None + system_ids = list(systems.values()) headers = {'Content-Type': 'application/json'} group_path = self.api_url + '/v1/groups' - group_get_path = group_path + ('?display_name=%s' % quote(group_name)) + # Description of the API is here (groups GET /groups): + # yeah, links with anchors do not work as expected :-/ + # https://console.redhat.com/docs/api/inventory/v1#operations-groups-api + group_get_path = group_path + ('?name=%s' % quote(group_name)) get_group = self.get(group_get_path) + + api_group_id = None if get_group.status_code == 200: - api_group_id = get_group.json()['id'] - - if get_group.status_code == 404: - # Group does not exist, POST to create - data = json.dumps({'display_name': group_name}) - post_group = self.post(group_path, - headers=headers, - data=data) + group_info = get_group.json() + if isinstance(group_info, dict): + try: + results = group_info.get("results", None) + if isinstance(results, list) and len(results) >= 1: + group = results[0] + if isinstance(group, dict): + api_group_id = group.get("id", None) + except KeyError: + logger.error("Group info does not contain information about ID") + + if api_group_id is None: + logger.warning("Unable to get ID of group: {group_name}".format(group_name=group_name)) + else: + logger.debug("Adding systems {systems} to the group {group_name}".format( + systems=", ".join(system_ids), group_name=group_name + )) + # Documentation for this REST API is here (groups POST /groups/{group_id}/hosts): + # yeah, links with anchors do not work as expected :-/ + # https://console.redhat.com/docs/api/inventory/v1#operations-groups-api + data = json.dumps(system_ids) + post_systems = self.post( + group_path + + ('/%s/hosts' % api_group_id), + headers=headers, + data=data + ) + if post_systems.status_code == 201: + logger.info("Successfully added systems {systems} to the group {group_name}".format( + systems=", ".join(system_ids), group_name=group_name + )) + else: + logger.error("Unable to add systems {systems} to the group {group_name}".format( + systems=", ".join(system_ids), group_name=group_name + )) + + if get_group.status_code == 404 or api_group_id is None: + logger.debug("Adding systems {systems} to new group {group_name}".format( + systems=", ".join(system_ids), group_name=group_name + )) + + # Group does not exist, POST to create new group with host_ids + # Documentation for this REST API end-point is here: + # https://console.redhat.com/docs/api/inventory/v1#operations-groups-api + data = json.dumps({"name": group_name, "host_ids": system_ids}) + post_group = self.post( + group_path, + headers=headers, + data=data + ) self.handle_fail_rcs(post_group) - api_group_id = post_group.json()['id'] - - data = json.dumps(systems) - self.put(group_path + - ('/%s/systems' % api_group_id), - headers=headers, - data=data) + if post_group.status_code == 201: + logger.info("Successfully created group {group_name} containing {systems}".format( + systems=", ".join(system_ids), group_name=group_name + )) + else: + logger.error("Unable to create group: {group_name}".format(group_name=group_name)) # -LEGACY- # Keeping this function around because it's not private and I don't know if anything else uses it diff --git a/insights/tests/client/connection/test_LEGACY_reg_check.py b/insights/tests/client/connection/test_LEGACY_reg_check.py index 5e628bd675..8d72c681d9 100644 --- a/insights/tests/client/connection/test_LEGACY_reg_check.py +++ b/insights/tests/client/connection/test_LEGACY_reg_check.py @@ -24,6 +24,209 @@ def test_registration_check_ok_reg(get_proxies, _init_session, _machine_id_exist assert conn.api_registration_check() +@patch("insights.client.connection.generate_machine_id", return_value="12345678-abcd-efgh-ijkl-mnopqstvwxyz") +@patch("insights.client.connection.InsightsConnection._init_session") +@patch("insights.client.connection.InsightsConnection.get_proxies") +@patch("insights.client.connection.InsightsConnection.create_system") +def test_register_ok_reg_group(mock_create_system, _get_proxies, _init_session, _generate_machine_id): + """ + Test the case, when we try to add system to existing group + """ + + # Mock creating connection + config = Mock( + legacy_upload=True, + base_url="example.com", + group="my_testing_group_name", + ) + conn = InsightsConnection(config) + + # Mock creating system on the server + mock_create_system.return_value = Mock(status_code=200) + + # Mock getting list of groups + get_groups_res = requests.Response() + group_list = { + "count": 1, + "page": 1, + "per_page": 1, + "total": 1, + "results": [ + { + "created": "2024-06-20T15:39:54.888Z", + "id": "87654321-dcba-efgh-ijkl-123456789abc", + "name": "my_testing_group_name", + "org_id": "000102", + "updated": "2024-06-20T15:39:54.888Z", + "host_count": 0 + }, + ] + } + get_groups_res.encoding = "utf-8" + get_groups_res._content = json.dumps(group_list).encode("utf-8") + get_groups_res.status_code = 200 + conn.get = MagicMock(return_value=get_groups_res) + + # Mock putting system to the group + updated_group = { + "created": "2024-06-21T10:32:27.135Z", + "id": "bA6deCFc19564430AB814bf8F70E8cEf", + "name": "my_testing_group_name", + "org_id": "000102", + "updated": "2024-06-21T10:32:27.135Z", + "host_count": 1 + } + post_system_to_group_res = requests.Response() + post_system_to_group_res.encoding = "utf-8" + post_system_to_group_res._content = json.dumps(updated_group).encode("utf-8") + post_system_to_group_res.status_code = 201 + conn.post = MagicMock(return_value=post_system_to_group_res) + + # Test the ^%&$#@* code + _, _, group_name, _ = conn.register() + assert group_name == "my_testing_group_name" + + +@patch("insights.client.connection.generate_machine_id", return_value="12345678-abcd-efgh-ijkl-mnopqstvwxyz") +@patch("insights.client.connection.InsightsConnection._init_session") +@patch("insights.client.connection.InsightsConnection.get_proxies") +@patch("insights.client.connection.InsightsConnection.create_system") +def test_register_ok_reg_empty_group(mock_create_system, _get_proxies, _init_session, _generate_machine_id): + """ + Test the case, when we try to add system to group that does not exist, + but server response with empty list and not 404 code + """ + + # Mock creating connection + config = Mock( + legacy_upload=True, + base_url="example.com", + group="my_testing_group_name", + ) + conn = InsightsConnection(config) + + # Mock creating system on the server + mock_create_system.return_value = Mock(status_code=200) + + # Mock getting the list of groups + get_groups_res = requests.Response() + group_list = { + "count": 0, + "page": 0, + "per_page": 0, + "total": 0, + "results": [] + } + get_groups_res.encoding = "utf-8" + get_groups_res._content = json.dumps(group_list).encode("utf-8") + get_groups_res.status_code = 200 + conn.get = MagicMock(return_value=get_groups_res) + + # Mock response of creating new group with new system + new_group = { + "created": "2024-06-21T12:25:21.009Z", + "id": "bA6deCFc19564430AB814bf8F70E8cEf", + "name": "my_testing_group_name", + "org_id": "000102", + "updated": "2024-06-21T12:25:21.009Z", + "host_count": 1 + } + post_system_to_group_res = requests.Response() + post_system_to_group_res.encoding = "utf-8" + post_system_to_group_res._content = json.dumps(new_group).encode("utf-8") + post_system_to_group_res.status_code = 201 + conn.post = MagicMock(return_value=post_system_to_group_res) + + # Test the code + _, _, group_name, _ = conn.register() + assert group_name == "my_testing_group_name" + + +@patch("insights.client.connection.generate_machine_id", return_value="12345678-abcd-efgh-ijkl-mnopqstvwxyz") +@patch("insights.client.connection.InsightsConnection._init_session") +@patch("insights.client.connection.InsightsConnection.get_proxies") +@patch("insights.client.connection.InsightsConnection.create_system") +def test_register_ok_reg_non_existing_group(mock_create_system, _get_proxies, _init_session, _generate_machine_id): + """ + Test the case, when we try to add the system to a group that has not been already created, + and it is necessary to create the group during registration process. + """ + + # Mock creating connection + config = Mock( + legacy_upload=True, + base_url="example.com", + group="my_testing_group_name", + ) + conn = InsightsConnection(config) + + # Mock creating system on the server + mock_create_system.return_value = Mock(status_code=200) + + # Mock getting list of groups + get_groups_res = requests.Response() + get_groups_res.encoding = "utf-8" + get_groups_res.status_code = 404 + conn.get = MagicMock(return_value=get_groups_res) + + # Mock response of creating new group with new system + new_group = { + "created": "2024-06-21T12:25:21.009Z", + "id": "bA6deCFc19564430AB814bf8F70E8cEf", + "name": "my_testing_group_name", + "org_id": "000102", + "updated": "2024-06-21T12:25:21.009Z", + "host_count": 1 + } + post_system_to_group_res = requests.Response() + post_system_to_group_res.encoding = "utf-8" + post_system_to_group_res._content = json.dumps(new_group).encode("utf-8") + post_system_to_group_res.status_code = 201 + conn.post = MagicMock(return_value=post_system_to_group_res) + + # Test the code + _, _, group_name, _ = conn.register() + assert group_name == "my_testing_group_name" + + +@patch("insights.client.connection.generate_machine_id", return_value="12345678-abcd-efgh-ijkl-mnopqstvwxyz") +@patch("insights.client.connection.InsightsConnection._init_session") +@patch("insights.client.connection.InsightsConnection.get_proxies") +@patch("insights.client.connection.InsightsConnection.create_system") +def test_register_failed_reg_non_existing_group(mock_create_system, _get_proxies, _init_session, _generate_machine_id): + """ + Test the case, when we try to add the system to a group that has not been already created, + but it is not possible to create the group during registration process. + """ + + # Mock creating connection + config = Mock( + legacy_upload=True, + base_url="example.com", + group="my_testing_group_name", + ) + conn = InsightsConnection(config) + + # Mock creating system on the server + mock_create_system.return_value = Mock(status_code=200) + + # Mock getting list of groups + get_groups_res = requests.Response() + get_groups_res.encoding = "utf-8" + get_groups_res.status_code = 404 + conn.get = MagicMock(return_value=get_groups_res) + + # Mock response of not creating new group due to lack of permissions + post_system_to_group_res = requests.Response() + post_system_to_group_res.encoding = "utf-8" + post_system_to_group_res.status_code = 403 + conn.post = MagicMock(return_value=post_system_to_group_res) + + # Test the code + _, _, group_name, _ = conn.register() + assert group_name == "my_testing_group_name" + + @patch("insights.client.connection.generate_machine_id", return_value='xxxxxx') @patch("insights.client.connection.machine_id_exists", return_value=True) @patch("insights.client.connection.InsightsConnection._init_session")