[Freeipa-devel] [PATCH] 153 Improve hostgroup/netgroup collision checks

[Martin Kosek]

On Mon, 2011-10-17 at 10:56 -0400, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Mon, 2011-10-17 at 10:22 -0400, Rob Crittenden wrote:
> >> Martin Kosek wrote:
> >>> When the NGP plugin is enabled, a managed netgroup is created for
> >>> every hostgroup. We already check that netgroup with the same
> >>> name does not exist and provide a meaningful error message.
> >>> However, this error message was also printed when a duplicate
> >>> hostgroup existed.
> >>>
> >>> This patch checks for duplicate hostgroup existence first and
> >>> netgroup on the second place. It also makes sure that when NGP
> >>> plugin is (temporarily) disabled, a colliding netgroup cannot
> >>> be created.
> >>>
> >>> https://fedorahosted.org/freeipa/ticket/1914
> >>
> >> NACK, you should use self.obj.handle_duplicate_entry and/or
> >> self.obj.already_exists_msg for reporting errors. See my patch 898 for
> >> an example of this.
> >>
> >> rob
> >
> > I was thinking about this too. My motivation was to add a bit of
> > information why we reported a colliding hostgroup/netgroup, that they
> > share a common namespace.
> >
> > I was afraid that the error "netgroup ... already exists" when user
> > tries to add a colliding hostgroup may rise questions.
> >
> > If we go your way, we may want to add a second check I included in my
> > patch - test that when adding a new netgroup, a hostgroup of the same
> > name does not exist. This would prevent name space collisions if user
> > decides to enable NGP plugin again.
> >
> > Additionally, the DuplicateEntry exception you are rising in your patch
> > may be simplified:
> >
> > self.api.Object['netgroup'].handle_duplicate_entry(keys[-1])
> >
> > Martin
> >
> 
> Ok, I see where you were going now. ACK to your patch as-is. Go ahead 
> and push to ipa-2-1 and master.
> 
> rob

Ok, thanks. Pushed to master, ipa-2-1.

Martin