[Freeipa-devel] [PATCH] 0055 Fix tests which fail after ipa-adtrust-install
Ana Krivokapic
akrivoka at redhat.com
Mon Aug 26 07:38:23 UTC 2013
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.
--
Regards,
Ana Krivokapic
Associate Software Engineer
FreeIPA team
Red Hat Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130826/f8339666/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-akrivoka-0055-03-Fix-tests-which-fail-after-ipa-adtrust-install.patch
Type: text/x-patch
Size: 49837 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20130826/f8339666/attachment.bin>
More information about the Freeipa-devel
mailing list