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

[pdnsutil] dedup in add-record #15170

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

Conversation

miodvallat
Copy link
Contributor

Short description

The discussion of #4727 suggests that adding some smarts to add-record for it not to insert duplicate content would be welcome.

This PR does exactly this, and also tries to make the "Thou shalt not mix CNAME with other records" logic easier to follow.

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)

@miodvallat miodvallat added this to the auth-5 milestone Feb 17, 2025
When adding records with pdnsutil, the combination of the existing and
to-be-added records will now be dedup'ed.

Fixes: PowerDNS#4727
@coveralls
Copy link

coveralls commented Feb 17, 2025

Pull Request Test Coverage Report for Build 13411269542

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 225 unchanged lines in 12 files lost coverage.
  • Overall coverage decreased (-0.001%) to 64.485%

Files with Coverage Reduction New Missed Lines %
pdns/dnsdistdist/dnsdist-backend.cc 1 66.19%
pdns/pollmplexer.cc 1 83.66%
modules/lmdbbackend/lmdbbackend.hh 2 90.0%
pdns/misc.cc 2 62.51%
pdns/recursordist/sortlist.cc 2 72.94%
pdns/tcpiohandler.cc 2 68.18%
pdns/recursordist/syncres.cc 3 80.09%
pdns/recursordist/test-syncres_cc2.cc 3 88.85%
pdns/remote_logger.cc 3 56.95%
pdns/dnsdistdist/dnsdist-tcp.cc 8 75.76%
Totals Coverage Status
Change from base Build 13368084756: -0.001%
Covered Lines: 127619
Relevant Lines: 166885

💛 - Coveralls

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.

unfinished review, no negative comments so far

pdns/pdnsutil.cc Show resolved Hide resolved
pdns/pdnsutil.cc Show resolved Hide resolved
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