Codesign: Report codesign errors from server to worker

Pass codesign errors (if any) from codesign buildbot server to the
buildbot worker, so that the latter one can abort build process if
the error happens. This solves issues when non-properly-notarized
DMG package gets uploaded to the buildbot website.

5 files changed, 233 insertions, 73 deletions

diff --git a/build_files/buildbot/codesign/archive_with_indicator.py b/build_files/buildbot/codesign/archive_with_indicator.py
index 085026fcf98..aebf5a15417 100644
--- a/build_files/buildbot/codesign/archive_with_indicator.py
+++ b/build_files/buildbot/codesign/archive_with_indicator.py
@@ -18,12 +18,72 @@
 
 # <pep8 compliant>
 
+import dataclasses
+import json
 import os
+
 from pathlib import Path
+from typing import Optional
 
 import codesign.util as util
 
 
+class ArchiveStateError(Exception):
+    message: str
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)
+
+
+@dataclasses.dataclass
+class ArchiveState:
+    """
+    Additional information (state) of the archive
+
+    Includes information like expected file size of the archive file in the case
+    the archive file is expected to be successfully created.
+
+    If the archive can not be created, this state will contain error message
+    indicating details of error.
+    """
+
+    # Size in bytes of the corresponding archive.
+    file_size: Optional[int] = None
+
+    # Non-empty value indicates that error has happenned.
+    error_message: str = ''
+
+    def has_error(self) -> bool:
+        """
+        Check whether the archive is at error state
+        """
+
+        return self.error_message
+
+    def serialize_to_string(self) -> str:
+        payload = dataclasses.asdict(self)
+        return json.dumps(payload, sort_keys=True, indent=4)
+
+    def serialize_to_file(self, filepath: Path) -> None:
+        string = self.serialize_to_string()
+        filepath.write_text(string)
+
+    @classmethod
+    def deserialize_from_string(cls, string: str) -> 'ArchiveState':
+        try:
+            object_as_dict = json.loads(string)
+        except json.decoder.JSONDecodeError:
+            raise ArchiveStateError('Error parsing JSON')
+
+        return cls(**object_as_dict)
+
+    @classmethod
+    def deserialize_from_file(cls, filepath: Path):
+        string = filepath.read_text()
+        return cls.deserialize_from_string(string)
+
+
 class ArchiveWithIndicator:
     """
     The idea of this class is to wrap around logic which takes care of keeping
@@ -79,6 +139,19 @@ class ArchiveWithIndicator:
         if not self.ready_indicator_filepath.exists():
             return False
 
+        try:
+            archive_state = ArchiveState.deserialize_from_file(
+                self.ready_indicator_filepath)
+        except ArchiveStateError as error:
+            print(f'Error deserializing archive state: {error.message}')
+            return False
+
+        if archive_state.has_error():
+            # If the error did happen during codesign procedure there will be no
+            # corresponding archive file.
+            # The caller code will deal with the error check further.
+            return True
+
         # Sometimes on macOS indicator file appears prior to the actual archive
         # despite the order of creation and os.sync() used in tag_ready().
         # So consider archive not ready if there is an indicator without an
@@ -88,23 +161,11 @@ class ArchiveWithIndicator:
                   f'({self.archive_filepath}) to appear.')
             return False
 
-        # Read archive size from indicator/
-        #
-        # Assume that file is either empty or is fully written. This is being checked
-        # by performing ValueError check since empty string will throw this exception
-        # when attempted to be converted to int.
-        expected_archive_size_str = self.ready_indicator_filepath.read_text()
-        try:
-            expected_archive_size = int(expected_archive_size_str)
-        except ValueError:
-            print(f'Invalid archive size "{expected_archive_size_str}"')
-            return False
-
         # Wait for until archive is fully stored.
         actual_archive_size = self.archive_filepath.stat().st_size
-        if  actual_archive_size != expected_archive_size:
+        if actual_archive_size != archive_state.file_size:
             print('Partial/invalid archive size (expected '
-                  f'{expected_archive_size} got {actual_archive_size})')
+                  f'{archive_state.file_size} got {actual_archive_size})')
             return False
 
         return True
@@ -129,7 +190,7 @@ class ArchiveWithIndicator:
             print(f'Exception checking archive: {e}')
             return False
 
-    def tag_ready(self) -> None:
+    def tag_ready(self, error_message='') -> None:
         """
         Tag the archive as ready by creating the corresponding indication file.
 
