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.

--
Martin^3 Babinsky
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index 93c4872..a274dbc 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -236,12 +236,31 @@ class ca_mod(LDAPUpdate):
         return dn
 
 
+class BaseCAQuery(LDAPQuery):
+    has_output = output.standard_value
+
+    def perform_action(self, cn, action='disable_ca', **options):
+        """
+        Perform action on lightweight CA using Dogtag RA backend
+        :param ca_id: sub-CA id
+        :param action: action to perform of sub-CA represented by
+            Dogtag API method
+        """
+        ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
+        with self.api.Backend.ra_lightweight_ca as ca_api:
+            getattr(ca_api, action)(ca_id)
+
+        return dict(
+            result=True,
+            value=pkey_to_value(cn, options),
+        )
+
+
 @register()
-class ca_disable(LDAPQuery):
+class ca_disable(BaseCAQuery):
     __doc__ = _('Disable a CA.')
 
     msg_summary = _('Disabled CA "%(value)s"')
-    has_output = output.standard_value
 
     def execute(self, cn, **options):
         ca_enabled_check()
@@ -252,31 +271,16 @@ class ca_disable(LDAPQuery):
                 key=cn,
                 reason=_("IPA CA cannot be disabled"))
 
-        ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
-        with self.api.Backend.ra_lightweight_ca as ca_api:
-            ca_api.disable_ca(ca_id)
-
-        return dict(
-            result=True,
-            value=pkey_to_value(cn, options),
-        )
+        return self.perform_action(cn, action='disable_ca', **options)
 
 
 @register()
-class ca_enable(LDAPQuery):
+class ca_enable(BaseCAQuery):
     __doc__ = _('Enable a CA.')
 
     msg_summary = _('Enabled CA "%(value)s"')
-    has_output = output.standard_value
 
     def execute(self, cn, **options):
         ca_enabled_check()
 
-        ca_id = self.api.Command.ca_show(cn)['result']['ipacaid'][0]
-        with self.api.Backend.ra_lightweight_ca as ca_api:
-            ca_api.enable_ca(ca_id)
-
-        return dict(
-            result=True,
-            value=pkey_to_value(cn, options),
-        )
+        return self.perform_action(cn, action='enable_ca', **options)
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to