[Freeipa-devel] [PATCH] 0051 Handle --subject option in ipa-server-install

[Martin Kosek]

On 08/08/2013 04:22 PM, Tomas Babej wrote:
> On 08/05/2013 07:05 PM, Ana Krivokapic wrote:
>> On 08/01/2013 04:52 PM, Rob Crittenden wrote:
>>> Petr Viktorin wrote:
>>>> On 08/01/2013 02:58 PM, Martin Kosek wrote:
>>>>> On 08/01/2013 02:54 PM, Petr Viktorin wrote:
>>>>>> On 07/31/2013 11:51 AM, Ana Krivokapic wrote:
>>>>>>> On 07/30/2013 06:24 PM, Petr Viktorin wrote:
>>>>>>>> On 07/30/2013 10:27 AM, Ana Krivokapic wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> This patch addresses ticket
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/3783.
>>>>>>>>>
>>>>>>>> Thanks for the patch, I have a concern below:
>>>>>>>>
>>>>>>>>> freeipa-akrivoka-0051-Handle-subject-option-in-ipa-server-install.patch
>>>>>>>>>
>>>>>>>>> diff --git a/install/tools/ipa-upgradeconfig
>>>>>>>>> b/install/tools/ipa-upgradeconfig
>>>>>>>>> index
>>>>>>>>> de17c5b23d79f31e8571a3400d44397630cadada..a2625e6198bcff0811c482e479c8af10716dcea1
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 100644
>>>>>>>>> --- a/install/tools/ipa-upgradeconfig
>>>>>>>>> +++ b/install/tools/ipa-upgradeconfig
>>>>>>>>> @@ -894,6 +895,7 @@ def main():
>>>>>>>>>          configured_constants = dogtag.configured_constants()
>>>>>>>>>          sub_dict = dict(
>>>>>>>>>              REALM=api.env.realm,
>>>>>>>>> +        SUBJECT_BASE=str(DN(('O', api.env.realm))),
>>>>>>>> When certmap.conf.template's version changes again, this will
>>>>>>>> rewrite the
>>>>>>>> subject to the default. Don't we want to use the subject base also
>>>>>>>> here?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>> You are right. The updated patch uses the current value of subject
>>>>>>> base from
>>>>>>> LDAP to update certmap.conf during upgrades.
>>>>>> When ipa-upgradeconfig is run while the DS is down, this results in a
>>>>>> small
>>>>>> warning, and very bad configuration:
>>>>>>       certmap ipaca           CN=Certificate Authority,None
>>>>>>
>>>>>>
>>>>>> I'm not sure how this should be handled. I'm adding Rob to the loop.
>>>>>> Rob, can we start the DS in ipa-upgradeconfig? That sounds quite
>>>>>> heavy-handed
>>>>>> for a RPM upgrade script.
>>>>>>
>>>>>> Maybe if the DS is unavailable, we should use the old value from the
>>>>>> config
>>>>>> file itself.
>>>>> Values can be stored/restored using a sysupgrade module.
>>>> The problem is that (AFAIK) old instances won't have this particular
>>>> value stored. Upgrading from 3.1 to a future version where certmap.conf
>>>> is modified again would fail.
>>>>
>>>>> I would not be so
>>>>> afraid of starting the DS module, we already do that exercise in
>>>>> ipa-ldap-updater, so adding it in ipa-upgradeconfig too does not
>>>>> change much.
>>>>>
>>>>> Question is, what should we do what DS cannot be started or cannot be
>>>>> read
>>>>> (e.g. when upgrade is run in fedup's chrooted environment) - we must
>>>>> make sure
>>>>> we don't mess the configuration up.
>>>> Again AFAIK, if the server is down, certmap.conf is the only place where
>>>> this value is available. I didn't check thoroughly, though.
>>> I think that's right.
>>>
>>> So the big problem here is we have no way of alerting users to possible
>>> problems found during the upgrade unless they happen to sift through
>>> ipaupgrade.log, which tends to be huge, and most users don't even know about.
>>>
>>> Part of this is due to not displaying anything during an rpm upgrade. I think
>>> we should print any exceptional errors found during the upgrade. Sure,
>>> something may eat the output, but it's a best effort sort of thing.
>>>
>>> I don't see us finding a perfect solution here. What I'd propose is this:
>>>
>>> - Store the value in sysupgrade from now on, even in new installs
>>> - Look there first on upgrade
>>> - Start dirsrv if necessary get the value
>>> - If we can't get the value, skip changing file and log loudly
>>> - Find out exactly this will present to a user by doing an upgrade from an old
>>> version to master and create an FAQ entry.
>>>
>>> I think that's about the best we can do.
>>>
>>> rob
>>>
>>> _______________________________________________
>>> Freeipa-devel mailing list
>>> Freeipa-devel at redhat.com
>>> https://www.redhat.com/mailman/listinfo/freeipa-devel
>> Updated patch tries to find the subject base value by searching in this order:
>> 1) sysupgrade
>> 2) dirsrv
>> 3) certmap.conf
>> If it cannot be found, an error is displayed and the file is not modified.
>>
>>
> 
> I tested exhaustively and patch held up against my efforts.
> 
> No objections to the code itself, so, ACK.
> 

Pushed to master, ipa-3-3.

Martin