[pdnsutil] Do not allow increase-serial on secondary zones

[miodvallat]

Short description

As reported in #15130, pdnsutil increase-serial should fail on secondary zones.

The approach in this PR is quite conservative, if the backend is not able to report whether the zone is primary or secondary, we'll try to increase anyway - have to trust the user at some point.

Similarly, there is no check added to pdnsutil edit-zone. We assume you know more or less what you are doing.

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)

[Habbie]

Similarly, there is no check added to pdnsutil edit-zone. We assume you know more or less what you are doing.

we got a complaint about add-record working on secondary zones recently. Perhaps we should have a separate function that we can call from all those places.

[miodvallat]

Why not. Let's make a list of the comamnds where it would apply, I can think of:

• add-record
• clear-zone
• delete-rrset
• increase-serial
• rectify-zone
• replace-rrset
• set-account

Maybe add-meta too?

[Habbie]

the meta stuff may be important for secondaries, like TSIG management, and for setups where a secondary receives a zone to then DNSSEC-sign it. (this means key management is also allowed)

clear-zone is a fun one, perhaps it should be allowed. At least it won't cause desynced data, and it will eventually cause a new transfer. People sometimes do the equivalent in SQL.

set-account seems harmless and perhaps useful.

[miodvallat]

So the list would be:

• add-record
• delete-rrset
• increase-serial
• rectify-zone
• replace-rrset

[Habbie]

if the backend is not able to report whether the zone is primary or secondary,

I -think- in this flow, it is impossible to have getDomainInfo fail. So if it does, it exposes a bug or corruption, we can just abort.

[miodvallat]

Well, until one week ago, the tinydns backend did not implement getDomainInfo. I have not checked if there remain backends which don't.

[Habbie]

oh of course. I wonder if there are backends without getDomainInfo, that do allow mutations. So I think your approach is fine, or we can go for 'error' and eventually find out we were too strict :)

[coveralls]

Pull Request Test Coverage Report for Build 13203634449

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 61 unchanged lines in 15 files lost coverage.
• Overall coverage decreased (-0.006%) to 64.722%

---

Files with Coverage Reduction	New Missed Lines	%
modules/gpgsqlbackend/gpgsqlbackend.cc	1	88.62%
pdns/pollmplexer.cc	1	83.66%
pdns/recursordist/recursor_cache.cc	1	84.09%
pdns/recursordist/aggressive_nsec.cc	2	66.39%
ext/json11/json11.cpp	2	62.72%
pdns/tcpiohandler.cc	2	68.18%
pdns/misc.hh	3	87.62%
pdns/misc.cc	3	63.38%
pdns/rcpgenerator.cc	3	90.37%
pdns/dnsdistdist/dnsdist-lbpolicies.cc	4	70.74%

Totals
Change from base Build 13202942838:	-0.006%
Covered Lines:	128283
Relevant Lines:	167092

---

💛 - Coveralls

[miodvallat]

Indeed most of these operations can't run if getDomainInfo fails. I have tried to make a wrapper to reduce code duplication, but I can't say I am happy with it. Feel free to suggest a nicer way to add as few lines of code as possible while still performing the check.

[mind04]

The use of those commands on a secondary zone is limited, but certainly not zero. If we do this there should be an option to force the action, for those who know what they are doing. Rectify on a secondary zone is something that should be possible anyway (inline signing)

[miodvallat]

So there's a --force option which is completely ignored at the moment. I have changed the logic to proceed on secondaries if the option is given, or fail with a message mentioning the possible use of --force if it isn't.

That ought to address all possible use cases... and give the users enough rope to shoot themselves in the feet.

[mind04]

rectify-zone, rectify-all-zones and now secure-zone must be possible without the force option or you will break about 3/4 of all the inline sign scripts on the planet with this pull.

The use of "non-primary" in the errors is confusing as it is excluding native zones. "secondary" will fit much better.

[miodvallat]

Only 3/4? I was aiming for more...

How about this new flavour?

[mind04]

Much better, two nits left.