[Freeipa-devel] [PATCH] 1012 validate domain in installer

[Rob Crittenden]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-rcrit-1012-3-validator.patch
Type: text/x-diff
Size: 2715 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20120516/6d7ad53b/attachment.bin>