From fde0a30492ad420ca81dc2ac4282075c5d93bfa5 Mon Sep 17 00:00:00 2001 From: jonathanmetzman <31354670+jonathanmetzman@users.noreply.github.com> Date: Wed, 21 Jul 2021 09:19:47 -0700 Subject: [PATCH] [CIFuzz] Support uploading coverage reports (#6078) --- infra/cifuzz/clusterfuzz_deployment.py | 8 +-- .../filestore/github_actions/__init__.py | 55 +++++++++++++------ infra/cifuzz/generate_coverage_report.py | 2 +- infra/cifuzz/run_fuzzers_test.py | 10 +++- 4 files changed, 51 insertions(+), 24 deletions(-) diff --git a/infra/cifuzz/clusterfuzz_deployment.py b/infra/cifuzz/clusterfuzz_deployment.py index 0e8f50873..c802f9060 100644 --- a/infra/cifuzz/clusterfuzz_deployment.py +++ b/infra/cifuzz/clusterfuzz_deployment.py @@ -178,10 +178,8 @@ class ClusterFuzzLite(BaseClusterFuzzDeployment): def upload_coverage(self): """Uploads the coverage report to the filestore.""" - # TODO(jonathanmetzman): Implement this. - raise NotImplementedError( - 'Not implemented yet. Waiting until we can specify a directory for ' - 'coverage report directories.') + self.filestore.upload_directory(self.COVERAGE_NAME, + self.workspace.coverage_report) def get_coverage(self, repo_path): """Returns the project coverage object for the project.""" @@ -314,7 +312,7 @@ class NoClusterFuzzDeployment(BaseClusterFuzzDeployment): """Noop Implementation of upload_crashes.""" logging.info('Not uploading crashes because no ClusterFuzz deployment.') - def download_corpus(self, target_name): # pylint: disable=no-self-use,unused-argument + def download_corpus(self, target_name): """Noop Implementation of download_corpus.""" logging.info('Not downloading corpus because no ClusterFuzz deployment.') return self.make_empty_corpus_dir(target_name) diff --git a/infra/cifuzz/filestore/github_actions/__init__.py b/infra/cifuzz/filestore/github_actions/__init__.py index 56d6c2b00..64be407f2 100644 --- a/infra/cifuzz/filestore/github_actions/__init__.py +++ b/infra/cifuzz/filestore/github_actions/__init__.py @@ -12,8 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. """Implementation of a filestore using Github actions artifacts.""" -import os import logging +import os +import shutil +import tarfile +import tempfile import http_utils import filestore @@ -21,6 +24,21 @@ from filestore.github_actions import github_api from third_party.github_actions_toolkit.artifact import artifact_client +def tar_directory(directory, archive_path): + """Tars a |directory| and stores archive at |archive_path|. |archive_path| + must end in .tar""" + assert archive_path.endswith('.tar') + # Do this because make_archive will append the extension to archive_path. + archive_path = os.path.splitext('.tar')[0] + + parent_directory = os.path.dirname(os.path.abspath(directory)) + basename = os.path.basename(directory) + shutil.make_archive(archive_path, + 'tar', + root_dir=parent_directory, + base_dir=basename) + + class GithubActionsFilestore(filestore.BaseFilestore): """Implementation of BaseFilestore using Github actions artifacts. Relies on github_actions_toolkit for using the GitHub actions API and the github_api @@ -35,20 +53,12 @@ class GithubActionsFilestore(filestore.BaseFilestore): def upload_directory(self, name, directory): # pylint: disable=no-self-use """Uploads |directory| as artifact with |name|.""" - directory = os.path.abspath(directory) + with tempfile.TemporaryDirectory() as temp_dir: + archive_path = os.path.join(temp_dir, name + '.tar') + tar_directory(directory, archive_path) + file_paths = [archive_path] - # Get file paths. - file_paths = [] - for root, _, curr_file_paths in os.walk(directory): - for file_path in curr_file_paths: - file_paths.append(os.path.join(root, file_path)) - - logging.debug('file_paths: %s', file_paths) - - # TODO(metzman): Zip so that we can upload directories within directories - # and save time? - - return artifact_client.upload_artifact(name, file_paths, directory) + return artifact_client.upload_artifact(name, file_paths, temp_dir) def download_corpus(self, name, dst_directory): # pylint: disable=unused-argument,no-self-use """Downloads the corpus located at |name| to |dst_directory|.""" @@ -69,8 +79,21 @@ class GithubActionsFilestore(filestore.BaseFilestore): logging.warning('Could not download artifact: %s.', name) return artifact download_url = artifact['archive_download_url'] - return http_utils.download_and_unpack_zip( - download_url, dst_directory, headers=self.github_api_http_headers) + with tempfile.TemporaryDirectory() as temp_dir: + if not http_utils.download_and_unpack_zip( + download_url, temp_dir, headers=self.github_api_http_headers): + return False + + artifact_tarfile_path = os.path.join(temp_dir, name + '.tar') + if not os.path.exists(artifact_tarfile_path): + logging.error('Artifact zip did not contain a tarfile.') + return False + + # TODO(jonathanmetzman): Replace this with archive.unpack from + # libClusterFuzz so we can avoid path traversal issues. + with tarfile.TarFile(artifact_tarfile_path) as artifact_tarfile: + artifact_tarfile.extractall(dst_directory) + return True def _list_artifacts(self): """Returns a list of artifacts.""" diff --git a/infra/cifuzz/generate_coverage_report.py b/infra/cifuzz/generate_coverage_report.py index 6c90ffada..7e427ab6d 100644 --- a/infra/cifuzz/generate_coverage_report.py +++ b/infra/cifuzz/generate_coverage_report.py @@ -44,4 +44,4 @@ def generate_coverage_report(fuzz_target_paths, workspace, """Generates a coverage report using Clang's source based coverage.""" download_corpora(fuzz_target_paths, clusterfuzz_deployment) run_coverage_command(workspace, config) - # TODO(metzman): Upload this build to the filestore. + clusterfuzz_deployment.upload_coverage() diff --git a/infra/cifuzz/run_fuzzers_test.py b/infra/cifuzz/run_fuzzers_test.py index b29c6b1a3..ddbaf4f9b 100644 --- a/infra/cifuzz/run_fuzzers_test.py +++ b/infra/cifuzz/run_fuzzers_test.py @@ -344,7 +344,10 @@ class CoverageReportIntegrationTest(unittest.TestCase): def setUp(self): test_helpers.patch_environ(self) - def test_coverage_report(self): + @mock.patch('third_party.github_actions_toolkit.artifact.artifact_client' + '.upload_artifact', + return_value=True) + def test_coverage_report(self, _): """Tests generation of coverage reports end-to-end, from building to generation.""" @@ -367,7 +370,10 @@ class CoverageReportIntegrationTest(unittest.TestCase): workspace=workspace, oss_fuzz_project_name=EXAMPLE_PROJECT, sanitizer=self.SANITIZER, - run_fuzzers_mode='coverage') + run_fuzzers_mode='coverage', + is_github=True, + # Set build integration path so it's not internal. + build_integration_path='/') result = run_fuzzers.run_fuzzers(run_config) self.assertEqual(result, run_fuzzers.RunFuzzersResult.NO_BUG_FOUND) expected_summary_path = os.path.join(