@@ -138,13 +199,34 @@ class ArchiveWithIndicator:
               If it is violated, an assert will fail.
         """
         assert not self.is_ready()
+
         # Try the best to make sure everything is synced to the file system,
         # to avoid any possibility of stamp appearing on a network share prior to
         # an actual file.
         if util.get_current_platform() != util.Platform.WINDOWS:
             os.sync()
-        archive_size = self.archive_filepath.stat().st_size
-        self.ready_indicator_filepath.write_text(str(archive_size))
+
+        archive_size = -1
+        if self.archive_filepath.exists():
+            archive_size = self.archive_filepath.stat().st_size
+
+        archive_info = ArchiveState(
+            file_size=archive_size, error_message=error_message)
+
+        self.ready_indicator_filepath.write_text(
+            archive_info.serialize_to_string())
+
+    def get_state(self) -> ArchiveState:
+        """
+        Get state object for this archive
+
+        The state is read from the corresponding state file.
+        """
+
+        try:
+            return ArchiveState.deserialize_from_file(self.ready_indicator_filepath)
+        except ArchiveStateError as error:
+            return ArchiveState(error_message=f'Error in information format: {error}')
 
     def clean(self) -> None:
         """
diff --git a/build_files/buildbot/codesign/base_code_signer.py b/build_files/buildbot/codesign/base_code_signer.py
index 66953bfc5e5..b816415e7e4 100644
--- a/build_files/buildbot/codesign/base_code_signer.py
+++ b/build_files/buildbot/codesign/base_code_signer.py
@@ -58,6 +58,7 @@ import codesign.util as util
 
 from codesign.absolute_and_relative_filename import AbsoluteAndRelativeFileName
 from codesign.archive_with_indicator import ArchiveWithIndicator
+from codesign.exception import CodeSignException
 
 
 logger = logging.getLogger(__name__)
@@ -145,13 +146,13 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
     def cleanup_environment_for_builder(self) -> None:
         # TODO(sergey): Revisit need of cleaning up the existing files.
         # In practice it wasn't so helpful, and with multiple clients
-        # talking to the same server it becomes even mor etricky.
+        # talking to the same server it becomes even more tricky.
         pass
 
     def cleanup_environment_for_signing_server(self) -> None:
         # TODO(sergey): Revisit need of cleaning up the existing files.
         # In practice it wasn't so helpful, and with multiple clients
-        # talking to the same server it becomes even mor etricky.
+        # talking to the same server it becomes even more tricky.
         pass
 
     def generate_request_id(self) -> str:
@@ -220,9 +221,15 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
         """
         Wait until archive with signed files is available.
 
+        Will only return if the archive with signed files is available. If there
+        was an error during code sign procedure the SystemExit exception is
+        raised, with the message set to the error reported by the codesign
+        server.
+
         Will only wait for the configured time. If that time exceeds and there
         is still no responce from the signing server the application will exit
         with a non-zero exit code.
+
         """
 
         signed_archive_info = self.signed_archive_info_for_request_id(
@@ -236,9 +243,17 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
             time.sleep(1)
             time_slept_in_seconds = time.monotonic() - time_start
             if time_slept_in_seconds > timeout_in_seconds:
+                signed_archive_info.clean()
                 unsigned_archive_info.clean()
                 raise SystemExit("Signing server didn't finish signing in "
-                                 f"{timeout_in_seconds} seconds, dying :(")
+                                 f'{timeout_in_seconds} seconds, dying :(')
+
+        archive_state = signed_archive_info.get_state()
+        if archive_state.has_error():
+            signed_archive_info.clean()
+            unsigned_archive_info.clean()
+            raise SystemExit(
+                f'Error happenned during codesign procedure: {archive_state.error_message}')
 
     def copy_signed_files_to_directory(
             self, signed_dir: Path, destination_dir: Path) -> None:
