Add initial support for the rules_fuzzing Bazel library. by stefanbucur · Pull Request #14834 · envoyproxy/envoy

stefanbucur · 2021-01-27T04:52:24Z

Commit Message: Introduce the new https://github.com/bazelbuild/rules_fuzzing library for fuzz testing support.

Additional Description: The new library provides new functionality (support for testing the fuzz test locally) and simplifies existing fuzzing tasks. This is the first PR in the series. See #14675 for the detailed rollout plan.

Risk Level: High (new dependencies and major change to the existing envoy_cc_fuzz_test macro)

Testing: The changes consist of either (a) code that should be a drop-in replacement to the existing functionality, or (b) new targets marked as "manual". For (a), the Envoy regression tests should catch deviations from the original behavior. The new functionality in (b) is either tested manually, or through the OSS-Fuzz integration (changes coming in future PR).

Docs Changes: None (this is test-only functionality).

Release Notes: N/A

Platform Specific Features: The fuzz tests run on Linux platforms only.

repokitteh-read-only · 2021-01-27T04:52:27Z

Hi @stefanbucur, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #14834 was opened by stefanbucur.

see: more, trace.

repokitteh-read-only · 2021-01-27T04:52:30Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #14834 was opened by stefanbucur.

see: more, trace.

Signed-off-by: Stefan Bucur <sbucur@google.com>

stefanbucur · 2021-01-27T04:56:08Z

@asraa As discussed in #14675, this is the first PR in the series. PTAL.

Signed-off-by: Stefan Bucur <sbucur@google.com>

asraa

Thanks! Just minor comments
/wait

bazel/envoy_test.bzl

.bazelrc

bazel/repository_locations.bzl

asraa · 2021-01-27T17:08:24Z

/assign htuch
For deps

Signed-off-by: Stefan Bucur <sbucur@google.com>

stefanbucur

PTAL.

bazel/envoy_test.bzl

bazel/repository_locations.bzl

.bazelrc

stefanbucur · 2021-01-28T01:01:56Z

I've just checked the failures and they don't seem to be fuzzing related. For the ASAN failure I also see there is a separate PR to address it (#14844).

asraa

Thanks so much! LGTM

Signed-off-by: Stefan Bucur <sbucur@google.com>

asraa · 2021-01-29T15:55:05Z

@moderation could you please take a look for deps please?

moderation

Question on sha256 pinning of Python dep

moderation · 2021-01-29T17:39:47Z

bazel/repositories_extra.bzl

+        requirements = "@rules_fuzzing//fuzzing:requirements.txt",
+
+        # project_name = "Abseil Python Common Libraries",
+        # project_url = "https://github.com/abseil/abseil-py",
+        # version = "0.11.0",
+        # release_date = "2020-10-27",
+        # use_category = ["test"],
+    )


This is a little different from our existing Python dependencies that normally have a requirements.txt file. The requirements.txt file allows us to pin to sha256 hashes with extra_pip_args = ["--require-hashes"],.

From what I can tell this is referring to https://github.com/bazelbuild/rules_fuzzing/blob/master/fuzzing/requirements.txt. Is there a way to pull in this dependency and pin to the sha256 hash (which is required by policy)?

I've just created bazel-contrib/rules_fuzzing#121 to address that.

Do you mean to keep using the requirements.txt file from the @rules_fuzzing dependency, or make a copy in this repository?

If it's the former, I will update the dependency version to incorporate this change and enable the --require-hashes flag here.

If it's the latter, I think a good location for this file would be //test/fuzz:rules_fuzzing_requirements.txt. WDYT?

I've just created bazelbuild/rules_fuzzing#121 to address that.

Do you mean to keep using the requirements.txt file from the @rules_fuzzing dependency, or make a copy in this repository?

If it's the former, I will update the dependency version to incorporate this change and enable the --require-hashes flag here.

This part is done - let me know if you'd prefer to go with the second approach instead.

moderation · 2021-01-29T17:51:18Z

bazel/repository_locations.bzl

+    rules_fuzzing = dict(
+        project_name = "Fuzzing Rules for Bazel",
+        project_desc = "Bazel rules for fuzz tests",
+        project_url = "https://github.com/bazelbuild/rules_fuzzing",
+        version = "2c97fcd74f20dce950c77129b9b9b31cc63a4c8e",
+        sha256 = "6764e198db6f84bd1f6bf9565ebd7bb532a73b5f99cfe19126a279f67cc9b633",
+        strip_prefix = "rules_fuzzing-{version}",
+        urls = ["https://github.com/bazelbuild/rules_fuzzing/archive/{version}.tar.gz"],
+        release_date = "2021-01-28",
+        use_category = ["test_only"],
+        implied_untracked_deps = [
+            # This is a repository rule generated to define an OSS-Fuzz fuzzing
+            # engine target from the CFLAGS/CXXFLAGS environment.
+            "rules_fuzzing_oss_fuzz",
+        ],
+    ),


Not a great OSSF score but this is test only. Looks OK to me. @htuch?

scorecard --repo=https://github.com/bazelbuild/rules_fuzzing RESULTS ------- Active: Pass 10 Branch-Protection: Fail 0 CI-Tests: Pass 10 CII-Best-Practices: Fail 10 Code-Review: Pass 10 Contributors: Fail 10 Frozen-Deps: Fail 5 Fuzzing: Pass 10 Packaging: Fail 10 Pull-Requests: Pass 10 SAST: Fail 10 Security-Policy: Fail 10 Signed-Releases: Fail 0 Signed-Tags: Fail 0

