Skip to content
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

Remove fuzzers from our binaries #4398

Closed
vaikas opened this issue Oct 27, 2020 · 10 comments
Closed

Remove fuzzers from our binaries #4398

vaikas opened this issue Oct 27, 2020 · 10 comments
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Milestone

Comments

@vaikas
Copy link
Contributor

vaikas commented Oct 27, 2020

Describe the bug
We discovered this while looking at some fuzzer code, it was dealt with in serving:
knative/serving#9725

Expected behavior
We should not be baking in our testing libraries into our binaries.

To Reproduce
There are other binaries, but just looking at one:

vaikas@vaikas-a01 eventing % go build ./cmd/controller/main.go
vaikas@vaikas-a01 eventing % go tool nm -size main | grep fuzz
143dbe0 32 T github.com/google/gofuzz.(*Continue).ExpFloat64
143dc00 32 T github.com/google/gofuzz.(*Continue).Float32
143dc20 32 T github.com/google/gofuzz.(*Continue).Float64
143dc40 128 T github.com/google/gofuzz.(*Continue).Fuzz
143dcc0 128 T github.com/google/gofuzz.(*Continue).FuzzNoCustom
... ...

Just recording this to see what impact this may have.

vaikas@vaikas-a01 eventing % ls -l main
-rwxr-xr-x 1 vaikas staff 61121452 Oct 27 11:25 main

Knative release version
all of them.

Additional context
Add any other context about the problem here such as proposed priority

@vaikas vaikas added the kind/bug Categorizes issue or PR as related to a bug. label Oct 27, 2020
@vaikas vaikas self-assigned this Oct 27, 2020
@vaikas
Copy link
Contributor Author

vaikas commented Oct 27, 2020

Moving things into _test files, results in a small amount of reduction in binary size:
vaikas@vaikas-a01 eventing % ls -l main
-rwxr-xr-x 1 vaikas staff 61103260 Oct 27 12:00 main

vaikas@vaikas-a01 mink % echo '61121452 61103260 - p' | dc
18192

However, that change alone does not get rid of all the fuzzer code, I think we need to do some additional work in the pkg code base.

@vaikas
Copy link
Contributor Author

vaikas commented Oct 27, 2020

Ok, we no longer have any knative.dev/* related fuzzery in our binary. However we'll tackle that later:

vaikas@vaikas-a01 eventing % go tool nm -size main | grep fuzz | wc -l
78
vaikas@vaikas-a01 eventing % go tool nm -size main | grep fuzz | grep -v github.com/google/gofuzz | wc -l
0

@vaikas
Copy link
Contributor Author

vaikas commented Nov 2, 2020

Looks like we need to pull this commit in to get rid of the gofuzz:
kubernetes/apimachinery@08191f1#diff-44ee78478149e26a30a4185475298d442bb9e53daa49e22bc355d436aff7b144

Currently we don't have that version and we're seeing this (after the PR to rename fuzzer -> fuzzer_test.go).

github.com/google/gofuzz

knative.dev/eventing/cmd/mtbroker
k8s.io/apimachinery/pkg/apis/meta/v1
github.com/google/gofuzz

And we're still using this version:
https://github.com/knative/eventing/blob/master/vendor/k8s.io/apimachinery/pkg/apis/meta/v1/micro_time.go

@n3wscott
Copy link
Contributor

We should still do this.

@vaikas
Copy link
Contributor Author

vaikas commented Jan 26, 2021

Yeah, absolutely, but if I read the upstream correctly, we need 1.20 of the apimachinery to get this.

@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 27, 2021
@devguyio
Copy link
Contributor

/remove-lifecycle stale

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 27, 2021
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 29, 2021
@vaikas
Copy link
Contributor Author

vaikas commented Aug 10, 2021

This is still present, even though we're using 1.20 of api machinery

vaikas@Villes-MacBook-Pro eventing % go build ./cmd/controller/main.go
vaikas@Villes-MacBook-Pro eventing % go tool nm -size main | grep fuzz | wc -l
      71

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 28, 2021
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

5 participants