On 10/10/2012 04:23 PM, Petr Viktorin wrote: > On 10/10/2012 03:37 PM, Martin Kosek wrote: >> On 10/10/2012 10:55 AM, Petr Viktorin wrote: >>> On 10/09/2012 06:01 PM, Petr Vobornik wrote: >>>> On 10/09/2012 05:26 PM, Petr Viktorin wrote: >>>>> On 10/09/2012 05:16 PM, Petr Viktorin wrote: >>>>>> https://fedorahosted.org/freeipa/ticket/3150 >>>>>> >>>>>> >>>>>> Patch 0086: >>>>>> I found an old unused function while working on this, the patch >>>>>> removes it. >>>>>> >>>>>> Patch 0087: >>>>>> Replica files generated on older masters don't contain the Firefox >>>>>> extension files. Skip installing them in this case. >>>>>> >>>>>> Patch 0088: >>>>>> Servers upgraded from IPA 2.2 need the Firefox extension installed. This >>>>>> is done in ipa-upgradeconfig if they're missing. >>>>>> I made the setup_firefox_extension method independent on the >>>>>> httpinstance state (which is mostly set in create_instance). >>>>>> Similarly, the files are installed ipa-replica-install if they're >>>>>> missing (i.e. skipped by the previous patch). >>>>>> If the Signing-Cert is not on this master, create an unsigned extension >>>>>> using the "zip" command. I needed to add Popen's `cwd` argument to >>>>>> ipautil.run() to get the right filenames out of zip. >>>>>> >>>>>> The patches add "copy_template_file" and "copy_file_if_exists" utilities >>>>>> I've written for some of my WIP patches, expect me to use them more when >>>>>> I get time to work on the installer code. >>>>>> >>>>> >>>>> In my previous mail I've attached an old version of patch 88. Please use >>>>> this one. Sorry! >>>>> >>>>> >>>> >>>> nack >>>> >>>> 1) patch 83-01 doesn't apply. >>> >>> There were conflicts with recent CRL and audit cert renewal patches. >>> Rebased. >>> >>>> 2) When pwd is supplied to setup_firefox_extension `db = >>>> certs.CertDB(realm, subject_base=subject_base)` is skipped and therefore >>>> `db.has_nickname` will fail. >>> >>> Thanks for the catch, fixed. >>> >> >> I tried different installation and upgrade procedures and it seems to work >> fine. I have found few minorish issues when inspecting the code: >> >> Patch 0086-01 looks OK. >> >> Patch 0087-01 looks OK. >> >> Patch 0088-02: >> >> 1) In http.setup_firefox_extension() - why do you require subject_base? >> AFAIK, >> it is not needed for the signtool and you do not have it right anyway: >> a) You use 0=$REALM, i.e. *zero*=$REALM, which would not be a valid subject >> base anyway >> b) Even when it would be used, a correct subject base is in IPA config (it >> does not have to be O=$REALM. >> >> Thus, I would not require it at all, it would safe us some code and potential >> confusion if subject base would be actually used. >> >> >> 2) [nitpick] In http.setup_firefox_extension() I would not format the string >> before logging: >> >> + root_logger.info( >> + '%s exists, skipping install of Firefox extension' % >> + target_fname) >> >> A desired pattern would be to pass formatting parameters as standard function >> parameters, it may save us few cycles in some situations. >> >> >> 3) In httpinstance.py, I would like to see an absolute path to zip >> executable. >> It is a common pattern in IPA and more secure: >> >> + ipautil.run(['zip', '-r', target_fname] + filenames, cwd=extdir) >> >> >> Martin >> > > Thanks, fixed in attached patch. >
ACK. Pushed all three patches to master, ipa-3-0. Martin _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel