test updates from review

craft_archives/repo/apt_key_manager.py

@@ -220,6 +220,11 @@ def install_key(self, *, key: str, key_id: Optional[str] = None) -> None:
         if not fingerprints:
             raise errors.AptGPGKeyInstallError("Invalid GPG key", key=key)
 
+        if key_id and key_id not in fingerprints:
+            raise errors.AptGPGKeyInstallError(
+                "Desired key_id not found in fingerprints", key=key
+            )
+
         self._create_keyrings_path()
         target_id = key_id or fingerprints[0]
         keyring_path = get_keyring_path(target_id, base_path=self._keyrings_path)

tests/integration/repo/test_apt_key_manager.py

@@ -127,18 +127,37 @@ def test_install_key_gpg_errors_valid(
 
 
 def test_install_key_gpg_errors_invalid_key_id(
-    apt_gpg, tmp_path, test_data_dir, caplog
+    apt_gpg, tmp_path, test_data_dir, caplog, mocker
 ):
     """Test that install_key() fails if the key contents are imported successfully but
     the key_id is *not* found in the imported file."""
     caplog.set_level(logging.DEBUG)
 
-    key_id = "NOT-IN-KEY"
+    missing_key_id = "NOT-IN-KEY"
     problem_key = test_data_dir / "multi-keys/9E61EF26.asc"
+    key_contents = problem_key.read_text()
+
+    original_get_fingerprints = AptKeyManager.get_key_fingerprints
+
+    # This is tricky: We want the install_key() to fail *after* the key is imported, but
+    # AptKeyManager.get_key_fingerprints() is called twice: once before, and once after,
+    # the actual installation. So this mock adds the missing key for the first call and
+    # not the second.
+    # A better test would need a key file that actually has this behavior, but we don't
+    # have one right now.
+    def fake_get_fingerprints(*, key: str) -> List[str]:
+        result = original_get_fingerprints(key=key)
+        if key is key_contents:
+            result.append(missing_key_id)
+        return result
+
+    mocker.patch.object(
+        AptKeyManager, "get_key_fingerprints", side_effect=fake_get_fingerprints
+    )
 
     expected_message = "Failed to install GPG key: key-id NOT-IN-KEY not imported."
     with pytest.raises(errors.AptGPGKeyInstallError, match=expected_message):
-        apt_gpg.install_key(key=problem_key.read_text(), key_id=key_id)
+        apt_gpg.install_key(key=key_contents, key_id=missing_key_id)
 
     # Check that log messages containing gpg's output were generated
     marker = caplog.messages.index("gpg stderr:")

tests/unit/repo/test_apt_key_manager.py

@@ -256,14 +256,19 @@ def test_is_key_installed_with_gpg_failure(
     mock_logger.warning.assert_called_once_with("gpg error: some error")
 
 
+@pytest.mark.parametrize("key_id", (None, "FAKE-KEY-ID-FROM-GNUPG"))
 def test_install_key(
-    apt_gpg, mock_run, mock_chmod, sample_key_string, sample_key_bytes
+    apt_gpg, mock_run, mock_chmod, sample_key_string, sample_key_bytes, tmp_path, key_id
 ):
     mock_run.return_value.stdout = SAMPLE_GPG_SHOW_KEY_OUTPUT
     mock_run.return_value.stderr = None
 
     apt_gpg.install_key(key=sample_key_string)
 
+    # The key should be imported to a file that contains the short-id of the
+    # "FAKE-KEY-ID-FROM-GNUPG" fingerprint, (so the last 8 characters).
+    expected_imported_keyring = f"gnupg-ring:{tmp_path / 'craft-OM-GNUPG.gpg'}"
+
     assert mock_run.mock_calls == [
         call(
             [
@@ -289,7 +294,7 @@ def test_install_key(
                 "--no-default-keyring",
                 "--with-colons",
                 "--keyring",
-                mock.ANY,
+                expected_imported_keyring,
                 "--homedir",
                 mock.ANY,
                 "--import",
@@ -347,20 +352,28 @@ def test_install_key_with_gpg_failure(apt_gpg, mock_run):
     assert str(raised.value) == "Failed to install GPG key: some error"
 
 
-@pytest.mark.parametrize(
-    "fingerprints,error",
-    [
-        pytest.param([], "Invalid GPG key", id="no keys"),
-    ],
-)
-def test_install_key_with_key_issue(apt_gpg, mocker, fingerprints, error):
+def test_install_key_with_no_fingerprints(apt_gpg, mocker):
+    """Test installing key contents that have no fingerprints at all."""
     mock_fingerprints = mocker.patch.object(apt_gpg, "get_key_fingerprints")
-    mock_fingerprints.return_value = fingerprints
+    mock_fingerprints.return_value = []
 
     with pytest.raises(errors.AptGPGKeyInstallError) as raised:
         apt_gpg.install_key(key="key")
 
-    assert str(raised.value) == f"Failed to install GPG key: {error}"
+    assert str(raised.value) == "Failed to install GPG key: Invalid GPG key"
+
+
+def test_install_key_with_invalid_key_id(apt_gpg, mocker):
+    """Test installing key contents where the desired key-id is *not* among the
+    existing fingerprints."""
+    mock_fingerprints = mocker.patch.object(apt_gpg, "get_key_fingerprints")
+    mock_fingerprints.return_value = ["FINGERPRINT-1", "FINGERPRINT-2"]
+
+    expected_error = (
+        "Failed to install GPG key: Desired key_id not found in fingerprints"
+    )
+    with pytest.raises(errors.AptGPGKeyInstallError, match=expected_error):
+        apt_gpg.install_key(key="key", key_id="IM-NOT-THERE")
 
 
 def test_install_key_from_keyserver(apt_gpg, mock_run, mock_chmod):