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

ci: add buildspec file for scheduled fuzzing #4763

Merged
merged 17 commits into from
Sep 24, 2024

Conversation

jouho
Copy link
Contributor

@jouho jouho commented Sep 10, 2024

Resolved issues:

Part of #4748

While migrating CI fuzz jobs from Make to CMake, I am adding buildspec files instead of defining them directly in the CodeBuild configuration.

Currently, the buildspec for scheduled fuzzing is defined within the CodeBuild configuration. To maintain better organization and make code changes more trackable, we should use a buildspec file, similar to what we currently do for the GeneralFuzzBatch job.

Description of changes:

  • Added buildspec_fuzz_scheduled.yml to be used by CodeBuild. This will replace buildspec that is defined on scheduled buzz build. The diff between the two is:
30c30
<             - aws-lc
---
>             - awslc
78,82c78,79
<         -DCMAKE_PREFIX_PATH=$LIBCRYPTO_ROOT \
<         -DS2N_FUZZ_TEST=on \
<         -DFUZZ_TIMEOUT_SEC=27000 \
<         -DCORPUS_UPLOAD_LOC=s3://s2n-tls-fuzz-corpus \
<         -DARTIFACT_UPLOAD_LOC=s3://s2n-build-artifacts/s2nFuzzScheduledArtifacts
---
>         -DCMAKE_PREFIX_PATH=/usr/local/$S2N_LIBCRYPTO \
>         -DS2N_FUZZ_TEST=on \
>         -DFUZZ_TIMEOUT_SEC=27000
  • Modified how environment variables are being initialized. Previously CodeBuild's environment variables were not being read by CMake properly

  • Removed unused variable FUZZ_TESTS

Call-outs:

  • The environment variable FUZZ_COVERAGE needs to be turned off on CodeBuild build configuration for the tests to run successfully. Currently, CMake is missing some CFlags required to generate a coverage report. I have added this as an action item in the tracking issue Port Fuzz Test to CMake #4748.

  • I am generating a batch job by specifying all the test names manually. This approach is inefficient because new fuzz tests need to be added to the list manually. I considered writing a script to parse through the test files and append them to this .yml file, but that might be overly complicated. I would appreciate suggestions for a better solution.

Testing:

Tested using this buildspec by overriding the nightly fuzz job with a 10-second timeout. Link to CodeBuild

s2nFuzzBatch also succeeds without error: Link to CodeBuild

Also confirmed corpus files are being updated on s3

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Sep 10, 2024
@jouho jouho mentioned this pull request Sep 11, 2024
9 tasks
@jouho jouho marked this pull request as ready for review September 11, 2024 19:50
@jouho jouho requested a review from dougch as a code owner September 11, 2024 19:50
@jouho jouho requested a review from maddeleine September 11, 2024 19:50
codebuild/spec/buildspec_fuzz_scheduled.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_fuzz_scheduled.yml Show resolved Hide resolved
codebuild/spec/buildspec_fuzz_scheduled.yml Outdated Show resolved Hide resolved
codebuild/spec/buildspec_fuzz_scheduled.yml Outdated Show resolved Hide resolved
- remove comment on unused var
- rephrase comment
- fix cmake prefix path
@jouho jouho requested a review from maddeleine September 11, 2024 23:18
CMakeLists.txt Outdated Show resolved Hide resolved
codebuild/spec/buildspec_fuzz.yml Outdated Show resolved Hide resolved
@jouho
Copy link
Contributor Author

jouho commented Sep 13, 2024

The scheduled fuzz tests were failing because ctest's default timeout is set to 1500 secs and our fuzz test run for ~7 hours. I made a change to override this for scheduled fuzz build so it is able to run up to 8 hours

@jouho jouho requested a review from maddeleine September 13, 2024 18:25
Copy link
Contributor

@lrstewart lrstewart left a comment

Choose a reason for hiding this comment

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

Tested using this buildspec by overriding the nightly fuzz job with a 30-second timeout

Have you tested it with no overrides? It might be a good idea to just let it run overnight to confirm the intended behavior.

codebuild/spec/buildspec_fuzz_scheduled.yml Show resolved Hide resolved
codebuild/spec/buildspec_fuzz_scheduled.yml Show resolved Hide resolved
@jouho
Copy link
Contributor Author

jouho commented Sep 14, 2024

Have you tested it with no overrides? It might be a good idea to just let it run overnight to confirm the intended behavior.

Sounds good. I'll edit the codebuild job to so the next build will run against this PR using the buildspec file

@jouho
Copy link
Contributor Author

jouho commented Sep 16, 2024

I let the scheduled fuzz test run against this PR using buildspec_fuzz_scheduled.yml and confirmed it runs as intended: Link to CodeBuild

@jouho jouho requested a review from lrstewart September 16, 2024 16:24
CMakeLists.txt Outdated Show resolved Hide resolved
@jouho jouho requested a review from lrstewart September 17, 2024 23:08
CMakeLists.txt Outdated Show resolved Hide resolved
codebuild/spec/buildspec_fuzz_scheduled.yml Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
codebuild/spec/buildspec_fuzz_scheduled.yml Show resolved Hide resolved
@jouho jouho requested a review from lrstewart September 19, 2024 22:45
@jouho jouho requested a review from lrstewart September 23, 2024 17:28
@jouho jouho enabled auto-merge (squash) September 24, 2024 18:13
@jouho jouho disabled auto-merge September 24, 2024 20:07
@jouho jouho enabled auto-merge (squash) September 24, 2024 20:21
@jouho jouho merged commit 0a887d7 into aws:main Sep 24, 2024
37 checks passed
@jouho jouho deleted the add-fuzz-scheduled-buildspec branch September 24, 2024 22:04
@jouho
Copy link
Contributor Author

jouho commented Sep 24, 2024

Final diff between the codebuild buildspec vs the new added buildspec_fuzz_scheduled.yml:

30c30
<             - aws-lc
---
>             - awslc
78c78
<         -DCMAKE_PREFIX_PATH=$LIBCRYPTO_ROOT \
---
>         -DCMAKE_PREFIX_PATH=/usr/local/$S2N_LIBCRYPTO \
80,82c80
<         -DFUZZ_TIMEOUT_SEC=27000 \
<         -DCORPUS_UPLOAD_LOC=s3://s2n-tls-fuzz-corpus \
<         -DARTIFACT_UPLOAD_LOC=s3://s2n-build-artifacts/s2nFuzzScheduledArtifacts
---
>         -DFUZZ_TIMEOUT_SEC=27000
88,91c86,88
<       # -R: Restrict tests to names containing the value of ${FUZZ_TEST_NAME}
<       - cmake --build build/ --target test -- ARGS="-L fuzz -R ${FUZZ_TESTS} --output-on-failure --timeout 28800"
<
< # Callout: FUZZ_COVERAGE needs to be turned off on CodeBuild config for this to work. Currently CMake is missing CFlags to generate coverage reprot.
\ No newline at end of file
---
>       # -R: Run the single fuzz test defined in ${FUZZ_TESTS}
>       # --timeout: override ctest's default timeout of 1500
>       - cmake --build build/ --target test -- ARGS="-L fuzz -R ${FUZZ_TESTS} --output-on-failure --timeout 28800"
\ No newline at end of file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants