From b2fa3547b235be1fceaa6629f2d68b4d91faecb8 Mon Sep 17 00:00:00 2001 From: jonathanmetzman <31354670+jonathanmetzman@users.noreply.github.com> Date: Thu, 28 Oct 2021 15:00:44 -0400 Subject: [PATCH] [clusterfuzzlite] Fixes for gsutil (#6683) 1 Fix usage of gsutil tool. 2 Get rid of run_fuzzers_mode and change to mode. Fixes: #6677 3 Install requirements before copying source code to make iterative development of cifuzz code faster. --- infra/cifuzz/actions/run_fuzzers/action.yml | 2 +- infra/cifuzz/cifuzz-base/Dockerfile | 6 ++++- infra/cifuzz/clusterfuzz_deployment_test.py | 2 +- infra/cifuzz/config_utils.py | 14 +++++------ infra/cifuzz/config_utils_test.py | 24 +++++++++---------- .../external-actions/run_fuzzers/action.yml | 2 +- infra/cifuzz/filestore/gsutil/__init__.py | 11 +++++---- infra/cifuzz/run_fuzzers.py | 9 ++++--- infra/cifuzz/run_fuzzers_test.py | 9 ++++--- 9 files changed, 41 insertions(+), 38 deletions(-) diff --git a/infra/cifuzz/actions/run_fuzzers/action.yml b/infra/cifuzz/actions/run_fuzzers/action.yml index deba50cb7..59309fd2d 100644 --- a/infra/cifuzz/actions/run_fuzzers/action.yml +++ b/infra/cifuzz/actions/run_fuzzers/action.yml @@ -49,7 +49,7 @@ runs: FUZZ_SECONDS: ${{ inputs.fuzz-seconds }} DRY_RUN: ${{ inputs.dry-run}} SANITIZER: ${{ inputs.sanitizer }} - RUN_FUZZERS_MODE: ${{ inputs.mode }} + MODE: ${{ inputs.mode }} GITHUB_TOKEN: ${{ inputs.github-token }} LOW_DISK_SPACE: 'True' REPORT_UNREPRODUCIBLE_CRASHES: ${{ inputs.report-unreproducible-crashes }} diff --git a/infra/cifuzz/cifuzz-base/Dockerfile b/infra/cifuzz/cifuzz-base/Dockerfile index a4773c1e1..66fd6b517 100644 --- a/infra/cifuzz/cifuzz-base/Dockerfile +++ b/infra/cifuzz/cifuzz-base/Dockerfile @@ -32,8 +32,12 @@ RUN apt-get update && \ ENV PATH=/opt/gcloud/google-cloud-sdk/bin/:$PATH ENV OSS_FUZZ_ROOT=/opt/oss-fuzz + +# Do this step before copying to make rebuilding faster when developing. +COPY ./infra/cifuzz/requirements.txt /tmp/requirements.txt +RUN python3 -m pip install -r /tmp/requirements.txt && rm /tmp/requirements.txt + ADD . ${OSS_FUZZ_ROOT} -RUN python3 -m pip install -r ${OSS_FUZZ_ROOT}/infra/cifuzz/requirements.txt RUN npm install ${OSS_FUZZ_ROOT}/infra/cifuzz # Python file to execute when the docker container starts up. diff --git a/infra/cifuzz/clusterfuzz_deployment_test.py b/infra/cifuzz/clusterfuzz_deployment_test.py index 9d74fbd57..e0e94095b 100644 --- a/infra/cifuzz/clusterfuzz_deployment_test.py +++ b/infra/cifuzz/clusterfuzz_deployment_test.py @@ -130,7 +130,7 @@ class ClusterFuzzLiteTest(fake_filesystem_unittest.TestCase): def setUp(self): self.setUpPyfakefs() - self.deployment = _create_deployment(run_fuzzers_mode='batch', + self.deployment = _create_deployment(mode='batch', oss_fuzz_project_name='', cloud_bucket='gs://bucket', is_github=True) diff --git a/infra/cifuzz/config_utils.py b/infra/cifuzz/config_utils.py index 4dd44a407..e2ce476b1 100644 --- a/infra/cifuzz/config_utils.py +++ b/infra/cifuzz/config_utils.py @@ -26,7 +26,6 @@ sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) import constants -RUN_FUZZERS_MODES = ['batch', 'code-change', 'coverage', 'prune'] SANITIZERS = ['address', 'memory', 'undefined', 'coverage'] # TODO(metzman): Set these on config objects so there's one source of truth. @@ -312,14 +311,15 @@ def _get_ci_environment(platform): class RunFuzzersConfig(BaseConfig): """Class containing constant configuration for running fuzzers in CIFuzz.""" + MODES = ['batch', 'code-change', 'coverage', 'prune'] + def __init__(self): super().__init__() # TODO(metzman): Pick a better default for pruning. self.fuzz_seconds = int(os.environ.get('FUZZ_SECONDS', 600)) - self.run_fuzzers_mode = os.environ.get('RUN_FUZZERS_MODE', - 'code-change').lower() + self.mode = os.environ.get('MODE', 'code-change').lower() if self.is_coverage: - self.run_fuzzers_mode = 'coverage' + self.mode = 'coverage' self.report_unreproducible_crashes = environment.get_bool( 'REPORT_UNREPRODUCIBLE_CRASHES', False) @@ -333,9 +333,9 @@ class RunFuzzersConfig(BaseConfig): """Do extra validation on RunFuzzersConfig.__init__(). Do not name this validate or else it will be called when using the parent's __init__ and will fail. Returns True if valid.""" - if self.run_fuzzers_mode not in RUN_FUZZERS_MODES: - logging.error('Invalid RUN_FUZZERS_MODE: %s. Must be one of %s.', - self.run_fuzzers_mode, RUN_FUZZERS_MODES) + if self.mode not in self.MODES: + logging.error('Invalid MODE: %s. Must be one of %s.', self.mode, + self.MODES) return False return True diff --git a/infra/cifuzz/config_utils_test.py b/infra/cifuzz/config_utils_test.py index 22d42d0bc..9d6fc8f30 100644 --- a/infra/cifuzz/config_utils_test.py +++ b/infra/cifuzz/config_utils_test.py @@ -135,20 +135,19 @@ class RunFuzzersConfigTest(unittest.TestCase): return config_utils.RunFuzzersConfig() def test_coverage(self): - """Tests that run_fuzzers_mode is overriden properly based on - is_coverage.""" + """Tests that mode is overriden properly based on is_coverage.""" # Test that it is overriden when it is supposed to be. os.environ['SANITIZER'] = 'coverage' - os.environ['RUN_FUZZERS_MODE'] = 'code-change' + os.environ['MODE'] = 'code-change' config = self._create_config() - self.assertEqual(config.run_fuzzers_mode, 'coverage') + self.assertEqual(config.mode, 'coverage') # Test that it isn't overriden when it isn't supposed to be. os.environ['SANITIZER'] = 'address' - run_fuzzers_mode = 'code-change' - os.environ['RUN_FUZZERS_MODE'] = run_fuzzers_mode + mode = 'code-change' + os.environ['MODE'] = mode config = self._create_config() - self.assertEqual(config.run_fuzzers_mode, run_fuzzers_mode) + self.assertEqual(config.mode, mode) def test_run_config_validate(self): """Tests that _run_config_validate returns True when the config is valid.""" @@ -156,14 +155,13 @@ class RunFuzzersConfigTest(unittest.TestCase): @mock.patch('logging.error') def test_run_config_invalid_mode(self, mock_error): - """Tests that _run_config_validate returns False when run_fuzzers_mode is - invalid.""" + """Tests that _run_config_validate returns False when mode is invalid.""" fake_mode = 'fake-mode' - os.environ['RUN_FUZZERS_MODE'] = fake_mode + os.environ['MODE'] = fake_mode self.assertFalse(self._create_config()._run_config_validate()) - mock_error.assert_called_with( - 'Invalid RUN_FUZZERS_MODE: %s. Must be one of %s.', fake_mode, - config_utils.RUN_FUZZERS_MODES) + mock_error.assert_called_with('Invalid MODE: %s. Must be one of %s.', + fake_mode, + config_utils.RunFuzzersConfig.MODES) class GetProjectRepoOwnerAndNameTest(unittest.TestCase): diff --git a/infra/cifuzz/external-actions/run_fuzzers/action.yml b/infra/cifuzz/external-actions/run_fuzzers/action.yml index 14ff0728d..eecd627ff 100644 --- a/infra/cifuzz/external-actions/run_fuzzers/action.yml +++ b/infra/cifuzz/external-actions/run_fuzzers/action.yml @@ -61,7 +61,7 @@ runs: FUZZ_SECONDS: ${{ inputs.fuzz-seconds }} DRY_RUN: ${{ inputs.dry-run}} SANITIZER: ${{ inputs.sanitizer }} - RUN_FUZZERS_MODE: ${{ inputs.mode }} + MODE: ${{ inputs.mode }} GITHUB_TOKEN: ${{ inputs.github-token }} LOW_DISK_SPACE: 'True' GIT_STORE_REPO: ${{ inputs.storage-repo }} diff --git a/infra/cifuzz/filestore/gsutil/__init__.py b/infra/cifuzz/filestore/gsutil/__init__.py index a52e91870..85f85508d 100644 --- a/infra/cifuzz/filestore/gsutil/__init__.py +++ b/infra/cifuzz/filestore/gsutil/__init__.py @@ -37,11 +37,14 @@ def _gsutil_execute(*args, parallel=True): return utils.execute(command, check_result=True) -def _rsync(src, dst, delete=False): +def _rsync(src, dst, recursive=True, delete=False): """Executes gsutil rsync on |src| and |dst|""" - args = ['rsync', src, dst] + args = ['rsync'] + if recursive: + args.append('-r') if delete: - args.append('--delete') + args.append('-d') + args += [src, dst] return _gsutil_execute(*args) @@ -75,7 +78,7 @@ class GSUtilFilestore(filestore.BaseFilestore): # 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.') + logging.info('Uploading crashes to %s.', gsutil_url) return _rsync(directory, gsutil_url) def upload_corpus(self, name, directory, replace=False): diff --git a/infra/cifuzz/run_fuzzers.py b/infra/cifuzz/run_fuzzers.py index 968fca645..aa0222adc 100644 --- a/infra/cifuzz/run_fuzzers.py +++ b/infra/cifuzz/run_fuzzers.py @@ -241,7 +241,7 @@ class BatchFuzzTargetRunner(BaseFuzzTargetRunner): fuzz_target_obj.free_disk_if_needed(delete_fuzz_target=False) -_RUN_FUZZERS_MODE_RUNNER_MAPPING = { +_MODE_RUNNER_MAPPING = { 'batch': BatchFuzzTargetRunner, 'coverage': CoverageTargetRunner, 'prune': PruneTargetRunner, @@ -250,11 +250,10 @@ _RUN_FUZZERS_MODE_RUNNER_MAPPING = { def get_fuzz_target_runner(config): - """Returns a fuzz target runner object based on the run_fuzzers_mode of + """Returns a fuzz target runner object based on the mode of |config|.""" - runner = _RUN_FUZZERS_MODE_RUNNER_MAPPING[config.run_fuzzers_mode](config) - logging.info('RUN_FUZZERS_MODE is: %s. Runner: %s.', config.run_fuzzers_mode, - runner) + runner = _MODE_RUNNER_MAPPING[config.mode](config) + logging.info('run fuzzers MODE is: %s. Runner: %s.', config.mode, runner) return runner diff --git a/infra/cifuzz/run_fuzzers_test.py b/infra/cifuzz/run_fuzzers_test.py index 2dafa11a6..7ccc3ffbf 100644 --- a/infra/cifuzz/run_fuzzers_test.py +++ b/infra/cifuzz/run_fuzzers_test.py @@ -356,7 +356,7 @@ class CoverageReportIntegrationTest(unittest.TestCase): run_config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS, workspace=temp_dir, sanitizer=self.SANITIZER, - run_fuzzers_mode='coverage', + mode='coverage', is_github=True) result = run_fuzzers.run_fuzzers(run_config) self.assertEqual(result, run_fuzzers.RunFuzzersResult.NO_BUG_FOUND) @@ -431,16 +431,15 @@ class GetFuzzTargetRunnerTest(unittest.TestCase): ('code-change', run_fuzzers.CiFuzzTargetRunner), ('coverage', run_fuzzers.CoverageTargetRunner) ]) - def test_get_fuzz_target_runner(self, run_fuzzers_mode, - fuzz_target_runner_cls): + def test_get_fuzz_target_runner(self, mode, fuzz_target_runner_cls): """Tests that get_fuzz_target_runner returns the correct runner based on the - specified run_fuzzers_mode.""" + specified mode.""" with tempfile.TemporaryDirectory() as tmp_dir: run_config = test_helpers.create_run_config( fuzz_seconds=FUZZ_SECONDS, workspace=tmp_dir, oss_fuzz_project_name='example', - run_fuzzers_mode=run_fuzzers_mode) + mode=mode) runner = run_fuzzers.get_fuzz_target_runner(run_config) self.assertTrue(isinstance(runner, fuzz_target_runner_cls))