allow recursion

[gcmalloc]

Allow recursion globally for certain ip or ranges.

[gcmalloc]

ok I was a little bit too much in need of a disable check name, so I also added it to this pull request :)
do you also need some test for that ?

[solarkennedy]

Yes of course, tests.

Acceptance criteria are:

• validate_array on recursion
• validate string on check_names
• Use @ syntax for variables: http://docs.puppetlabs.com/guides/templating.html#referencing-variables
• The check_names var should default to false, but have accept strings for maximum flexibility (you can do more than ignore)
• Having allow recursion logic adding in "commented" lines isn't very obvious. You should just not add them, and then write a tests. The first test is that, by default, there is no recursion lines. Then the second test, when recursion lines are requested, that they show up in the file as expected.

[solarkennedy]

I would prefer. To have one feature per pull request, but I'll keep going.

Lets start with validation. Can you add validation on the arrays to start?

[gcmalloc]

I changed the whole check name to allow for more freedom. You can specify each level independently.
The default is also enforced at puppet defautl argument level.
I added a set of test for the option, not sure it covers everything.
I totally agreed with you previous remarks and fixed them accordingly.

[gcmalloc]

test written.

[solarkennedy]

can you add additionally that the allow-recursion is in there too?

In these controlled tests, it is not going to be possible for that ip to be leaked from other things, but it might in the future.

[solarkennedy]

In the last 8 days things have changed. Can you merge or rebase against master to fix merge conflicts so Travis can run?

[gcmalloc]

rebased

[gcmalloc]

so if we are backward compatible to bind 4.9.4, the check name options is a bit different. ubuntu 12.04 is new enough to not have a bind that is that old.

[gcmalloc]

Also thanks for the changes in rspec, the test are now much simpler to read and write.

[kubashin-a]

I have a question, not on this pull request, and on dns::server::options as a whole...
Separate define is really necessary for us for implementation of dns-options or we can transfer this functionality to dns::server class?

In my current implementation setting properties of dns-options looks so:

  ## Bind (DNS) settings
  class{ "dns::server":
    forwarders      => ['8.8.8.8', '4.2.2.2'],
    allow_query     => ['172.16.0.0/12', '10.0.0.0/8', '127.0.0.1'],
    allow_recursion => ['172.16.0.0/12', '10.0.0.0/8', '127.0.0.1'],
    listen_on       => ['127.0.0.1', '10.25.2.20'],
  }

We don't need more then one block of dns-options in real life. Can be to declare this define as a class or really to move it functionality to dns::server?

[gcmalloc]

Personally, I never used multiple instance of the option defined type.

[solarkennedy]

This looks really good to me. It's got good validation, good tests, and the implementation leaves the zone file clean when you don't set anything special.

@ajjahn @danzilio Unless you have any objections I will merge.

[danzilio]

I haven't had a chance to look very closely but I trust your judgment!

On Apr 24, 2014, at 9:44 PM, Kyle Anderson notifications@github.com wrote:

This looks really good to me. It's got good validation, good tests, and the implementation leaves the zone file clean when you don't set anything special.

@ajjahn @danzilio Unless you have any objections I will merge.

—
Reply to this email directly or view it on GitHub.

[solarkennedy]

@gcmalloc Thank you for this contribution!

You worked hard to take it from a few commits to adding a lot of extra "polish" to make it a really good PR. I appreciate that.

[ajjahn]

Do what you got to do! It's great you guys are keeping things moving along.

On Apr 24, 2014, at 11:45 PM, David Danzilio notifications@github.com wrote:

I haven't had a chance to look very closely but I trust your judgment!

On Apr 24, 2014, at 9:44 PM, Kyle Anderson notifications@github.com wrote:

This looks really good to me. It's got good validation, good tests, and the implementation leaves the zone file clean when you don't set anything special.

@ajjahn @danzilio Unless you have any objections I will merge.

—
Reply to this email directly or view it on GitHub.
—
Reply to this email directly or view it on GitHub.

[solarkennedy]

😄

[gcmalloc]

thanks for everything :)