Hi,

On 27.11.2015 07:57, David Kupka wrote:
On 26/11/15 15:22, David Kupka wrote:
On 26/11/15 15:13, David Kupka wrote:
On 26/11/15 15:01, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5441


Replaced accidentally inserted tabs.



Fixed indentation I screwed up when replacing tabs :-/

1) The deprecated --*_pkcs12 and --*_pin aliases should not be supported in ipa-replica-install.


2) This check from ipa-replica-prepare should be added to Replica.__init__() as well:

        # If any of the PKCS#12 options are selected, all are required.
cert_file_req = (options.dirsrv_cert_files, options.http_cert_files)
        cert_file_opt = (options.pkinit_cert_files,)
        if any(cert_file_req + cert_file_opt) and not all(cert_file_req):
            self.option_parser.error(
"--dirsrv-cert-file and --http-cert-file are required if any "
                "PKCS#12 options are used.")


3) This check from ipa-replica-prepare should be added below the pkcs12_info initialization block in promote_check():

        if (options.http_cert_files and options.dirsrv_cert_files and
            http_ca_cert != dirsrv_ca_cert):
            raise admintool.ScriptError(
                "Apache Server SSL certificate and Directory Server SSL "
                 "certificate are not signed by the same CA certificate")


4) This check should use the same message as ipa-replica-prepare: "Cannot issue certificates: a CA is not installed. Use the --http-cert-file, --dirsrv-cert-file options to provide custom certificates.":

+            if not options.dirsrv_cert_files:
+                root_logger.error("The remote master does not have a CA "
+                                  "installed, can't proceed without certs")
+                sys.exit(3)


5) Please use the common "You cannot specify a --option together with replica file" error message here:

+            if any(self.ca.dirsrv_pkcs12_file, self.ca.http_pkcs12_file,
+                self.ca.pkinit_pkcs12_file):
+ raise RuntimeError("You cannot provide certificates together "
+                                   "with replica file")


6) Please make the ca_is_enabled argument of install_replica_ds() and install_http() mandatory and fill as appropriate when called, it will make the code more readable.


7)

$ git diff -U0 | pep8 --diff
./ipaserver/install/server/replicainstall.py:99:80: E501 line too long (82 > 79 characters) ./ipaserver/install/server/replicainstall.py:161:80: E501 line too long (82 > 79 characters) ./ipaserver/install/server/replicainstall.py:1289:13: E265 block comment should start with '# ' ./ipaserver/install/server/replicainstall.py:1291:17: E125 continuation line with same indent as next logical line ./ipaserver/install/server/replicainstall.py:1291:17: E128 continuation line under-indented for visual indent


8) Nitpicks:

s/ca_configured/ca_is_configured/ in install_replica_ds(), for consistency.

Set ca_enabled = False in the else branch rather than before the if statement in promote_check().

Put the "#pylint: disable=no-member" in Replica.__init__() in the same spot as it is in Server.__init__().


Honza

--
Jan Cholasta

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