@@ -391,7 +406,13 @@ class BaseCodeSigner(metaclass=abc.ABCMeta):
                 temp_dir)
 
             logger_server.info('Signing all requested files...')
-            self.sign_all_files(files)
+            try:
+                self.sign_all_files(files)
+            except CodeSignException as error:
+                signed_archive_info.tag_ready(error_message=error.message)
+                unsigned_archive_info.clean()
+                logger_server.info('Signing is complete with errors.')
+                return
 
             logger_server.info('Packing signed files...')
             pack_files(files=files,
diff --git a/build_files/buildbot/codesign/exception.py b/build_files/buildbot/codesign/exception.py
new file mode 100644
index 00000000000..6c8a9f262a5
--- /dev/null
+++ b/build_files/buildbot/codesign/exception.py
@@ -0,0 +1,26 @@
+# ##### BEGIN GPL LICENSE BLOCK #####
+#
+#  This program is free software; you can redistribute it and/or
+#  modify it under the terms of the GNU General Public License
+#  as published by the Free Software Foundation; either version 2
+#  of the License, or (at your option) any later version.
+#
+#  This program is distributed in the hope that it will be useful,
+#  but WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#  GNU General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with this program; if not, write to the Free Software Foundation,
+#  Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+#
+# ##### END GPL LICENSE BLOCK #####
+
+# <pep8 compliant>
+
+class CodeSignException(Exception):
+    message: str
+
+    def __init__(self, message):
+        self.message = message
+        super().__init__(self.message)
diff --git a/build_files/buildbot/codesign/macos_code_signer.py b/build_files/buildbot/codesign/macos_code_signer.py
index 44677339afa..f03dad8e1d6 100644
--- a/build_files/buildbot/codesign/macos_code_signer.py
+++ b/build_files/buildbot/codesign/macos_code_signer.py
@@ -33,6 +33,7 @@ from buildbot_utils import Builder
 
 from codesign.absolute_and_relative_filename import AbsoluteAndRelativeFileName
 from codesign.base_code_signer import BaseCodeSigner
+from codesign.exception import CodeSignException
 
 logger = logging.getLogger(__name__)
 logger_server = logger.getChild('server')
@@ -45,6 +46,10 @@ EXTENSIONS_TO_BE_SIGNED = {'.dylib', '.so', '.dmg'}
 NAME_PREFIXES_TO_BE_SIGNED = {'python'}
 
 
+class NotarizationException(CodeSignException):
+    pass
+
+
 def is_file_from_bundle(file: AbsoluteAndRelativeFileName) -> bool:
     """
     Check whether file is coming from an .app bundle
@@ -186,7 +191,7 @@ class MacOSCodeSigner(BaseCodeSigner):
                    file.absolute_filepath]
         self.run_command_or_mock(command, util.Platform.MACOS)
 
-    def codesign_all_files(self, files: List[AbsoluteAndRelativeFileName]) -> bool:
+    def codesign_all_files(self, files: List[AbsoluteAndRelativeFileName]) -> None:
         """
         Run codesign tool on all eligible files in the given list.
 
@@ -225,8 +230,6 @@ class MacOSCodeSigner(BaseCodeSigner):
                     file_index + 1, num_signed_files,
                     signed_file.relative_filepath)
 
