fix(client): Ensure unregistration is always explicit

* Card ID: CCT-1122

Some commands, such as `--status`, could unintentionally unregister the
system if the host was missing from Inventory, which led to the creation
of a `.unregistered` file without user intent. This change ensures that
unregistration only occurs when explicitly triggered via the
`--unregister` command.

Signed-off-by: pkoprda <[email protected]>

insights/client/connection.py

@@ -26,6 +26,7 @@
 from .utilities import (determine_hostname,
                         generate_machine_id,
                         machine_id_exists,
+                        write_to_disk,
                         write_unregistered_file,
                         write_registered_file,
                         os_release_info,
@@ -818,6 +819,11 @@ def unregister(self):
 
         results = self._fetch_system_by_machine_id()
         if not results:
+            if machine_id_exists() or os.path.exists(constants.registered_files[0]):
+                write_unregistered_file()
+                write_to_disk(constants.machine_id_file, delete=True)
+                logger.info("Successfully unregistered from the Red Hat Insights Service")
+                return True
             logger.info('This host could not be found.')
             return False
         try:
@@ -1050,6 +1056,10 @@ def set_display_name(self, display_name):
 
         system = self._fetch_system_by_machine_id()
         if not system:
+            if machine_id_exists() or os.path.exists(constants.registered_files[0]):
+                logger.error("Could not update display name.\n"
+                             "The system was not found in Inventory. Please, register the system again:\n"
+                             "# insights-client --register")
             return system
         inventory_id = system['id']
 
@@ -1071,6 +1081,10 @@ def set_ansible_host(self, ansible_host):
         '''
         system = self._fetch_system_by_machine_id()
         if not system:
+            if machine_id_exists() or os.path.exists(constants.registered_files[0]):
+                logger.error("Could not update Ansible hostname.\n"
+                             "The system was not found in Inventory. Please, register the system again:\n"
+                             "# insights-client --register")
             return system
         inventory_id = system['id']
 

insights/client/support.py

@@ -15,7 +15,7 @@
 
 from .constants import InsightsConstants as constants
 from .connection import InsightsConnection
-from .utilities import write_registered_file, write_unregistered_file, write_to_disk
+from .utilities import machine_id_exists, write_registered_file, write_unregistered_file, write_to_disk
 
 APP_NAME = constants.app_name
 logger = logging.getLogger(__name__)
@@ -58,6 +58,7 @@ def _legacy_registration_check(api_reg_status):
 
 def registration_check(pconn):
     status = pconn.api_registration_check()
+
     # Legacy code
     if pconn.config.legacy_upload:
         status = _legacy_registration_check(status)
@@ -67,6 +68,8 @@ def registration_check(pconn):
             reg_status = None
     # --- end legacy ---
     else:
+        if not status and machine_id_exists() and os.path.exists(constants.registered_files[0]):
+            status = True
         reg_status = status
     if reg_status:
         write_registered_file()

insights/tests/client/support/test_collect_support_info.py

@@ -3,6 +3,7 @@
 from insights.client.config import InsightsConfig
 from insights.client.connection import InsightsConnection
 from insights.client.support import InsightsSupport, registration_check
+from insights.client.constants import InsightsConstants as constants
 from mock.mock import Mock, patch
 
 
@@ -112,6 +113,22 @@ def test_registration_check_registered_unreach(_, __):
     conn.api_registration_check.assert_called_once()
 
 
+@patch("insights.client.connection.InsightsConnection._init_session")
+@patch("insights.client.connection.InsightsConnection.get_proxies")
+@patch("insights.client.connection.InsightsConnection.api_registration_check", return_value=False)
+@patch("insights.client.support.machine_id_exists", return_value=True)
+@patch("os.path.exists", side_effect=lambda path: path == constants.registered_files[0])
+def test_registration_check_registered_no_inventory(mock_os_path_exists, machine_id_exists, api_registration_check, get_proxies, _init_session):
+    '''
+    Test the registration check function when the system does not exist in Inventory,
+    but the machine-id and .registered files are present locally on the system.
+    '''
+    config = Mock(base_url=None, legacy_upload=False)
+    conn = InsightsConnection(config)
+    check = registration_check(conn)
+    assert check is True
+
+
 @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")

insights/tests/client/test_client.py

@@ -8,6 +8,7 @@
 from insights.client.client import get_file_handler, RotatingFileHandlerWithUMask, FileHandlerWithUMask
 from insights.client.archive import InsightsArchive
 from insights.client.config import InsightsConfig
+from insights.client.connection import InsightsConnection
 from insights import package_info
 from insights.client.constants import InsightsConstants as constants
 from mock.mock import patch, Mock, call, ANY
@@ -251,6 +252,27 @@ def test_unregister_legacy(date, client_write, utilities_write, _delete_cache_fi
     ))
 
 
+@patch("insights.client.connection.InsightsConnection._init_session")
+@patch("insights.client.connection.InsightsConnection.get_proxies")
+@patch("insights.client.connection.machine_id_exists", return_value=True)
+@patch("insights.client.connection.write_unregistered_file")
+@patch("insights.client.connection.write_to_disk")
+@patch("os.path.exists", side_effect=lambda path: path == constants.registered_files[0])
+@patch("insights.client.connection.InsightsConnection._fetch_system_by_machine_id", return_value=False)
+def test_unregister_no_inventory(fetch_system_by_machine_id, os_path_exists, write_to_disk, write_unregistered_file, machine_id_exists, get_proxies, _init_session):
+    '''
+    Test unregister when the system does not exist in Inventory,
+    but local registration files exist.
+    '''
+    config = InsightsConfig(unregister=True, legacy_upload=False)
+    connection = InsightsConnection(config)
+    result = connection.unregister()
+
+    write_unregistered_file.assert_called_once()
+    write_to_disk.assert_called_once_with(constants.machine_id_file, delete=True)
+    assert result is True
+
+
 def test_register_container():
     with pytest.raises(ValueError):
         InsightsConfig(register=True, analyze_container=True)