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

Fix building our fuzzing targets from a dist tarball #13145

Merged
merged 3 commits into from Sep 7, 2023

Conversation

rgacogne
Copy link
Member

Short description

Until now all of our fuzzing targets where built with the authoritative server, even though one of them is specific to dnsdist.
This made it easy to build all of them at once, especially for OSS-Fuzz and CI-Fuzz, but had the unfortunate drawback of pulling several dnsdist-specific files into the main pdns/ directory for no good reason. It also prevented building the fuzzing targets from a dist tarball/directory.
This commit moves the dnsdist-specific fuzzing target to the dnsdist build process, and ensure that the standalone_fuzz_target_runner.cc file is part of the dist tarball, making it possible to build the fuzzing targets from the dist.
It does not move the dnsdist-specific files to the pdns/dnsdistdist/ directory yet because this would conflict with existing PRs.
This will require a follow-up pull request for OSS-Fuzz and CI-Fuzz to pick up the dnsdist fuzzing target once this PR has been merged.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • compiled this code
  • tested this code
  • included documentation (including possible behaviour changes)
  • documented the code
  • added or modified regression test(s)
  • added or modified unit test(s)

Until now all of our fuzzing targets where built with the authoritative
server, even though one of them is specific to dnsdist.
This made it easy to build all of them at once, especially for OSS-Fuzz
and CI-Fuzz, but had the unfortunate drawback of pulling several
dnsdist-specific files into the main pdns/ directory for no good
reason. It also prevented building the fuzzing targets from a dist
tarball/directory.
This commit moves the dnsdist-specific fuzzing target to the dnsdist
build process, and ensure that the standalone_fuzz_target_runner.cc
file is part of the dist tarball, making it possible to build the
fuzzing targets from the dist.
It does not move the dnsdist-specific files to the pdns/dnsdistdist/
directory yet because this would conflict with existing PRs.
Copy link
Member

@Habbie Habbie left a comment

Choose a reason for hiding this comment

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

one nit (that really is older than this PR), approved otherwise. Feel free to just merge :)

fuzzing/README.md Outdated Show resolved Hide resolved
@rgacogne
Copy link
Member Author

rgacogne commented Sep 7, 2023

I'm going to merge this pull request, which means that the dnsdist fuzzing will no longer get built either OSS-fuzz or CI-fuzz. It should not cause any error, and I'll open a pull request against the OSS-fuzz repository right away.

@rgacogne rgacogne merged commit a61f498 into PowerDNS:master Sep 7, 2023
74 checks passed
@rgacogne rgacogne deleted the fuzz-targets-dist branch September 7, 2023 09:36
rgacogne added a commit to rgacogne/oss-fuzz that referenced this pull request Sep 7, 2023
PowerDNS recently [1] refactored the way fuzzing targets were handled,
and decided to split them per-product inside the monorepo. We thus now
need to build the DNSDist targets separately from the Authoritative
Server's ones.

[1]: PowerDNS/pdns#13145
jonathanmetzman pushed a commit to google/oss-fuzz that referenced this pull request Sep 7, 2023
PowerDNS recently [1] refactored the way fuzzing targets were handled,
and decided to split them per-product inside the monorepo. We thus now
need to build the DNSDist targets separately from the Authoritative
Server's ones.

[1]: PowerDNS/pdns#13145
gedigi pushed a commit to gedigi/oss-fuzz that referenced this pull request Sep 27, 2023
PowerDNS recently [1] refactored the way fuzzing targets were handled,
and decided to split them per-product inside the monorepo. We thus now
need to build the DNSDist targets separately from the Authoritative
Server's ones.

[1]: PowerDNS/pdns#13145
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants