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 >>>>> Patch 65: >>>>> Remove global variable dns_forwarders from ipaserver.install.dns >>>>> It seems to me that the global thingy is not necessary, so I've ripped >>>>> it out. >>>> >>>> ACK. Rebased version of patch 65 is attached. -- Petr^2 Spacek
From ad97c62d747eed85505d5a2a54bdca1cad531d36 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 da24a6f2f4872581f4c0dc6194614b27a4006a0d..8e2f1ba28180c4356d82a9caa17d491889c36558 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 @@ -232,8 +235,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.default_forwarders: + if options.forwarders: + dns_forwarders = options.forwarders + else: + dns_forwarders = [] + if options.default_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 1d3551f8bb9cfcac1f6fa24043aea4b5d0a07719..39b5ba6eb2f3ddbe5fd6d68537330a482e966aec 100644 --- a/ipaserver/install/installutils.py +++ b/ipaserver/install/installutils.py @@ -295,6 +295,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..45dd34c7a2ee13e0ce4fbad7fb8461fbdcb767af 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', ) + default_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.default_forwarders: + raise RuntimeError( + "You cannot specify a --default-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.default_forwarders and self.dns.no_forwarders: + raise RuntimeError( + "You cannot specify a --default-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.default_forwarders = self.dns.default_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 16539892dcffb3ad0e95aab0c5a3d85f3bb44c48..a9e02691fbd967d0558b8816bb07027260b78e80 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -1283,10 +1283,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.default_forwarders): raise RuntimeError( - "You must specify at least one --forwarder option or " - "--no-forwarders option") + "You must specify at least one of --forwarder, " + "--default-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 b01df9526bb3a7dce7abca67f85fc54cd95f6218..76dd04108f48456fd2c2862c833faa70a6604a62 100644 --- a/ipaserver/install/server/replicainstall.py +++ b/ipaserver/install/server/replicainstall.py @@ -1215,10 +1215,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.default_forwarders): raise RuntimeError( - "You must specify at least one --forwarder option or " - "--no-forwarders option") + "You must specify at least one of --forwarder, " + "--default-forwarders, or --no-forwarders options") self.password = self.dm_password -- 2.4.3
From 8c5c391ec8aca34c03f6ca49d139802a5ea51c05 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 8e2f1ba28180c4356d82a9caa17d491889c36558..43550000c3ac34a2bda898b068aa4814480c9d25 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 = [] NEW_MASTER_MARK = 'NEW_DNSSEC_MASTER' @@ -102,7 +101,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) @@ -234,25 +232,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.default_forwarders: - if options.forwarders: - dns_forwarders = options.forwarders - else: - dns_forwarders = [] + if not options.forwarders: + options.forwarders = [] if options.default_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 @@ -275,7 +272,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), @@ -294,7 +290,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 a9e02691fbd967d0558b8816bb07027260b78e80..2d0d643327944bd46d3e8b2f82ffdd307f5b2754 100644 --- a/ipaserver/install/server/install.py +++ b/ipaserver/install/server/install.py @@ -630,8 +630,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 @@ -769,7 +769,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
