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

types: refactor multiple fuzzers #1258

Merged
merged 1 commit into from Jan 6, 2023
Merged

types: refactor multiple fuzzers #1258

merged 1 commit into from Jan 6, 2023

Conversation

AdamKorcz
Copy link
Contributor

Signed-off-by: AdamKorcz adam@adalogics.com

Summary

Adds a fuzzer for intoto v0.0.2

Does the following changes to multiple types fuzzers:

  1. Silences the logger.
  2. Creates the artifact file if the artifact bytes are nil for the properties.
  3. Hardcodes the version to avoid unnecessary time in the semver dependency.
  4. Moves the fuzzers to their v0.0.1 directory to initiate the relevant entry in the VersionMap. For example here:
    func init() {
    if err := helm.VersionMap.SetEntryFactory(APIVERSION, NewEntry); err != nil {
    log.Logger.Panic(err)
    }
    }

Release Note

Documentation

@AdamKorcz AdamKorcz requested review from asraa, bobcallaway and a team as code owners December 29, 2022 15:57
@codecov-commenter
Copy link

codecov-commenter commented Dec 29, 2022

Codecov Report

Merging #1258 (2ecbafd) into main (e8ba0ac) will decrease coverage by 21.14%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             main    #1258       +/-   ##
===========================================
- Coverage   63.25%   42.11%   -21.15%     
===========================================
  Files          82       74        -8     
  Lines        7670     7225      -445     
===========================================
- Hits         4852     3043     -1809     
- Misses       2205     3874     +1669     
+ Partials      613      308      -305     
Flag Coverage Δ
e2etests ?
unittests 42.11% <ø> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/api/metrics.go 0.00% <0.00%> (-100.00%) ⬇️
cmd/rekor-cli/app/useragent.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/api/trillian_client.go 0.00% <0.00%> (-66.31%) ⬇️
pkg/api/entries.go 0.00% <0.00%> (-64.65%) ⬇️
pkg/api/api.go 0.00% <0.00%> (-64.29%) ⬇️
pkg/types/types.go 0.00% <0.00%> (-63.64%) ⬇️
pkg/types/versionmap.go 0.00% <0.00%> (-63.42%) ⬇️
pkg/api/error.go 0.00% <0.00%> (-58.70%) ⬇️
cmd/rekor-cli/app/verify.go 2.47% <0.00%> (-53.72%) ⬇️
cmd/rekor-cli/app/log_info.go 2.50% <0.00%> (-52.50%) ⬇️
... and 43 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

bobcallaway
bobcallaway previously approved these changes Jan 3, 2023
Signed-off-by: AdamKorcz <adam@adalogics.com>
Copy link
Contributor

@asraa asraa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Feel free to address the review in a follow-up, they're minor.

func FuzzCreateProposedEntry(f *testing.F) {
f.Fuzz(func(t *testing.T, version string) {
initter.Do(fuzzUtils.SetFuzzLogger)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be moved to cose v0.0.1 and populate the props with CreateProps like other types?

if err != nil {
return props, nil, err
}
props.ArtifactPath = artifactURL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases, if ArtifactBytes are written, then we won't even look at the ArtifactPath (

if artifactBytes == nil {
var artifactReader io.ReadCloser
if props.ArtifactPath == nil {
) -- maybe instead you can take some input from ff to either use ArtifactBytes directly or use it from ArtifactPath and clear ArtifactBytes?

The same goes for SignatureBytes and SignaturePath/PublicKeyBytes and PublicKeyPaths

Copy link
Contributor

@asraa asraa Jan 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I misread this! If the ArtifactBytes are nil -- lgtm! (Just see about the Signature/PublicKey if there are any issues)

@asraa asraa merged commit 610b728 into sigstore:main Jan 6, 2023
@github-actions github-actions bot added this to the v1.1.0 milestone Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants