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

dnsdist: Add the ability to change the qname and owner names in DNS packets #12417

Merged
merged 5 commits into from
Jan 31, 2023

Conversation

rgacogne
Copy link
Member

@rgacogne rgacogne commented Jan 13, 2023

Short description

In some setups, we might need to get the result for a different qname and change the response to set the owner names of the records matching the changed qname back to the initial qname. Note that this is brittle and should only be used in very specific cases.
This PR is based on top of #12388 and will have to be rebased.

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)

@rgacogne rgacogne added this to the dnsdist-1.8.0 milestone Jan 13, 2023
@rgacogne rgacogne force-pushed the ddist-async-alternate-name branch from 7464710 to b84ac3b Compare January 24, 2023 12:45
@rgacogne rgacogne force-pushed the ddist-async-alternate-name branch from b84ac3b to 56287dc Compare January 24, 2023 16:36
@rgacogne rgacogne changed the title dnsdist: Add the ability to rebase (switch from one name to another) DNS packets dnsdist: Add the ability to change the qname and owner names in DNS packets Jan 24, 2023
@rgacogne rgacogne marked this pull request as ready for review January 24, 2023 16:37
@rgacogne rgacogne requested a review from omoerbeek January 25, 2023 08:30
Copy link
Member

@omoerbeek omoerbeek left a comment

Choose a reason for hiding this comment

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

Is it intentional the new ffi APIs are undocumented?

@rgacogne
Copy link
Member Author

Is it intentional the new ffi APIs are undocumented?

Yes and no, the gist of it is that dnsdist's FFI API is not documented at all, which is not great. It's also very low-level, by design, so it's hard to document in a meaningful way, but in the future we should do better than no documentation at all.

@rgacogne rgacogne requested a review from omoerbeek January 26, 2023 08:52
@chbruyand
Copy link
Member

Looked good from what I read

@rgacogne rgacogne merged commit 3af9f13 into PowerDNS:master Jan 31, 2023
@rgacogne rgacogne deleted the ddist-async-alternate-name branch January 31, 2023 08:40
rgacogne added a commit to rgacogne/pdns that referenced this pull request Jan 31, 2023
I removed the pdns_string_view alias in PowerDNS#12417 as it was no longer
used, but merging PowerDNS#11511 actually reintroduced it, breaking the build.
rgacogne added a commit that referenced this pull request Jan 31, 2023
omoerbeek pushed a commit to omoerbeek/pdns that referenced this pull request Feb 10, 2023
I removed the pdns_string_view alias in PowerDNS#12417 as it was no longer
used, but merging PowerDNS#11511 actually reintroduced it, breaking the build.
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.

3 participants