TrueCloudLab/frostfs-sdk-go
30
0
Fork 21
Code Issues 29 Pull requests 2 Releases Activity Actions

Add policy decode fuzz tests #219

Merged
fyrchik merged 1 commit from dstepanov-yadro/frostfs-sdk-go:feat/policy_fuzz_tests into master 2024-05-07 11:10:41 +00:00
Conversation 10 Commits 1 Files changed 5 +45 -13
dstepanov-yadro commented 2024-05-02 13:40:53 +00:00
Member
Copy link

Relates #216

Fuzz test for policy decode added.

Relates #216 Fuzz test for policy decode added.
dstepanov-yadro force-pushed feat/policy_fuzz_tests from 380b21d160 to 6d8fffd9f9 2024-05-02 13:41:45 +00:00 Compare
dstepanov-yadro requested review from storage-core-committers 2024-05-02 13:42:16 +00:00
dstepanov-yadro requested review from storage-core-developers 2024-05-02 13:42:17 +00:00
fyrchik reviewed 2024-05-02 13:45:41 +00:00
netmap/policy_fuzz_test.go Outdated
@ -0,0 +1,22 @@
//go:build gofuzz
fyrchik commented 2024-05-02 13:45:41 +00:00
Owner
Copy link

We will use this with libFuzzer https://github.com/AdamKorcz/go-118-fuzz-build?tab=readme-ov-file#using-test-utils-from-other-_testgo-files

So the logic should be in the main package without _test prefix

We will use this with libFuzzer https://github.com/AdamKorcz/go-118-fuzz-build?tab=readme-ov-file#using-test-utils-from-other-_testgo-files So the logic should be in the main package without `_test` prefix
dstepanov-yadro commented 2024-05-02 13:48:11 +00:00
Author
Member
Copy link

done

done
fyrchik marked this conversation as resolved
dstepanov-yadro force-pushed feat/policy_fuzz_tests from 6d8fffd9f9 to c2cdb82617 2024-05-02 13:48:07 +00:00 Compare
ognev commented 2024-05-02 14:09:12 +00:00
Member
Copy link

  1. Предлагаю прийти к такому способу хранения фаззинг тестов - <pkg_root>/tests/fuzzing/<pkg_name>/[pkg_test.go, pkg_fuzz.go]

  2. хочется чтобы сам фаззинг тест имел следующую структуру:
    f.Fuzz(func(t *testing.T, data []byte) {
    DoFuzzMyFunc(data)
    })
    Это позволит использовать и go test (сохранение совместимости на перспективу), и libfuzzer (более мощный движок + возможность сбора покрытия кода)

