[Freeipa-devel] [PATCH 0069] ipa-replica-install support caless install with promotion.
Jan Cholasta
jcholast at redhat.com
Mon Nov 30 16:24:56 UTC 2015
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
More information about the Freeipa-devel
mailing list