On 9.6.2016 08:14, David Kupka wrote:
On 08/06/16 14:44, Jan Cholasta wrote:
On 8.6.2016 14:40, Martin Babinsky wrote:
On 06/08/2016 02:11 PM, David Kupka wrote:
On 28/04/16 14:45, Jan Cholasta wrote:
Hi,

I have pushed my thin client WIP branch to GitHub:
<https://github.com/jcholast/freeipa/tree/trac-4739>.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to change
(WARNING: I will force push into this branch).

Honza


Hello!

Patch set:

spec file: require correct packages to get API plugins
schema: fix typo
schema: fix topic command output
replica install: use remote server API to create service entries
This one is not complete since it breaks replica install w/ --setup-dns.
Removing this line seems to fix this case:

"""
diff --git a/ipaserver/install/server/replicainstall.py
b/ipaserver/install/server/replicainstall.py
index 41eee96..d695a15 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1478,7 +1478,6 @@ def promote(installer):
     server_api.finalize()

     if options.setup_dns:
-        server_api.Backend.rpcclient.connect()
         server_api.Backend.ldap2.connect(autobind=True)
         dns.install(False, True, options, server_api
"""

Fixed.



schema: do not validate unrequested params in command_defaults

fixes some regressions introduced by previous patches. Works for me and
I haven't met any new regression or bug, ACK.






Thanks for the catch, Martin. Now it's really fixed and can be pushed.
Obligatory keyword to trigger the push: ACK

Thanks. Attaching the patches for reference.

Pushed to master: 0f995312565e69768c660b85476b1efe1f62fb84


BTW, I've discovered other related issue [1] that is there since 4.2. I
will send patch soon.

[1] https://fedorahosted.org/freeipa/ticket/5945



--
Jan Cholasta
From 327c3e7309f3f18536eb964afdd25f7d211ad239 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Mon, 6 Jun 2016 12:14:21 +0200
Subject: [PATCH 1/5] schema: do not validate unrequested params in
 command_defaults

Request specific params when getting the defaults instead of getting
defaults for all params and filtering the result.

This fixes command_defaults failing with validation errors on unrequested
params.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipalib/frontend.py          | 8 +++++---
 ipaserver/plugins/schema.py | 3 +--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/ipalib/frontend.py b/ipalib/frontend.py
index 81e9cd4..ffcf71b 100644
--- a/ipalib/frontend.py
+++ b/ipalib/frontend.py
@@ -670,7 +670,7 @@ class Command(HasParam):
             if kw.get(param.name, None) is None:
                 continue
 
-    def get_default(self, **kw):
+    def get_default(self, _params=None, **kw):
         """
         Return a dictionary of defaults for all missing required values.
 
@@ -687,8 +687,10 @@ class Command(HasParam):
         >>> c.get_default(color=u'Yellow')
         {}
         """
