[Freeipa-devel] [PATCH] 938 consolidate external member code
Martin Kosek
mkosek at redhat.com
Mon Feb 6 14:40:25 UTC 2012
On Mon, 2012-02-06 at 09:28 -0500, Rob Crittenden wrote:
> Martin Kosek wrote:
> > On Wed, 2012-02-01 at 16:45 -0500, Rob Crittenden wrote:
> >> We had code all over the place to handle adding and removing external
> >> members from a variety of attributes. I consolidated these all into two
> >> functions in baseldap.py.
> >>
> >> This obsoletes my patch 920 but this patch includes the improved error
> >> reporting that was present.
> >>
> >> rob
> >
> > Hm, good patch! 89 insertions and 283 deletions, I like that.
> >
> > Still, I saw some minor issues that this patch introduced:
> >
> > 1) Extraneous line in failed list:
> >
> > # ipa hbacrule-show foo
> > Rule name: foo
> > Enabled: TRUE
> > External host: foo.example.com
> > # ipa hbacrule-add-sourcehost foo --hosts=foo.example.com
> > Rule name: foo
> > Enabled: TRUE
> > External host: foo.example.com
> > Failed source hosts/hostgroups:
> > member host: foo.example.com: This entry is already a member
> > member host group:<<<<<<<<
> > -------------------------
> > Number of members added 0
> > -------------------------
> >
> > 2) Empty external host list when all of its values was removed:
> >
> > # ipa hbacrule-remove-sourcehost foo --hosts=foo.example.com
> > Rule name: foo
> > Enabled: TRUE
> > External host:<<<<<<<< Empty list
> > ---------------------------
> > Number of members removed 1
> > ---------------------------
> >
> > 3) sudorule-{add|remove}-runasuser does not show failed additions:
> >
> > # ipa sudorule-add-runasuser foo --users=admin,foo --groups=admins
> > Rule name: foo
> > Enabled: TRUE
> > RunAs Users: admin
> > Groups of RunAs Users: admins
> > RunAs External User: foo
> > -------------------------
> > Number of members added 3
> > -------------------------
> >
> > # ipa sudorule-add-runasuser foo --users=admin,foo --groups=admins,foo
> > Rule name: foo
> > Enabled: TRUE
> > RunAs Users: admin
> > Groups of RunAs Users: admins
> > -------------------------
> > Number of members added 0<<<< Error messages missing
> > -------------------------
> >
> > 4) The same issue is with sudorule-{add|remove}-runasgroup:
> > # ipa sudorule-remove-runasgroup foo --groups=admins,foo
> > Rule name: foo
> > Enabled: TRUE
> > ---------------------------
> > Number of members removed 0
> > ---------------------------
> >
> > Although this problem was there before your patch, we may create a
> > separate ticket if you want.
> >
> > Martin
> >
>
> I don't think that #1 and #2 are problems. It is extraneous perhaps.
These are not major problems, but it is still an inconsistency with the
rest of the plugins. I can image tickets for that in the future.
>
> I don't see any failures in #3, what errors would you expect?
>
> In any case I don't think any of of these are caused by my changes, I'd
> prefer to open new tickets on them (and we'll have only one place to fix
> some of them).
>
> rob
In 3) I miss error message that a member could not be added. Just like
other plugin commands do. I'd expect something like this:
# ipa hbacrule-add-sourcehost foo --hosts=foo.example.com
Rule name: foo
Enabled: TRUE
Source Hosts: vm-068.idm.lab.bos.redhat.com
External host: foo.example.com
Failed source hosts/hostgroups:
member host: foo.example.com: This entry is already a member <<<<
-------------------------
Number of members added 0
-------------------------
But you are right this issue was present there before your patch. I am
OK with creating another ticket too.
Martin
More information about the Freeipa-devel
mailing list