launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad:copy-signing-key-overwrite into launchpad:master.

Commit message:
Adding --overwrite option to copy-signingkeys script

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400277
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:copy-signing-key-overwrite into launchpad:master.

diff --git a/lib/lp/archivepublisher/scripts/copy_signingkeys.py b/lib/lp/archivepublisher/scripts/copy_signingkeys.py
index e4a46bd..acd159f 100644
--- a/lib/lp/archivepublisher/scripts/copy_signingkeys.py
+++ b/lib/lp/archivepublisher/scripts/copy_signingkeys.py
@@ -42,6 +42,10 @@ class CopySigningKeysScript(LaunchpadScript):
             "-n", "--dry-run", action="store_true", default=False,
             help="Report what would be done, but don't actually copy keys.")
 
+        self.parser.add_option(
+            "-o", "--overwrite", action="store_true", default=False,
+            help="Overwrite existing keys when executing the copy.")
+
     def getArchive(self, reference):
         archive = getUtility(IArchiveSet).getByReference(reference)
         if archive is None:
@@ -78,7 +82,8 @@ class CopySigningKeysScript(LaunchpadScript):
         self.key_types = self.getKeyTypes(self.options.key_type)
         self.series = self.getSeries(self.options.series)
 
-    def copy(self, from_archive, to_archive, key_type, series=None):
+    def copy(self, from_archive, to_archive, key_type, series=None,
+             overwrite=False):
         series_name = series.name if series else None
         from_archive_signing_key = getUtility(IArchiveSigningKeySet).get(
             key_type, from_archive, series, exact_match=True)
@@ -90,10 +95,17 @@ class CopySigningKeysScript(LaunchpadScript):
         to_archive_signing_key = getUtility(IArchiveSigningKeySet).get(
             key_type, to_archive, series, exact_match=True)
         if to_archive_signing_key is not None:
+            if not overwrite:
+                # If it already exists and we do not force overwrite,
+                # abort this signing key copy.
+                self.logger.warning(
+                    "%s signing key for %s / %s already exists",
+                    key_type, to_archive.reference, series_name)
+                return
             self.logger.warning(
-                "%s signing key for %s / %s already exists",
+                "%s signing key for %s / %s being overwritten",
                 key_type, to_archive.reference, series_name)
-            return
+            to_archive_signing_key.destroySelf()
         self.logger.info(
             "Copying %s signing key %s from %s / %s to %s / %s",
             key_type, from_archive_signing_key.signing_key.fingerprint,
@@ -107,7 +119,7 @@ class CopySigningKeysScript(LaunchpadScript):
         for key_type in self.key_types:
             self.copy(
                 self.from_archive, self.to_archive, key_type,
-                series=self.series)
+                series=self.series, overwrite=self.options.overwrite)
         if self.options.dry_run:
             self.logger.info("Dry run requested.  Not committing changes.")
             transaction.abort()
diff --git a/lib/lp/archivepublisher/tests/test_copy_signingkeys.py b/lib/lp/archivepublisher/tests/test_copy_signingkeys.py
index 56862a7..f993c19 100644
--- a/lib/lp/archivepublisher/tests/test_copy_signingkeys.py
+++ b/lib/lp/archivepublisher/tests/test_copy_signingkeys.py
@@ -306,6 +306,42 @@ class TestCopySigningKeysScript(TestCaseWithFactory):
                 MatchesStructure.byEquality(
                     archive=archives[1], earliest_distro_series=None,
                     key_type=SigningKeyType.UEFI, signing_key=signing_keys[1]),
+            ))
+
+    def test_copy_forced_overwrite(self):
+        archives = [self.factory.makeArchive() for _ in range(2)]
+        signing_keys = [
+            self.factory.makeSigningKey(key_type=SigningKeyType.UEFI)
+            for _ in range(2)]
+        for archive, signing_key in zip(archives, signing_keys):
+            self.factory.makeArchiveSigningKey(
+                archive=archive, signing_key=signing_key)
+        script = self.makeScript(
+            test_args=["--key-type", "UEFI", "--overwrite"],
+            archives=archives[:2])
+        script.main()
+
+        expected_log = [
+            "WARNING UEFI signing key for %s / None being overwritten" % (
+                archives[1].reference),
+            "INFO Copying UEFI signing key %s from %s / %s to %s / %s" % (
+                signing_keys[0].fingerprint,
+                archives[0].reference, None,
+                archives[1].reference, None),
+            ]
+        self.assertEqual(
+            expected_log, script.logger.content.as_text().splitlines())
+        self.assertThat(
+            self.findKeys(archives),
+            MatchesSetwise(
+                # First archive keeps its signing keys.
+                MatchesStructure.byEquality(
+                    archive=archives[0], earliest_distro_series=None,
+                    key_type=SigningKeyType.UEFI, signing_key=signing_keys[0]),
+                # Second archive uses the same signing_key from first archive.
+                MatchesStructure.byEquality(
+                    archive=archives[1], earliest_distro_series=None,
+                    key_type=SigningKeyType.UEFI, signing_key=signing_keys[0]),
                 ))
 
     def runScript(self, args=None):
diff --git a/lib/lp/services/signing/interfaces/signingkey.py b/lib/lp/services/signing/interfaces/signingkey.py
index 8548fa8..b714d42 100644
--- a/lib/lp/services/signing/interfaces/signingkey.py
+++ b/lib/lp/services/signing/interfaces/signingkey.py
@@ -1,4 +1,4 @@
-# Copyright 2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2020-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interfaces for signing keys stored at the signing service."""
@@ -126,6 +126,9 @@ class IArchiveSigningKey(Interface):
         ISigningKey, title=_("Signing key"), required=True, readonly=True,
         description=_("Which signing key should be used by this archive"))
 
+    def destroySelf():
+        """Removes the ArchiveSigningKey from the database."""
+
 
 class IArchiveSigningKeySet(Interface):
     """Management class to deal with ArchiveSigningKey objects
diff --git a/lib/lp/services/signing/model/signingkey.py b/lib/lp/services/signing/model/signingkey.py
index ee77613..fdbc199 100644
--- a/lib/lp/services/signing/model/signingkey.py
+++ b/lib/lp/services/signing/model/signingkey.py
@@ -1,4 +1,4 @@
-# Copyright 2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2020-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes to manage signing keys stored at the signing service."""
@@ -177,6 +177,9 @@ class ArchiveSigningKey(StormBase):
         self.key_type = signing_key.key_type
         self.earliest_distro_series = earliest_distro_series
 
+    def destroySelf(self):
+        IStore(self).remove(self)
+
 
 @implementer(IArchiveSigningKeySet)
 class ArchiveSigningKeySet: