[Freeipa-devel] [PATCH] 151 Add --zonemgr validator

[Martin Kosek]

On Tue, 2011-10-25 at 15:57 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Mon, 2011-10-24 at 17:08 +0200, Martin Kosek wrote:
> >> On Mon, 2011-10-24 at 09:02 -0400, Rob Crittenden wrote:
> >>> Martin Kosek wrote:
> >>>> On Fri, 2011-10-21 at 11:31 -0400, Rob Crittenden wrote:
> >>>>> Martin Kosek wrote:
> >>>>>> On Fri, 2011-10-14 at 14:11 -0400, Rob Crittenden wrote:
> >>>>>>> Martin Kosek wrote:
> >>>>>>>> Do at least a basic validation of DNS zone manager mail address.
> >>>>>>>>
> >>>>>>>> Do not require '@' to be in the mail address as it is not used
> >>>>>>>> in common DNS zone configuration (in bind for example) and people
> >>>>>>>> may be used to configure it that way. '@' is always removed by the
> >>>>>>>> installer before the DNS zone is created.
> >>>>>>>>
> >>>>>>>> https://fedorahosted.org/freeipa/ticket/1966
> >>>>>>>
> >>>>>>> There is already a zonemgr_callback defined for this option, can the
> >>>>>>> verify_zonemgr call be either integrated or called from that?
> >>>>>>>
> >>>>>>> rob
> >>>>>>>
> >>>>>>
> >>>>>> Right. Please, try this one. I also added a parser error when more than
> >>>>>> one '@' is in the checked value.
> >>>>>>
> >>>>>> Martin
> >>>>>
> >>>>> A couple of things:
> >>>>>
> >>>>> In the block where you are counting @ why not add an :
> >>>>>
> >>>>> else:
> >>>>>        raise ValueError('address is not fully qualified')
> >>>>>
> >>>>> rather than looking for '.' in the result? I think it will be clearer
> >>>>> that way. I wonder if the error should contain an example as well, are
> >>>>> people going to know what a fully-qualified means?
> >>>>>
> >>>>> The regex is very strict for e-mail addresses, perhaps too much so. It
> >>>>> doesn't allow upper-case characters, periods or _, both of which are
> >>>>> allowed in login names. A common e-mail format is first.last at domain.
> >>>>>
> >>>>> rob
> >>>>
> >>>> I reorganized the validator a little and let people enter _ in the
> >>>> domain name. I also added a small explanation of what we mean by
> >>>> fully-qualified.
> >>>>
> >>>> Since we have the zonemgr validator available, why not use it for the
> >>>> DNS plugin too? I enhanced the plugin to use this validator too. Please,
> >>>> see attached patch.
> >>>>
> >>>> Martin
> >>>
> >>> NACK, a client might not have the server sub-package installed so the
> >>> import of bindinstance will fail.
> >>>
> >>> I think that moving the validator into dns.py as a central location
> >>> should work though.
> >>>
> >>> Otherwise looks good.
> >>>
> >>> rob
> >>
> >> Thanks. I didn't realize that user can have freeipa-admintools without
> >> freeipa-server installed.
> >>
> >> I placed the validator to ipalib/util.py as we cannot place it to the
> >> dns plugin directly as all our plugins assume that we have initialized
> >> api.
> >>
> >> Updated patch attached.
> >>
> >> Martin
> >
> > And now also with API.txt change. This patch must be cursed or
> > something.
> >
> > No need to bump API version, just the validator has been added.
> >
> > Martin
> 
> ACK

Pushed to master.

Martin