Martin Kosek wrote:
On Fri, 2012-02-03 at 16:58 -0500, Rob Crittenden wrote:There is some validation that we only need to apply when an entry is being created, namely the key itself. This is to allow us to manage an otherwise illegal entry that finds its way into the system (i.e. migration).Consider this. We migrate a group with a space in it. This isn't allowed in IPA but we also provide no way to delete it because the cn regex kicks out the group-del command. The trick is adding appropriate context so we can know during validation how we got here. A command object has a bases field which contains the base classes associated with it, which appears to contain only the leaf baseclass. So using this we can tell how we got to validation and can skip based on that baseclass name. I went back and forth a bit on where the right place to put this was, I'm open to more fine tuning. I initially skipped just the pattern validation then expanded it to apply to all validation in the Parameter. rob1) This patch breaks API.txt, it needs re-generating. 2) This may be a matter of opinion but I think that + if self.onlyvalidateclasses and \ + not any(x in self.onlyvalidateclasses for x in classbases): + return would be more readable than: + if self.onlyvalidateclasses and \ + not [x for x in classbases if x in self.onlyvalidateclasses]: + return 3) I would move the entire self.onlyvalidateclasses up to the validate() method. It would have several benefits: - the validation skip would be done just once, not for every value as the decision does not use the value at all - we would not have to modify _validate_scalar() methods at all since they won't need classbases 4) I think it would be better to keep validation for --rename options. As it is generated from RDN attribute parameter, it inherits onlyvalidateclasses list as well. Otherwise your patch would let user to rename a valid object to an invalid one: # ipa user-mod fbar --rename="bad boy" -------------------- Modified user "fbar" -------------------- User login: bad boy First name: Foo Last name: Bar Home directory: /home/fbar Login shell: /bin/sh UID: 480800040 GID: 480800040 Account disabled: False Password: False Member of groups: ipausers Kerberos keys available: False Martin
This should address your concerns. rob
freeipa-rcrit-940-2-validation.patch
Description: application/mbox
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
