New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fuzzing: run latest fuzzers in OSS-Fuzz #22580
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! I have some questions, I think I don't fully understand how are we running the fuzzers.
@@ -1,3 +1,13 @@ | |||
#/bin/bash -eu | |||
printf "package policy\nimport _ \"github.com/AdamKorcz/go-118-fuzz-build/testing\"\n" > $SRC/cilium/pkg/policy/registerfuzzdep.go | |||
go mod tidy && go mod vendor | |||
mv $SRC/cilium/pkg/policy/l4_filter_test.go $SRC/cilium/pkg/policy/l4_filter_test_fuzz.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why we need to move these files around and where is this file being run? Can it be run locally by developers?
Maybe we should just move all fuzzing tests from l4_filter_test.go
to l4_filter_test_fuzz.go
and not bother with moving the files around?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the oss-fuzz-build.sh is meant to only be run inside the OSS-Fuzz build environment. In that environment, the fuzzers are built with go build which doesn't include _test.go
files in its scope and they are therefore renamed. When run locally by developers with go test -fuzz
then the renaming is not necessary.
@@ -1,3 +1,13 @@ | |||
#/bin/bash -eu | |||
printf "package policy\nimport _ \"github.com/AdamKorcz/go-118-fuzz-build/testing\"\n" > $SRC/cilium/pkg/policy/registerfuzzdep.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that this is done to not import fuzzing libs in policy package for building agent/whatever else might use policy package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is done because OSS-Fuzz rewrites the testing types to ones compatible with libFuzzer which is the fuzzing engine used by OSS-Fuzz.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the changes, can you provide an explanation on why we need this change in the commit message as well as provide some comments in the script file?
Friendly ping 👋 |
This PR adds multiple fuzzers to Ciliums OSS-Fuzz build script. OSS-Fuzz will build the fuzzers and run them continuously. Ciliums OSS-Fuzz integration can be found here: https://github.com/google/oss-fuzz/tree/master/projects/cilium - the build script will invoke Ciliums upstream OSS-Fuzz build script Signed-off-by: AdamKorcz <adam@adalogics.com>
Done! |
Signed-off-by: AdamKorcz adam@adalogics.com
Builds the recently added fuzzers in OSS-Fuzz.