Skip to content

auth: Überbackend code cleanup #16542

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.

Sign up for GitHub

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

Jump to bottom
Open
miodvallat wants to merge 4 commits into
base: master
Choose a base branch
from
Open

auth: Überbackend code cleanup #16542

miodvallat wants to merge 4 commits into from

Conversation

@miodvallat
Copy link
Contributor

@miodvallat miodvallat commented Nov 24, 2025

Short description

This apparently simple PR reworks the behaviour of the handle internal object of UeberBackend to:

  • factor the code performing backend selection and lookup call.
  • make the current backend pointer a private field to guarantee that the above factored logic is the only thing which decides which backend to use.

It is not supposed to introduce any behaviour change. But it will help make further (possibly breaking) changes in UeberBackend with confidence.

P.S.: do we really need to keep the instances atomic counter? It doesn't seem to be used by anything and its value is not retrievable either.

Checklist

I have:

  • read the CONTRIBUTING.md document
  • read and accepted the Developer Certificate of Origin document, including the AI Policy, and added a "Signed-off-by" to my commits
  • 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)

@coveralls
Copy link

coveralls commented Nov 24, 2025 •
edited
Loading

Pull Request Test Coverage Report for Build 19757600681

Details

  • 32 of 32 (100.0%) changed or added relevant lines in 2 files are covered.
  • 35 unchanged lines in 9 files lost coverage.
  • Overall coverage increased (+0.04%) to 73.146%
Files with Coverage Reduction New Missed Lines %
pdns/pollmplexer.cc 1 84.97%
modules/godbcbackend/sodbc.cc 2 71.43%
pdns/recursordist/rec-tcpout.cc 2 79.53%
pdns/axfr-retriever.cc 3 66.94%
pdns/recursordist/test-syncres_cc2.cc 3 89.12%
pdns/tsigverifier.cc 3 77.22%
pdns/recursordist/syncres.cc 6 81.28%
pdns/recursordist/test-syncres_cc1.cc 7 90.23%
pdns/packethandler.cc 8 72.44%
Totals Coverage Status
Change from base Build 19741851688: 0.04%
Covered Lines: 128173
Relevant Lines: 164538
💛 - Coveralls

@Habbie
Copy link
Member

Habbie commented Nov 25, 2025

P.S.: do we really need to keep the instances atomic counter? It doesn't seem to be used by anything and its value is not retrievable either.

the only thing using it appears to be a // commented out log message. I think it can go.

Habbie
Habbie previously approved these changes Nov 25, 2025
View reviewed changes
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.

nice cleanup

Habbie
Habbie reviewed Nov 25, 2025
View reviewed changes
@miodvallat
Factor code handling backends in Ueberbackend::handle.
218d6df 
This allows making the current backend pointer private and makes the
behaviour more explicit:
- all backends will be queried in sequence.
- as soon as one backend answers the given (qname, qtype) query, none of
  the remaining backends will be queried until the next UeberBackend::lookup
  call (which starts a fresh new query).

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
@miodvallat
Drop Ueberbackend handle instance counter, nothing uses it.
31ea89b 
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
Habbie
Habbie previously approved these changes Nov 25, 2025
View reviewed changes
@miodvallat
Remove dead variable pointed by clang-tidy
9dd451f 
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
@miodvallat miodvallat force-pushed the aschung branch 3 times, most recently from 3e81f60 to a212ab4 Compare November 28, 2025 07:46
@miodvallat
Make UeberBackend::handle even more opaque. NFCI
06e3dd6 
Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Habbie Habbie Habbie left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

auth

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@miodvallat @coveralls @Habbie