[Freeipa-devel] [PATCH] 0101 Add ca-disable and ca-enable commands

[Martin Babinsky]

On 09/06/2016 04:49 PM, Fraser Tweedale wrote:
> On Tue, Aug 30, 2016 at 10:23:10AM +0200, Martin Babinsky wrote:
>> On 08/30/2016 10:09 AM, Jan Cholasta wrote:
>>> Hi,
>>>
>>> On 30.8.2016 09:56, Martin Babinsky wrote:
>>>> On 08/25/2016 10:25 AM, Fraser Tweedale wrote:
>>>>> Hi team,
>>>>>
>>>>> The attached patch fixes
>>>>> https://fedorahosted.org/freeipa/ticket/6257.
>>>>>
>>>>> The behaviour of cert-request when the CA is disabled is not very
>>>>> nice (it reports a server error from Dogtag).  The Dogtag REST
>>>>> interface gives much better errors so I plan to move to it in a
>>>>> later change (which will also address
>>>>> https://fedorahosted.org/freeipa/ticket/3473, in part).
>>>>>
>>>>> Thanks,
>>>>> Fraser
>>>>>
>>>>>
>>>>>
>>>>
>>>> HI Fraser,
>>>>
>>>> I have a couple of comments below:
>>>>
>>>> 1.)
>>>> @@ -25,6 +33,10 @@ EXAMPLES:
>>>>      ipa ca-add puppet --desc "Puppet" \\
>>>>          --subject "CN=Puppet CA,O=EXAMPLE.COM"
>>>>
>>>> +  Disable a CA.
>>>> +
>>>> +    ipa ca-disable puppet
>>>> +
>>>>  """)
>>>>
>>>> You missed an example of `ca-enable` command in the doc string.
>>>>
>>>> 2.)
>>>>
>>>> Regarding implementation of ca_enable/disable, I think you can reduce
>>>> the amount of code duplication by employing a base class which will look
>>>> up the required sub-CA and call the RA backend method required by the
>>>> subclass. See the attached untested diff (passes lint) for details.
>>
>> Looks like I forgot how to OOP while on PTO :) Honza is right, of course,
>> see the example code in the attached diff (again not tested, just a quick
>> example).
>>
> Updated patch attached, implemented inheritance suggestion and
> expanding plugin help.
>
> Thanks,
> Fraser
>

Thanks, ACK.

Pushed to:
master: c7e0dbc4e174d0bb7577de18cdb2f414f4199c57
ipa-4-4: b037e54e457d731cd16144df7573f4c85d79368a

-- 
Martin^3 Babinsky