apport: only prompt for cloud_name when instance-data.json is absent

Do not prompt for CloudName when instance-data.json exists and is valid
YAML.

When instance-data.json exists, general-hooks/cloud_init.py will add the
following fields to bug reports:
   CloudName, CloudID, CloudPlatform and CloudSubplatform.

Downstream ubuntu packaging braches deliver:
   debian/apport-general-hook.py to
       /usr/share/apport/general-hooks/cloud-init.py

Only prompt in during apport bug when the general-hook can't
process instance-data.json.

2 files changed, 86 insertions, 13 deletions

diff --git a/cloudinit/apport.py b/cloudinit/apport.py
index e42ecf8e..dead3059 100644
--- a/cloudinit/apport.py
+++ b/cloudinit/apport.py
@@ -4,6 +4,7 @@
 
 """Cloud-init apport interface"""
 
+import json
 import os
 
 from cloudinit.cmd.devel import read_cfg_paths
@@ -101,8 +102,29 @@ def attach_hwinfo(report, ui=None):
 
 
 def attach_cloud_info(report, ui=None):
-    """Prompt for cloud details if available."""
+    """Prompt for cloud details if instance-data unavailable.
+
+    When we have valid _get_instance_data, apport/generic-hooks/cloud_init.py
+    provides CloudName, CloudID, CloudPlatform and CloudSubPlatform.
+
+    Apport/generic-hooks are delivered by cloud-init's downstream branches
+    ubuntu/(devel|kinetic|jammy|focal|bionic) so they will not be represented
+    in upstream main.
+
+    In absence of viable instance-data.json format, prompt for the cloud below.
+    """
+
     if ui:
+        paths = read_cfg_paths()
+        try:
+            with open(paths.get_runpath("instance_data")) as file:
+                instance_data = json.load(file)
+                assert instance_data.get("v1", {}).get("cloud_name")
+                return  # Valid instance-data means generic-hooks will report
+        except (IOError, json.decoder.JSONDecodeError, AssertionError):
+            pass
+
+        # No valid /run/cloud/instance-data.json on system. Prompt for cloud.
         prompt = "Is this machine running in a cloud environment?"
         response = ui.yesno(prompt)
         if response is None:
diff --git a/tests/unittests/test_apport.py b/tests/unittests/test_apport.py
index 1876c1be..c731a30a 100644
--- a/tests/unittests/test_apport.py
+++ b/tests/unittests/test_apport.py
@@ -1,3 +1,5 @@
+import os
+
 import pytest
 
 from tests.unittests.helpers import mock
@@ -5,24 +7,74 @@ from tests.unittests.helpers import mock
 M_PATH = "cloudinit.apport."
 
 
+@pytest.fixture()
+def apport(request, mocker, paths):
+    """Mock apport.hookutils before importing cloudinit.apport.
+
+    This avoids our optional import dependency on apport, providing tests with
+    mocked apport.hookutils function call counts.
+    """
+    m_hookutils = mock.Mock()
+    mocker.patch.dict("sys.modules", {"apport.hookutils": m_hookutils})
+    mocker.patch(M_PATH + "read_cfg_paths", return_value=paths)
+    from cloudinit import apport
+
+    yield apport
+
+
 class TestApport:
-    def test_attach_user_data(self, mocker, tmpdir):
-        m_hookutils = mock.Mock()
-        mocker.patch.dict("sys.modules", {"apport.hookutils": m_hookutils})
-        user_data_file = tmpdir.join("instance", "user-data.txt")
-        mocker.patch(
-            M_PATH + "_get_user_data_file", return_value=user_data_file
-        )
+    @pytest.mark.parametrize(
+        "instance_data,choice_idx,expected_report",
+        (
+            pytest.param(
+                '{"v1": {"cloud_name": "mycloud"}}',
+                None,
+                {},
+                id="v1_cloud_name_exists",
+            ),
+            pytest.param(
+                '{"v1": {"cloud_id": "invalid"}}',
+                1,
+                {"CloudName": "Azure"},
+                id="v1_no_cloud_name_present",
+            ),
+            pytest.param("{}", 0, {"CloudName": "AliYun"}, id="no_v1_key"),
+            pytest.param(
+                "{", 22, {"CloudName": "Oracle"}, id="not_valid_json"
+            ),
+        ),
+    )
+    def test_attach_cloud_info(
+        self, instance_data, choice_idx, expected_report, apport, paths
+    ):
+        """Prompt for cloud name when instance-data.json is not-json/absent."""
 
-        from cloudinit import apport
+        instance_data_file = paths.get_runpath("instance_data")
+        if instance_data is None:
+            assert not os.path.exists(instance_data_file)
+        else:
+            with open(instance_data_file, "w") as stream:
+                stream.write(instance_data)
+        ui = mock.Mock()
+        ui.yesno.return_value = True
+        ui.choice.return_value = (choice_idx, "")
+        report = {}
+        apport.attach_cloud_info(report, ui)
+        if choice_idx is not None:
+            assert ui.choice.call_count == 1
+            assert report["CloudName"] == apport.KNOWN_CLOUD_NAMES[choice_idx]
+        else:
+            assert ui.choice.call_count == 0
 
+    def test_attach_user_data(self, apport, paths):
+        user_data_file = paths.get_ipath_cur("userdata_raw")
         ui = mock.Mock()
         ui.yesno.return_value = True
         report = object()
         apport.attach_user_data(report, ui)
         assert [
             mock.call(report, user_data_file, "user_data.txt"),
-        ] == m_hookutils.attach_file.call_args_list
+        ] == apport.attach_file.call_args_list
         assert [
             mock.call(
                 report,
@@ -35,7 +87,7 @@ class TestApport:
                 "/etc/cloud/cloud.cfg.d/99-installer.cfg",
                 "InstallerCloudCfg",
             ),
-        ] == m_hookutils.attach_file_if_exists.call_args_list
+        ] == apport.attach_file_if_exists.call_args_list
 
     @pytest.mark.parametrize(
         "report,tags",
@@ -52,9 +104,8 @@ class TestApport:
             ),
         ),
     )
-    def test_add_bug_tags_assigns_proper_tags(self, report, tags):
+    def test_add_bug_tags_assigns_proper_tags(self, report, tags, apport):
         """Tags are assigned based on non-empty project report key values."""
-        from cloudinit import apport
 
         apport.add_bug_tags(report)
         assert report.get("Tags", "") == tags