-        return True
-
     def codesign_bundles(
             self, files: List[AbsoluteAndRelativeFileName]) -> None:
         """
@@ -273,8 +276,6 @@ class MacOSCodeSigner(BaseCodeSigner):
 
         files.extend(extra_files)
 
-        return True
-
     ############################################################################
     # Notarization.
 
@@ -334,7 +335,40 @@ class MacOSCodeSigner(BaseCodeSigner):
         logger_server.error('xcrun command did not report RequestUUID')
         return None
 
-    def notarize_wait_result(self, request_uuid: str) -> bool:
+    def notarize_review_status(self, xcrun_output: str) -> bool:
+        """
+        Review status returned by xcrun's notarization info
+
+        Returns truth if the notarization process has finished.
+        If there are errors during notarization, a NotarizationException()
+        exception is thrown with status message from the notarial office.
+        """
+
+        # Parse status and message
+        status = xcrun_field_value_from_output('Status', xcrun_output)
+        status_message = xcrun_field_value_from_output(
+            'Status Message', xcrun_output)
+
+        if status == 'success':
+            logger_server.info(
+                'Package successfully notarized: %s', status_message)
+            return True
+
+        if status == 'invalid':
+            logger_server.error(xcrun_output)
+            logger_server.error(
+                'Package notarization has failed: %s', status_message)
+            raise NotarizationException(status_message)
+
+        if status == 'in progress':
+            return False
+
+        logger_server.info(
+            'Unknown notarization status %s (%s)', status, status_message)
+
+        return False
+
+    def notarize_wait_result(self, request_uuid: str) -> None:
         """
         Wait for until notarial office have a reply
         """
@@ -351,29 +385,11 @@ class MacOSCodeSigner(BaseCodeSigner):
         timeout_in_seconds = self.config.MACOS_NOTARIZE_TIMEOUT_IN_SECONDS
 
         while True:
-            output = self.check_output_or_mock(
+            xcrun_output = self.check_output_or_mock(
                 command, util.Platform.MACOS, allow_nonzero_exit_code=True)
-            # Parse status and message
-            status = xcrun_field_value_from_output('Status', output)
-            status_message = xcrun_field_value_from_output(
-                'Status Message', output)
-
-            # Review status.
-            if status:
-                if status == 'success':
-                    logger_server.info(
-                        'Package successfully notarized: %s', status_message)
-                    return True
-                elif status == 'invalid':
-                    logger_server.error(output)
-                    logger_server.error(
-                        'Package notarization has failed: %s', status_message)
-                    return False
-                elif status == 'in progress':
-                    pass
-                else:
-                    logger_server.info(
-                        'Unknown notarization status %s (%s)', status, status_message)
+
+            if self.notarize_review_status(xcrun_output):
+                break
 
             logger_server.info('Keep waiting for notarization office.')
             time.sleep(30)
@@ -394,8 +410,6 @@ class MacOSCodeSigner(BaseCodeSigner):
         command = ['xcrun', 'stapler', 'staple', '-v', file.absolute_filepath]
         self.check_output_or_mock(command, util.Platform.MACOS)
 
-        return True
-
     def notarize_dmg(self, file: AbsoluteAndRelativeFileName) -> bool:
         """
         Run entire pipeline to get DMG notarized.
@@ -414,10 +428,7 @@ class MacOSCodeSigner(BaseCodeSigner):
             return False
 
         # Staple.
-        if not self.notarize_staple(file):
-            return False
-
-        return True
+        self.notarize_staple(file)
 
     def notarize_all_dmg(
             self, files: List[AbsoluteAndRelativeFileName]) -> bool:
@@ -432,10 +443,7 @@ class MacOSCodeSigner(BaseCodeSigner):
             if not self.check_file_is_to_be_signed(file):
                 continue
 
-            if not self.notarize_dmg(file):
-                return False
-
-        return True
+            self.notarize_dmg(file)
 
     ############################################################################
     # Entry point.
@@ -443,11 +451,6 @@ class MacOSCodeSigner(BaseCodeSigner):
     def sign_all_files(self, files: List[AbsoluteAndRelativeFileName]) -> None:
         # TODO(sergey): Handle errors somehow.
 
