On 26.11.2015 09:01, Jan Cholasta wrote: > On 11.11.2015 15:27, Petr Spacek wrote: >> On 11.11.2015 09:36, Martin Babinsky wrote: >>> On 11/11/2015 09:32 AM, Jan Cholasta wrote: >>>> On 11.11.2015 09:27, Martin Babinsky wrote: >>>>> On 11/11/2015 08:12 AM, Jan Cholasta wrote: >>>>>> On 10.11.2015 16:58, Petr Spacek wrote: >>>>>>> Hello, >>>>>>> >>>>>>> Patch 64: >>>>>>> ipa-dns-install offer IP addresses from resolv.conf as default >>>>>>> forwarders >>>>>>> >>>>>>> In non-interactive more option --auto-forwarders can be used to do the >>>>>>> same. --forward option can be used to supply additional IP addresses. >>>>>>> >>>>>>> https://fedorahosted.org/freeipa/ticket/5438 >>>>>> >>>>>> IMO it's perverse to add option which effectively means "use default >>>>>> value" instead of actually using the value as default. This is >>>>>> inconsistent with every other option and I don't see what makes >>>>>> forwarders so special to require this. >>>>>> >>>>>> NACK unless you have a strong justification for this. >> >> Motivation: >> /etc/resolv.conf holds nearest DNS servers. On the other hand, you want to >> have backup forwarder which may not be local but could work even if local >> ones >> fail. >> >> Option --default-forwarders reads list of "local" servers from resolv.conf >> and >> --forwarder option allows you to add additional IP addresses to it. >> >> So your Ansible script can contain call like: >> ipa-server-install --setup-dns --default-forwarder >> --forwarder=<company-wide-fallback> >> and you do not need to worry about mapping sites to nearest servers etc. >> >>>>> Is it possible to use default_getter decorator to fetch defaults for the >>>>> 'forwarders' knob from the resolver if it is avaliable like so (warning: >>>>> untested and possibly wrong)? >>>> >>>> Yes, this is exactly how it should be used (although the exception >>>> handling could be better). >>>> >>> That was just a quick example off the top of my head without much thought >>> going into it. >>> >>> Anyway, when running in interactive mode the installer can inform the user >>> that he found these forwarders as defaults and prompt whether they shoud be >>> used. >> >> After discussion in person we decided to not use default_getter decorator >> because that would change current behavior in an unexpected way. >> >> Original option --auto-forwarders was renamed to --default-forwarders because >> it sounds nicer :-D > > Turns out I misunderstood the intent here and after another discussion in > person we decided to go with the --auto-forwarders option. > > ACK on the original patch. > > Petr, could you please rebase patch 65 on top of current master?
Sure. I'm sorry for the delay! -- Petr^2 Spacek
From 6e046e7ecd59777f52d9ae703e1a4e1245a3af3a Mon Sep 17 00:00:00 2001 From: Petr Spacek <[email protected]> Date: Tue, 10 Nov 2015 11:22:43 +0100 Subject: [PATCH] ipa-dns-install offer IP addresses from resolv.conf as default forwarders In non-interactive more option --auto-forwarders can be used to do the same. --forward option can be used to supply additional IP addresses. https://fedorahosted.org/freeipa/ticket/5438 --- ipaserver/install/dns.py | 12 ++++++++++-- ipaserver/install/installutils.py | 7 +++++++ ipaserver/install/server/common.py | 14 ++++++++++++++ ipaserver/install/server/install.py | 7 ++++--- ipaserver/install/server/replicainstall.py | 7 ++++--- 5 files changed, 39 insertions(+), 8 deletions(-) diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py index 615bd557b2e675c3e5fa362d338a35c6b697932d..6c8e952f4ba44d64875f207c15c6a0f4bfcb05ec 100644 --- a/ipaserver/install/dns.py +++ b/ipaserver/install/dns.py @@ -2,8 +2,11 @@ # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # +from __future__ import absolute_import from __future__ import print_function +# absolute import is necessary because IPA module dns clashes with python-dns +from dns import resolver import sys from subprocess import CalledProcessError @@ -230,8 +233,13 @@ def install_check(standalone, replica, options, hostname): if options.no_forwarders: dns_forwarders = () - elif options.forwarders: - dns_forwarders = options.forwarders + elif options.forwarders or options.auto_forwarders: + if options.forwarders: + dns_forwarders = options.forwarders + else: + dns_forwarders = [] + if options.auto_forwarders: + dns_forwarders += resolver.get_default_resolver().nameservers elif standalone or not replica: dns_forwarders = read_dns_forwarders() diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py index 489d03bdad0909ffe78736f720e4376206689a6a..156c8a5eb01a2e66f4dccc4b4d71605f09406dba 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -282,6 +282,13 @@ def read_ip_addresses(): def read_dns_forwarders(): addrs = [] if ipautil.user_input("Do you want to configure DNS forwarders?", True): + print("Following DNS servers are configured in /etc/resolv.conf: %s" % + ", ".join(resolver.get_default_resolver().nameservers)) + if ipautil.user_input("Do you want to configure these servers as DNS " + "forwarders?", True): + addrs = resolver.default_resolver.nameservers[:] + print("All DNS servers from /etc/resolv.conf were added. You can " + "enter additional addresses now:") while True: ip = ipautil.user_input("Enter an IP address for a DNS forwarder, " "or press Enter to skip", allow_empty=True) diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py index 93c95dd8e8d2b24af193ee19368959188bcd6cb9..82c2c9eac253f82baeffbebfa388718dcc30d14a 100644 --- a/ipaserver/install/server/common.py +++ b/ipaserver/install/server/common.py @@ -167,6 +167,11 @@ class BaseServerDNS(common.Installable, core.Group, core.Composite): cli_name='forwarder', ) + auto_forwarders = Knob( + bool, False, + description="Use DNS forwarders configured in /etc/resolv.conf", + ) + no_forwarders = Knob( bool, False, description="Do not add any DNS forwarders, use root servers instead", @@ -395,6 +400,10 @@ class BaseServer(common.Installable, common.Interactive, core.Composite): raise RuntimeError( "You cannot specify a --forwarder option without the " "--setup-dns option") + if self.dns.auto_forwarders: + raise RuntimeError( + "You cannot specify a --auto-forwarders option without " + "the --setup-dns option") if self.dns.no_forwarders: raise RuntimeError( "You cannot specify a --no-forwarders option without the " @@ -415,6 +424,10 @@ class BaseServer(common.Installable, common.Interactive, core.Composite): raise RuntimeError( "You cannot specify a --forwarder option together with " "--no-forwarders") + elif self.dns.auto_forwarders and self.dns.no_forwarders: + raise RuntimeError( + "You cannot specify a --auto-forwarders option together with " + "--no-forwarders") elif self.dns.reverse_zones and self.dns.no_reverse: raise RuntimeError( "You cannot specify a --reverse-zone option together with " @@ -441,6 +454,7 @@ class BaseServer(common.Installable, common.Interactive, core.Composite): self.skip_schema_check = self.ca.skip_schema_check self.forwarders = self.dns.forwarders + self.auto_forwarders = self.dns.auto_forwarders self.no_forwarders = self.dns.no_forwarders self.reverse_zones = self.dns.reverse_zones self.no_reverse = self.dns.no_reverse diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 6ecb87ac9b27c78579a08de79c9df8f5ed5f114d..4af70005d45adf3d3fe98e4b77620c1458b6fe07 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -1267,10 +1267,11 @@ class Server(BaseServer): "and -a options") if self.setup_dns: #pylint: disable=no-member - if not self.dns.forwarders and not self.dns.no_forwarders: + if (not self.dns.forwarders and not self.dns.no_forwarders + and not self.dns.auto_forwarders): raise RuntimeError( - "You must specify at least one --forwarder option or " - "--no-forwarders option") + "You must specify at least one of --forwarder, " + "--auto-forwarders, or --no-forwarders options") if self.idmax < self.idstart: raise RuntimeError( diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py index e6d96bbe62c6960ebe94c529a8dac9dd0468d734..eac42dab2a3f94c4e9c4f0f2d0d1b84d4a1f0847 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -1199,10 +1199,11 @@ class Replica(BaseServer): if self.setup_dns: #pylint: disable=no-member - if not self.dns.forwarders and not self.dns.no_forwarders: + if (not self.dns.forwarders and not self.dns.no_forwarders + and not self.dns.auto_forwarders): raise RuntimeError( - "You must specify at least one --forwarder option or " - "--no-forwarders option") + "You must specify at least one of --forwarder, " + "--auto-forwarders, or --no-forwarders options") self.password = self.dm_password -- 2.4.3
From 08eab73a9a210325fa13e7a57297b41b6ad765f9 Mon Sep 17 00:00:00 2001 From: Petr Spacek <[email protected]> Date: Tue, 10 Nov 2015 16:53:10 +0100 Subject: [PATCH] Remove global variable dns_forwarders from ipaserver.install.dns --- ipaserver/install/dns.py | 27 ++++++++++++--------------- ipaserver/install/server/install.py | 5 ++--- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py index 6c8e952f4ba44d64875f207c15c6a0f4bfcb05ec..258bf5dbe46e2167e07a62127c7fd8fd4be23ee6 100644 --- a/ipaserver/install/dns.py +++ b/ipaserver/install/dns.py @@ -32,7 +32,6 @@ from ipaserver.install import odsexporterinstance from ipaserver.install import opendnssecinstance ip_addresses = [] -dns_forwarders = [] reverse_zones = [] @@ -100,7 +99,6 @@ def _disable_dnssec(): def install_check(standalone, replica, options, hostname): global ip_addresses - global dns_forwarders global reverse_zones fstore = sysrestore.FileStore(paths.SYSRESTORE) @@ -232,25 +230,24 @@ def install_check(standalone, replica, options, hostname): True, options.ip_addresses) if options.no_forwarders: - dns_forwarders = () + options.forwarders = [] elif options.forwarders or options.auto_forwarders: - if options.forwarders: - dns_forwarders = options.forwarders - else: - dns_forwarders = [] + if not options.forwarders: + options.forwarders = [] if options.auto_forwarders: - dns_forwarders += resolver.get_default_resolver().nameservers + options.forwarders += resolver.get_default_resolver().nameservers elif standalone or not replica: - dns_forwarders = read_dns_forwarders() + options.forwarders = read_dns_forwarders() # test DNSSEC forwarders - if dns_forwarders: - if (not bindinstance.check_forwarders(dns_forwarders, root_logger) and - not options.no_dnssec_validation): + if options.forwarders: + if (not bindinstance.check_forwarders(options.forwarders, + root_logger) + and not options.no_dnssec_validation): options.no_dnssec_validation = True print("WARNING: DNSSEC validation will be disabled") - root_logger.debug("will use dns_forwarders: %s\n", dns_forwarders) + root_logger.debug("will use DNS forwarders: %s\n", options.forwarders) if not standalone: search_reverse_zones = False @@ -273,7 +270,6 @@ def install_check(standalone, replica, options, hostname): def install(standalone, replica, options, api=api): global ip_addresses - global dns_forwarders global reverse_zones local_dnskeysyncd_dn = DN(('cn', 'DNSKeySync'), ('cn', api.env.host), @@ -292,7 +288,8 @@ def install(standalone, replica, options, api=api): bind = bindinstance.BindInstance(fstore, ldapi=True, api=api, autobind=AUTOBIND_ENABLED) bind.setup(api.env.host, ip_addresses, api.env.realm, api.env.domain, - dns_forwarders, conf_ntp, reverse_zones, zonemgr=options.zonemgr, + options.forwarders, conf_ntp, reverse_zones, + zonemgr=options.zonemgr, no_dnssec_validation=options.no_dnssec_validation, ca_configured=options.setup_ca) diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py index 4af70005d45adf3d3fe98e4b77620c1458b6fe07..8b33cd4b878d951949047826227342e09fa1dede 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -628,8 +628,8 @@ def install_check(installer): if options.setup_dns: print("BIND DNS server will be configured to serve IPA domain with:") print("Forwarders: %s" % ( - "No forwarders" if not dns.dns_forwarders - else ", ".join([str(ip) for ip in dns.dns_forwarders]) + "No forwarders" if not options.forwarders + else ", ".join([str(ip) for ip in options.forwarders]) )) print("Reverse zone(s): %s" % ( "No reverse zone" if options.no_reverse or not dns.reverse_zones @@ -765,7 +765,6 @@ def install(installer): options.dm_password = dm_password options.admin_password = admin_password options.host_name = host_name - options.forwarders = dns.dns_forwarders options.reverse_zones = dns.reverse_zones cache_vars = {n: getattr(options, n) for o, n in installer.knobs()} write_cache(cache_vars) -- 2.4.3
-- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
