search

kubernetes-sigs / cluster-api

Home for Cluster API, a subproject of sig-cluster-lifecycle
https://cluster-api.sigs.k8s.io
Apache License 2.0
3.52k stars 1.3k forks source link

Consider dropping external fuzz testing from oss-fuzz #9548

Open killianmuldoon opened 11 months ago

killianmuldoon commented 11 months ago

We added fuzz testing from the oss-fuzz project as part of https://github.com/kubernetes-sigs/cluster-api/issues/6059, with the implementation done here: https://github.com/cncf/cncf-fuzzing/tree/main/projects/cluster-api

The implementation is currently undermaintained as it's completely external to the project and has low visibility. Building fails after changes are made to method signatures and need to be fixed. It is the responsibility of CAPI maintainers to keep these tests healthy, but they've been failing for a long time without being fixed (I'm definitely part of the problem here :sweat_smile: )

These tests are also complicated to maintain, and maintenance hasn't been done on them. They currently provide no signal for CAPI as they're failing to build.

I think we should try to replace these fuzz tests with an in-repo version.

This would involve:

sbueringer commented 11 months ago

Absolutely agree with that what we currently have is not very useful in its current state. I think right now we simply don't have the bandwith to deal with this, also not sure if it's worth a significant investment.

killianmuldoon commented 11 months ago

/assign

killianmuldoon commented 11 months ago

/triage accepted

I opened PRs on the relevant repos to remove the CAPI project and fuzzers.

killianmuldoon commented 11 months ago

Update with some context: https://github.com/cncf/cncf-fuzzing/pull/461#issuecomment-1802734619

One of the folks working on CNCF fuzzing has volunteered to bring the fuzzers upstream. Let's see what that looks like in implementation, but it could be a step forward.

@AdamKorcz it would be good if you could explain what bringing the fuzzers upstream would look like to make sure it's acceptable in upstream Cluster API.

AdamKorcz commented 10 months ago

I was planning on rewriting the fuzzers to native Go fuzz tests that can be run from the command line with go test -fuzz= and place the fuzzers in the directories that they test which is the recommended way of structuring tests in go. I would also add the build.sh file to CAPI so that maintainers easily can modify it.

sbueringer commented 10 months ago

Sounds like it would be a huge improvement. In any case we'll need someone who has the bandwith to fix potential findings, but maybe that's easier to achieve if we have an easy way to reproduce the failures.

Would be probably also good if there's an easy way to ignore findings if we think a specific issue doesn't have to be fixed. But I assume if the test is part of the CAPI repo we can just modify the test code for that.

fabriziopandini commented 5 months ago

/kind cleanup /priority backlog