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.

NACK, please don't use getattr() this way. Since you are subclassing
here, you should use polymorphism:

    class CAQuery(LDAPQuery):
        def execute(...):
            ...
            self.perform_action(ca_api, ca_id)
            ...

        def perform_action(self, ca_api, ca_id):
            raise NotImplementedError()

    class ca_enable(CAQuery):
        def perform_action(self, ca_api, ca_id):
            ca_api.enable_ca(ca_id)

Honza


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).

--
Martin^3 Babinsky
diff --git a/ipaserver/plugins/ca.py b/ipaserver/plugins/ca.py
index 93c4872..efeab74 100644
--- a/ipaserver/plugins/ca.py
+++ b/ipaserver/plugins/ca.py
@@ -236,25 +236,18 @@ class ca_mod(LDAPUpdate):
         return dn
 
 
-@register()
-class ca_disable(LDAPQuery):
-    __doc__ = _('Disable a CA.')
-
-    msg_summary = _('Disabled CA "%(value)s"')
+class CAQuery(LDAPQuery):
     has_output = output.standard_value
 
+    def perform_action(self, ca_api, ca_id):
+        raise NotImplementedError()
+
     def execute(self, cn, **options):
         ca_enabled_check()
 
-        if cn == IPA_CA_CN:
-            raise errors.ProtectedEntryError(
-                label=_("CA"),
-                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)
+            self.perform_action(ca_api, ca_id)
 
         return dict(
             result=True,
@@ -263,20 +256,29 @@ class ca_disable(LDAPQuery):
 
 
 @register()
-class ca_enable(LDAPQuery):
-    __doc__ = _('Enable a CA.')
+class ca_disable(CAQuery):
+    __doc__ = _('Disable a CA.')
 
-    msg_summary = _('Enabled CA "%(value)s"')
-    has_output = output.standard_value
+    msg_summary = _('Disabled CA "%(value)s"')
 
     def execute(self, cn, **options):
-        ca_enabled_check()
+        if cn == IPA_CA_CN:
+            raise errors.ProtectedEntryError(
+                label=_("CA"),
+                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.enable_ca(ca_id)
+        return super(ca_disable, self).execute(cn, **options)
 
-        return dict(
-            result=True,
-            value=pkey_to_value(cn, options),
-        )
+    def perform_action(self, ca_api, ca_id):
+        ca_api.disable_ca(ca_id)
+
+
+@register()
+class ca_enable(CAQuery):
+    __doc__ = _('Enable a CA.')
+
+    msg_summary = _('Enabled CA "%(value)s"')
+
+    def perform_action(self, ca_api, ca_id):
+        ca_api.enable_ca(ca_id)
-- 
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