URL: https://github.com/freeipa/freeipa/pull/492
Author: HonzaCholasta
Title: #492: [WIP] config: remove meaningless defaults
Action: synchronized
To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/492/head:pr492
git checkout pr492
From f054783ed7f632f6f676fe2d3fec1c486163e956 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 23 Feb 2017 09:44:04 +0000
Subject: [PATCH 1/6] user, migration: use LDAPClient for ad-hoc LDAP
connections
Use LDAPClient instead of ldap2 for ad-hoc remote LDAP connections in the
user_status and migrate-ds plugins.
---
ipaserver/plugins/migration.py | 15 +++++----------
ipaserver/plugins/user.py | 11 ++++-------
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/ipaserver/plugins/migration.py b/ipaserver/plugins/migration.py
index 72abd14..e8d102a 100644
--- a/ipaserver/plugins/migration.py
+++ b/ipaserver/plugins/migration.py
@@ -28,13 +28,9 @@
from ipalib.cli import to_cli
from ipalib.plugable import Registry
from .user import NO_UPG_MAGIC
-if api.env.in_server and api.env.context in ['lite', 'server']:
- try:
- from ipaserver.plugins.ldap2 import ldap2
- except Exception as e:
- raise e
from ipalib import _
from ipapython.dn import DN
+from ipapython.ipaldap import LDAPClient
from ipapython.ipautil import write_tmp_file
from ipapython.kerberos import Principal
import datetime
@@ -885,8 +881,6 @@ def execute(self, ldapuri, bindpw, **options):
return dict(result={}, failed={}, enabled=False, compat=True)
# connect to DS
- ds_ldap = ldap2(self.api, ldap_uri=ldapuri)
-
cacert = None
if options.get('cacertfile') is not None:
# store CA cert into file
@@ -894,12 +888,13 @@ def execute(self, ldapuri, bindpw, **options):
cacert = tmp_ca_cert_f.name
# start TLS connection
- ds_ldap.connect(bind_dn=options['binddn'], bind_pw=bindpw,
- cacert=cacert)
+ ds_ldap = LDAPClient(ldapuri, cacert=cacert)
+ ds_ldap.simple_bind(options['binddn'], bindpw)
tmp_ca_cert_f.close()
else:
- ds_ldap.connect(bind_dn=options['binddn'], bind_pw=bindpw)
+ ds_ldap = LDAPClient(ldapuri, cacert=cacert)
+ ds_ldap.simple_bind(options['binddn'], bindpw)
# check whether the compat plugin is enabled
if not options.get('compat'):
diff --git a/ipaserver/plugins/user.py b/ipaserver/plugins/user.py
index 88171cf..4c4b7d3 100644
--- a/ipaserver/plugins/user.py
+++ b/ipaserver/plugins/user.py
@@ -21,7 +21,6 @@
import time
from time import gmtime, strftime
import posixpath
-import os
import six
@@ -62,12 +61,10 @@
from ipalib import output
from ipaplatform.paths import paths
from ipapython.dn import DN
+from ipapython.ipaldap import LDAPClient
from ipapython.ipautil import ipa_generate_password, TMP_PWD_ENTROPY_BITS
from ipalib.capabilities import client_has_capability
-if api.env.in_server:
- from ipaserver.plugins.ldap2 import ldap2
-
if six.PY3:
unicode = str
@@ -1115,9 +1112,9 @@ def execute(self, *keys, **options):
if host == api.env.host:
other_ldap = self.obj.backend
else:
- other_ldap = ldap2(self.api, ldap_uri='ldap://%s' % host)
try:
- other_ldap.connect(ccache=os.environ['KRB5CCNAME'])
+ other_ldap = LDAPClient(ldap_uri='ldap://%s' % host)
+ other_ldap.gssapi_bind()
except Exception as e:
self.error("user_status: Connecting to %s failed with %s" % (host, str(e)))
newresult = {'dn': dn}
@@ -1162,7 +1159,7 @@ def execute(self, *keys, **options):
count += 1
if host != api.env.host:
- other_ldap.disconnect()
+ other_ldap.close()
return dict(result=entries,
count=count,
From 6ea9124b9bec0abd30236082d692f12e2b2fec1f Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 23 Feb 2017 09:52:51 +0000
Subject: [PATCH 2/6] {ca,kra}instance: drop redundant URI argument from ad-hoc
ldap2 connections
Use the default URI from api.env.ldap_uri, as it is the same as the URI
provided using the argument.
---
ipaserver/install/cainstance.py | 19 +++++--------------
ipaserver/install/krainstance.py | 4 +---
2 files changed, 6 insertions(+), 17 deletions(-)
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 6e3f995..e464263 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -706,9 +706,7 @@ def __create_ca_agent(self):
cert = x509.load_certificate(cert_data, x509.DER)
# connect to CA database
- server_id = installutils.realm_to_serverid(api.env.realm)
- dogtag_uri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % server_id
- conn = ldap2.ldap2(api, ldap_uri=dogtag_uri)
+ conn = ldap2.ldap2(api)
conn.connect(autobind=True)
# create ipara user with ipaCert certificate
@@ -1341,14 +1339,12 @@ def __update_entry_from_cert(make_filter, make_entry, dercert):
base_dn = DN(('o', 'ipaca'))
attempts = 0
- server_id = installutils.realm_to_serverid(api.env.realm)
- dogtag_uri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % server_id
updated = False
while attempts < 10:
conn = None
try:
- conn = ldap2.ldap2(api, ldap_uri=dogtag_uri)
+ conn = ldap2.ldap2(api)
conn.connect(autobind=True)
db_filter = make_filter(dercert)
@@ -1378,7 +1374,7 @@ def __update_entry_from_cert(make_filter, make_entry, dercert):
except errors.NetworkError:
syslog.syslog(
syslog.LOG_ERR,
- 'Connection to %s failed, sleeping 30s' % dogtag_uri)
+ 'Connection to %s failed, sleeping 30s' % api.env.ldap_uri)
time.sleep(30)
attempts += 1
except Exception as e:
@@ -1468,10 +1464,7 @@ def ensure_entry(dn, **attrs):
otherwise add the entry and return ``True``.
"""
- server_id = installutils.realm_to_serverid(api.env.realm)
- dogtag_uri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % server_id
-
- conn = ldap2.ldap2(api, ldap_uri=dogtag_uri)
+ conn = ldap2.ldap2(api)
if not conn.isconnected():
conn.connect(autobind=True)
@@ -1558,9 +1551,7 @@ def __get_profile_config(profile_id):
'/usr/share/ipa/profiles/{}.cfg'.format(profile_id), sub_dict)
def import_included_profiles():
- server_id = installutils.realm_to_serverid(api.env.realm)
- dogtag_uri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % server_id
- conn = ldap2.ldap2(api, ldap_uri=dogtag_uri)
+ conn = ldap2.ldap2(api)
if not conn.isconnected():
conn.connect(autobind=True)
diff --git a/ipaserver/install/krainstance.py b/ipaserver/install/krainstance.py
index d7ab6fd..8272702 100644
--- a/ipaserver/install/krainstance.py
+++ b/ipaserver/install/krainstance.py
@@ -302,9 +302,7 @@ def __create_kra_agent(self):
cert = x509.load_certificate(cert_data, x509.DER)
# connect to KRA database
- server_id = installutils.realm_to_serverid(api.env.realm)
- dogtag_uri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % server_id
- conn = ldap2.ldap2(api, ldap_uri=dogtag_uri)
+ conn = ldap2.ldap2(api)
conn.connect(autobind=True)
# create ipakra user with ipaCert certificate
From cb3108d87ea407683809545f76c921156709070d Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 23 Feb 2017 10:14:41 +0000
Subject: [PATCH 3/6] test_ldap: drop redundant URI argument
Use the default URI from api.env.ldap_uri, as it is the same as the URI
provided using the argument.
---
ipatests/test_ipaserver/test_ldap.py | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/ipatests/test_ipaserver/test_ldap.py b/ipatests/test_ipaserver/test_ldap.py
index 3a0b4b2..3975e60 100644
--- a/ipatests/test_ipaserver/test_ldap.py
+++ b/ipatests/test_ipaserver/test_ldap.py
@@ -35,7 +35,7 @@
import six
from ipaplatform.paths import paths
-from ipaserver.plugins.ldap2 import ldap2
+from ipaserver.plugins.ldap2 import ldap2, AUTOBIND_DISABLED
from ipalib import api, x509, create_api, errors
from ipapython import ipautil
from ipapython.dn import DN
@@ -52,7 +52,7 @@ class test_ldap(object):
def setup(self):
self.conn = None
- self.ldapuri = 'ldap://%s' % ipautil.format_netloc(api.env.host)
+ self.ldapuri = api.env.ldap_uri
nss.nss_init_nodb()
self.dn = DN(('krbprincipalname','ldap/%s@%s' % (api.env.host, api.env.realm)),
('cn','services'),('cn','accounts'),api.env.basedn)
@@ -65,8 +65,8 @@ def test_anonymous(self):
"""
Test an anonymous LDAP bind using ldap2
"""
- self.conn = ldap2(api, ldap_uri=self.ldapuri)
- self.conn.connect()
+ self.conn = ldap2(api)
+ self.conn.connect(autobind=AUTOBIND_DISABLED)
dn = api.env.basedn
entry_attrs = self.conn.get_entry(dn, ['associateddomain'])
domain = entry_attrs.single_value['associateddomain']
@@ -76,8 +76,8 @@ def test_GSSAPI(self):
"""
Test a GSSAPI LDAP bind using ldap2
"""
- self.conn = ldap2(api, ldap_uri=self.ldapuri)
- self.conn.connect()
+ self.conn = ldap2(api)
+ self.conn.connect(autobind=AUTOBIND_DISABLED)
entry_attrs = self.conn.get_entry(self.dn, ['usercertificate'])
cert = entry_attrs.get('usercertificate')
cert = cert[0]
@@ -95,7 +95,7 @@ def test_simple(self):
fp.close()
else:
raise nose.SkipTest("No directory manager password in %s" % pwfile)
- self.conn = ldap2(api, ldap_uri=self.ldapuri)
+ self.conn = ldap2(api)
self.conn.connect(bind_dn=DN(('cn', 'directory manager')), bind_pw=dm_password)
entry_attrs = self.conn.get_entry(self.dn, ['usercertificate'])
cert = entry_attrs.get('usercertificate')
@@ -135,8 +135,7 @@ def test_autobind(self):
"""
Test an autobind LDAP bind using ldap2
"""
- ldapuri = 'ldapi://%%2fvar%%2frun%%2fslapd-%s.socket' % api.env.realm.replace('.','-')
- self.conn = ldap2(api, ldap_uri=ldapuri)
+ self.conn = ldap2(api)
try:
self.conn.connect(autobind=True)
except errors.ACIError:
@@ -159,9 +158,9 @@ class test_LDAPEntry(object):
dn2 = DN(('cn', cn2[0]))
def setup(self):
- self.ldapuri = 'ldap://%s' % ipautil.format_netloc(api.env.host)
- self.conn = ldap2(api, ldap_uri=self.ldapuri)
- self.conn.connect()
+ self.ldapuri = api.env.ldap_uri
+ self.conn = ldap2(api)
+ self.conn.connect(autobind=AUTOBIND_DISABLED)
self.entry = self.conn.make_entry(self.dn1, cn=self.cn1)
From 75a7c9241a7b2df5c3340071f62c2054f133136b Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Thu, 23 Feb 2017 10:15:52 +0000
Subject: [PATCH 4/6] ldap2: remove unused URI argument from ldap2 constructor
---
ipapython/ipaldap.py | 19 ++++++++++---------
ipaserver/plugins/ldap2.py | 11 ++++++-----
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 82d45b9..3897b5b 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -700,7 +700,16 @@ def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False,
If true, attributes are decoded to Python types according to their
syntax.
"""
- self.ldap_uri = ldap_uri
+ if ldap_uri is not None:
+ self.ldap_uri = ldap_uri
+ self.host = 'localhost'
+ self.port = None
+ url_data = urlparse(ldap_uri)
+ self._protocol = url_data.scheme
+ if self._protocol in ('ldap', 'ldaps'):
+ self.host = url_data.hostname
+ self.port = url_data.port
+
self._start_tls = start_tls
self._force_schema_updates = force_schema_updates
self._no_schema = no_schema
@@ -708,14 +717,6 @@ def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False,
self._cacert = cacert
self._sasl_nocanon = sasl_nocanon
- self.host = 'localhost'
- self.port = None
- url_data = urlparse(ldap_uri)
- self._protocol = url_data.scheme
- if self._protocol in ('ldap', 'ldaps'):
- self.host = url_data.hostname
- self.port = url_data.port
-
self.log = log_mgr.get_logger(self)
self._has_schema = False
self._schema = None
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index e671ecb..3cff47b 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -67,20 +67,21 @@ class ldap2(CrudBackend, LDAPClient):
LDAP Backend Take 2.
"""
- def __init__(self, api, ldap_uri=None):
- if ldap_uri is None:
- ldap_uri = api.env.ldap_uri
-
+ def __init__(self, api):
force_schema_updates = api.env.context in ('installer', 'updates')
CrudBackend.__init__(self, api)
- LDAPClient.__init__(self, ldap_uri,
+ LDAPClient.__init__(self, None,
force_schema_updates=force_schema_updates)
self.__time_limit = float(LDAPClient.time_limit)
self.__size_limit = int(LDAPClient.size_limit)
@property
+ def ldap_uri(self):
+ return self.api.env.ldap_uri
+
+ @property
def time_limit(self):
if self.__time_limit is None:
return float(self.get_ipa_config().single_value.get(
From cf6b7fd5571c79a4aaf3d0f4daa4079659fff1c0 Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Tue, 10 May 2016 14:20:15 +0200
Subject: [PATCH 5/6] ipalib.constants: Remove default domain, realm, basedn,
xmlrpc_uri, ldap_uri
Domain, realm, basedn, xmlrpc_uri, ldap_uri do not have any reasonable default.
This patch removes hardcoded default so the so the code which depends
on these values blows up early and does not do crazy stuff
with default values instead of real ones.
This should help to uncover issues caused by improper ipalib
initialization.
---
ipalib/constants.py | 16 +++++++++++-----
makeaci | 3 +++
makeapi | 6 ++++++
3 files changed, 20 insertions(+), 5 deletions(-)
diff --git a/ipalib/constants.py b/ipalib/constants.py
index e64324f..0269959 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -67,9 +67,12 @@
('version', VERSION),
# Domain, realm, basedn:
- ('domain', 'example.com'),
- ('realm', 'EXAMPLE.COM'),
- ('basedn', DN(('dc', 'example'), ('dc', 'com'))),
+ # Following values do not have any reasonable default.
+ # Do not initialize them so the code which depends on them blows up early
+ # and does not do crazy stuff with default values instead of real ones.
+ # ('domain', 'example.com'),
+ # ('realm', 'EXAMPLE.COM'),
+ # ('basedn', DN(('dc', 'example'), ('dc', 'com'))),
# LDAP containers:
('container_accounts', DN(('cn', 'accounts'))),
@@ -124,9 +127,12 @@
('container_sysaccounts', DN(('cn', 'sysaccounts'), ('cn', 'etc'))),
# Ports, hosts, and URIs:
- ('xmlrpc_uri', 'http://localhost:8888/ipa/xml'),
+ # Following values do not have any reasonable default.
+ # Do not initialize them so the code which depends on them blows up early
+ # and does not do crazy stuff with default values instead of real ones.
+ # ('xmlrpc_uri', 'http://localhost:8888/ipa/xml'),
# jsonrpc_uri is set in Env._finalize_core()
- ('ldap_uri', 'ldap://localhost:389'),
+ # ('ldap_uri', 'ldap://localhost:389'),
('rpc_protocol', 'jsonrpc'),
diff --git a/makeaci b/makeaci
index 98b199c..813ecc7 100755
--- a/makeaci
+++ b/makeaci
@@ -98,6 +98,9 @@ def main(options):
plugins_on_demand=False,
basedn=DN('dc=ipa,dc=example'),
realm='IPA.EXAMPLE',
+ domain="example.com",
+ xmlrpc_uri="http://localhost:8888/ipa/xml";,
+ ldap_uri="ldap://localhost:389";,
)
from ipaserver.install.plugins import update_managed_permissions
diff --git a/makeapi b/makeapi
index d0a7295..729b922 100755
--- a/makeapi
+++ b/makeapi
@@ -40,6 +40,7 @@ from ipalib.parameters import Param
from ipalib.output import Output
from ipalib.text import Gettext, NGettext, ConcatenatedLazyText
from ipalib.capabilities import capabilities
+from ipapython.dn import DN
API_FILE='API.txt'
@@ -513,6 +514,11 @@ def main():
enable_ra=True,
mode='developer',
plugins_on_demand=False,
+ basedn=DN(('dc', 'example'), ('dc', 'com')),
+ realm="EXAMPLE.COM",
+ domain="example.com",
+ xmlrpc_uri="http://localhost:8888/ipa/xml";,
+ ldap_uri="ldap://localhost:389";,
)
api.bootstrap(**cfg)
From f9549515beb003dd2b48417736690e1e9aa23445 Mon Sep 17 00:00:00 2001
From: Jan Cholasta <jchol...@redhat.com>
Date: Tue, 21 Feb 2017 13:24:51 +0000
Subject: [PATCH 6/6] config: provide defaults for `xmlrpc_uri`, `ldap_uri` and
`basedn`
Derive the default value of `xmlrpc_uri` and `ldap_uri` from `server`.
Derive the default value of `basedn` from `domain`.
---
ipalib/config.py | 14 ++++++++++++++
ipalib/constants.py | 5 ++---
makeaci | 2 --
makeapi | 4 ----
pylint_plugins.py | 4 +++-
5 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/ipalib/config.py b/ipalib/config.py
index 388ffe8..28ed5ea 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -565,6 +565,20 @@ def _finalize_core(self, **defaults):
if 'log' not in self:
self.log = self._join('logdir', '%s.log' % self.context)
+ if 'basedn' not in self and 'domain' in self:
+ self.basedn = DN(*(('dc', dc) for dc in self.domain.split('.')))
+
+ # Derive xmlrpc_uri from server
+ # (Note that this is done before deriving jsonrpc_uri from xmlrpc_uri
+ # and server from jsonrpc_uri so that when only server or xmlrpc_uri
+ # is specified, all 3 keys have a value.)
+ if 'xmlrpc_uri' not in self and 'server' in self:
+ self.xmlrpc_uri = 'https://{}/ipa/xml'.format(self.server)
+
+ # Derive ldap_uri from server
+ if 'ldap_uri' not in self and 'server' in self:
+ self.ldap_uri = 'ldap://{}'.format(self.server)
+
# Derive jsonrpc_uri from xmlrpc_uri
if 'jsonrpc_uri' not in self:
if 'xmlrpc_uri' in self:
diff --git a/ipalib/constants.py b/ipalib/constants.py
index 0269959..71b5aa4 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -130,8 +130,9 @@
# Following values do not have any reasonable default.
# Do not initialize them so the code which depends on them blows up early
# and does not do crazy stuff with default values instead of real ones.
+ # ('server', 'localhost'),
# ('xmlrpc_uri', 'http://localhost:8888/ipa/xml'),
- # jsonrpc_uri is set in Env._finalize_core()
+ # ('jsonrpc_uri', 'http://localhost:8888/ipa/json'),
# ('ldap_uri', 'ldap://localhost:389'),
('rpc_protocol', 'jsonrpc'),
@@ -237,8 +238,6 @@
('in_server', object), # Whether or not running in-server (bool)
('logdir', object), # Directory containing log files
('log', object), # Path to context specific log file
- ('jsonrpc_uri', object), # derived from xmlrpc_uri in Env._finalize_core()
- ('server', object), # derived from jsonrpc_uri in Env._finalize_core()
)
diff --git a/makeaci b/makeaci
index 813ecc7..57622fe 100755
--- a/makeaci
+++ b/makeaci
@@ -99,8 +99,6 @@ def main(options):
basedn=DN('dc=ipa,dc=example'),
realm='IPA.EXAMPLE',
domain="example.com",
- xmlrpc_uri="http://localhost:8888/ipa/xml";,
- ldap_uri="ldap://localhost:389";,
)
from ipaserver.install.plugins import update_managed_permissions
diff --git a/makeapi b/makeapi
index 729b922..2b1d154 100755
--- a/makeapi
+++ b/makeapi
@@ -40,7 +40,6 @@ from ipalib.parameters import Param
from ipalib.output import Output
from ipalib.text import Gettext, NGettext, ConcatenatedLazyText
from ipalib.capabilities import capabilities
-from ipapython.dn import DN
API_FILE='API.txt'
@@ -514,11 +513,8 @@ def main():
enable_ra=True,
mode='developer',
plugins_on_demand=False,
- basedn=DN(('dc', 'example'), ('dc', 'com')),
realm="EXAMPLE.COM",
domain="example.com",
- xmlrpc_uri="http://localhost:8888/ipa/xml";,
- ldap_uri="ldap://localhost:389";,
)
api.bootstrap(**cfg)
diff --git a/pylint_plugins.py b/pylint_plugins.py
index 45adf71..5d7da73 100644
--- a/pylint_plugins.py
+++ b/pylint_plugins.py
@@ -94,7 +94,9 @@ def fake_class(name_or_class_obj, members=()):
'xmlrpc_uri',
'validate_api',
'startup_traceback',
- 'verbose'
+ 'verbose',
+ 'server',
+ {'domain': dir(str)},
] + LOGGING_ATTRS,
'ipalib.errors.ACIError': [
'info',
--
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
