Re: [Freeipa-devel] [PATCH] 153 Improve hostgroup/netgroup collision checks
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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 153 Improve hostgroup/netgroup collision checks
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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 153 Improve hostgroup/netgroup collision checks
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 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
