On 11/30/2015 09:25 AM, Jan Cholasta wrote:
> On 26.11.2015 14:36, Tomas Babej wrote:
>>
>>
>> On 11/26/2015 01:33 PM, Jan Cholasta wrote:
>>> On 25.11.2015 09:01, Jan Cholasta wrote:
>>>> On 24.11.2015 15:56, Tomas Babej wrote:
>>>>>
>>>>>
>>>>> On 11/23/2015 04:43 PM, Jan Cholasta wrote:
>>>>>> 3)
>>>>>>
>>>>>> +    host_name = Knob(
>>>>>> +        str, None,
>>>>>> +        description="fully qualified name of this host",
>>>>>> +        cli_name='hostname',
>>>>>> +    )
>>>>>>
>>>>>> This knob is already defined in BaseServer, there's no need to
>>>>>> redefine
>>>>>> it here (just remove the "host_name = None").
>>>>>>
>>>>>> If you want to change the description, change it in BaseServer.
>>>>>
>>>>> Yep, that was the reasoning here. Changed in BaseServer.
> 
> You can remove the duplicate knob from Replica, since it is inherited
> from BaseServer.
> 

Yeah, this is mine brainfart. Removed.

>>>>>> 5) The replica file argument conflicts with the --realm, --domain,
>>>>>> --server, --admin-password and --principal options. This should be
>>>>>> checked in Replica.__init__().
>>>>>>
>>>>>> The --hostname option should either be conflicting as well or be
>>>>>> implemented properly for legacy replica install.
>>>>>>
>>>>>
>>>>> All of them now conflict --replica-file.
>>>>
>>>> Replica file is not an option but rather an argument in
>>>> ipa-replica-install.
>>>>
>>>> I think the error message should use the same wording which is used for
>>>> --forwarder vs. --no-forwarder and --reverse-zone vs. --no-reverse:
>>>> "You
>>>> cannot specify a --something option together with replica file".
> 
> Nitpick: the namedtuple seems like an overkill, given it's only used in
> a single place. (But I'm fine with keeping it.)
> 

This is just to improve the readability (by a small bit).

>>>>
>>>>>
>>>>>>
>>>>>> 6) I think --admin-password should be renamed to --password and the
>>>>>> description changed, since it now also allows OTP enrollment.
>>>>>>
>>>>>
>>>>> I changed the requirements to mandate --principal when --password is
>>>>> used, as we discussed.
>>>>
>>>> I was wrong here, sorry.
>>>>
>>>> ipa-replica-install assumes "admin" for principal by default, so we
>>>> just
>>>> need to make sure ipa-client-install is called with --principal when
>>>> password is used, whether it's the provided principal or the default
>>>> "admin".
> 
> ipa-replica-install assumes "admin", but it's not the default value of
> --principal, so it may be None here:
> 
> +        if installer.admin_password:
> +            args.extend(["--password", installer.admin_password])
> +            args.extend(["--principal", installer.principal])
> 
> 
> This should be removed:
> 
> +            # Make sure --password is not used without principal,
> +            # that would mean OTP join for ipa-client-install
> +
> +            password_and_principal = [self.admin_password, self.principal]
> +            if any(password_and_principal) and not
> all(password_and_principal):
> +                raise RuntimeError(
> +                    "The --password and --principal options must be used "
> +                    "together."
> +                )
> 

Same here, fixed.

Updated patch attached.

Tomas
From c7662f4ec93c1cff954fce10c87b2e05027e51ec Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Mon, 23 Nov 2015 12:46:15 +0100
Subject: [PATCH] replicainstall: Add possiblity to install client in one
 command

https://fedorahosted.org/freeipa/ticket/5310
---
 ipaserver/install/server/common.py         |  2 +-
 ipaserver/install/server/replicainstall.py | 92 +++++++++++++++++++++++++++---
 2 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py
index 93c95dd8e8d2b24af193ee19368959188bcd6cb9..6116ebc25fc1d439a515d260feed82fc134f1d97 100644
--- a/ipaserver/install/server/common.py
+++ b/ipaserver/install/server/common.py
@@ -275,7 +275,7 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
 
     host_name = Knob(
         str, None,
-        description="fully qualified name of server",
+        description="fully qualified name of this host",
         cli_name='hostname',
     )
 
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index e6d96bbe62c6960ebe94c529a8dac9dd0468d734..6aaf587002473aeab4eda35e7bf49f2292360c03 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -4,6 +4,7 @@
 
 from __future__ import print_function
 
+import collections
 import dns.exception as dnsexception
 import dns.name as dnsname
 import dns.resolver as dnsresolver
@@ -751,6 +752,51 @@ def install(installer):
     remove_replica_info_dir(installer)
 
 
