Merge ~cjwatson/launchpad:oval-check-changes into launchpad:master

[Colin Watson]

Compare incoming OVAL data with already published one

I adopted https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/441101 and fixed up a number of issues I found while writing tests for it. Normally I'd squash all this before proposing it, but in this case I've kept the commits separate to make it clear what Jürgen did and what I did.

[Guruprasad (lgp171188)]

LGTM 👍🏼

review: Approve

[Jürgen Gmach (jugmac00)]

Thanks for picking this up!

diff --git a/lib/lp/archivepublisher/scripts/publishdistro.py b/lib/lp/archivepublisher/scripts/publishdistro.py
index 419b1ef..87e9a7d 100644
--- a/lib/lp/archivepublisher/scripts/publishdistro.py
+++ b/lib/lp/archivepublisher/scripts/publishdistro.py
@@ -8,13 +8,16 @@ __all__ = [
 ]
 
 import os
+from filecmp import dircmp
 from optparse import OptionValueError
+from pathlib import Path
 from subprocess import CalledProcessError, check_call
 
 from storm.store import Store
 from zope.component import getUtility
 
 from lp.app.errors import NotFoundError
+from lp.archivepublisher.config import getPubConfig
 from lp.archivepublisher.publishing import (
     GLOBAL_PUBLISHER_LOCK,
     cannot_modify_suite,
@@ -46,6 +49,19 @@ def is_ppa_public(ppa):
     return not ppa.private
 
 
+def has_oval_data_changed(incoming_dir, published_dir):
+    """Compare the incoming data with the already published one."""
+    # XXX cjwatson 2023-04-19: `dircmp` in Python < 3.6 doesn't accept
+    # path-like objects.
+    compared = dircmp(str(incoming_dir), str(published_dir))
+    return (
+        bool(compared.left_only)
+        or bool(compared.right_only)
+        or bool(compared.diff_files)
+        or bool(compared.funny_files)
+    )
+
+
 class PublishDistro(PublisherScript):
     """Distro publisher."""
 
@@ -507,56 +523,108 @@ class PublishDistro(PublisherScript):
                 # store and cause performance problems.
                 Store.of(archive).reset()
 
-    def rsync_oval_data(self):
-        if config.archivepublisher.oval_data_rsync_endpoint:
-            # Ensure that the rsync paths have a trailing slash.
-            rsync_src = os.path.join(
-                config.archivepublisher.oval_data_rsync_endpoint, ""
-            )
-            rsync_dest = os.path.join(
-                config.archivepublisher.oval_data_root, ""
+    def rsyncOVALData(self):
+        # Ensure that the rsync paths have a trailing slash.
+        rsync_src = os.path.join(
+            config.archivepublisher.oval_data_rsync_endpoint, ""
+        )
+        rsync_dest = os.path.join(config.archivepublisher.oval_data_root, "")
+        rsync_command = [
+            "/usr/bin/rsync",
+            "-a",
+            "-q",
+            "--timeout={}".format(
+                config.archivepublisher.oval_data_rsync_timeout
+            ),
+            "--delete",
+            "--delete-after",
+            rsync_src,
+            rsync_dest,
+        ]
+        try:
+            self.logger.info(
+                "Attempting to rsync the OVAL data from '%s' to '%s'",
+                rsync_src,
+                rsync_dest,
             )
-            rsync_command = [
-                "/usr/bin/rsync",
-                "-a",
-                "-q",
-                "--timeout={}".format(
-                    config.archivepublisher.oval_data_rsync_timeout
-                ),
-                "--delete",
-                "--delete-after",
+            check_call(rsync_command)
+        except CalledProcessError:
+            self.logger.exception(
+                "Failed to rsync OVAL data from '%s' to '%s'",
                 rsync_src,
                 rsync_dest,
-            ]
-            try:
-                self.logger.info(
-                    "Attempting to rsync the OVAL data from '%s' to '%s'",
-                    rsync_src,
-                    rsync_dest,
-                )
-                check_call(rsync_command)
-            except CalledProcessError:
-                self.logger.exception(
-                    "Failed to rsync OVAL data from '%s' to '%s'",
-                    rsync_src,
-                    rsync_dest,
-                )
-                raise
-        else:
-            self.logger.info(
-                "Skipping the OVAL data sync as no rsync endpoint"
-                " has been configured."
             )
+            raise
+
+    def checkForUpdatedOVALData(self, distribution):
+        """Compare the published OVAL files with the incoming one."""
+        start_dir = Path(config.archivepublisher.oval_data_root)
+        archive_set = getUtility(IArchiveSet)
+        for owner_path in start_dir.iterdir():
+            if not owner_path.name.startswith("~"):
+                continue
+            distribution_path = owner_path / distribution.name
+            if not distribution_path.is_dir():
+                continue
+            for archive_path in distribution_path.iterdir():
+                archive = archive_set.getPPAByDistributionAndOwnerName(
+                    distribution, owner_path.name[1:], archive_path.name
+                )
+                if archive is None:
+                    self.logger.info(
+                        "Skipping OVAL data for '~%s/%s/%s' "
+                        "(no such archive).",
+                        owner_path.name[1:],
+                        distribution.name,
+                        archive_path.name,
+                    )
+                    continue
+                for suite_path in archive_path.iterdir():
+                    try:
+                        series, pocket = distribution.getDistroSeriesAndPocket(
+                            suite_path.name
+                        )
+                    except NotFoundError:
+                        self.logger.info(
+                            "Skipping OVAL data for '%s:%s' (no such suite).",
+                            archive.reference,
+                            suite_path.name,
+                        )
+                        continue
+                    for component in archive.getComponentsForSeries(series):
+                        incoming_dir = suite_path / component.name
+                        published_dir = (
+                            Path(getPubConfig(archive).distsroot)
+                            / series.name
+                            / component.name
+                            / "oval"
+                        )
+                        if not published_dir.is_dir() or has_oval_data_changed(
+                            incoming_dir=incoming_dir,
+                            published_dir=published_dir,
+                        ):
+                            archive.markSuiteDirty(
+                                distroseries=series, pocket=pocket
+                            )
+                            break
 
     def main(self, reset_store_between_archives=True):
         """See `LaunchpadScript`."""
         self.validateOptions()
         self.logOptions()
 
-        self.rsync_oval_data()
+        if config.archivepublisher.oval_data_rsync_endpoint:
+            self.rsyncOVALData()
+        else:
+            self.logger.info(
+                "Skipping the OVAL data sync as no rsync endpoint"
+                " has been configured."
+            )
 
         archive_ids = []
         for distribution in self.findDistros():
+            if config.archivepublisher.oval_data_rsync_endpoint:
+                self.checkForUpdatedOVALData(distribution)
             for archive in self.getTargetArchives(distribution):
                 if archive.distribution != distribution:
                     raise AssertionError(
diff --git a/lib/lp/archivepublisher/tests/test_publishdistro.py b/lib/lp/archivepublisher/tests/test_publishdistro.py
index 3d9e0cc..049dccb 100644
--- a/lib/lp/archivepublisher/tests/test_publishdistro.py
+++ b/lib/lp/archivepublisher/tests/test_publishdistro.py
@@ -7,6 +7,7 @@ import os
 import shutil
 import subprocess
 from optparse import OptionValueError
+from pathlib import Path
 
 from fixtures import MockPatch
 from storm.store import Store
@@ -33,6 +34,7 @@ from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.config import config
 from lp.services.database.interfaces import IStore
 from lp.services.log.logger import BufferLogger, DevNullLogger
+from lp.services.osutils import write_file
 from lp.services.scripts.base import LaunchpadScriptFailure
 from lp.soyuz.enums import (
     ArchivePublishingMethod,
@@ -258,10 +260,11 @@ class TestPublishDistro(TestNativePublishingBase):
         self.assertEqual(PackagePublishingStatus.PENDING, pub_source.status)
 
     def setUpOVALDataRsync(self):
+        self.oval_data_root = self.makeTemporaryDirectory()
         self.pushConfig(
             "archivepublisher",
             oval_data_rsync_endpoint="oval.internal::oval/",
-            oval_data_root="/tmp/oval-data",
+            oval_data_root=self.oval_data_root,
             oval_data_rsync_timeout=90,
         )
 
@@ -299,7 +302,7 @@ class TestPublishDistro(TestNativePublishingBase):
             "--delete",
             "--delete-after",
             "oval.internal::oval/",
-            "/tmp/oval-data/",
+            self.oval_data_root + "/",
         ]
         mock_subprocess_check_call.assert_called_once_with(call_args)
 
@@ -322,15 +325,167 @@ class TestPublishDistro(TestNativePublishingBase):
             "--delete",
             "--delete-after",
             "oval.internal::oval/",
-            "/tmp/oval-data/",
+            self.oval_data_root + "/",
         ]
         mock_subprocess_check_call.assert_called_once_with(call_args)
         expected_log_line = (
             "ERROR Failed to rsync OVAL data from "
-            "'oval.internal::oval/' to '/tmp/oval-data/'"
+            "'oval.internal::oval/' to '%s/'" % self.oval_data_root
         )
         self.assertTrue(expected_log_line in self.logger.getLogBuffer())
 
+    def test_checkForUpdatedOVALData_new(self):
+        self.setUpOVALDataRsync()
+        self.useFixture(
+            MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+        )
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        # Disable normal publication so that dirty_suites isn't cleared.
+        archive.publish = False
+        incoming_dir = (
+            Path(self.oval_data_root)
+            / archive.reference
+            / "breezy-autotest"
+            / "main"
+        )
+        write_file(str(incoming_dir), b"test")
+        self.runPublishDistro(
+            extra_args=["--ppa"], distribution=archive.distribution.name
+        )
+        self.assertEqual(["breezy-autotest"], archive.dirty_suites)
+
+    def test_checkForUpdatedOVALData_unchanged(self):
+        self.setUpOVALDataRsync()
+        self.useFixture(
+            MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+        )
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        # Disable normal publication so that dirty_suites isn't cleared.
+        archive.publish = False
+        incoming_dir = (
+            Path(self.oval_data_root)
+            / archive.reference
+            / "breezy-autotest"
+            / "main"
+        )
+        published_dir = (
+            Path(getPubConfig(archive).distsroot)
+            / "breezy-autotest"
+            / "main"
+            / "oval"
+        )
+        write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+        shutil.copytree(str(incoming_dir), str(published_dir))
+        self.runPublishDistro(
+            extra_args=["--ppa"], distribution=archive.distribution.name
+        )
+        self.assertIsNone(archive.dirty_suites)
+
+    def test_checkForUpdatedOVALData_updated(self):
+        self.setUpOVALDataRsync()
+        self.useFixture(
+            MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+        )
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        # Disable normal publication so that dirty_suites isn't cleared.
+        archive.publish = False
+        incoming_dir = (
+            Path(self.oval_data_root)
+            / archive.reference
+            / "breezy-autotest"
+            / "main"
+        )
+        published_dir = (
+            Path(getPubConfig(archive).distsroot)
+            / "breezy-autotest"
+            / "main"
+            / "oval"
+        )
+        write_file(str(published_dir / "foo.oval.xml.bz2"), b"old")
+        mtime = (published_dir / "foo.oval.xml.bz2").stat().st_mtime
+        os.utime(
+            str(published_dir / "foo.oval.xml.bz2"), (mtime - 1, mtime - 1)
+        )
+        write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"new")
+        self.runPublishDistro(
+            extra_args=["--ppa"], distribution=archive.distribution.name
+        )
+        self.assertEqual(["breezy-autotest"], archive.dirty_suites)
+
+    def test_checkForUpdatedOVALData_new_files(self):
+        self.setUpOVALDataRsync()
+        self.useFixture(
+            MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+        )
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        # Disable normal publication so that dirty_suites isn't cleared.
+        archive.publish = False
+        incoming_dir = (
+            Path(self.oval_data_root)
+            / archive.reference
+            / "breezy-autotest"
+            / "main"
+        )
+        published_dir = (
+            Path(getPubConfig(archive).distsroot)
+            / "breezy-autotest"
+            / "main"
+            / "oval"
+        )
+        write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+        shutil.copytree(str(incoming_dir), str(published_dir))
+        write_file(str(incoming_dir / "bar.oval.xml.bz2"), b"test")
+        self.runPublishDistro(
+            extra_args=["--ppa"], distribution=archive.distribution.name
+        )
+        self.assertEqual(["breezy-autotest"], archive.dirty_suites)
+
+    def test_checkForUpdatedOVALData_nonexistent_archive(self):
+        self.setUpOVALDataRsync()
+        self.useFixture(
+            MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+        )
+        incoming_dir = (
+            Path(self.oval_data_root)
+            / "~nonexistent"
+            / "ubuntutest"
+            / "archive"
+            / "breezy-autotest"
+            / "main"
+        )
+        write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+        self.runPublishDistro(extra_args=["--ppa"], distribution="ubuntutest")
+        self.assertIn(
+            "INFO Skipping OVAL data for '~nonexistent/ubuntutest/archive' "
+            "(no such archive).",
+            self.logger.getLogBuffer(),
+        )
+
+    def test_checkForUpdatedOVALData_nonexistent_suite(self):
+        self.setUpOVALDataRsync()
+        self.useFixture(
+            MockPatch("lp.archivepublisher.scripts.publishdistro.check_call")
+        )
+        archive = self.factory.makeArchive(purpose=ArchivePurpose.PPA)
+        # Disable normal publication so that dirty_suites isn't cleared.
+        archive.publish = False
+        incoming_dir = (
+            Path(self.oval_data_root)
+            / archive.reference
+            / "nonexistent"
+            / "main"
+        )
+        write_file(str(incoming_dir / "foo.oval.xml.bz2"), b"test")
+        self.runPublishDistro(
+            extra_args=["--ppa"], distribution=archive.distribution.name
+        )
+        self.assertIn(
+            "INFO Skipping OVAL data for '%s:nonexistent' (no such suite)."
+            % archive.reference,
+            self.logger.getLogBuffer(),
+        )
+        self.assertIsNone(archive.dirty_suites)
+
     @defer.inlineCallbacks
     def testForPPA(self):
         """Try to run publish-distro in PPA mode.