On 02/18/2013 08:39 PM, Rob Crittenden wrote:
Rob Crittenden wrote:
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

And the patch.

rob


ACK, thank you!

--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to