On Wed, 2012-05-16 at 14:04 -0400, Rob Crittenden wrote: > 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
Yup, this one is ok :-) ACK, pushed to master. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
