<html> <head> <meta content="text/html; charset=windows-1252" http-equiv="Content-Type"> </head> <body bgcolor="#FFFFFF" text="#000000"> <div class="moz-cite-prefix">On 04/05/15 15:36, Fraser Tweedale wrote: </div> <blockquote cite="mid:20150504133641.GQ16379@dhcp-40-8.bne.redhat.com" type="cite"> <pre wrap="">Hello, Please review the first cut of the 'certprofile' command and other changes associated with the Certificate Profiles feature[1]. Custom profiles can't be used yet because 'cert-request' has not been updated, but you can manage the profiles (find, show, import, modify, delete). There's a bit more work to do on profile management and a lot more to do for using profiles and sub-CAs. I am tracking my progress on etherpad[2] so if you are reviewing check there for the TODO list and some commentary. If you want to test: for f21, please use Dogtag from my copr[2]. For f22 the required version is in updates-testing (or my copr). In summary: this is not the whole feature, just the first functional part. Since it is my first experience developing in the IPA framework I want to get patches out so you can point out all the things I did wrong or overlooked, and I can fix them. Don't hold back :) [1] <a class="moz-txt-link-freetext" href="http://www.freeipa.org/page/V4/Certificate_Profiles">http://www.freeipa.org/page/V4/Certificate_Profiles</a> [2] <a class="moz-txt-link-freetext" href="http://idm.etherpad.corp.redhat.com/rhel72-cert-mgmt-progress">http://idm.etherpad.corp.redhat.com/rhel72-cert-mgmt-progress</a> [3] <a class="moz-txt-link-freetext" href="http://copr.fedoraproject.org/coprs/ftweedal/freeipa/">http://copr.fedoraproject.org/coprs/ftweedal/freeipa/</a> </pre> <fieldset class="mimeAttachmentHeader"></fieldset> </blockquote> Thank you for patches, I have no idea what kind of dogtag magic is happening there, but I have a few comments related to IPA: Upgrade: 1) <pre wrap="">+ config.set("CA", "pki_profiles_in_ldap", "True")</pre> IMO this will work only for new installations. For upgrade you may need to add this to ipa-upgradeconfig 2) +dn: cn=certprofiles,cn=etc,$SUFFIX +changetype: add +objectClass: nsContainer +objectClass: top +cn: certprofiles IMO this will work only for new installations. For upgrade you may need to add it into update file as well, with the 'default' keyword 3) Your patch 0004 will work on new installations only. You may need to add that new step into ipa-upgradeconfig. Must be that step there during installation? If not you can create just one update file, which will be applied at the end of installation and during upgrade. Other issues: 1) I do not see modifications in API.txt file 2) We use new shorter license header # # Copyright (C) 2015 FreeIPA Contributors see COPYING for license # 3) +from ipalib.plugins.baseldap import \ + LDAPObject, LDAPSearch, LDAPCreate, LDAPDelete, LDAPUpdate, LDAPRetrieve please use 'import ( modules, .. )' instead of '\' 4) + if method == 'POST' \ + and 'content-type' not in (str(k).lower() for k in headers.viewkeys()): again, please use if ( ... ): instead \ 5) +import ipalib.errors as errors in dogtag.py Can you use from ipalib import errors, instead? 6) + def __exit__(self, exc_type, exc_value, traceback): + """Log out of the REST API""" + # TODO logout + self.cookie = None This is forgotten, or will be this fixed later? 7) + if not explanation: print resp_body # NOCOMMIT Martin^2 <pre class="moz-signature" cols="72">-- Martin Basti</pre> </body> </html>