[Freeipa-devel] [PATCH] 1085 cert-find command

[Rob Crittenden]

Petr Viktorin wrote:
> On 02/15/2013 10:43 PM, Rob Crittenden wrote:
>> Petr Viktorin wrote:
>>> On 02/06/2013 07:23 PM, Rob Crittenden wrote:
>>>> Petr Viktorin wrote:
>>>>> On 02/06/2013 12:44 AM, Rob Crittenden wrote:
>>>>>> This adds a cert-find command for the dogtag backend.
>>>>>>
>>>>>> Searches can be done by serial number, by subject, revocation reason,
>>>>>> issue date, notbefore, notafter and revocation dates.
>>>>>>
>>>>>> I added some basic tests for this. I made it a separate test file
>>>>>> because the cert plugin tests do not use the declarative format and
>>>>>> rely
>>>>>> on the selfsign backend by default.
>>>>>>
>>>>>> rob
>>>>>
>>>>> Thanks! The code works well, but I found a few issues.
>>>>>
>>>>>
>>>>> These tests don't work when the full test suite is run: test_cert adds
>>>>> and revokes additional certs that throw the code off.
>>>>> Perhaps have the tests only query valid certs? I don't see that option
>>>>> but I think it would be helpful to support.
>>>>
>>>> I added some rather nasty hacks to the test to make things pass. I
>>>> limit
>>>> the search to 10 certificates, which is the number start with by
>>>> default. There is an open dogtag ticket to return only VALID
>>>> certificates so we should be able to drop this eventually.
>>>>
>>>> I had to go further on one of the revocation tests, limiting it to a
>>>> sizelimit of 1. The count changes every time the suite runs so this is
>>>> the safest for now. It also means that one test will fail if this is
>>>> the
>>>> only part of the suite executed.
>>>
>>> This gets rid of most of the failures, but it still fails the "certs for
>>> this IPA server/short name" tests if the cert from ./make-cert is
>>> present (creating it is part of `make test`).
>>> If make-cert is run more times, it'll revoke the previous cert, so the
>>> test for revocation reason 4 fails as well.
>>>
>>> It looks like when using sizelimit Dogtag will always discard *newer*
>>> certs, ones with higher serials. Is it documented behavior or does
>>> Dogtag just happen to do that?
>>
>> It isn't documented anywhere I could find, it is just what dogtag returns
>>
>>> I wonder how other people run their tests. This solution looks like it
>>> could break easily if people do something differently :(
>>>
>>> I'm not sure how to solve this properly. Perhaps not using Declarative,
>>> and checking "by hand" that the wanted certs are in the response and the
>>> unwanted ones are not, would work better.
>>
>> I ended up switching the test class. It is not a very fine-grained set
>> of tests, mostly searching with limits and verifying that we fall within
>> a reasonable range, but it is better than nothing.
>>
>> rob
>
> This works much better, thanks! Just two nitpicks now.
>
> The patch doesn't apply well, there's a conflict in VERSION and some
> added trailing whitespace,
>
> AFAIK this would be the only (first?) test that relies on Nose's
> ordering of test modules -- tests 0011 and 0030 rely on the other cert
> tests running first. Please at least mention that in a comment. Or
> better, move class test_cert_find to test_cert.py
>
>

Rebased the patch and removed whitespace.

I went ahead and combined this with the existing test_cert file. 
Originally test_cert was only tested against lite-server but since it 
works against a live dogtag server too it makes sense to combine them.

I improved the set up documentation a little bit and tried to handle all 
the different configurations that one might see so that this should be 
runnable against either a live server or the lite-server for both the 
selfsign and dogtag backends.

This relies on the user configuring ~/.ipa/default.conf to match the 
remote server. There is no way from a client to know what kind of CA 
backend a server is running.

rob