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

[Tomas Babej]

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.

-- 
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org