[cifuzz] Create validate method on BaseConfig (#6135)

* [cifuzz] Create validate method on BaseConfig

Use it to validate that either OSS_FUZZ_PROJECT_NAME
or BUILD_INTEGRATION_PATH is set.
Also use it to validate that workspace is set
(rather than duplicate code).
Add tests.

* Use env var hack to bypass valdiation

* fix

* fix

* fmt

* fix

* tmp

* fix

infra/cifuzz/build_fuzzers_entrypoint.py

@@ -37,10 +37,6 @@ def build_fuzzers_entrypoint():
     # The default return code when an error occurs.
     returncode = 1
 
-  if not config.workspace:
-    logging.error('This script needs to be run within Github actions.')
-    return returncode
-
   if not build_fuzzers.build_fuzzers(config):
     logging.error('Error building fuzzers for (commit: %s, pr_ref: %s).',
                   config.commit_sha, config.pr_ref)

infra/cifuzz/config_utils.py

@@ -88,6 +88,10 @@ def _get_language():
 # pylint: disable=too-few-public-methods,too-many-instance-attributes
 
 
+class ConfigurationError(Exception):
+  """Error for invalid configuration."""
+
+
 class BaseConfig:
   """Object containing constant configuration for CIFuzz."""
 
@@ -108,10 +112,8 @@ class BaseConfig:
     self.dry_run = _is_dry_run()
 
     self.sanitizer = _get_sanitizer()
-    # TODO(ochang): Error out if both oss_fuzz and build_integration_path is not
-    # set.
-    self.build_integration_path = os.getenv('BUILD_INTEGRATION_PATH')
 
+    self.build_integration_path = os.getenv('BUILD_INTEGRATION_PATH')
     self.language = _get_language()
     event_path = os.getenv('GITHUB_EVENT_PATH')
     self.is_github = bool(event_path)
@@ -124,6 +126,28 @@ class BaseConfig:
     self.git_store_branch_coverage = os.environ.get('GIT_STORE_BRANCH_COVERAGE',
                                                     self.git_store_branch)
 
+    # TODO(metzman): Fix tests to create valid configurations and get rid of
+    # CIFUZZ_TEST here and in presubmit.py.
+    if not self.validate() and not os.getenv('CIFUZZ_TEST'):
+      raise ConfigurationError('Invalid Configuration.')
+
+  def validate(self):
+    """Returns False if the configuration is invalid."""
+    # Do validation here so that unittests don't need to make a fully-valid
+    # config.
+    if (self.build_integration_path is None and
+        self.oss_fuzz_project_name is None):
+      logging.error('Must set OSS_FUZZ_PROJECT_NAME if OSS-Fuzz user. '
+                    'Otherwise must set BUILD_INTEGRATION_PATH. '
+                    'Neither is set.')
+      return False
+
+    if not self.workspace:
+      logging.error('Must set GITHUB_WORKSPACE.')
+      return False
+
+    return True
+
   @property
   def is_internal(self):
     """Returns True if this is an OSS-Fuzz project."""

infra/cifuzz/config_utils_test.py

@@ -14,6 +14,7 @@
 """Module for getting the configuration CIFuzz needs to run."""
 import os
 import unittest
+from unittest import mock
 
 import config_utils
 import test_helpers
@@ -56,6 +57,34 @@ class BaseConfigTest(unittest.TestCase):
     config = self._create_config()
     self.assertFalse(config.is_coverage)
 
+  @mock.patch('logging.error')
+  def test_validate_oss_fuzz_project_name_or_build_integration_path(
+      self, mocked_error):
+    """Tests that validate returns False if neither OSS_FUZZ_PROJECT_NAME or
+    BUILD_INTEGRATION_PATH is set."""
+    os.environ['GITHUB_WORKSPACE'] = '/workspace'
+    config = self._create_config()
+    self.assertFalse(config.validate())
+    mocked_error.assert_called_with(
+        'Must set OSS_FUZZ_PROJECT_NAME if OSS-Fuzz user. '
+        'Otherwise must set BUILD_INTEGRATION_PATH. '
+        'Neither is set.')
+
+  @mock.patch('logging.error')
+  def test_validate_no_workspace(self, mocked_error):
+    """Tests that validate returns False if GITHUB_WORKSPACE isn't set."""
+    os.environ['OSS_FUZZ_PROJECT_NAME'] = 'example'
+    config = self._create_config()
+    self.assertFalse(config.validate())
+    mocked_error.assert_called_with('Must set GITHUB_WORKSPACE.')
+
+  def test_validate(self):
+    """Tests that validate returns True if config is valid."""
+    os.environ['OSS_FUZZ_PROJECT_NAME'] = 'example'
+    os.environ['GITHUB_WORKSPACE'] = '/workspace'
+    config = self._create_config()
+    self.assertTrue(config.validate())
+
 
 class BuildFuzzersConfigTest(unittest.TestCase):
   """Tests for BuildFuzzersConfig."""

infra/cifuzz/docker_test.py

@@ -17,9 +17,11 @@ from unittest import mock
 
 import config_utils
 import docker
+import test_helpers
 
 CONTAINER_NAME = 'example-container'
-config = config_utils.RunFuzzersConfig()
+config = test_helpers.create_run_config(oss_fuzz_project_name='project',
+                                        workspace='/workspace')
 config.workspace = '/workspace'
 WORKSPACE = config_utils.Workspace(config)
 SANITIZER = 'example-sanitizer'

infra/cifuzz/run_fuzzers_entrypoint.py

@@ -52,10 +52,6 @@ def run_fuzzers_entrypoint():
     # Sets the default return code on error to success.
     returncode = 0
 
-  if not config.workspace:
-    logging.error('This script needs to be run within Github actions.')
-    return returncode
-
   delete_unneeded_docker_images(config)
   # Run the specified project's fuzzers from the build.
   result = run_fuzzers.run_fuzzers(config)

infra/cifuzz/run_fuzzers_test.py

@@ -368,7 +368,6 @@ class CoverageReportIntegrationTest(unittest.TestCase):
         run_config = test_helpers.create_run_config(
             fuzz_seconds=FUZZ_SECONDS,
             workspace=workspace,
-            oss_fuzz_project_name=EXAMPLE_PROJECT,
             sanitizer=self.SANITIZER,
             run_fuzzers_mode='coverage',
             is_github=True,

infra/cifuzz/test_helpers.py

@@ -22,19 +22,19 @@ from unittest import mock
 import config_utils
 
 
-def _create_config(config_cls, **kwargs):
+@mock.patch('config_utils._is_dry_run', return_value=True)
+@mock.patch('config_utils.get_project_src_path', return_value=None)
+@mock.patch('os.path.basename', return_value=None)
+def _create_config(config_cls, _, __, ___, **kwargs):
   """Creates a config object from |config_cls| 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):
+  with mock.patch('config_utils.BaseConfig.validate', return_value=True):
     config = config_cls()
-
   for key, value in kwargs.items():
     assert hasattr(config, key), 'Config doesn\'t have attribute: ' + key
     setattr(config, key, value)
+
   return config
 
 

infra/presubmit.py

@@ -403,7 +403,12 @@ def run_nonbuild_tests(parallel):
     command.extend(['-n', 'auto'])
   command += list(relevant_dirs)
   print('Running non-build tests.')
-  return subprocess.run(command, check=False).returncode == 0
+
+  # TODO(metzman): Get rid of this once config_utils stops using it.
+  env = os.environ.copy()
+  env['CIFUZZ_TEST'] = '1'
+
+  return subprocess.run(command, check=False, env=env).returncode == 0
 
 
 def run_tests(_=None, parallel=False, skip_build_tests=False):