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

Add a new array allow_update_cidr in zone management #64

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

Conversation

vide
Copy link
Contributor

@vide vide commented Jan 11, 2016

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

@vide vide force-pushed the extend_allow_update branch from a5d9d1d to 64b2d53 Compare January 11, 2016 15:36
@cjeanneret
Copy link
Contributor

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}'")
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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? ?

Copy link
Contributor

Choose a reason for hiding this comment

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

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice! Thank you very much :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants