Martin Kosek wrote:
On Mon, 2012-05-14 at 17:29 -0400, Rob Crittenden wrote:
Martin Kosek wrote:
On Fri, 2012-05-11 at 16:03 -0400, Rob Crittenden wrote:
Use our domain validator to validate the domain name we get in the
installer.
rob
I found few issues with the patch:
1) The "unexpected error" is not very user friendly error message:
# ipa-server-install
...
Server host name [vm-109.idm.lab.bos.redhat.com]:
The domain name has been calculated based on the host name.
Please confirm the domain name [idm.lab.bos.redhat.com]: bar-.com
Unexpected error - see ipaserver-install.log for details:
only letters, numbers, and - are allowed. DNS label may not start or
end with -
I think we should make it consistent with hostname validation error
message format:
# ipa-server-install
...
Server host name [vm-109.idm.lab.bos.redhat.com]: foo-
Invalid hostname 'foo-', must be fully-qualified.
2) The error is also confusing when domain is passed as an option, user
won't know what actually failed:
# ipa-server-install --domain ..
...
Server host name [vm-109.idm.lab.bos.redhat.com]:
Unexpected error - see ipaserver-install.log for details:
empty DNS label
As for value passed via option, I think we should quit immediately and
not in second or third interactive step - like we do for --zonemgr
validation:
# ipa-server-install --zonemgr=foo
Usage: ipa-server-install [options]
ipa-server-install: error: invalid zonemgr: missing address domain
This applies both for --hostname and --domain options.
Martin
Done.
There is a separate ticket to validate hostname in the parser. It is a
bit more complicated since it depends on other options so I punted on that.
rob
Nack. Domain name passed via option is not used. I assume this is the
reason:
+def domain_callback(option, opt_str, value, parser):
...
+
+ parser.values.zonemgr = value
+
Btw. callback is not necessary for domain validation, it may be done
right in parse_options() in ipa-server-install with other checks.
Martin
Ouch, that was embarrassing. Took your advise and dumped the validator,
it isn't like this is going to be shared so yeah, it's overkill.
I went back and forth whether to validate in read_domain_name() or not
and decided against it.
rob
>From c9ea84baf4294abcb6496446f995613643063fe5 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Fri, 11 May 2012 15:56:43 -0400
Subject: [PATCH] Validate on the user-provided domain name in the installer.
Wrap printing exceptions in unicode() to do Gettext conversion.
https://fedorahosted.org/freeipa/ticket/2196
---
install/tools/ipa-server-install | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 1dd02ba870a02e902c4c345d9f5802ef09f78a7a..2b78acb5e7a5cca42d906ab6427edb135f731a1a 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -62,6 +62,7 @@ from ipapython.config import IPAOptionParser
from ipalib.dn import DN
from ipalib.x509 import load_certificate_from_file, load_certificate_chain_from_file
from ipalib.constants import DNS_ZONE_REFRESH
+from ipalib.util import validate_domain_name
from ipapython import services as ipaservices
from ipapython.ipa_log_manager import *
@@ -225,6 +226,12 @@ def parse_options():
if options.admin_password is not None and len(options.admin_password) < 8:
parser.error("Admin user password must be at least 8 characters long")
+ if options.domain_name is not None:
+ try:
+ validate_domain_name(options.domain_name)
+ except ValueError, e:
+ parser.error("invalid domain: " + unicode(e))
+
if not options.setup_dns:
if options.forwarders:
parser.error("You cannot specify a --forwarder option without the --setup-dns option")
@@ -723,6 +730,10 @@ def main():
if not options.domain_name:
domain_name = read_domain_name(host_name[host_name.find(".")+1:], options.unattended)
root_logger.debug("read domain_name: %s\n" % domain_name)
+ try:
+ validate_domain_name(domain_name)
+ except ValueError, e:
+ sys.exit("Invalid domain name: %s" % unicode(e))
else:
domain_name = options.domain_name
@@ -1108,9 +1119,9 @@ try:
except Exception, e:
success = False
if uninstalling:
- message = "Unexpected error - see ipaserver-uninstall.log for details:\n %s" % str(e)
+ message = "Unexpected error - see ipaserver-uninstall.log for details:\n %s" % unicode(e)
else:
- message = "Unexpected error - see ipaserver-install.log for details:\n %s" % str(e)
+ message = "Unexpected error - see ipaserver-install.log for details:\n %s" % unicode(e)
print message
message = str(e)
for str in traceback.format_tb(sys.exc_info()[2]):
--
1.7.10.1
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
