[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Pki-devel] [PATCH] 0017 Enable Authority Key Identifier CRL extension



On 12/18/2014 7:59 AM, Fraser Tweedale wrote:
On Wed, Dec 17, 2014 at 10:13:04AM -0800, Christina Fu wrote:
Hi Fraser,
Regarding CRL, I found the following:
https://groups.google.com/forum/#!topic/mozilla.dev.security.policy/ilOoDiCU4JM
So I think we can just forget it then, unless you want to install old FF to
try.
You have an ACK on this patch now.

About upgrade,  I can see that you are on the right path there with the
upgrade script, and it looks to do the thing, but since I don't have much
experience with Python, could you please ask Endi to take a closer look?

Thanks Christina.

Endi, any comments on upgrade script?

Currently if you opt out of an upgrade step it aborts the whole
process.  I think there could be scope for marking upgrade steps as
optional so that the process doesn't bail out, but I haven't
addressed that in the patch - wanted to solicit feedback first.

Cheers,

Fraser

I have some comments:

1. The upgrade script will run automatically when you install the RPM. There's no opt-out mechanism with automatic upgrade, so the behavior of existing instances will change. If this is not what we want, we should not add an upgrade script.

2. The path to CS.cfg can be constructed like this:
    cfg_path = os.path.join(subsystem.conf_dir, 'CS.cfg')

3. The existing CS.cfg should be backed up before doing anything with it using this command:
    self.backup(cfg_path)

4. Ideally the CS.cfg should be read with a proper CS.cfg parser (e.g. in case it has multi-line properties). But since the parser only exists in Java and we're only modifying a simple property this is fine.

5. If this is going to be added into 10.2.2 you should create an empty common/upgrade/10.2.2 folder with a .gitignore file (just copy from another folder).

If this is going to be added into 10.2.1 the script should be moved into server/upgrade/10.2.1 and be renamed to 02-EnableCRLAKIExtension.

This patch is conditionally ACKed pending changes to address item #2, #3, and #5.

--
Endi S. Dewata


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]