-        if not self.codesign_all_files(files):
-            return
-
-        if not self.codesign_bundles(files):
-            return
-
-        if not self.notarize_all_dmg(files):
-            return
+        self.codesign_all_files(files)
+        self.codesign_bundles(files)
+        self.notarize_all_dmg(files)
diff --git a/build_files/buildbot/codesign/windows_code_signer.py b/build_files/buildbot/codesign/windows_code_signer.py
index 2557d3c0b68..251dd856c8a 100644
--- a/build_files/buildbot/codesign/windows_code_signer.py
+++ b/build_files/buildbot/codesign/windows_code_signer.py
@@ -29,6 +29,7 @@ from buildbot_utils import Builder
 
 from codesign.absolute_and_relative_filename import AbsoluteAndRelativeFileName
 from codesign.base_code_signer import BaseCodeSigner
+from codesign.exception import CodeSignException
 
 logger = logging.getLogger(__name__)
 logger_server = logger.getChild('server')
@@ -40,6 +41,9 @@ BLACKLIST_FILE_PREFIXES = (
     'api-ms-', 'concrt', 'msvcp', 'ucrtbase', 'vcomp', 'vcruntime')
 
 
+class SigntoolException(CodeSignException):
+    pass
+
 class WindowsCodeSigner(BaseCodeSigner):
     def check_file_is_to_be_signed(
             self, file: AbsoluteAndRelativeFileName) -> bool:
@@ -50,12 +54,41 @@ class WindowsCodeSigner(BaseCodeSigner):
 
         return file.relative_filepath.suffix in EXTENSIONS_TO_BE_SIGNED
 
+
     def get_sign_command_prefix(self) -> List[str]:
         return [
             'signtool', 'sign', '/v',
             '/f', self.config.WIN_CERTIFICATE_FILEPATH,
             '/tr', self.config.WIN_TIMESTAMP_AUTHORITY_URL]
 
+
+    def run_codesign_tool(self, filepath: Path) -> None:
+        command = self.get_sign_command_prefix() + [filepath]
+        codesign_output = self.check_output_or_mock(command, util.Platform.WINDOWS)
+        logger_server.info(f'signtool output:\n{codesign_output}')
+
+        got_number_of_success = False
+
+        for line in codesign_output.split('\n'):
+            line_clean = line.strip()
+            line_clean_lower = line_clean.lower()
+
+            if line_clean_lower.startswith('number of warnings') or \
+               line_clean_lower.startswith('number of errors'):
+                 number = int(line_clean_lower.split(':')[1])
+                 if number != 0:
+                     raise SigntoolException('Non-clean success of signtool')
+
+            if line_clean_lower.startswith('number of files successfully signed'):
+                 got_number_of_success = True
+                 number = int(line_clean_lower.split(':')[1])
+                 if number != 1:
+                     raise SigntoolException('Signtool did not consider codesign a success')
+
+        if not got_number_of_success:
+            raise SigntoolException('Signtool did not report number of files signed')
+
+
     def sign_all_files(self, files: List[AbsoluteAndRelativeFileName]) -> None:
         # NOTE: Sign files one by one to avoid possible command line length
         # overflow (which could happen if we ever decide to sign every binary
@@ -73,12 +106,7 @@ class WindowsCodeSigner(BaseCodeSigner):
                     file_index + 1, num_files, file.relative_filepath)
                 continue
 
-            command = self.get_sign_command_prefix()
-            command.append(file.absolute_filepath)
             logger_server.info(
                 'Running signtool command for file [%d/%d] %s...',
                 file_index + 1, num_files, file.relative_filepath)
-            # TODO(sergey): Check the status somehow. With a missing certificate
-            # the command still exists with a zero code.
-            self.run_command_or_mock(command, util.Platform.WINDOWS)
-        # TODO(sergey): Report number of signed and ignored files.
+            self.run_codesign_tool(file.absolute_filepath)