Rooba
/
oss-fuzz
mirror of https://github.com/google/oss-fuzz.git
1
1
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[CIFuzz] Refactor in preparation for filestore (#5841) 

1. Moving the _create_config and create_config functions from build_fuzzers_test.py and run_fuzzers_test.py into test_helpers.py (which is now part of cifuzz instead of infra) and share code between them.
2. Rename artifacts_dir to crashes_dir in run_fuzzers.py. "artifacts" is ambiguous.
3. Make some small changes to pytest.ini to improve debugging.
This commit is contained in:
jonathanmetzman 2021-05-26 09:45:22 -07:00 committed by GitHub
parent 8d313798ab
commit 8c4ad095e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 146 additions and 153 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
infra/cifuzz/actions/run_fuzzers/action.yml
View File

@ -30,6 +30,14 @@ inputs:
"batch" is in alpha and should not be used in production.
required: false
default: 'ci'
github-token:
description: |
Token for GitHub API. WARNING: THIS SHOULD NOT BE USED IN PRODUCTION YET
You should use "secrets.GITHUB_TOKEN" in your workflow file, do not
hardcode the token.
TODO(https://github.com/google/oss-fuzz/pull/5841#discussion_r639393361):
Document locking this down.
required: false
runs:
using: 'docker'
image: '../../../run_fuzzers.Dockerfile'
@ -44,4 +52,5 @@ runs:
# for running because we use it to distinguish OSS-Fuzz from non-OSS-Fuzz.
# We should do something explicit instead.
BUILD_INTEGRATION_PATH: ${{ inputs.build-integration-path }}
GITHUB_TOKEN: ${{ inputs.github-token }}
LOW_DISK_SPACE: 'True'

44
infra/cifuzz/build_fuzzers_test.py
View File

@ -28,7 +28,6 @@ sys.path.append(INFRA_DIR)
OSS_FUZZ_DIR = os.path.dirname(INFRA_DIR)
import build_fuzzers
import config_utils
import continuous_integration
import repo_manager
import test_helpers
@ -57,22 +56,6 @@ EXAMPLE_BUILD_FUZZER = 'do_stuff_fuzzer'
# pylint: disable=no-self-use,protected-access,too-few-public-methods
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.BuildFuzzersConfig()
for key, value in kwargs.items():
assert hasattr(config, key), 'Config doesn\'t have attribute: ' + key
setattr(config, key, value)
return config
class BuildFuzzersTest(unittest.TestCase):
"""Unit tests for build_fuzzers."""
@ -86,7 +69,7 @@ class BuildFuzzersTest(unittest.TestCase):
with tempfile.TemporaryDirectory() as tmp_dir:
build_fuzzers.build_fuzzers(
create_config(project_name=EXAMPLE_PROJECT,
test_helpers.create_build_config(project_name=EXAMPLE_PROJECT,
project_repo_name=EXAMPLE_PROJECT,
workspace=tmp_dir,
pr_ref='refs/pull/1757/merge'))
@ -114,7 +97,8 @@ class InternalGithubBuildTest(unittest.TestCase):
def _create_builder(self, tmp_dir):
"""Creates an InternalGithubBuilder and returns it."""
config = create_config(project_name=self.PROJECT_NAME,
config = test_helpers.create_build_config(
project_name=self.PROJECT_NAME,
project_repo_name=self.PROJECT_REPO_NAME,
workspace=tmp_dir,
sanitizer=self.SANITIZER,
@ -164,7 +148,8 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
git_url = 'https://github.com/jonathanmetzman/cifuzz-external-example.git'
# This test is dependant on the state of
# github.com/jonathanmetzman/cifuzz-external-example.
config = create_config(project_name=project_name,
config = test_helpers.create_build_config(
project_name=project_name,
project_repo_name=project_name,
workspace=self.workspace,
build_integration_path=build_integration_path,
@ -187,7 +172,8 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
'https://github.com/jonathanmetzman/cifuzz-external-example',
self.tmp_dir_obj.name)
project_src_path = manager.repo_dir
config = create_config(project_name=project_name,
config = test_helpers.create_build_config(
project_name=project_name,
project_repo_name=project_name,
workspace=self.workspace,
build_integration_path=build_integration_path,
@ -201,7 +187,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_valid_commit(self):
"""Tests building fuzzers with valid inputs."""
config = create_config(
config = test_helpers.create_build_config(
project_name=EXAMPLE_PROJECT,
project_repo_name='oss-fuzz',
workspace=self.workspace,
@ -215,7 +201,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_valid_pull_request(self):
"""Tests building fuzzers with valid pull request."""
# TODO(metzman): What happens when this branch closes?
config = create_config(project_name=EXAMPLE_PROJECT,
config = test_helpers.create_build_config(project_name=EXAMPLE_PROJECT,
project_repo_name='oss-fuzz',
workspace=self.workspace,
pr_ref='refs/pull/1757/merge',
@ -227,7 +213,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_invalid_pull_request(self):
"""Tests building fuzzers with invalid pull request."""
config = create_config(project_name=EXAMPLE_PROJECT,
config = test_helpers.create_build_config(project_name=EXAMPLE_PROJECT,
project_repo_name='oss-fuzz',
workspace=self.workspace,
pr_ref='ref-1/merge',
@ -237,7 +223,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_invalid_project_name(self):
"""Tests building fuzzers with invalid project name."""
config = create_config(
config = test_helpers.create_build_config(
project_name='not_a_valid_project',
project_repo_name='oss-fuzz',
workspace=self.workspace,
@ -246,7 +232,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_invalid_repo_name(self):
"""Tests building fuzzers with invalid repo name."""
config = create_config(
config = test_helpers.create_build_config(
project_name=EXAMPLE_PROJECT,
project_repo_name='not-real-repo',
workspace=self.workspace,
@ -255,7 +241,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_invalid_commit_sha(self):
"""Tests building fuzzers with invalid commit SHA."""
config = create_config(project_name=EXAMPLE_PROJECT,
config = test_helpers.create_build_config(project_name=EXAMPLE_PROJECT,
project_repo_name='oss-fuzz',
workspace=self.workspace,
commit_sha='',
@ -265,7 +251,7 @@ class BuildFuzzersIntegrationTest(unittest.TestCase):
def test_invalid_workspace(self):
"""Tests building fuzzers with invalid workspace."""
config = create_config(
config = test_helpers.create_build_config(
project_name=EXAMPLE_PROJECT,
project_repo_name='oss-fuzz',
workspace=os.path.join(self.workspace, 'not', 'a', 'dir'),
@ -329,7 +315,7 @@ class BuildSantizerIntegrationTest(unittest.TestCase):
@classmethod
def _create_config(cls, tmp_dir, sanitizer):
return create_config(project_name=cls.PROJECT_NAME,
return test_helpers.create_build_config(project_name=cls.PROJECT_NAME,
project_repo_name=cls.PROJECT_NAME,
workspace=tmp_dir,
pr_ref=cls.PR_REF,

13
infra/cifuzz/clusterfuzz_deployment_test.py
View File

@ -21,7 +21,7 @@ import urllib.error
from pyfakefs import fake_filesystem_unittest
import clusterfuzz_deployment
import config_utils
import test_helpers
# NOTE: This integration test relies on
# https://github.com/google/oss-fuzz/tree/master/projects/example project.
@ -40,16 +40,7 @@ def _create_config(**kwargs):
if default_key not in kwargs:
kwargs[default_key] = default_value
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
return test_helpers.create_run_config(**kwargs)
def _create_deployment(**kwargs):

11
infra/cifuzz/config_utils.py
View File

@ -21,8 +21,10 @@ import json
import environment
def _get_project_repo_name():
return os.path.basename(environment.get('GITHUB_REPOSITORY', ''))
def _get_project_repo_owner_and_name():
# Includes owner and repo name.
github_repository = os.getenv('GITHUB_REPOSITORY', '')
return os.path.split(github_repository)
def _get_pr_ref(event):
@ -93,6 +95,8 @@ class BaseConfig:
def __init__(self):
self.workspace = os.getenv('GITHUB_WORKSPACE')
self.project_name = _get_project_name()
self.project_repo_owner, self.project_repo_name = (
_get_project_repo_owner_and_name())
# Check if failures should not be reported.
self.dry_run = _is_dry_run()
self.sanitizer = _get_sanitizer()
@ -104,6 +108,8 @@ class BaseConfig:
# TODO(metzman): Parse env like we do in ClusterFuzz.
self.low_disk_space = environment.get('LOW_DISK_SPACE', False)
self.github_token = os.environ.get('GITHUB_TOKEN')
@property
def is_internal(self):
"""Returns True if this is an OSS-Fuzz project."""
@ -162,7 +168,6 @@ class BuildFuzzersConfig(BaseConfig):
# TODO(metzman): Some of this config is very CI-specific. Move it into the
# CI class.
super().__init__()
self.project_repo_name = _get_project_repo_name()
self.commit_sha = os.getenv('GITHUB_SHA')
event = os.getenv('GITHUB_EVENT_NAME')

13
infra/cifuzz/fuzz_target_test.py
View File

@ -22,8 +22,8 @@ import parameterized
from pyfakefs import fake_filesystem_unittest
import clusterfuzz_deployment
import config_utils
import fuzz_target
import test_helpers
# NOTE: This integration test relies on
# https://github.com/google/oss-fuzz/tree/master/projects/example project.
@ -48,16 +48,7 @@ def _create_config(**kwargs):
if default_key not in kwargs:
kwargs[default_key] = default_value
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
return test_helpers.create_run_config(**kwargs)
def _create_deployment(**kwargs):

15
infra/cifuzz/run_fuzzers.py
View File

@ -46,7 +46,7 @@ class BaseFuzzTargetRunner:
# Set by the initialize method.
self.out_dir = None
self.fuzz_target_paths = None
self.artifacts_dir = None
self.crashes_dir = None
def initialize(self):
"""Initialization method. Must be called before calling run_fuzz_targets.
@ -69,13 +69,12 @@ class BaseFuzzTargetRunner:
logging.error('Out directory: %s does not exist.', self.out_dir)
return False
self.artifacts_dir = os.path.join(self.out_dir, 'artifacts')
if not os.path.exists(self.artifacts_dir):
os.mkdir(self.artifacts_dir)
elif (not os.path.isdir(self.artifacts_dir) or
os.listdir(self.artifacts_dir)):
self.crashes_dir = os.path.join(self.out_dir, 'artifacts')
if not os.path.exists(self.crashes_dir):
os.mkdir(self.crashes_dir)
elif (not os.path.isdir(self.crashes_dir) or os.listdir(self.crashes_dir)):
logging.error('Artifacts path: %s exists and is not an empty directory.',
self.artifacts_dir)
self.crashes_dir)
return False
self.fuzz_target_paths = utils.get_fuzz_targets(self.out_dir)
@ -108,7 +107,7 @@ class BaseFuzzTargetRunner:
target_name=target.target_name,
sanitizer=self.config.sanitizer,
artifact_name=artifact_name)
return os.path.join(self.artifacts_dir, artifact_name)
return os.path.join(self.crashes_dir, artifact_name)
def create_fuzz_target_obj(self, target_path, run_seconds):
"""Returns a fuzz target object."""

43
infra/cifuzz/run_fuzzers_test.py
View File

@ -22,7 +22,6 @@ from unittest import mock
import parameterized
from pyfakefs import fake_filesystem_unittest
import config_utils
import fuzz_target
import run_fuzzers
@ -49,22 +48,6 @@ UNDEFINED_FUZZER = 'curl_fuzzer_undefined'
FUZZ_SECONDS = 10
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."""
@ -76,7 +59,7 @@ 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:
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS,
workspace=fuzzer_dir_copy,
project_name='curl',
sanitizer=sanitizer)
@ -119,7 +102,7 @@ class BaseFuzzTargetRunnerTest(unittest.TestCase):
if default_key not in kwargs:
kwargs[default_key] = default_value
config = _create_config(**kwargs)
config = test_helpers.create_run_config(**kwargs)
return run_fuzzers.BaseFuzzTargetRunner(config)
def _test_initialize_fail(self, expected_error_args, **create_runner_kwargs):
@ -219,8 +202,8 @@ class BaseFuzzTargetRunnerTest(unittest.TestCase):
def test_get_fuzz_target_artifact(self):
"""Tests that get_fuzz_target_artifact works as intended."""
runner = self._create_runner()
artifacts_dir = 'artifacts-dir'
runner.artifacts_dir = artifacts_dir
crashes_dir = 'crashes-dir'
runner.crashes_dir = crashes_dir
artifact_name = 'artifact-name'
target = mock.MagicMock()
target_name = 'target_name'
@ -228,7 +211,7 @@ class BaseFuzzTargetRunnerTest(unittest.TestCase):
fuzz_target_artifact = runner.get_fuzz_target_artifact(
target, artifact_name)
expected_fuzz_target_artifact = (
'artifacts-dir/target_name-address-artifact-name')
'crashes-dir/target_name-address-artifact-name')
self.assertEqual(fuzz_target_artifact, expected_fuzz_target_artifact)
@ -248,7 +231,7 @@ class CiFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
workspace = 'workspace'
out_path = os.path.join(workspace, 'out')
self.fs.create_dir(out_path)
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS,
workspace=workspace,
project_name=EXAMPLE_PROJECT)
runner = run_fuzzers.CiFuzzTargetRunner(config)
@ -264,7 +247,7 @@ class CiFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
magic_mock.target_name = 'target1'
mocked_create_fuzz_target_obj.return_value = magic_mock
self.assertTrue(runner.run_fuzz_targets())
self.assertIn('target1-address-testcase', os.listdir(runner.artifacts_dir))
self.assertIn('target1-address-testcase', os.listdir(runner.crashes_dir))
self.assertEqual(mocked_run_fuzz_target.call_count, 1)
@ -284,7 +267,7 @@ class BatchFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
workspace = 'workspace'
out_path = os.path.join(workspace, 'out')
self.fs.create_dir(out_path)
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS,
workspace=workspace,
project_name=EXAMPLE_PROJECT)
runner = run_fuzzers.BatchFuzzTargetRunner(config)
@ -314,7 +297,7 @@ class BatchFuzzTargetRunnerTest(fake_filesystem_unittest.TestCase):
mocked_create_fuzz_target_obj.return_value = magic_mock
self.assertTrue(runner.run_fuzz_targets())
self.assertIn('target1-address-testcase-aaa',
os.listdir(runner.artifacts_dir))
os.listdir(runner.crashes_dir))
self.assertEqual(mocked_run_fuzz_target.call_count, 2)
@ -336,7 +319,7 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
with tempfile.TemporaryDirectory() as tmp_dir:
workspace = os.path.join(tmp_dir, 'workspace')
shutil.copytree(TEST_DATA_PATH, workspace)
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS,
workspace=workspace,
project_name=EXAMPLE_PROJECT)
result = run_fuzzers.run_fuzzers(config)
@ -350,13 +333,13 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
side_effect=[True, True])
def test_old_bug_found(self, _):
"""Tests run_fuzzers with a bug found in OSS-Fuzz before."""
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS,
workspace=TEST_DATA_PATH,
project_name=EXAMPLE_PROJECT)
with tempfile.TemporaryDirectory() as tmp_dir:
workspace = os.path.join(tmp_dir, 'workspace')
shutil.copytree(TEST_DATA_PATH, workspace)
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS,
workspace=TEST_DATA_PATH,
project_name=EXAMPLE_PROJECT)
result = run_fuzzers.run_fuzzers(config)
@ -370,7 +353,7 @@ class RunAddressFuzzersIntegrationTest(RunFuzzerIntegrationTestMixin,
with tempfile.TemporaryDirectory() as tmp_dir:
out_path = os.path.join(tmp_dir, 'out')
os.mkdir(out_path)
config = _create_config(fuzz_seconds=FUZZ_SECONDS,
config = test_helpers.create_run_config(fuzz_seconds=FUZZ_SECONDS,
workspace=tmp_dir,
project_name=EXAMPLE_PROJECT)
result = run_fuzzers.run_fuzzers(config)

28
infra/test_helpers.py → infra/cifuzz/test_helpers.py
View File

@ -19,6 +19,34 @@ import shutil
import tempfile
from unittest import mock
import config_utils
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):
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
def create_build_config(**kwargs):
"""Wrapper around _create_config for build configs."""
return _create_config(config_utils.BuildFuzzersConfig, **kwargs)
def create_run_config(**kwargs):
"""Wrapper around _create_config for run configs."""
return _create_config(config_utils.RunFuzzersConfig, **kwargs)
def patch_environ(testcase_obj, env=None):
"""Patch environment."""

1
infra/pytest.ini
View File

@ -1,2 +1,3 @@
[pytest]
python_files = *_test.py
log_cli = true

2
infra/retry.py
View File

@ -56,7 +56,7 @@ def wrap(retries,
"""Handle retry."""
if (exception is None or
isinstance(exception, exception_type)) and num_try < tries:
logging.log('Retrying on %s failed with %s. Retrying again.',
logging.info('Retrying on %s failed with %s. Retrying again.',
function_with_type,
sys.exc_info()[1])
sleep(get_delay(num_try, delay, backoff))