+def ensure_enrolled(installer):
+    config = installer._config
+
+    # Perform only if we have the necessary options
+    if not any([installer.admin_password, installer.keytab]):
+        sys.exit("IPA client is not configured on this system.\n"
+                 "You must use a replica file or join the system "
+                 "either by using by running 'ipa-client-install'. "
+                 "Alternatively, you may specify enrollment related options "
+                 "directly, see man ipa-replica-install.")
+
+    # Call client install script
+    service.print_msg("Configuring client side components")
+    try:
+        args = [paths.IPA_CLIENT_INSTALL, "--unattended"]
+        if installer.domain_name:
+            args.extend(["--domain", installer.domain_name])
+        if installer.server:
+            args.extend(["--server", installer.server])
+        if installer.realm_name:
+            args.extend(["--realm", installer.realm_name])
+        if installer.host_name:
+            args.extend(["--hostname", installer.host_name])
+
+        if installer.admin_password:
+            args.extend(["--password", installer.admin_password])
+            args.extend(["--principal", installer.principal or "admin"])
+        if installer.keytab:
+            args.extend(["--keytab", installer.keytab])
+
+        if installer.no_dns_sshfp:
+            args.append("--no-dns-sshfp")
+        if installer.ssh_trust_dns:
+            args.append("--ssh-trust-dns")
+        if installer.no_ssh:
+            args.append("--no-ssh")
+        if installer.no_sshd:
+            args.append("--no-sshd")
+        if installer.mkhomedir:
+            args.append("--mkhomedir")
+        ipautil.run(args)
+    except Exception as e:
+        sys.exit("Configuration of client side components failed!\n"
+                 "ipa-client-install returned: " + str(e))
+
 @common_cleanup
 def promote_check(installer):
     options = installer
@@ -761,9 +807,7 @@ def promote_check(installer):
 
     client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
     if not client_fstore.has_files():
-        sys.exit("IPA client is not configured on this system.\n"
-                 "You must use a replica file or join the system "
-                 "using 'ipa-client-install'.")
+        ensure_enrolled(installer)
 
     sstore = sysrestore.StateFile(paths.SYSRESTORE)
 
@@ -1108,9 +1152,6 @@ class Replica(BaseServer):
         description="a file generated by ipa-replica-prepare",
     )
 
-    realm_name = None
-    domain_name = None
-
     setup_ca = Knob(BaseServer.setup_ca)
     setup_kra = Knob(BaseServer.setup_kra)
     setup_dns = Knob(BaseServer.setup_dns)
@@ -1130,12 +1171,16 @@ class Replica(BaseServer):
 
     admin_password = Knob(
         BaseServer.admin_password,
-        description="Admin user Kerberos password used for connection check",
+        description="Kerberos password for the specified admin principal",
         cli_short_name='w',
     )
 
+    server = Knob(
+        str, None,
+        description="fully qualified name of IPA server to enroll to",
+    )
+
     mkhomedir = Knob(BaseServer.mkhomedir)
-    host_name = None
     no_host_dns = Knob(BaseServer.no_host_dns)
     no_ntp = Knob(BaseServer.no_ntp)
     no_pkinit = Knob(BaseServer.no_pkinit)
@@ -1153,10 +1198,17 @@ class Replica(BaseServer):
     principal = Knob(
         str, None,
         sensitive=True,
-        description="User Principal allowed to promote replicas",
+        description="User Principal allowed to promote replicas "
+                    "and join IPA realm",
         cli_short_name='P',
     )
 
+    keytab = Knob(
+        str, None,
+        description="path to backed up keytab from previous enrollment",
+        cli_short_name='k',
+    )
+
     promote = False
 
     # ca
@@ -1197,6 +1249,28 @@ class Replica(BaseServer):
                 raise RuntimeError("Replica file %s does not exist"
                                    % self.replica_file)
 
+            CLIKnob = collections.namedtuple('CLIKnob', ('value', 'name'))
+
+            conflicting_knobs = (
+                CLIKnob(self.realm_name, '--realm'),
+                CLIKnob(self.domain_name, '--domain'),
+                CLIKnob(self.host_name, '--hostname'),
+                CLIKnob(self.server, '--server'),
+                CLIKnob(self.admin_password, '--admin-password'),
+                CLIKnob(self.principal, '--principal'),
+            )
+
+            if any([k.value is not None for k in conflicting_knobs]):
+                conflicting_knob_names = [
+                    knob.name for knob in conflicting_knobs
+                    if knob.value is not None
+                ]
+
+                raise RuntimeError(
+                    "You cannot specify '{0}' option(s) with replica file."
+                    .format(", ".join(conflicting_knob_names))
+                    )
+
         if self.setup_dns:
             #pylint: disable=no-member
             if not self.dns.forwarders and not self.dns.no_forwarders:
-- 
2.5.0

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