[Freeipa-devel] [PATCH] 0055 Fix tests which fail after ipa-adtrust-install
Tomas Babej
tbabej at redhat.com
Thu Aug 22 16:13:19 UTC 2013
On 08/20/2013 04:14 PM, Ana Krivokapic wrote:
> On 08/09/2013 05:35 PM, Tomas Babej wrote:
>> On 08/09/2013 04:03 PM, Ana Krivokapic wrote:
>>> On 08/09/2013 09:39 AM, Tomas Babej wrote:
>>>> On 08/08/2013 04:09 PM, Ana Krivokapic wrote:
>>>>> Hello,
>>>>>
>>>>> This patch should fix the failing unit tests.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/3852
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Freeipa-devel mailing list
>>>>> Freeipa-devel at redhat.com
>>>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>>>>
>>>> There are two tests failing on my machine when running the tests
>>>> after ipa-adtrust-install with your patch applied:
>>>
>>> You say there are two tests failing but I only see one below.
>>>
>>
>> That was just debris from trying to break your patch too much, one of
>> my comments rendered invalid in the end :)
>>
>>>>
>>>> ======================================================================
>>>> FAIL: test_group[24]: group_find: Search for POSIX groups
>>>> ----------------------------------------------------------------------
>>>> Traceback (most recent call last):
>>>> [...]
>>>> AssertionError: assert_deepequal: dict keys mismatch.
>>>> test_group[24]: group_find: Search for POSIX groups
>>>> missing keys = []
>>>> extra keys = ['ipantsecurityidentifier']
>>>> expected = {'dn':
>>>> ipapython.dn.DN('cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com'),
>>>> 'cn': [u'editors'], 'objectclass': Fuzzy(None, None, <function
>>>> <lambda> at 0x3768c08>), 'gidnumber': [Fuzzy('^\\d+$', <type
>>>> 'basestring'>, None)], 'ipauniqueid':
>>>> [Fuzzy('^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$',
>>>> <type 'unicode'>, None)], 'description': [u'Limited admins who can
>>>> edit other users']}
>>>> got = {'dn':
>>>> u'cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com',
>>>> 'cn': (u'editors',), 'objectclass': (u'top', u'groupofnames',
>>>> u'posixgroup', u'ipausergroup', u'ipaobject', u'nestedGroup',
>>>> u'ipantgroupattrs'), 'ipantsecurityidentifier':
>>>> (u'S-1-5-21-1457515837-642396627-3509099663-1002',), 'gidnumber':
>>>> (u'1804600002',), 'ipauniqueid':
>>>> (u'7c6e1672-0039-11e3-9567-001a4a2221fb',), 'description':
>>>> (u'Limited admins who can edit other users',)}
>>>> path = ('result', 1)
>>>>
>>>> I think you need the wrap the dictionary discribing the editor's
>>>> group entry with the add_sid wrapper, and its objectclasses using
>>>> the add_oc wrapper.
>>>>
>>>> [tbabej at vm-139 freeipa]$ git diff
>>>> diff --git a/ipatests/test_xmlrpc/test_group_plugin.py
>>>> b/ipatests/test_xmlrpc/test_group_plugin.py
>>>> index d380fe5..14c70cd 100644
>>>> --- a/ipatests/test_xmlrpc/test_group_plugin.py
>>>> +++ b/ipatests/test_xmlrpc/test_group_plugin.py
>>>> @@ -447,14 +447,15 @@ class test_group(Declarative):
>>>> objectclasses.posixgroup,
>>>> u'ipantgroupattrs')),
>>>> 'ipauniqueid': [fuzzy_uuid],
>>>> }),
>>>> - {
>>>> + add_sid({
>>>> 'dn': get_group_dn('editors'),
>>>> 'gidnumber': [fuzzy_digits],
>>>> 'cn': [u'editors'],
>>>> 'description': [u'Limited admins who can
>>>> edit other users'],
>>>> - 'objectclass':
>>>> fuzzy_set_ci(objectclasses.posixgroup),
>>>> + 'objectclass': fuzzy_set_ci(add_oc(
>>>> + objectclasses.posixgroup,
>>>> u'ipantgroupattrs')),
>>>> 'ipauniqueid': [fuzzy_uuid],
>>>> - },
>>>> + }),
>>>> dict(
>>>> dn=get_group_dn(group1),
>>>> cn=[group1],
>>>>
>>>>
>>>> These changes were sufficient for me to have the unit test suite
>>>> run without errors.
>>>> --
>>>> Tomas Babej
>>>> Associate Software Engeneer | Red Hat | Identity Management
>>>> RHCE | Brno Site | IRC: tbabej | freeipa.org
>>>
>>> I retested the patch and the tests are passing in my setup. The
>>> editors group definitely does not have the ipantsecurityidentifier
>>> attribute nor the ipantgroupattrs objectclass:
>>>
>>> [akrivoka at vm-181 freeipa]$ ipa group-show editors --all
>>> dn:
>>> cn=editors,cn=groups,cn=accounts,dc=idm,dc=lab,dc=eng,dc=brq,dc=redhat,dc=com
>>> Group name: editors
>>> Description: Limited admins who can edit other users
>>> GID: 1977000002
>>> ipauniqueid: 91b3597e-00f3-11e3-92ae-001a4a22217b
>>> objectclass: top, groupofnames, posixgroup, ipausergroup,
>>> ipaobject, nestedGroup
>>>
>>> What I noticed though, is that if I delete and re-create the editors
>>> group (after ipa-adtrust-install has been run), it then gets the
>>> above mentioned attribute and objectclass. Maybe you did some
>>> similar manipulation in your setup, resulting in the test failing?
>>>
>> I think it does depend on whether you have ran the ipa-sidgen task
>> when running the ipa-adtrust-install.
>>
>> Do you think we can cover both cases here?
>>
>>>
>>> --
>>> Regards,
>>>
>>> Ana Krivokapic
>>> Associate Software Engineer
>>> FreeIPA team
>>> Red Hat Inc.
>>
>>
>> --
>> Tomas Babej
>> Associate Software Engeneer | Red Hat | Identity Management
>> RHCE | Brno Site | IRC: tbabej | freeipa.org
>
> Updated patch should detect the situation when ipa-sidgen task was
> run, and add the required attribute/objectclass accordingly.
>
> --
> Regards,
>
> Ana Krivokapic
> Associate Software Engineer
> FreeIPA team
> Red Hat Inc.
+
+
+class sidgen_was_run(Command):
+ NO_CLI = True
+
+ __doc__ = _('Determine whether ipa-adtrust-install has been run with '
+ 'sidgen task')
+
+ def execute(self, *keys, **options):
+ ldap = self.api.Backend.ldap2
+ editors_dn = DN(
+ ('cn', 'editors'),
+ ('cn', 'groups'),
+ ('cn', 'accounts'),
+ api.env.basedn
+ )
+
+ try:
+ editors_entry = ldap.get_entry(editors_dn)
+ except errors.NotFound:
+ return dict(result=False)
+
+ attr = editors_entry.get('ipaNTSecurityIdentifier')
+ if not attr:
+ return dict(result=False)
+
+ return dict(result=True)
+
+api.register(sidgen_was_run)
The problem with this detection is that it uses the editors group, which
might not exist,
and in such case, it might return improper result (false negative for
IPA server without
editors group).
It works well for our use case in the unit tests, since other unit tests
expect that this
group exists as well. However, if we're adding a new command to the API,
I'd prefer
something more reliable, so that it can be reused it the future.
Unfortunately, I was unable to find any way how to detect this. Running
ipa-sidgen-task
on the server leaves no permanent trace in the LDAP, and we setup the
task configuration
in either case.
So without setting a flag somewhere (which would be IMHO overkill for
this use case)
I guess we're left with the approach implemented in the patch.
I'm personally OK with that, but we should include docstrings that point
out that this
command might provide false negatives in case the editors group does not
exist.
If nobody has objections, with the documentation improved this patch has
an ACK
from me.
--
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130822/0cf88496/attachment.htm>
More information about the Freeipa-devel
mailing list