[Freeipa-devel] [PATCH] 715 ensure required variables are required
Yi found a tricky way to remove required attributes that aren't required in the schema. The problem was we weren't enforcing parameter.required in mods (because it was enforcing that every variable with required be provided). I added a new check routine that is executed after setattr/addattr does its work and verifies that no required parameters get skipped. ticket 852 rob freeipa-rcrit-715-required.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 715 ensure required variables are required
Rob Crittenden wrote: > Yi found a tricky way to remove required attributes that aren't required > in the schema. The problem was we weren't enforcing parameter.required > in mods (because it was enforcing that every variable with required be > provided). > > I added a new check routine that is executed after setattr/addattr does > its work and verifies that no required parameters get skipped. > > ticket 852 > > rob Looks fine, works as expected. ACK I'm just not sure whether is is necessary to call the function twice - once on self.params and once on self.obj.params (I get the latter one, but I'm not sure whether the former one is necessary). Jan ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 715 ensure required variables are required
Jan Zelený wrote: > Rob Crittenden wrote: > > Yi found a tricky way to remove required attributes that aren't required > > in the schema. The problem was we weren't enforcing parameter.required > > in mods (because it was enforcing that every variable with required be > > provided). > > > > I added a new check routine that is executed after setattr/addattr does > > its work and verifies that no required parameters get skipped. > > > > ticket 852 > > > > rob > > Looks fine, works as expected. ACK > > I'm just not sure whether is is necessary to call the function twice - once > on self.params and once on self.obj.params (I get the latter one, but I'm > not sure whether the former one is necessary). > > Jan One more thing - I'm not sure whether it is necessary to add the check to LDAPCreate - I tried to create role with empty description and it failed as expected. Jan ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 715 ensure required variables are required
Jan Zelený wrote: Jan Zelený wrote: Rob Crittenden wrote: Yi found a tricky way to remove required attributes that aren't required in the schema. The problem was we weren't enforcing parameter.required in mods (because it was enforcing that every variable with required be provided). I added a new check routine that is executed after setattr/addattr does its work and verifies that no required parameters get skipped. ticket 852 rob Looks fine, works as expected. ACK I'm just not sure whether is is necessary to call the function twice - once on self.params and once on self.obj.params (I get the latter one, but I'm not sure whether the former one is necessary). Hmm, you may be right. I did it in case any of self.params had a requires on it, but since this is a mod operation then I think by definition it can't. Jan One more thing - I'm not sure whether it is necessary to add the check to LDAPCreate - I tried to create role with empty description and it failed as expected. I think you're. I did it to prevent something like this: # ipa group-add --desc='foo' --setattr description='' foo but it is already handled. I'll work up a new patch. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 715 ensure required variables are required
Rob Crittenden wrote: Jan Zelený wrote: Jan Zelený wrote: Rob Crittenden wrote: Yi found a tricky way to remove required attributes that aren't required in the schema. The problem was we weren't enforcing parameter.required in mods (because it was enforcing that every variable with required be provided). I added a new check routine that is executed after setattr/addattr does its work and verifies that no required parameters get skipped. ticket 852 rob Looks fine, works as expected. ACK I'm just not sure whether is is necessary to call the function twice - once on self.params and once on self.obj.params (I get the latter one, but I'm not sure whether the former one is necessary). Hmm, you may be right. I did it in case any of self.params had a requires on it, but since this is a mod operation then I think by definition it can't. Jan One more thing - I'm not sure whether it is necessary to add the check to LDAPCreate - I tried to create role with empty description and it failed as expected. I think you're. I did it to prevent something like this: # ipa group-add --desc='foo' --setattr description='' foo but it is already handled. I'll work up a new patch. rob Updated patch attached. rob freeipa-rcrit-715-2-required.patch Description: application/mbox ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 715 ensure required variables are required
Rob Crittenden wrote: > Rob Crittenden wrote: > > Jan Zelený wrote: > >> Jan Zelený wrote: > >>> Rob Crittenden wrote: > Yi found a tricky way to remove required attributes that aren't > required > in the schema. The problem was we weren't enforcing parameter.required > in mods (because it was enforcing that every variable with required be > provided). > > I added a new check routine that is executed after setattr/addattr > does its work and verifies that no required parameters get skipped. > > ticket 852 > > rob > >>> > >>> Looks fine, works as expected. ACK > >>> > >>> I'm just not sure whether is is necessary to call the function twice > >>> - once > >>> on self.params and once on self.obj.params (I get the latter one, but > >>> I'm > >>> not sure whether the former one is necessary). > > > > Hmm, you may be right. I did it in case any of self.params had a > > requires on it, but since this is a mod operation then I think by > > definition it can't. > > > >>> Jan > >> > >> One more thing - I'm not sure whether it is necessary to add the check > >> to LDAPCreate - I tried to create role with empty description and it > >> failed as > >> expected. > > > > I think you're. I did it to prevent something like this: > > > > # ipa group-add --desc='foo' --setattr description='' foo > > > > but it is already handled. > > > > I'll work up a new patch. > > > > rob > > Updated patch attached. > > rob ack Jan ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 715 ensure required variables are required
Jan Zeleny wrote: Rob Crittenden wrote: Rob Crittenden wrote: Jan Zelený wrote: Jan Zelený wrote: Rob Crittenden wrote: Yi found a tricky way to remove required attributes that aren't required in the schema. The problem was we weren't enforcing parameter.required in mods (because it was enforcing that every variable with required be provided). I added a new check routine that is executed after setattr/addattr does its work and verifies that no required parameters get skipped. ticket 852 rob Looks fine, works as expected. ACK I'm just not sure whether is is necessary to call the function twice - once on self.params and once on self.obj.params (I get the latter one, but I'm not sure whether the former one is necessary). Hmm, you may be right. I did it in case any of self.params had a requires on it, but since this is a mod operation then I think by definition it can't. Jan One more thing - I'm not sure whether it is necessary to add the check to LDAPCreate - I tried to create role with empty description and it failed as expected. I think you're. I did it to prevent something like this: # ipa group-add --desc='foo' --setattr description='' foo but it is already handled. I'll work up a new patch. rob Updated patch attached. rob ack Jan pushed to master ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
