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 a5bfc0b734466ea5a8a9447fd1a916fa85462922 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 f77a3d6f811c20e46b6a61e4e8a20b1e447b0ed5 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 3c86b91..b79b432 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -703,9 +703,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
@@ -1347,14 +1345,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)
@@ -1384,7 +1380,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:
@@ -1474,10 +1470,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)
@@ -1564,9 +1557,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 ec38801..ec5a9ae 100644
--- a/ipaserver/install/krainstance.py
+++ b/ipaserver/install/krainstance.py
@@ -296,9 +296,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 b5c5eab22107b89aba0fe0451ae84425f87c2fdd 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 from ldap2
connections
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 ae97a4b01e89435c9a50592ce1e9df4d0f75f132 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
---
ipaserver/plugins/ldap2.py | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index e671ecb..43516c2 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -67,14 +67,11 @@ 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, api.env.ldap_uri,
force_schema_updates=force_schema_updates)
self.__time_limit = float(LDAPClient.time_limit)
From b278245984cfe5bf865c146f4289a7a558a78901 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 6ec29663bf3067be72368df5f589e31ada3b1fb4 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 | 9 +++++++++
ipalib/constants.py | 5 ++---
makeaci | 2 --
makeapi | 4 ----
pylint_plugins.py | 4 +++-
5 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/ipalib/config.py b/ipalib/config.py
index 388ffe8..7ac1ca3 100644
--- a/ipalib/config.py
+++ b/ipalib/config.py
@@ -565,6 +565,15 @@ 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('.')))
+
+ if 'xmlrpc_uri' not in self and 'server' in self:
+ self.xmlrpc_uri = 'https://{}/ipa/xml'.format(self.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
