Add a new array allow_update_cidr in zone management#64
Add a new array allow_update_cidr in zone management#64vide wants to merge 2 commits intocamptocamp:masterfrom
Conversation
vide
force-pushed
the
extend_allow_update
branch
from
a5d9d1d to
64b2d53
Compare
|
Hello, Sorry for the delay — you might want to rebase on latest master so that we should be able to merge your PR. Cheers, C. |
| if @is_dynamic and @allow_update.empty? | ||
| raise(Puppet::ParseError, "allow_update is empty for dynamic zone '#{name}'") | ||
| if @is_dynamic and (@allow_update.empty? and @allow_update_cidr.empty?) | ||
| raise(Puppet::ParseError, "Both allow_update and allow_update_cidr are empty for dynamic zone '#{name}'") |
There was a problem hiding this comment.
Not sure if allowing both parameters to be set is wise…
I think we should allow only Array XOR string.
Also, we might is is_a?(Array) in order to re-use the standard parameter, avoiding the hassle to check one more param. Of course, doing so will change the check l51-52 in the bind::zone class.
There was a problem hiding this comment.
Well, IMO it's a perfectly normal scenario: I can both accept updates from a secure network via CIDR validation and from an unsecure one with the secret key.
I don't understand what you mean to use is_a?(Array), do you mean instead of .empty? ?
There was a problem hiding this comment.
Forget it, the change I proposed would break existing configuration in fact.
I'll merge shortly, and we should get a new (working) release shortly (have to add a new feature, for logging support).
There was a problem hiding this comment.
Nice! Thank you very much :)
2 participants
Add a new, dedicated array so we can specify more allow-update values like networks, a part from keys, as per http://www.zytrax.com/books/dns/ch7/xfer.html#allow-update
I created a new array to maintain backwards compatibility with the keys-only array and because they have slightly different syntax + different requirements