TIL about Scorecard, this is awesome! :) Just as a heads-up, we are planning to improve upon a number of these items in the short term, and will update the Envoy dependencies when this happens.

Signed-off-by: Stefan Bucur <sbucur@google.com>

htuch

LGTM! I think since this is test-only we will not block on OSSF Scorecard, but it would be ideal to see whatever practical steps taken here as a nice-to-have.

htuch · 2021-01-29T21:00:00Z

.bazelrc

+build:oss-fuzz --linkopt=-fno-sanitize=vptr
+build:oss-fuzz --define=tcmalloc=disabled
+build:oss-fuzz --define=signal_trace=disabled
+build:oss-fuzz --define=ENVOY_CONFIG_ASAN=1


Nit: this is in the base fuzzing config.

Ah, good catch - fixed.

It seems the last push has undone the previous LGTM. Could you approve again?

Signed-off-by: Stefan Bucur <sbucur@google.com>

htuch · 2021-02-02T02:30:00Z

/lgtm deps

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jan 27, 2021

First step of integrating the rules_fuzzing support.

573c239

Signed-off-by: Stefan Bucur <sbucur@google.com>

stefanbucur force-pushed the rules-fuzzing-phase-one branch from 30acee0 to 573c239 Compare January 27, 2021 04:54

Fix release date.

80781ca

Signed-off-by: Stefan Bucur <sbucur@google.com>

Fix the dependency release date.

61a19e0

Signed-off-by: Stefan Bucur <sbucur@google.com>

yanavlasov assigned asraa Jan 27, 2021

asraa reviewed Jan 27, 2021

View reviewed changes

bazel/envoy_test.bzl Show resolved Hide resolved

.bazelrc Outdated Show resolved Hide resolved

bazel/repository_locations.bzl Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Jan 27, 2021

repokitteh-read-only bot assigned htuch Jan 27, 2021

stefanbucur added 2 commits January 27, 2021 12:27

Clarify the use of rules_fuzzing_oss_fuzz.

e199d9a

Signed-off-by: Stefan Bucur <sbucur@google.com>

Reorganize the .bazelrc file.

4bfb765

Signed-off-by: Stefan Bucur <sbucur@google.com>

repokitteh-read-only bot removed the waiting label Jan 27, 2021

stefanbucur commented Jan 27, 2021

View reviewed changes

bazel/envoy_test.bzl Show resolved Hide resolved

bazel/repository_locations.bzl Show resolved Hide resolved

.bazelrc Outdated Show resolved Hide resolved

asraa previously approved these changes Jan 28, 2021

View reviewed changes

Update rules_fuzzing dep to incorporate latest fixes.

61739b8

Signed-off-by: Stefan Bucur <sbucur@google.com>

stefanbucur dismissed asraa’s stale review via 61739b8 January 28, 2021 17:13

stefanbucur mentioned this pull request Jan 28, 2021

Simplify the Envoy integration by using the new rules_fuzzing primitives. google/oss-fuzz#5062

Merged

asraa assigned moderation Jan 29, 2021

moderation suggested changes Jan 29, 2021

View reviewed changes

stefanbucur mentioned this pull request Jan 29, 2021

Pin the hashes of all Python requirements. bazel-contrib/rules_fuzzing#121

Merged

Pin all rules_fuzzing Python deps.

854bec7

Signed-off-by: Stefan Bucur <sbucur@google.com>

htuch previously approved these changes Jan 29, 2021

View reviewed changes

Remove redundant oss-fuzz option.

930631e

Signed-off-by: Stefan Bucur <sbucur@google.com>

stefanbucur dismissed htuch’s stale review via 930631e February 1, 2021 15:27

htuch approved these changes Feb 2, 2021

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Feb 2, 2021

htuch merged commit 461caf0 into envoyproxy:main Feb 2, 2021

stefanbucur mentioned this pull request Feb 2, 2021

The first working draft of the rules_fuzzing integration in Envoy. #14675

Closed

stefanbucur mentioned this pull request Mar 2, 2021

Update the Envoy fuzz target implementation to use the new cc_fuzz_test components in this repo. bazel-contrib/rules_fuzzing#21

Closed

stefanbucur mentioned this pull request Apr 1, 2021

Exegesis google/oss-fuzz#5541

Closed

Conversation

stefanbucur commented Jan 27, 2021

Uh oh!

repokitteh-read-only bot commented Jan 27, 2021

Uh oh!

repokitteh-read-only bot commented Jan 27, 2021

Uh oh!

stefanbucur commented Jan 27, 2021

Uh oh!

asraa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

asraa commented Jan 27, 2021

Uh oh!

stefanbucur left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stefanbucur commented Jan 28, 2021

Uh oh!

asraa left a comment

Choose a reason for hiding this comment

Uh oh!

asraa commented Jan 29, 2021

Uh oh!

moderation left a comment

Choose a reason for hiding this comment

Uh oh!

moderation Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

stefanbucur Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

stefanbucur Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

moderation Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

stefanbucur Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

htuch Jan 29, 2021

Choose a reason for hiding this comment

Uh oh!

stefanbucur Feb 1, 2021

Choose a reason for hiding this comment

Uh oh!

htuch commented Feb 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants