[CIFuzz] Move run_fuzzers to new config system (#5063)

Also, decide is_github based on something not used/faked by Skia.

8 files changed, 116 insertions, 83 deletions

diff --git a/infra/cifuzz/build_fuzzers_entrypoint.py b/infra/cifuzz/build_fuzzers_entrypoint.py
index 7a2ee303e..837ed2a39 100644
--- a/infra/cifuzz/build_fuzzers_entrypoint.py
+++ b/infra/cifuzz/build_fuzzers_entrypoint.py
@@ -52,7 +52,7 @@ def main():
   Returns:
     0 on success or 1 on failure.
   """
-  config = config_utils.Config()
+  config = config_utils.BuildFuzzersConfig()
 
   if config.dry_run:
     # Sets the default return code on error to success.
diff --git a/infra/cifuzz/cifuzz.py b/infra/cifuzz/cifuzz.py
index e4af7f31c..f4f4ea24c 100644
--- a/infra/cifuzz/cifuzz.py
+++ b/infra/cifuzz/cifuzz.py
@@ -146,6 +146,8 @@ class Builder:  # pylint: disable=too-many-instance-attributes
 
   def remove_unaffected_fuzz_targets(self):
     """Removes the fuzzers unaffected by the patch."""
+    if self.config.keep_unaffected_fuzz_targets:
+      return True
     changed_files = self.ci_system.get_changed_code_under_test(
         self.repo_manager)
     affected_fuzz_targets.remove_unaffected_fuzz_targets(
diff --git a/infra/cifuzz/cifuzz_test.py b/infra/cifuzz/cifuzz_test.py
index 9790426af..a8fe1395a 100644
--- a/infra/cifuzz/cifuzz_test.py
+++ b/infra/cifuzz/cifuzz_test.py
@@ -63,7 +63,7 @@ def create_config(**kwargs):
       'config_utils.get_project_src_path',
       return_value=None), mock.patch('config_utils._is_dry_run',
                                      return_value=True):
-    config = config_utils.Config()
+    config = config_utils.BuildFuzzersConfig()
 
   for key, value in kwargs.items():
     assert hasattr(config, key), 'Config doesn\'t have attribute: ' + key
diff --git a/infra/cifuzz/config_utils.py b/infra/cifuzz/config_utils.py
index 99ebf85c8..e0babda41 100644
--- a/infra/cifuzz/config_utils.py
+++ b/infra/cifuzz/config_utils.py
@@ -62,7 +62,10 @@ def get_project_src_path(workspace):
   return os.path.join(workspace, path)
 
 
-class Config:  # pylint: disable=too-few-public-methods,too-many-instance-attributes
+# pylint: disable=too-few-public-methods,too-many-instance-attributes
+
+
+class BaseConfig:
   """Object containing constant configuration for CIFuzz."""
 
   class Platform(enum.Enum):
@@ -71,6 +74,38 @@ class Config:  # pylint: disable=too-few-public-methods,too-many-instance-attrib
     INTERNAL_GITHUB = 1  # OSS-Fuzz on GitHub actions.
     INTERNAL_GENERIC_CI = 2  # OSS-Fuzz on any CI.
 
+  def __init__(self):
+    self.workspace = os.getenv('GITHUB_WORKSPACE')
+    self.project_name = _get_project_name()
+    # Check if failures should not be reported.
+    self.dry_run = _is_dry_run()
+    self.sanitizer = _get_sanitizer()
+    self.build_integration_path = os.getenv('BUILD_INTEGRATION_PATH')
+    event_path = os.getenv('GITHUB_EVENT_PATH')
+    self.is_github = bool(event_path)
+    logging.debug('Is github: %s.', self.is_github)
+
+  @property
+  def platform(self):
+    """Returns the platform CIFuzz is runnning on."""
+    if self.build_integration_path:
+      return self.Platform.EXTERNAL_GITHUB
+    if self.is_github:
+      return self.Platform.INTERNAL_GITHUB
+    return self.Platform.INTERNAL_GENERIC_CI
+
+
+class RunFuzzersConfig(BaseConfig):
+  """Class containing constant configuration for running fuzzers in CIFuzz."""
+
+  def __init__(self):
+    super().__init__()
+    self.fuzz_seconds = int(os.environ.get('FUZZ_SECONDS', 600))
+
+
+class BuildFuzzersConfig(BaseConfig):
+  """Class containing constant configuration for building fuzzers in CIFuzz."""
+
   def _get_config_from_event_path(self, event):
     event_path = os.getenv('GITHUB_EVENT_PATH')
     if not event_path:
@@ -92,13 +127,10 @@ class Config:  # pylint: disable=too-few-public-methods,too-many-instance-attrib
     are set by GitHub or the user."""
     # TODO(metzman): Some of this config is very CI-specific. Move it into the
     # CI class.
-    self.project_name = _get_project_name()
+    super().__init__()
     self.project_repo_name = _get_project_repo_name()
     self.commit_sha = os.getenv('GITHUB_SHA')
-
     event = os.getenv('GITHUB_EVENT_NAME')
-    self.is_github = bool(event)
-    logging.debug('Is github: %s.', self.is_github)
 
     self.pr_ref = None
     self.git_url = None
@@ -106,21 +138,10 @@ class Config:  # pylint: disable=too-few-public-methods,too-many-instance-attrib
     self._get_config_from_event_path(event)
 
     self.base_ref = os.getenv('GITHUB_BASE_REF')
-    self.workspace = os.getenv('GITHUB_WORKSPACE')
     self.project_src_path = get_project_src_path(self.workspace)
 
-    self.sanitizer = _get_sanitizer()
-    self.build_integration_path = os.getenv('BUILD_INTEGRATION_PATH')
     self.allowed_broken_targets_percentage = os.getenv(
         'ALLOWED_BROKEN_TARGETS_PERCENTAGE')
-    # Check if failures should not be reported.
-    self.dry_run = _is_dry_run()
 
-  @property
-  def platform(self):
-    """Returns the platform CIFuzz is runnning on."""
-    if self.build_integration_path:
-      return self.Platform.EXTERNAL_GITHUB
-    if self.is_github:
-      return self.Platform.INTERNAL_GITHUB
-    return self.Platform.INTERNAL_GENERIC_CI
+    self.keep_unaffected_fuzz_targets = bool(
+        os.getenv('KEEP_UNAFFECTED_FUZZERS', 'False'))
diff --git a/infra/cifuzz/config_utils_test.py b/infra/cifuzz/config_utils_test.py
index c97ebd15b..12daf7eea 100644
--- a/infra/cifuzz/config_utils_test.py
+++ b/infra/cifuzz/config_utils_test.py
@@ -26,14 +26,14 @@ import test_helpers
 # pylint: disable=no-self-use
 
 
-class ConfigTest(unittest.TestCase):
-  """Tests for Config."""
+class BuildFuzzersConfigTest(unittest.TestCase):
+  """Tests for BuildFuzzersConfig."""
 
   def setUp(self):
     test_helpers.patch_environ(self)
 
   def _create_config(self):
-    return config_utils.Config()
+    return config_utils.BuildFuzzersConfig()
 
   def test_base_ref(self):
     """Tests that base_ref is set properly."""
diff --git a/infra/cifuzz/run_fuzzers.py b/infra/cifuzz/run_fuzzers.py
index f3dfcca37..1bb08fe6c 100644
--- a/infra/cifuzz/run_fuzzers.py
+++ b/infra/cifuzz/run_fuzzers.py
@@ -27,12 +27,8 @@ sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
 import utils
 
 
-def run_fuzzers(  # pylint: disable=too-many-arguments,too-many-locals
-    fuzz_seconds,
-    workspace,
-    project_name,
-    sanitizer='address'):
-  """Runs all fuzzers for a specific OSS-Fuzz project.
+def run_fuzzers(config):  # pylint: disable=too-many-locals
+  """Runs fuzzers for a specific OSS-Fuzz project.
 
   Args:
     fuzz_seconds: The total time allotted for fuzzing.
@@ -45,18 +41,15 @@ def run_fuzzers(  # pylint: disable=too-many-arguments,too-many-locals
     (True if run was successful, True if bug was found).
   """
   # Validate inputs.
-  if not os.path.exists(workspace):
-    logging.error('Invalid workspace: %s.', workspace)
-    return False, False
-
-  logging.info('Using %s sanitizer.', sanitizer)
+  logging.info('Using %s sanitizer.', config.sanitizer)
 
-  out_dir = os.path.join(workspace, 'out')
+  out_dir = os.path.join(config.workspace, 'out')
   artifacts_dir = os.path.join(out_dir, 'artifacts')
   os.makedirs(artifacts_dir, exist_ok=True)
-  if not fuzz_seconds or fuzz_seconds < 1:
+
+  if not config.fuzz_seconds or config.fuzz_seconds < 1:
     logging.error('Fuzz_seconds argument must be greater than 1, but was: %s.',
-                  fuzz_seconds)
+                  config.fuzz_seconds)
     return False, False
 
   # Get fuzzer information.
@@ -68,19 +61,19 @@ def run_fuzzers(  # pylint: disable=too-many-arguments,too-many-locals
   # Run fuzzers for allotted time.
   total_num_fuzzers = len(fuzzer_paths)
   fuzzers_left_to_run = total_num_fuzzers
-  min_seconds_per_fuzzer = fuzz_seconds // total_num_fuzzers
+  min_seconds_per_fuzzer = config.fuzz_seconds // total_num_fuzzers
   for fuzzer_path in fuzzer_paths:
-    run_seconds = max(fuzz_seconds // fuzzers_left_to_run,
+    run_seconds = max(config.fuzz_seconds // fuzzers_left_to_run,
                       min_seconds_per_fuzzer)
 
     target = fuzz_target.FuzzTarget(fuzzer_path,
                                     run_seconds,
                                     out_dir,
-                                    project_name,
-                                    sanitizer=sanitizer)
+                                    config.project_name,
+                                    sanitizer=config.sanitizer)
     start_time = time.time()
     testcase, stacktrace = target.fuzz()
-    fuzz_seconds -= (time.time() - start_time)
+    config.fuzz_seconds -= (time.time() - start_time)
     if not testcase or not stacktrace:
       logging.info('Fuzzer %s, finished running.', target.target_name)
     else:
diff --git a/infra/cifuzz/run_fuzzers_entrypoint.py b/infra/cifuzz/run_fuzzers_entrypoint.py
index 86a5abede..48d745fd8 100644
--- a/infra/cifuzz/run_fuzzers_entrypoint.py
+++ b/infra/cifuzz/run_fuzzers_entrypoint.py
@@ -13,9 +13,9 @@
 # limitations under the License.
 """Runs specific OSS-Fuzz project's fuzzers for CI tools."""
 import logging
-import os
 import sys
 
+import config_utils
 import run_fuzzers
 
 # pylint: disable=c-extension-no-member
@@ -52,41 +52,26 @@ def main():
   Returns:
     0 on success or 1 on failure.
   """
-  fuzz_seconds = int(os.environ.get('FUZZ_SECONDS', 600))
-  workspace = os.environ.get('GITHUB_WORKSPACE')
-  oss_fuzz_project_name = os.environ.get('OSS_FUZZ_PROJECT_NAME')
-  sanitizer = os.environ.get('SANITIZER').lower()
-
-  # Check if failures should not be reported.
-  dry_run = (os.environ.get('DRY_RUN').lower() == 'true')
-
+  config = config_utils.RunFuzzersConfig()
   # The default return code when an error occurs.
   returncode = 1
-  if dry_run:
-    # A testcase file is required in order for CIFuzz to surface bugs.
-    # If the file does not exist, the action will crash attempting to upload it.
-    # The dry run needs this file because it is set to upload a testcase both
-    # on successful runs and on failures.
-    out_dir = os.path.join(workspace, 'out', 'artifacts')
-    os.makedirs(out_dir, exist_ok=True)
-
+  if config.dry_run:
     # Sets the default return code on error to success.
     returncode = 0
 
-  if not workspace:
-    logging.error('This script needs to be run in the Github action context.')
+  if not config.workspace:
+    logging.error('This script needs to be run within Github actions.')
     return returncode
+
   # Run the specified project's fuzzers from the build.
-  run_status, bug_found = run_fuzzers.run_fuzzers(fuzz_seconds,
-                                                  workspace,
-                                                  oss_fuzz_project_name,
-                                                  sanitizer=sanitizer)
+  run_status, bug_found = run_fuzzers.run_fuzzers(config)
   if not run_status:
-    logging.error('Error occurred while running in workspace %s.', workspace)
+    logging.error('Error occurred while running in workspace %s.',
+                  config.workspace)
     return returncode
   if bug_found:
     logging.info('Bug found.')
-    if not dry_run:
+    if not config.dry_run:
       # Return 2 when a bug was found by a fuzzer causing the CI to fail.
       return 2
   return 0
diff --git a/infra/cifuzz/run_fuzzers_test.py b/infra/cifuzz/run_fuzzers_test.py
index 9c33dde52..75b27b104 100644
--- a/infra/cifuzz/run_fuzzers_test.py
+++ b/infra/cifuzz/run_fuzzers_test.py
@@ -18,12 +18,14 @@ import tempfile
 import unittest
 from unittest import mock
 
+import config_utils
+import fuzz_target
+import run_fuzzers
+
 # pylint: disable=wrong-import-position
 INFRA_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.append(INFRA_DIR)
 
-import fuzz_target
-import run_fuzzers
 import test_helpers
 
 # NOTE: This integration test relies on
@@ -41,6 +43,22 @@ UNDEFINED_FUZZER_DIR = os.path.join(TEST_FILES_PATH, 'undefined')
 UNDEFINED_FUZZER = 'curl_fuzzer_undefined'
 
 
+def create_config(**kwargs):
+  """Creates a config object and then sets every attribute that is a key in
+  |kwargs| to the corresponding value. Asserts that each key in |kwargs| is an
+  attribute of Config."""
+  with mock.patch('os.path.basename', return_value=None), mock.patch(
+      'config_utils.get_project_src_path',
+      return_value=None), mock.patch('config_utils._is_dry_run',
+                                     return_value=True):
+    config = config_utils.RunFuzzersConfig()
+
+  for key, value in kwargs.items():
+    assert hasattr(config, key), 'Config doesn\'t have attribute: ' + key
+    setattr(config, key, value)
+  return config
+
+
 class RunFuzzerIntegrationTestMixin:  # pylint: disable=too-few-public-methods,invalid-name
   """Mixin for integration test classes that runbuild_fuzzers on builds of a
   specific sanitizer."""
@@ -52,10 +70,11 @@ class RunFuzzerIntegrationTestMixin:  # pylint: disable=too-few-public-methods,i
     """Calls run_fuzzers on fuzzer_dir and |sanitizer| and asserts
     the run succeeded and that no bug was found."""
     with test_helpers.temp_dir_copy(fuzzer_dir) as fuzzer_dir_copy:
-      run_success, bug_found = run_fuzzers.run_fuzzers(10,
-                                                       fuzzer_dir_copy,
-                                                       'curl',
-                                                       sanitizer=sanitizer)
+      config = create_config(fuzz_seconds=10,
+                             workspace=fuzzer_dir_copy,
+                             project_name='curl',
+                             sanitizer=sanitizer)
+      run_success, bug_found = run_fuzzers.run_fuzzers(config)
     self.assertTrue(run_success)
     self.assertFalse(bug_found)
 
@@ -100,8 +119,10 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
     with mock.patch.object(fuzz_target.FuzzTarget,
                            'is_reproducible',
                            side_effect=[True, False]):
-      run_success, bug_found = run_fuzzers.run_fuzzers(10, TEST_FILES_PATH,
-                                                       EXAMPLE_PROJECT)
+      config = create_config(fuzz_seconds=10,
+                             workspace=TEST_FILES_PATH,
+                             project_name=EXAMPLE_PROJECT)
+      run_success, bug_found = run_fuzzers.run_fuzzers(config)
       build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest')
       self.assertTrue(os.path.exists(build_dir))
       self.assertNotEqual(0, len(os.listdir(build_dir)))
@@ -112,11 +133,16 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
                    'INTEGRATION_TESTS=1 not set')
   def test_old_bug_found(self):
     """Tests run_fuzzers with a bug found in OSS-Fuzz before."""
+    config = create_config(fuzz_seconds=10,
+                           workspace=TEST_FILES_PATH,
+                           project_name=EXAMPLE_PROJECT)
     with mock.patch.object(fuzz_target.FuzzTarget,
                            'is_reproducible',
                            side_effect=[True, True]):
-      run_success, bug_found = run_fuzzers.run_fuzzers(10, TEST_FILES_PATH,
-                                                       EXAMPLE_PROJECT)
+      config = create_config(fuzz_seconds=10,
+                             workspace=TEST_FILES_PATH,
+                             project_name=EXAMPLE_PROJECT)
+      run_success, bug_found = run_fuzzers.run_fuzzers(config)
       build_dir = os.path.join(TEST_FILES_PATH, 'out', 'oss_fuzz_latest')
       self.assertTrue(os.path.exists(build_dir))
       self.assertNotEqual(0, len(os.listdir(build_dir)))
@@ -128,8 +154,10 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
     with tempfile.TemporaryDirectory() as tmp_dir:
       out_path = os.path.join(tmp_dir, 'out')
       os.mkdir(out_path)
-      run_success, bug_found = run_fuzzers.run_fuzzers(10, tmp_dir,
-                                                       EXAMPLE_PROJECT)
+      config = create_config(fuzz_seconds=10,
+                             workspace=tmp_dir,
+                             project_name=EXAMPLE_PROJECT)
+      run_success, bug_found = run_fuzzers.run_fuzzers(config)
     self.assertFalse(run_success)
     self.assertFalse(bug_found)
 
@@ -138,15 +166,19 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
     with tempfile.TemporaryDirectory() as tmp_dir:
       out_path = os.path.join(tmp_dir, 'out')
       os.mkdir(out_path)
-      run_success, bug_found = run_fuzzers.run_fuzzers(0, tmp_dir,
-                                                       EXAMPLE_PROJECT)
+      config = create_config(fuzz_seconds=0,
+                             workspace=tmp_dir,
+                             project_name=EXAMPLE_PROJECT)
+      run_success, bug_found = run_fuzzers.run_fuzzers(config)
     self.assertFalse(run_success)
     self.assertFalse(bug_found)
 
   def test_invalid_out_dir(self):
     """Tests run_fuzzers with an invalid out directory."""
-    run_success, bug_found = run_fuzzers.run_fuzzers(10, 'not/a/valid/path',
-                                                     EXAMPLE_PROJECT)
+    config = create_config(fuzz_seconds=10,
+                           workspace='not/a/valid/path',
+                           project_name=EXAMPLE_PROJECT)
+    run_success, bug_found = run_fuzzers.run_fuzzers(config)
     self.assertFalse(run_success)
     self.assertFalse(bug_found)