Enhance warning about RPMs that were not validate by RPM

DNF5 informs about number of packages that signature was not verified, but without any additional detail. The ID of repository provides a good hint for user why the check was skipped. Closes: #1311
rpm-software-management · May 21, 2024 · 2ddb0b3 · 2ddb0b3
1 parent ae824d8
commit 2ddb0b3
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 4 deletions.
diff --git a/libdnf5/base/transaction.cpp b/libdnf5/base/transaction.cpp
@@ -1264,7 +1264,8 @@ bool Transaction::Impl::check_gpg_signatures() {
     // TODO(mblaha): DNSsec key verification
     libdnf5::rpm::RpmSignature rpm_signature(base);
     std::set<std::string> processed_repos{};
-    int num_checks_skipped = 0;
+    unsigned long num_checks_skipped = 0;
+    std::set<std::string> repos_with_skipped_checks;
     for (const auto & trans_pkg : packages) {
         if (transaction_item_action_is_inbound(trans_pkg.get_action())) {
             auto const & pkg = trans_pkg.get_package();
@@ -1277,6 +1278,7 @@ bool Transaction::Impl::check_gpg_signatures() {
             auto check_result = rpm_signature.check_package_signature(pkg);
             if (check_result == libdnf5::rpm::RpmSignature::CheckResult::SKIPPED) {
                 num_checks_skipped += 1;
+                repos_with_skipped_checks.insert(pkg.get_repo_id());
             } else if (check_result != libdnf5::rpm::RpmSignature::CheckResult::OK) {
                 // these two errors are possibly recoverable by importing the correct public key
                 auto is_error_recoverable =
@@ -1315,7 +1317,17 @@ bool Transaction::Impl::check_gpg_signatures() {
         }
     }
     if (num_checks_skipped > 0) {
-        auto warning_msg = utils::sformat(_("Warning: skipped PGP checks for {} package(s)."), num_checks_skipped);
+        auto repo_string = libdnf5::utils::string::join(
+            repos_with_skipped_checks, C_("It is a joining character for repositories IDs", ", "));
+        auto warning_msg =
+            (num_checks_skipped == 1)
+                ? utils::sformat(_("Warning: skipped PGP checks for 1 package from {} repository."), repo_string)
+                : utils::sformat(
+                      P_("Warning: skipped PGP checks for {0} packages from {1} repository.",
+                         "Warning: skipped PGP checks for {0} packages from {1} repositories.",
+                         repos_with_skipped_checks.size()),
+                      num_checks_skipped,
+                      repo_string);
         signature_problems.push_back(warning_msg);
     }
     return result;

diff --git a/test/libdnf5/base/test_transaction.cpp b/test/libdnf5/base/test_transaction.cpp
@@ -38,7 +38,8 @@ void BaseTransactionTest::test_check_gpg_signatures_no_gpgcheck() {
     CPPUNIT_ASSERT(transaction.check_gpg_signatures());
     CPPUNIT_ASSERT_EQUAL((size_t)1, transaction.get_gpg_signature_problems().size());
     CPPUNIT_ASSERT_EQUAL(
-        std::string("Warning: skipped PGP checks for 1 package(s)."), transaction.get_gpg_signature_problems()[0]);
+        std::string("Warning: skipped PGP checks for 1 package from repomd-repo1 repository."),
+        transaction.get_gpg_signature_problems()[0]);
 }
 
 void BaseTransactionTest::test_check_gpg_signatures_fail() {

diff --git a/test/python3/libdnf5/base/test_transaction.py b/test/python3/libdnf5/base/test_transaction.py
@@ -32,7 +32,7 @@ def test_check_gpg_signatures_no_gpgcheck(self):
 
         self.assertEqual(1, transaction.get_transaction_packages_count())
         self.assertTrue(transaction.check_gpg_signatures())
-        self.assertEqual(('Warning: skipped PGP checks for 1 package(s).',),
+        self.assertEqual(('Warning: skipped PGP checks for 1 package from repomd-repo1 repository.',),
                          transaction.get_gpg_signature_problems())
 
     def test_check_gpg_signatures_fail(self):