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

[Martin Kosek]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-mkosek-151-5-add-zonemgr-validator.patch
Type: text/x-patch
Size: 13536 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111024/fddda8a2/attachment.bin>