-
Notifications
You must be signed in to change notification settings - Fork 86
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
Refactor Fuzz implementation #212
Conversation
81793b0
to
b728528
Compare
Signed-off-by: AdamKorcz <adam@adalogics.com>
bc730e6
to
bb09769
Compare
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 looks good to me, thank you very much @pjbgf 🙇
runtime/events/events_fuzzer.go
Outdated
|
||
if os.Getenv("KUBEBUILDER_ASSETS") == "" { | ||
cmd := exec.Command("/usr/bin/bash", "-c", `go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest && \ | ||
/root/go/bin/setup-envtest use -p path 1.21.x`) |
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 we move the Kubernetes version into the Makefile?
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.
@stefanprodan good point. I changed it to use the value from ENVTEST_BIN_VERSION
.
The default value of 1.23
is to keep the calls from oss-fuzz
deterministic. PTAL
runtime/go.sum
Outdated
github.com/containerd/console v0.0.0-20181022165439-0650fd9eeb50/go.mod h1:Tj/on1eG8kiEhd0+fhSDzsPAFESxzBBvdyEgyryXffw= | ||
github.com/containerd/console v0.0.0-20191206165004-02ecf6a7291e/go.mod h1:8Pf4gM6VEbTNRIT26AyyU7hxdQU3MvAvxVI0sc00XBE= | ||
github.com/containerd/console v1.0.1/go.mod h1:XUsP6YE/mKtz6bxc+I8UiKKTP04qjQL4qcS3XoQ5xkw= | ||
github.com/containerd/console v1.0.2/go.mod h1:ytZPjGgY2oeTkAONYafi2kSj0aYggsf8acV1PGKCbzQ= |
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.
How did we end up with 500 new dependencies in here?
@pjbgf would it be possible to have a dedicated module for fuzz that imports all our packages and the fuzz specific ones? This would keep the runtime dependencies to a minimum. |
Sure, will move them all to the fuzz directory and move them back at execution time. |
Structure the fuzz implementation to be closer to what go native will support. Add Makefile target to enable smoketesting fuzzers. Add github workflow to test and upload crash results. Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
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.
LGTM
Thanks @pjbgf 🏅
Following-up from the security audit, I reviewed the initial Fuzz PR and created this PR that supersedes it.
Main changes:
FuzzEventf
target.The changes aim to make it easier to test fuzzers locally. And decrease the amount of changes to adopt built-in support for fuzzing.
Oss-fuzz aims to add support to the golang native fuzzing once 1.18 is released officially.