Например, тут [https://github.com/etclabscore/core-geth/blob/master/tests/fuzzers/bitutil/compress_test.go] можно посмотреть как это реализовано в других проектах (в данном случае все лежит в одном test файле)

1) Предлагаю прийти к такому способу хранения фаззинг тестов - <pkg_root>/tests/fuzzing/<pkg_name>/[pkg_test.go, pkg_fuzz.go] 2) хочется чтобы сам фаззинг тест имел следующую структуру: f.Fuzz(func(t *testing.T, data []byte) { DoFuzzMyFunc(data) }) Это позволит использовать и go test (сохранение совместимости на перспективу), и libfuzzer (более мощный движок + возможность сбора покрытия кода) Например, тут [https://github.com/etclabscore/core-geth/blob/master/tests/fuzzers/bitutil/compress_test.go] можно посмотреть как это реализовано в других проектах (в данном случае все лежит в одном test файле)
dstepanov-yadro force-pushed feat/policy_fuzz_tests from c2cdb82617 to fa2f14c5a3 2024-05-02 14:13:19 +00:00 Compare
dstepanov-yadro commented 2024-05-02 14:24:12 +00:00
Author
Member
Copy link

Например, тут [https://github.com/etclabscore/core-geth/blob/master/tests/fuzzers/bitutil/compress_test.go] можно посмотреть как это реализовано в других проектах (в данном случае все лежит в одном test файле)

Поправил

> Например, тут [https://github.com/etclabscore/core-geth/blob/master/tests/fuzzers/bitutil/compress_test.go] можно посмотреть как это реализовано в других проектах (в данном случае все лежит в одном test файле) Поправил
ognev commented 2024-05-02 15:02:07 +00:00
Member
Copy link

я имел ввиду frostfs-sdk-go/tests/fuzzing/netmap/...

я имел ввиду frostfs-sdk-go/tests/fuzzing/netmap/...
dstepanov-yadro force-pushed feat/policy_fuzz_tests from fa2f14c5a3 to 1a9de18eb8 2024-05-02 15:07:11 +00:00 Compare
dstepanov-yadro commented 2024-05-02 15:07:33 +00:00
Author
Member
Copy link

я имел ввиду frostfs-sdk-go/tests/fuzzing/netmap/...

done

> я имел ввиду frostfs-sdk-go/tests/fuzzing/netmap/... done
🚀 1
fyrchik commented 2024-05-02 15:18:20 +00:00
Owner
Copy link

Do we really need to use fzgen for simple types ([]byte, string)?

Do we really need to use fzgen for simple types ([]byte, string)?
fyrchik commented 2024-05-03 05:39:26 +00:00
Owner
Copy link

Frankly I would rather not have a separate tests package and put tests together with the code (it may reuse helpers, for example).
It there any reason why you want this besides "one specific not very popular project does this"? @ognev

Frankly I would rather not have a separate `tests` package and put tests together with the code (it may reuse helpers, for example). It there any reason why you want this besides "one specific not very popular project does this"? @ognev
ognev commented 2024-05-03 06:12:09 +00:00
Member
Copy link

No, I don't see any major reasons.
The main reason is cluttering of the packages with code.
In addition to tests, we will also need to store a corpus of input data and (possibly) a dictionary (various string values and magic numbers) for more efficient fuzzing.

No, I don't see any major reasons. The main reason is cluttering of the packages with code. In addition to tests, we will also need to store a corpus of input data and (possibly) a dictionary (various string values and magic numbers) for more efficient fuzzing.
ognev commented 2024-05-03 06:15:28 +00:00
Member
Copy link

Do we really need to use fzgen for simple types ([]byte, string)?

To generate coverage, we need to take an array of bytes as input regardless of the code.
Of course, we can generate simple types from it ourselves.

> Do we really need to use fzgen for simple types ([]byte, string)? To generate coverage, we need to take an array of bytes as input regardless of the code. Of course, we can generate simple types from it ourselves.
fyrchik commented 2024-05-03 07:11:16 +00:00
Owner
Copy link

we will also need to store a corpus of input data

This is my argument in favor of storing them together with code. Usually we have unexported test helpers which can generate this corpus. And because structures evolve, static corpus will also need to change.

For an example, see TrueCloudLab/policy-engine#72/files

>we will also need to store a corpus of input data This is my argument in favor of storing them together with code. Usually we have unexported test helpers which can generate this corpus. And because structures evolve, static corpus will also need to change. For an example, see https://git.frostfs.info/TrueCloudLab/policy-engine/pulls/72/files
dstepanov-yadro force-pushed feat/policy_fuzz_tests from 1a9de18eb8 to 1bea561f81 2024-05-03 07:23:14 +00:00 Compare
dstepanov-yadro commented 2024-05-03 07:23:33 +00:00
Author
Member
Copy link

we will also need to store a corpus of input data

This is my argument in favor of storing them together with code. Usually we have unexported test helpers which can generate this corpus. And because structures evolve, static corpus will also need to change.

Done

> >we will also need to store a corpus of input data > > This is my argument in favor of storing them together with code. Usually we have unexported test helpers which can generate this corpus. And because structures evolve, static corpus will also need to change. Done
fyrchik approved these changes 2024-05-03 07:48:03 +00:00
dstepanov-yadro force-pushed feat/policy_fuzz_tests from 1bea561f81 to eb02e1e595 2024-05-03 07:53:44 +00:00 Compare
fyrchik merged commit 02c936f397 into master 2024-05-07 11:10:41 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
fyrchik
TrueCloudLab/storage-core-developers
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
good first issue

Good for newcomers

  
pool

Tasks related to client pool

  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
good first issue
pool
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-sdk-go#219
No description provided.
Delete branch "dstepanov-yadro/frostfs-sdk-go:feat/policy_fuzz_tests"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?