| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Remove hacks around contrib/fuzz #7087
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
Conversation
|
Skipping CI for Draft Pull Request. |
kzys
force-pushed
the
fuzz-less-hacks
branch
7 times, most recently
from
dcdd645 to
5f7f75e
Compare
kzys
force-pushed
the
fuzz-less-hacks
branch
3 times, most recently
from
33a97ca to
f11d284
Compare
containerd/containerd#7087 will move the fuzzer to the upstream. Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
|
Really nice work @kzys! Let me know if I should review anything. |
| @@ -40,14 +40,19 @@ var ( | |||
| initDaemon sync.Once | |||
| ) | |||
|
|
|||
| func startDaemonForFuzzing(arguments []string) { | |||
| app := command.App() | |||
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.
Note to reviewers: This fuzzer is starting containerd by instantiating command.App directly. To have all containerd's plugins, it has to have all of _ "github.com/containerd/containerd/.../plugin" import and that's why we need builtins 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.
@AdamKorcz Regarding directly instantiating fuzzers, does it help coverage-guided fuzzers to know more about containerd, compared to launching containerd as an external process?
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.
Regarding directly instantiating fuzzers, does it help coverage-guided fuzzers to know more about containerd, compared to launching containerd as an external process?
Yes. Interacting with an executable is less effective than the approach of launcing command.App
| "github.com/containerd/containerd/pkg/registrar" | ||
|
|
||
| "github.com/containerd/containerd" | ||
| _ "github.com/containerd/containerd/cmd/containerd/builtins" |
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.
Because this is using builtins package, adding CRI into builtins makes a circular dependency between builtins and CRI, hence avoided.
|
needs rebase due to conflict |
|
one more rebase to pick up the fix for windows integration #7106 |
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
Signed-off-by: Kazuyoshi Kato <katokazu@amazon.com>
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
I think we can init plugins and test it in out of cri package in the future.
This PR is removing some hacks from contrib/fuzz/oss_fuzz_build.sh. Rewriting Go source code with regex is too error-prone.