-        params = [p.name for p in self.params() if p.name not in kw and (p.required or p.autofill)]
-        return dict(self.__get_default_iter(params, kw))
+        if _params is None:
+            _params = [p.name for p in self.params()
+                       if p.name not in kw and (p.required or p.autofill)]
+        return dict(self.__get_default_iter(_params, kw))
 
     def get_default_of(self, _name, **kw):
         """
diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index 8bc2303..d66943c 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -229,8 +229,7 @@ class command_defaults(PKQuery):
             raise errors.ConversionError(name=name,
                                          error=_("must be a dictionary"))
 
-        result = command.get_default(**kw)
-        result = {n: v for n, v in result.items() if n in params}
+        result = command.get_default(params, **kw)
 
         return dict(result=result)
 
-- 
2.7.4

From 3ed6d04a80a6a7130481e23786eda19d0228bf15 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Mon, 6 Jun 2016 13:35:20 +0200
Subject: [PATCH 2/5] replica install: use remote server API to create service
 entries

Use the existing remote server API to create service entries instead of a
client API.

This fixes a crash during replica promotion due to unavailable schema.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaserver/install/dsinstance.py            |  6 +-
 ipaserver/install/installutils.py          | 23 ++------
 ipaserver/install/server/replicainstall.py | 90 +++++++++++++-----------------
 ipaserver/plugins/service.py               |  2 +-
 4 files changed, 48 insertions(+), 73 deletions(-)

diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 00ef5f3..da0e743 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -351,7 +351,7 @@ class DsInstance(service.Service):
         self.start_creation(runtime=10)
 
     def create_replica(self, realm_name, master_fqdn, fqdn,
-                       domain_name, dm_password, subject_base,
+                       domain_name, dm_password, subject_base, api,
                        pkcs12_info=None, ca_file=None,
                        ca_is_configured=None, promote=False):
         # idstart and idmax are configured so that the range is seen as
@@ -376,6 +376,7 @@ class DsInstance(service.Service):
         if ca_is_configured is not None:
             self.ca_is_configured = ca_is_configured
         self.promote = promote
+        self.api = api
 
         self.__common_setup(enable_ssl=(not self.promote))
         self.step("restarting directory server", self.__restart_instance)
@@ -1215,7 +1216,8 @@ class DsInstance(service.Service):
         except OSError:
             pass
 
-        installutils.install_service_keytab(self.principal,
+        installutils.install_service_keytab(self.api,
+                                            self.principal,
                                             self.master_fqdn,
                                             paths.DS_KEYTAB,
                                             force_service_add=True)
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 2a71ef7..b8ce068 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -49,7 +49,7 @@ from ipapython.admintool import ScriptError
 from ipapython.ipa_log_manager import root_logger
 from ipalib.util import validate_hostname
 from ipapython import config
-from ipalib import api, errors, x509
+from ipalib import errors, x509
 from ipapython.dn import DN
 from ipaserver.install import certs, service, sysupgrade
 from ipaplatform import services
@@ -1090,27 +1090,12 @@ def realm_to_ldapi_uri(realm_name):
     return 'ldapi://' + ldapurl.ldapUrlEscape(socketname)
 
 
-def install_service_keytab(principal, server, path, force_service_add=False):
-
+def install_service_keytab(api, principal, server, path,
+                           force_service_add=False):
     try:
-        api.Backend.rpcclient.connect()
-
-        # Create services if none exists (we use the .forward method
-        # here so that we can control the client version number and avoid
-        # errors. This is a workaround until the API becomes version
-        # independent: FIXME
-
-        api.Backend.rpcclient.forward(
-            'service_add',
-            krbprincipalname=principal,
-            force=force_service_add,
-            version=u'2.112'    # All the way back to 3.0 servers
-        )
+        api.Command.service_add(principal, force=force_service_add)
     except errors.DuplicateEntry:
         pass
-    finally:
-        if api.Backend.rpcclient.isconnected():
-            api.Backend.rpcclient.disconnect()
 
     args = [paths.IPA_GETKEYTAB, '-k', path, '-p', principal, '-s', server]
     ipautil.run(args)
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index e3052c1..6c0ad69 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -70,7 +70,7 @@ def make_pkcs12_info(directory, cert_name, password_name):
         return None
 
 
-def install_http_certs(config, fstore):
+def install_http_certs(config, fstore, remote_api):
 
     # Obtain keytab for the HTTP service
     fstore.backup_file(paths.IPA_KEYTAB)
@@ -80,7 +80,8 @@ def install_http_certs(config, fstore):
         pass
 
     principal = 'HTTP/%s@%s' % (config.host_name, config.realm_name)
-    installutils.install_service_keytab(principal,
+    installutils.install_service_keytab(remote_api,
+                                        principal,
                                         config.master_host_name,
                                         paths.IPA_KEYTAB,
                                         force_service_add=True)
@@ -93,8 +94,8 @@ def install_http_certs(config, fstore):
     # FIXME: need Signing-Cert too ?
 
 
-def install_replica_ds(config, options, ca_is_configured, promote=False,
-                       pkcs12_info=None):
+def install_replica_ds(config, options, ca_is_configured, remote_api,
+                       promote=False, pkcs12_info=None):
     dsinstance.check_ports()
 
     # if we have a pkcs12 file, create the cert db from
@@ -122,6 +123,7 @@ def install_replica_ds(config, options, ca_is_configured, promote=False,
         ca_is_configured=ca_is_configured,
         ca_file=ca_file,
         promote=promote,
+        api=remote_api,
     )
 
     return ds
@@ -323,19 +325,6 @@ def check_dns_resolution(host_name, dns_servers):
     return no_errors
 
 
-def check_ca_enabled(api):
-    try:
-        api.Backend.rpcclient.connect()
-        result = api.Backend.rpcclient.forward(
-            'ca_is_enabled',
-            version=u'2.112'    # All the way back to 3.0 servers
-        )
-        return result['result']
-    finally:
-        if api.Backend.rpcclient.isconnected():
-            api.Backend.rpcclient.disconnect()
-
-
 def configure_certmonger():
     messagebus = services.knownservices.messagebus
     try:
@@ -789,7 +778,7 @@ def install(installer):
             ntp.create_instance()
 
         # Configure dirsrv
-        ds = install_replica_ds(config, options, ca_enabled)
+        ds = install_replica_ds(config, options, ca_enabled, remote_api)
 
         # Always try to install DNS records
         install_dns_records(config, options, remote_api)
@@ -984,7 +973,7 @@ def promote_check(installer):
         except ipaclient.ntpconf.NTPConfigurationError:
             pass
 
-    api.bootstrap(context='installer')
+    api.bootstrap(in_server=True, context='installer')
     api.finalize()
 
     config = ReplicaConfig()
@@ -1314,58 +1303,58 @@ def promote(installer):
     pkinit_pkcs12_file = installer._pkinit_pkcs12_file
     pkinit_pkcs12_info = installer._pkinit_pkcs12_info
 
-    if installer._add_to_ipaservers:
-        ccache = os.environ['KRB5CCNAME']
-        remote_api = installer._remote_api
-        conn = remote_api.Backend.ldap2
-        try:
-            conn.connect(ccache=installer._ccache)
+    ccache = os.environ['KRB5CCNAME']
+    remote_api = installer._remote_api
+    conn = remote_api.Backend.ldap2
+    try:
+        conn.connect(ccache=installer._ccache)
 
+        if installer._add_to_ipaservers:
             remote_api.Command['hostgroup_add_member'](
                 u'ipaservers',
                 host=[unicode(api.env.host)],
             )
-        finally:
-            if conn.isconnected():
-                conn.disconnect()
-            os.environ['KRB5CCNAME'] = ccache
 
-    # Save client file and merge in server directives
-    target_fname = paths.IPA_DEFAULT_CONF
-    fstore.backup_file(target_fname)
-    ipaconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Replica Promote")
-    ipaconf.setOptionAssignment(" = ")
-    ipaconf.setSectionNameDelimiters(("[", "]"))
+        # Save client file and merge in server directives
+        target_fname = paths.IPA_DEFAULT_CONF
+        fstore.backup_file(target_fname)
+        ipaconf = ipaclient.ipachangeconf.IPAChangeConf("IPA Replica Promote")
+        ipaconf.setOptionAssignment(" = ")
+        ipaconf.setSectionNameDelimiters(("[", "]"))
 
-    config.promote = installer.promote
-    config.dirman_password = hexlify(ipautil.ipa_generate_password())
+        config.promote = installer.promote
+        config.dirman_password = hexlify(ipautil.ipa_generate_password())
 
-    # FIXME: allow to use passed in certs instead
-    if installer._ca_enabled:
-        configure_certmonger()
+        # FIXME: allow to use passed in certs instead
+        if installer._ca_enabled:
+            configure_certmonger()
 
-    # Create DS user/group if it doesn't exist yet
-    dsinstance.create_ds_user()
+        # Create DS user/group if it doesn't exist yet
+        dsinstance.create_ds_user()
 
-    # Configure ntpd
-    if not options.no_ntp:
-        ipaclient.ntpconf.force_ntpd(sstore)
-        ntp = ntpinstance.NTPInstance()
-        ntp.create_instance()
+        # Configure ntpd
+        if not options.no_ntp:
+            ipaclient.ntpconf.force_ntpd(sstore)
+            ntp = ntpinstance.NTPInstance()
+            ntp.create_instance()
 
-    try:
         # Configure dirsrv
         ds = install_replica_ds(config, options, installer._ca_enabled,
+                                remote_api,
                                 promote=True, pkcs12_info=dirsrv_pkcs12_info)
 
         # Always try to install DNS records
-        install_dns_records(config, options, api)
+        install_dns_records(config, options, remote_api)
 
         # Must install http certs before changing ipa configuration file
         # or certmonger will fail to contact the peer master
-        install_http_certs(config, fstore)
+        install_http_certs(config, fstore, remote_api)
 
     finally:
+        if conn.isconnected():
+            conn.disconnect()
+        os.environ['KRB5CCNAME'] = ccache
+
         # Create the management framework config file
         # do this regardless of the state of DS installation. Even if it fails,
         # we need to have master-like configuration in order to perform a
@@ -1476,7 +1465,6 @@ def promote(installer):
     server_api.finalize()
 
     if options.setup_dns:
-        server_api.Backend.rpcclient.connect()
         server_api.Backend.ldap2.connect(autobind=True)
         dns.install(False, True, options, server_api)
 
diff --git a/ipaserver/plugins/service.py b/ipaserver/plugins/service.py
index 80cf393..7b8f2a7 100644
--- a/ipaserver/plugins/service.py
+++ b/ipaserver/plugins/service.py
@@ -556,7 +556,7 @@ class service_add(LDAPCreate):
             raise errors.HostService()
 
         try:
-            hostresult = api.Command['host_show'](hostname)['result']
+            hostresult = self.api.Command['host_show'](hostname)['result']
         except errors.NotFound:
             raise errors.NotFound(
                 reason=_("The host '%s' does not exist to add a service to.") %
-- 
2.7.4

From 168c794ba2e74b903aebb91b10019b279e00cf43 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 7 Jun 2016 12:51:06 +0200
Subject: [PATCH 3/5] schema: fix topic command output

Return topic names as text instead of binary blob.

This fixes ipa help topic display.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/remote_plugins/schema.py |  2 ++
 ipaserver/plugins/schema.py        | 16 +++++++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index 7d1b1e4..c84994d 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -237,6 +237,8 @@ def _create_topic(schema):
         topic['doc'] = ConcatenatedLazyText(schema['doc'])
     if 'topic_topic' in schema:
         topic['topic'] = str(schema['topic_topic'])
+    else:
+        topic['topic'] = None
 
     return topic
 
diff --git a/ipaserver/plugins/schema.py b/ipaserver/plugins/schema.py
index d66943c..80c485d 100644
--- a/ipaserver/plugins/schema.py
+++ b/ipaserver/plugins/schema.py
@@ -248,9 +248,12 @@ class topic_(MetaObject):
             object.__setattr__(self, '_topic___topics', topics)
 
             for command in self.api.Command():
-                topic_name = command.topic
+                topic_value = command.topic
+                if topic_value is None:
+                    continue
+                topic_name = unicode(topic_value)
 
-                while topic_name is not None and topic_name not in topics:
+                while topic_name not in topics:
                     topic = topics[topic_name] = {'name': topic_name}
 
                     for package in self.api.packages:
@@ -267,11 +270,14 @@ class topic_(MetaObject):
                             topic['doc'] = unicode(module.__doc__).strip()
 
                         try:
-                            topic_name = module.topic
+                            topic_value = module.topic
                         except AttributeError:
-                            topic_name = None
-                        else:
+                            continue
+                        if topic_value is not None:
+                            topic_name = unicode(topic_value)
                             topic['topic_topic'] = topic_name
+                        else:
+                            topic.pop('topic_topic', None)
 
         return self.__topics
 
-- 
2.7.4

From de4f425808e1fea34153723bacfc13abf0bf49b2 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 7 Jun 2016 13:09:42 +0200
Subject: [PATCH 4/5] schema: fix typo

This fixes summary lines for commands in the help command.

https://fedorahosted.org/freeipa/ticket/4739
---
 ipaclient/remote_plugins/schema.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipaclient/remote_plugins/schema.py b/ipaclient/remote_plugins/schema.py
index c84994d..c817562 100644
--- a/ipaclient/remote_plugins/schema.py
+++ b/ipaclient/remote_plugins/schema.py
@@ -207,7 +207,7 @@ def _create_command(schema):
     command = {}
     command['name'] = name
     if 'doc' in schema:
-        command['doc'] = ConcatenatedLazyText(['doc'])
+        command['doc'] = ConcatenatedLazyText(schema['doc'])
     if 'topic_topic' in schema:
         command['topic'] = str(schema['topic_topic'])
     else:
-- 
2.7.4

From b355e4592a236330f7410dc9c4e8d697e7fddc75 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Wed, 8 Jun 2016 10:58:05 +0200
Subject: [PATCH 5/5] spec file: require correct packages to get API plugins

Since ipalib.plugins was split into ipaserver.plugins and
ipaclient.plugins, require python-ipaserver and/or python-ipaclient instead
of python-ipalib where appropriate.

https://fedorahosted.org/freeipa/ticket/4739
---
 freeipa.spec.in | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index d5d78f8..dc0ca56 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -421,11 +421,10 @@ installed on every client machine.
 Summary: IPA administrative tools
 Group: System Environment/Base
 BuildArch: noarch
-Requires: %{name}-client-common = %{version}-%{release}
 %if 0%{?with_python3}
-Requires: python3-ipalib = %{version}-%{release}
+Requires: python3-ipaclient = %{version}-%{release}
 %else
-Requires: python2-ipalib = %{version}-%{release}
+Requires: python2-ipaclient = %{version}-%{release}
 %endif
 Requires: python-ldap
 
@@ -596,8 +595,8 @@ BuildArch: noarch
 Obsoletes: %{name}-tests < 4.2.91
 Provides: %{name}-tests = %{version}-%{release}
 %{?python_provide:%python_provide python2-ipatests}
-Requires: %{name}-client-common = %{version}-%{release}
-Requires: python2-ipalib = %{version}-%{release}
+Requires: python2-ipaclient = %{version}-%{release}
+Requires: python2-ipaserver = %{version}-%{release}
 Requires: tar
 Requires: xz
 Requires: python-nose
@@ -630,8 +629,9 @@ This package contains tests that verify IPA functionality.
 Summary: IPA tests and test tools
 BuildArch: noarch
 %{?python_provide:%python_provide python3-ipatests}
-Requires: %{name}-client-common = %{version}-%{release}
-Requires: python3-ipalib = %{version}-%{release}
+Requires: python3-ipaclient = %{version}-%{release}
+# FIXME: uncomment once there's python3-ipaserver
+#Requires: python3-ipaserver = %{version}-%{release}
 Requires: tar
 Requires: xz
 Requires: python3-nose
-- 
2.7.4

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