[Freeipa-devel] [PATCH] 0055 Fix tests which fail after ipa-adtrust-install
Tomas Babej
tbabej at redhat.com
Wed Aug 28 13:03:05 UTC 2013
On 08/26/2013 09:38 AM, Ana Krivokapic wrote:
> On 08/22/2013 06:13 PM, Tomas Babej wrote:
>> 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
>
> I added a docstring explaining that the command depends on the
> presence of "editors" group. I also changed the behavior so the
> command now raises an exception if the "editors" group does not exist,
> instead of returning false negative.
>
> Also, please note that this command is hidden from the CLI (it can
> only be called using the API).
>
> Updated patch is attached.
>
I think this should be sufficient.
Retested, ACK!
--
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/20130828/a97df2ef/attachment.htm>
More information about the Freeipa-devel
mailing list