[ClusterFuzzLite] Support GCB and gsutil/gcs as filestore. (#6629)

* add gsutil filestore

* lint

* Fix

* Add build image script

* get gcb fuzzing working

* fmt and fix config_utils_test

* Check that crashes are uploaded

* Add no_filestore

* fix test

* fix tests

* fix

* Print crash URL

* Fix

* fix

* fmt

* lnt

* fix

* fmt

.dockerignore

@@ -1,4 +1,5 @@
 .git
+.venv
 infra/cifuzz/test_data/*
 docs/*
 
@@ -8,4 +9,6 @@ docs/*
 build
 *~
 .DS_Store
-*.swp
+*.swp
+.pytest_cache
+*__pycache__/*

infra/cifuzz/build-images.sh

@@ -0,0 +1,28 @@
+#! /bin/bash -eux
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Script for building the docker images for cifuzz.
+
+CIFUZZ_DIR=$(dirname "$0")
+CIFUZZ_DIR=$(realpath $CIFUZZ_DIR)
+INFRA_DIR=$(realpath $CIFUZZ_DIR/..)
+OSS_FUZZ_ROOT=$(realpath $INFRA_DIR/..)
+
+# Build cifuzz-base.
+docker build --tag gcr.io/oss-fuzz-base/cifuzz-base --file $CIFUZZ_DIR/cifuzz-base/Dockerfile $OSS_FUZZ_ROOT
+
+# Build run-fuzzers and build-fuzzers images.
+docker build --tag gcr.io/oss-fuzz-base/cifuzz-build-fuzzers:v1 --file $INFRA_DIR/build_fuzzers.Dockerfile $INFRA_DIR
+docker build --tag gcr.io/oss-fuzz-base/cifuzz-run-fuzzers:v1 --file $INFRA_DIR/run_fuzzers.Dockerfile $INFRA_DIR

infra/cifuzz/build_fuzzers_test.py

@@ -201,6 +201,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
         project_repo_name=project_repo_name,
         workspace=self.workspace,
         git_url=git_url,
+        filestore='no_filestore',
         commit_sha='HEAD',
         project_src_path=project_src_path,
         base_commit='HEAD^1')

infra/cifuzz/cifuzz-base/Dockerfile

@@ -20,8 +20,17 @@ RUN apt-get update && \
     apt-get install -y systemd && \
     apt-get install -y --no-install-recommends nodejs npm && \
     wget https://download.docker.com/linux/ubuntu/dists/focal/pool/stable/amd64/docker-ce-cli_20.10.8~3-0~ubuntu-focal_amd64.deb -O /tmp/docker-ce.deb && \
-    dpkg -i /tmp/docker-ce.deb && rm /tmp/docker-ce.deb
+    dpkg -i /tmp/docker-ce.deb && \
+    rm /tmp/docker-ce.deb && \
+    mkdir -p /opt/gcloud && \
+    wget -qO- https://dl.google.com/dl/cloudsdk/release/google-cloud-sdk.tar.gz | tar zxv -C /opt/gcloud && \
+    /opt/gcloud/google-cloud-sdk/install.sh --usage-reporting=false --bash-completion=false --disable-installation-options && \
+    apt-get -y install gcc python3-dev && \
+    pip3 install -U crcmod && \
+    apt-get autoremove -y gcc python3-dev
 
+
+ENV PATH=/opt/gcloud/google-cloud-sdk/bin/:$PATH
 ENV OSS_FUZZ_ROOT=/opt/oss-fuzz
 ADD . ${OSS_FUZZ_ROOT}
 RUN python3 -m pip install -r ${OSS_FUZZ_ROOT}/infra/cifuzz/requirements.txt

infra/cifuzz/cifuzz_end_to_end_test.py

@@ -39,6 +39,8 @@ class EndToEndTest(unittest.TestCase):
     """Simple end-to-end test using run_cifuzz.main()."""
     os.environ['REPOSITORY'] = 'external-project'
     os.environ['PROJECT_SRC_PATH'] = EXTERNAL_PROJECT_PATH
+    os.environ['FILESTORE'] = 'no_filestore'
+    os.environ['NO_CLUSTERFUZZ_DEPLOYMENT'] = 'True'
 
     with test_helpers.docker_temp_dir() as temp_dir:
       os.environ['WORKSPACE'] = temp_dir

infra/cifuzz/clusterfuzz_deployment.py

@@ -100,7 +100,7 @@ class ClusterFuzzLite(BaseClusterFuzzDeployment):
       # called if multiple bugs are found.
       return self.workspace.clusterfuzz_build
 
-    repo_dir = self.ci_system.repo_dir()
+    repo_dir = self.ci_system.repo_dir
     if not repo_dir:
       raise RuntimeError('Repo checkout does not exist.')
 
@@ -355,20 +355,19 @@ class NoClusterFuzzDeployment(BaseClusterFuzzDeployment):
 
 
 _PLATFORM_CLUSTERFUZZ_DEPLOYMENT_MAPPING = {
-    config_utils.BaseConfig.Platform.INTERNAL_GENERIC_CI:
+    config_utils.BaseConfig.Platform.INTERNAL_GENERIC_CI: OSSFuzz,
-        OSSFuzz,
+    config_utils.BaseConfig.Platform.INTERNAL_GITHUB: OSSFuzz,
-    config_utils.BaseConfig.Platform.INTERNAL_GITHUB:
+    config_utils.BaseConfig.Platform.EXTERNAL_GENERIC_CI: ClusterFuzzLite,
-        OSSFuzz,
+    config_utils.BaseConfig.Platform.EXTERNAL_GITHUB: ClusterFuzzLite,
-    config_utils.BaseConfig.Platform.EXTERNAL_GENERIC_CI:
-        NoClusterFuzzDeployment,
-    config_utils.BaseConfig.Platform.EXTERNAL_GITHUB:
-        ClusterFuzzLite,
 }
 
 
 def get_clusterfuzz_deployment(config, workspace):
   """Returns object reprsenting deployment of ClusterFuzz used by |config|."""
   deployment_cls = _PLATFORM_CLUSTERFUZZ_DEPLOYMENT_MAPPING[config.platform]
+  if config.no_clusterfuzz_deployment:
+    logging.info('Overriding ClusterFuzzDeployment. Using None.')
+    deployment_cls = NoClusterFuzzDeployment
   result = deployment_cls(config, workspace)
   logging.info('ClusterFuzzDeployment: %s.', result)
   return result

infra/cifuzz/clusterfuzz_deployment_test.py

@@ -132,6 +132,7 @@ class ClusterFuzzLiteTest(fake_filesystem_unittest.TestCase):
     self.setUpPyfakefs()
     self.deployment = _create_deployment(run_fuzzers_mode='batch',
                                          oss_fuzz_project_name='',
+                                         cloud_bucket='gs://bucket',
                                          is_github=True)
     self.corpus_dir = os.path.join(self.deployment.workspace.corpora,
                                    EXAMPLE_FUZZER)
@@ -157,7 +158,7 @@ class ClusterFuzzLiteTest(fake_filesystem_unittest.TestCase):
               side_effect=[False, True])
   @mock.patch('repo_manager.RepoManager.get_commit_list',
               return_value=['commit1', 'commit2'])
-  @mock.patch('continuous_integration.BaseCi.repo_dir',
+  @mock.patch('continuous_integration.GithubCiMixin.repo_dir',
               return_value='/path/to/repo')
   def test_download_latest_build(self, mock_repo_dir, mock_get_commit_list,
                                  mock_download_build):
@@ -173,7 +174,7 @@ class ClusterFuzzLiteTest(fake_filesystem_unittest.TestCase):
               side_effect=Exception)
   @mock.patch('repo_manager.RepoManager.get_commit_list',
               return_value=['commit1', 'commit2'])
-  @mock.patch('continuous_integration.BaseCi.repo_dir',
+  @mock.patch('continuous_integration.GithubCiMixin.repo_dir',
               return_value='/path/to/repo')
   def test_download_latest_build_fail(self, mock_repo_dir, mock_get_commit_list,
                                       _):
@@ -195,10 +196,13 @@ class NoClusterFuzzDeploymentTest(fake_filesystem_unittest.TestCase):
   def setUp(self):
     self.setUpPyfakefs()
     config = test_helpers.create_run_config(workspace=WORKSPACE,
-                                            is_github=False)
+                                            is_github=False,
+                                            filestore='no_filestore',
+                                            no_clusterfuzz_deployment=True)
     workspace = workspace_utils.Workspace(config)
     self.deployment = clusterfuzz_deployment.get_clusterfuzz_deployment(
         config, workspace)
+
     self.corpus_dir = os.path.join(workspace.corpora, EXAMPLE_FUZZER)
 
   @mock.patch('logging.info')
@@ -241,7 +245,7 @@ class GetClusterFuzzDeploymentTest(unittest.TestCase):
       (config_utils.BaseConfig.Platform.INTERNAL_GITHUB,
        clusterfuzz_deployment.OSSFuzz),
       (config_utils.BaseConfig.Platform.EXTERNAL_GENERIC_CI,
-       clusterfuzz_deployment.NoClusterFuzzDeployment),
+       clusterfuzz_deployment.ClusterFuzzLite),
       (config_utils.BaseConfig.Platform.EXTERNAL_GITHUB,
        clusterfuzz_deployment.ClusterFuzzLite),
   ])

infra/cifuzz/config_utils.py

@@ -236,7 +236,12 @@ class BaseConfig:
     self.git_store_branch = os.environ.get('GIT_STORE_BRANCH')
     self.git_store_branch_coverage = os.environ.get('GIT_STORE_BRANCH_COVERAGE',
                                                     self.git_store_branch)
+    self.project_src_path = self._ci_env.project_src_path
     self.docker_in_docker = os.environ.get('DOCKER_IN_DOCKER')
+    self.filestore = os.environ.get('FILESTORE')
+    self.cloud_bucket = os.environ.get('CLOUD_BUCKET')
+    self.no_clusterfuzz_deployment = os.environ.get('NO_CLUSTERFUZZ_DEPLOYMENT',
+                                                    False)
 
     # TODO(metzman): Fix tests to create valid configurations and get rid of
     # CIFUZZ_TEST here and in presubmit.py.
@@ -261,6 +266,10 @@ class BaseConfig:
                     constants.LANGUAGES)
       return False
 
+    if not self.project_repo_name:
+      logging.error('Must set REPOSITORY.')
+      return False
+
     return True
 
   @property
@@ -361,7 +370,6 @@ class BuildFuzzersConfig(BaseConfig):
     self._get_config_from_event_path(event)
 
     self.base_ref = os.getenv('GITHUB_BASE_REF')
-    self.project_src_path = self._ci_env.project_src_path
 
     self.allowed_broken_targets_percentage = os.getenv(
         'ALLOWED_BROKEN_TARGETS_PERCENTAGE')

infra/cifuzz/config_utils_test.py

@@ -91,6 +91,7 @@ class BaseConfigTest(unittest.TestCase):
     """Tests that validate returns True if config is valid."""
     os.environ['OSS_FUZZ_PROJECT_NAME'] = 'example'
     os.environ['WORKSPACE'] = '/workspace'
+    os.environ['REPOSITORY'] = 'repo'
     config = self._create_config()
     self.assertTrue(config.validate())
 

infra/cifuzz/continuous_integration.py

@@ -53,23 +53,13 @@ class BaseCi:
   def __init__(self, config):
     self.config = config
     self.workspace = workspace_utils.Workspace(config)
+    self._repo_dir = None
 
+  @property
   def repo_dir(self):
     """Returns the source repo path, if it has been checked out. None is
     returned otherwise."""
-    if not os.path.exists(self.workspace.repo_storage):
+    raise NotImplementedError('Child class must implement method.')
-      return None
-
-    # Note: this assumes there is only one repo checked out here.
-    listing = os.listdir(self.workspace.repo_storage)
-    if len(listing) != 1:
-      raise RuntimeError('Invalid repo storage.')
-
-    repo_path = os.path.join(self.workspace.repo_storage, listing[0])
-    if not os.path.isdir(repo_path):
-      raise RuntimeError('Repo is not a directory.')
-
-    return repo_path
 
   def prepare_for_fuzzer_build(self):
     """Builds the fuzzer builder image and gets the source code we need to
@@ -152,6 +142,31 @@ def checkout_specified_commit(repo_manager_obj, pr_ref, commit_sha):
 class GithubCiMixin:
   """Mixin for Github based CI systems."""
 
+  def __init__(self, config):
+    super().__init__(config)
+    # Unlike in other classes, here _repo_dir is the parent directory of the
+    # repo, not its actual directory.
+    self._repo_dir = self.workspace.repo_storage
+
+  @property
+  def repo_dir(self):
+    """Returns the source repo path, if it has been checked out. None is
+    returned otherwise."""
+    if not os.path.exists(self._repo_dir):
+      logging.warning('Repo dir: %s does not exist.', self._repo_dir)
+      return None
+
+    # Note: this assumes there is only one repo checked out here.
+    listing = os.listdir(self._repo_dir)
+    if len(listing) != 1:
+      raise RuntimeError('Invalid repo directory.')
+
+    repo_path = os.path.join(self._repo_dir, listing[0])
+    if not os.path.isdir(repo_path):
+      raise RuntimeError('Repo is not a directory.')
+
+    return repo_path
+
   def get_diff_base(self):
     """Returns the base to diff against with git to get the change under
     test."""
@@ -217,6 +232,16 @@ class InternalGeneric(BaseCi):
   """Class representing CI for an OSS-Fuzz project on a CI other than Github
   actions."""
 
+  def __init__(self, config):
+    super().__init__(config)
+    self._repo_dir = config.project_src_path
+
+  @property
+  def repo_dir(self):
+    """Returns the source repo path, if it has been checked out. None is
+    returned otherwise."""
+    return self._repo_dir
+
   def prepare_for_fuzzer_build(self):
     """Builds the project builder image for an OSS-Fuzz project outside of
     GitHub actions. Returns the repo_manager. Does not checkout source code
@@ -263,6 +288,16 @@ def build_external_project_docker_image(project_src, build_integration_path):
 class ExternalGeneric(BaseCi):
   """CI implementation for generic CI for external (non-OSS-Fuzz) projects."""
 
+  def __init__(self, config):
+    super().__init__(config)
+    self._repo_dir = config.project_src_path
+
+  @property
+  def repo_dir(self):
+    """Returns the source repo path, if it has been checked out. None is
+    returned otherwise."""
+    return self._repo_dir
+
   def get_diff_base(self):
     return 'origin...'
 

infra/cifuzz/filestore/__init__.py

@@ -49,6 +49,6 @@ class BaseFilestore:
     """Downloads the build with |name| to |dst_directory|."""
     raise NotImplementedError('Child class must implement method.')
 
-  def download_coverage(self, dst_directory):
+  def download_coverage(self, name, dst_directory):
     """Downloads the latest project coverage report."""
     raise NotImplementedError('Child class must implement method.')

infra/cifuzz/filestore/github_actions/__init__.py

@@ -21,8 +21,9 @@ import tempfile
 
 # pylint: disable=wrong-import-position,import-error
 sys.path.append(
-    os.path.join(os.path.pardir, os.path.pardir, os.path.pardir,
+    os.path.abspath(
-                 os.path.dirname(os.path.abspath(__file__))))
+        os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir,
+                     os.path.pardir)))
 
 import utils
 import http_utils

infra/cifuzz/filestore/github_actions/github_actions_test.py

@@ -24,8 +24,7 @@ from pyfakefs import fake_filesystem_unittest
 
 # pylint: disable=wrong-import-position
 INFRA_DIR = os.path.dirname(
-    os.path.dirname(os.path.dirname(os.path.dirname(
+    os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
-        os.path.abspath(__file__)))))
 sys.path.append(INFRA_DIR)
 
 from filestore import github_actions

infra/cifuzz/filestore/github_actions/github_api_test.py

@@ -12,8 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """Tests for github_api."""
+import os
+import sys
 import unittest
 
+# pylint: disable=wrong-import-position,import-error
+sys.path.append(
+    os.path.abspath(
+        os.path.join(os.path.dirname(__file__), os.path.pardir, os.path.pardir,
+                     os.path.pardir)))
+
 from filestore.github_actions import github_api
 import test_helpers
 

infra/cifuzz/filestore/gsutil/__init__.py

@@ -0,0 +1,106 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Filestore implementation using gsutil."""
+import logging
+import os
+import posixpath
+import subprocess
+import sys
+
+# pylint: disable=wrong-import-position,import-error
+sys.path.append(
+    os.path.join(os.path.dirname(os.path.abspath(__file__)), os.pardir,
+                 os.pardir, os.pardir))
+import filestore
+import utils
+
+
+def _gsutil_execute(*args, parallel=True):
+  """Executes a gsutil command, passing |*args| to gsutil and returns the
+  stdout, stderr and returncode. Exceptions on failure."""
+  command = ['gsutil']
+  if parallel:
+    command.append('-m')
+  command += list(args)
+  logging.info('Executing gsutil command: %s', command)
+  return utils.execute(command, check_result=True)
+
+
+def _rsync(src, dst, delete=False):
+  """Executes gsutil rsync on |src| and |dst|"""
+  args = ['rsync', src, dst]
+  if delete:
+    args.append('--delete')
+  return _gsutil_execute(*args)
+
+
+class GSUtilFilestore(filestore.BaseFilestore):
+  """Filestore implementation using gsutil."""
+  BUILD_DIR = 'build'
+  CRASHES_DIR = 'crashes'
+  CORPUS_DIR = 'corpus'
+  COVERAGE_DIR = 'coverage'
+
+  def __init__(self, config):
+    super().__init__(config)
+    self._cloud_bucket = self.config.cloud_bucket
+
+  def _get_gsutil_url(self, name, prefix_dir):
+    """Returns the gsutil URL for |name| and |prefix_dir|."""
+    if not prefix_dir:
+      return posixpath.join(self._cloud_bucket, name)
+    return posixpath.join(self._cloud_bucket, prefix_dir, name)
+
+  def _upload_directory(self, name, directory, prefix, delete=False):
+    gsutil_url = self._get_gsutil_url(name, prefix)
+    return _rsync(directory, gsutil_url, delete=delete)
+
+  def _download_directory(self, name, dst_directory, prefix):
+    gsutil_url = self._get_gsutil_url(name, prefix)
+    return _rsync(gsutil_url, dst_directory)
+
+  def upload_crashes(self, name, directory):
+    """Uploads the crashes at |directory| to |name|."""
+    # Name is going to be "current". I don't know if this makes sense outside of
+    # GitHub Actions.
+    gsutil_url = self._get_gsutil_url(name, self.CRASHES_DIR)
+    logging.info('Uploading crashes to %s.')
+    return _rsync(directory, gsutil_url)
+
+  def upload_corpus(self, name, directory, replace=False):
+    """Uploads the crashes at |directory| to |name|."""
+    return self._upload_directory(name,
+                                  directory,
+                                  self.CORPUS_DIR,
+                                  delete=replace)
+
+  def upload_build(self, name, directory):
+    """Uploads the build located at |directory| to |name|."""
+    return self._upload_directory(name, directory, self.BUILD_DIR)
+
+  def upload_coverage(self, name, directory):
+    """Uploads the coverage report at |directory| to |name|."""
+    return self._upload_directory(name, directory, self.COVERAGE_DIR)
+
+  def download_corpus(self, name, dst_directory):
+    """Downloads the corpus located at |name| to |dst_directory|."""
+    return self._download_directory(name, dst_directory, self.CORPUS_DIR)
+
+  def download_build(self, name, dst_directory):
+    """Downloads the build with |name| to |dst_directory|."""
+    return self._download_directory(name, dst_directory, self.BUILD_DIR)
+
+  def download_coverage(self, name, dst_directory):
+    """Downloads the latest project coverage report."""
+    return self._download_directory(name, dst_directory, self.COVERAGE_DIR)

infra/cifuzz/filestore/no_filestore/__init__.py

@@ -0,0 +1,51 @@
+# Copyright 2021 Google LLC
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+"""Empty filestore implementation for platforms that haven't implemented it."""
+import logging
+
+import filestore
+
+# pylint:disable=no-self-use,unused-argument
+
+
+class NoFilestore(filestore.BaseFilestore):
+  """Empty Filestore implementation."""
+
+  def upload_crashes(self, name, directory):
+    """Noop implementation of upload_crashes."""
+    logging.info('Not uploading crashes because no Filestore.')
+
+  def upload_corpus(self, name, directory):
+    """Noop implementation of upload_corpus."""
+    logging.info('Not uploading corpus because no Filestore.')
+
+  def upload_build(self, name, directory):
+    """Noop implementation of upload_build."""
+    logging.info('Not uploading build because no Filestore.')
+
+  def upload_coverage(self, name, directory):
+    """Noop implementation of upload_coverage."""
+    logging.info('Not uploading coverage because no Filestore.')
+
+  def download_corpus(self, name, dst_directory):
+    """Noop implementation of download_corpus."""
+    logging.info('Not downloading corpus because no Filestore.')
+
+  def download_build(self, name, dst_directory):
+    """Noop implementation of download_build."""
+    logging.info('Not downloading build because no Filestore.')
+
+  def download_coverage(self, name, dst_directory):
+    """Noop implementation of download_coverage."""
+    logging.info('Not downloading coverage because no Filestore.')

infra/cifuzz/filestore_utils.py

@@ -15,12 +15,20 @@
 import filestore
 import filestore.git
 import filestore.github_actions
+import filestore.gsutil
+import filestore.no_filestore
+
+FILESTORE_MAPPING = {
+    'gsutil': filestore.gsutil.GSUtilFilestore,
+    'github-actions': filestore.github_actions.GithubActionsFilestore,
+    'git': filestore.git.GitFilestore,
+    'no_filestore': filestore.no_filestore.NoFilestore,
+}
 
 
 def get_filestore(config):
-  """Returns the correct filestore based on the platform in |config|.
+  """Returns the correct filestore object based on the platform in |config|.
   Raises an exception if there is no correct filestore for the platform."""
-  # TODO(metzman): Force specifying of filestore.
   if config.platform == config.Platform.EXTERNAL_GITHUB:
     ci_filestore = filestore.github_actions.GithubActionsFilestore(config)
     if not config.git_store_repo:
@@ -28,4 +36,7 @@ def get_filestore(config):
 
     return filestore.git.GitFilestore(config, ci_filestore)
 
-  raise filestore.FilestoreError('Filestore doesn\'t support platform.')
+  filestore_cls = FILESTORE_MAPPING.get(config.filestore)
+  if filestore_cls is None:
+    raise filestore.FilestoreError('Filestore doesn\'t exist.')
+  return filestore_cls(config)

infra/cifuzz/run_cifuzz.py

@@ -36,7 +36,8 @@ def docker_run(name, workspace, project_src_path):
   command = [
       'docker', 'run', '--name', name, '--rm', '-e', 'PROJECT_SRC_PATH', '-e',
       'OSS_FUZZ_PROJECT_NAME', '-e', 'WORKSPACE', '-e', 'REPOSITORY', '-e',
-      'DRY_RUN', '-e', 'CI', '-e', 'SANITIZER', '-e', 'GIT_SHA'
+      'DRY_RUN', '-e', 'CI', '-e', 'SANITIZER', '-e', 'GIT_SHA', '-e',
+      'FILESTORE', '-e', 'NO_CLUSTERFUZZ_DEPLOYMENT'
   ]
   if project_src_path:
     command += ['-v', f'{project_src_path}:{project_src_path}']

infra/cifuzz/run_fuzzers.py

@@ -143,8 +143,9 @@ class BaseFuzzTargetRunner:
       bug_found = True
       if self.quit_on_bug_found:
         logging.info('Bug found. Stopping fuzzing.')
-        return bug_found
+        break
 
+    self.clusterfuzz_deployment.upload_crashes()
     return bug_found
 
 
@@ -239,11 +240,6 @@ class BatchFuzzTargetRunner(BaseFuzzTargetRunner):
     # because it is needed when we upload the build.
     fuzz_target_obj.free_disk_if_needed(delete_fuzz_target=False)
 
-  def run_fuzz_targets(self):
-    result = super().run_fuzz_targets()
-    self.clusterfuzz_deployment.upload_crashes()
-    return result
-
 
 _RUN_FUZZERS_MODE_RUNNER_MAPPING = {
     'batch': BatchFuzzTargetRunner,

infra/cifuzz/run_fuzzers_test.py

@@ -218,11 +218,13 @@ class CiFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
   def setUp(self):
     self.setUpPyfakefs()
 
+  @mock.patch('clusterfuzz_deployment.OSSFuzz.upload_crashes')
   @mock.patch('utils.get_fuzz_targets')
   @mock.patch('run_fuzzers.CiFuzzTargetRunner.run_fuzz_target')
   @mock.patch('run_fuzzers.CiFuzzTargetRunner.create_fuzz_target_obj')
   def test_run_fuzz_targets_quits(self, mock_create_fuzz_target_obj,
-                                  mock_run_fuzz_target, mock_get_fuzz_targets):
+                                  mock_run_fuzz_target, mock_get_fuzz_targets,
+                                  mock_upload_crashes):
     """Tests that run_fuzz_targets quits on the first crash it finds."""
     workspace = 'workspace'
     out_path = os.path.join(workspace, 'build-out')
@@ -247,6 +249,7 @@ class CiFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
     mock_create_fuzz_target_obj.return_value = magic_mock
     self.assertTrue(runner.run_fuzz_targets())
     self.assertEqual(mock_run_fuzz_target.call_count, 1)
+    self.assertEqual(mock_upload_crashes.call_count, 1)
 
 
 class BatchFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
@@ -268,12 +271,11 @@ class BatchFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
                                                  is_github=True)
 
   @mock.patch('utils.get_fuzz_targets', return_value=['target1', 'target2'])
-  @mock.patch('clusterfuzz_deployment.ClusterFuzzLite.upload_build',
+  @mock.patch('clusterfuzz_deployment.ClusterFuzzLite.upload_crashes')
-              return_value=True)
   @mock.patch('run_fuzzers.BatchFuzzTargetRunner.run_fuzz_target')
   @mock.patch('run_fuzzers.BatchFuzzTargetRunner.create_fuzz_target_obj')
   def test_run_fuzz_targets_quits(self, mock_create_fuzz_target_obj,
-                                  mock_run_fuzz_target, _, __):
+                                  mock_run_fuzz_target, mock_upload_crashes, _):
     """Tests that run_fuzz_targets doesn't quit on the first crash it finds."""
     runner = run_fuzzers.BatchFuzzTargetRunner(self.config)
     runner.initialize()
@@ -298,18 +300,6 @@ class BatchFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
     mock_create_fuzz_target_obj.return_value = magic_mock
     self.assertTrue(runner.run_fuzz_targets())
     self.assertEqual(mock_run_fuzz_target.call_count, 2)
-
-  @mock.patch('run_fuzzers.BaseFuzzTargetRunner.run_fuzz_targets',
-              return_value=False)
-  @mock.patch('clusterfuzz_deployment.ClusterFuzzLite.upload_crashes')
-  def test_run_fuzz_targets_upload_crashes_and_builds(self, mock_upload_crashes,
-                                                      _):
-    """Tests that run_fuzz_targets uploads crashes and builds correctly."""
-    runner = run_fuzzers.BatchFuzzTargetRunner(self.config)
-    # TODO(metzman): Don't rely on this failing gracefully.
-    runner.initialize()
-
-    self.assertFalse(runner.run_fuzz_targets())
     self.assertEqual(mock_upload_crashes.call_count, 1)