Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fixes #3850 - Accept an array of reverse DNS zones #90

Open
wants to merge 1 commit into from

3 participants

@GregSutcliffe
Collaborator

This allows you to use a network like 10.1.0.0/23 and specify something like:

--foreman-proxy-dns-reverse=0.1.10.in-addr.arpa --foreman-proxy-dns-reverse=1.1.10.in-addr.arpa

to the installer, which then correctly generates both zones.

manifests/init.pp
@@ -109,7 +109,8 @@
#
# $dns_zone:: DNS zone name
@domcleal Owner

Shall we do the same here?

@GregSutcliffe Collaborator

Can do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@domcleal
Owner

:+1:

@GregSutcliffe
Collaborator

@domcleal dns_zone is done

@domcleal
Owner

The tests exploded.

@GregSutcliffe
Collaborator

Gah, 2 character test fix - seems to be passing now :)

@GregSutcliffe
Collaborator

@domcleal ping?

@domcleal
Owner

:+1:

@ekohl ekohl commented on the diff
manifests/init.pp
@@ -109,9 +109,11 @@
#
# $dns_interface:: DNS interface
#
-# $dns_zone:: DNS zone name
+# $dns_zone:: Array of DNS zone names
@ekohl Collaborator
ekohl added a note

Not sure I like the name being singular while the value is plural.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@GregSutcliffe
Collaborator

@ekohl shall I fix that?

@ekohl
Collaborator

@GregSutcliffe I wonder how much compatibility we want to keep. I'd prefer to have a parameter dns_zones, but not sure if we should keep dns_zone to stay compatible. Same for reverse.

@GregSutcliffe
Collaborator

I'm in favour of moving to plural and not worrying too much about backwards compat here. @domcleal ?

@domcleal
Owner

Compatibility would be a nice bonus, then we won't have to bump the module by a major release.

@GregSutcliffe
Collaborator

hmm how would we do it? $dns_zone = inline_template("<%= dns_zones.first %>") ?

@ekohl
Collaborator

I'd default dns_zone and dns_zones to some values ('' and [] come to mind), error if both are set. You can use the first non-empty value and show a deprecation notice if dns_zone is set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
10 manifests/init.pp
@@ -109,9 +109,11 @@
#
# $dns_interface:: DNS interface
#
-# $dns_zone:: DNS zone name
+# $dns_zone:: Array of DNS zone names
@ekohl Collaborator
ekohl added a note

Not sure I like the name being singular while the value is plural.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+# type:array
#
-# $dns_reverse:: DNS reverse zone name
+# $dns_reverse:: Array of DNS reverse zone names
+# type:array
#
# $dns_server:: Address of DNS server to manage
#
@@ -218,8 +220,8 @@
# Validate dns params
validate_bool($dns)
- validate_string($dns_interface, $dns_provider, $dns_reverse, $dns_server, $keyfile)
- validate_array($dns_forwarders)
+ validate_string($dns_interface, $dns_provider, $dns_server, $keyfile)
+ validate_array($dns_zone, $dns_forwarders, $dns_reverse)
# Validate bmc params
validate_bool($bmc)
View
4 manifests/params.pp
@@ -121,8 +121,8 @@
$dns_managed = true
$dns_provider = 'nsupdate'
$dns_interface = 'eth0'
- $dns_zone = $::domain
- $dns_reverse = '100.168.192.in-addr.arpa'
+ $dns_zone = [$::domain]
+ $dns_reverse = ['100.168.192.in-addr.arpa']
# localhost can resolve to ipv6 which ruby doesn't handle well
$dns_server = '127.0.0.1'
case $::operatingsystem {
View
2  spec/classes/foreman_proxy__proxydns__spec.rb
@@ -48,7 +48,7 @@
context 'with dns_zone overridden' do
let :pre_condition do
- "class {'foreman_proxy': dns_zone => 'something.example.com' }"
+ "class {'foreman_proxy': dns_zone => ['something.example.com'] }"
end
it 'should include the forward zone' do
Something went wrong with that request. Please try again.