Jan Zeleny <jzel...@redhat.com> wrote: > Jan Zelený <jzel...@redhat.com> wrote: > > Jan Zelený <jzel...@redhat.com> wrote: > > > Rob Crittenden <rcrit...@redhat.com> wrote: > > > > Jan Zelený wrote: > > > > > Rob Crittenden<rcrit...@redhat.com> wrote: > > > > >> Jan Zelený wrote: > > > > >>> Loading of the schema is now performed in the first request that > > > > >>> requires it. > > > > >>> > > > > >>> https://fedorahosted.org/freeipa/ticket/583 > > > > >>> > > > > >>> Jan > > > > >> > > > > >> We still need to enforce that we get the schema, some low-level > > > > >> functions depend on it. Also, if the UI doesn't get its aciattrs > > > > >> (which are derived from the schema) then nothing will be editable. > > > > >> > > > > >> I'm getting this backtrace if I force no schema by disabling > > > > get_schema: > > > > > Ok, I'm sending new version, it should handle these exceptions > > > > > better and the operation should fail if it needs the schema and > > > > > the schema is not available for some reason. > > > > > > > > This breaks the XML-RPC server. I fixed one problem: > > > > --- a/ipaserver/plugins/ldap2.py > > > > +++ b/ipaserver/plugins/ldap2.py > > > > > > > > @@ -253,9 +253,10 @@ class ldap2(CrudBackend, Encoder): > > > > def get_syntax(self, attr, value): > > > > if not self.schema: > > > > - self.schema = get_schema(self.ldap_uri, self.conn) > > > > - if not self.schema: > > > > + schema = get_schema(self.ldap_uri, self.conn) > > > > > > > > + if not schema: > > > > return None > > > > > > > > + object.__setattr__(self, 'schema', schema) > > > > > > > > obj = self.schema.get_obj(_ldap.schema.AttributeType, attr) > > > > > > > > if obj is not None: > > > > return obj.syntax > > > > > > > > But simply things like get_entry() return an InternalError now. I'm > > > > not sure where you were going by adding this. > > > > > > > > rob > > > > > > Ok, no problem. It's possible that I simply did a mistake thinking I > > > can do something in Python what is not really possible. > > > > > > About that InternalError: I think raising InternalError when we cannot > > > load the schema to do the decoding is the right thing to do. Do you > > > have a better solution? I thought about returning empty result, but > > > that would mean we have to check the result in every funtction that is > > > calling them and raising InternalError there. > > > > I'm sending updated patch. I modified the get_syntax() as you suggested > > and I slightly modified raising that InternalError - currently it isn't > > raised when results from get_entry() are not required by calling method. > > Currently I'm running some tests, preliminary results looked ok. > > self-nack > > I discovered some issues discovered by internal test suite, I'm working on > them > > Jan
Ok, everything is solved, I'm sending final version of the patch in the attachment. But I still think this should go to 2.1, since it's quite extensive patch in the core of IPA server and it has potential to break many things. Jan
From 8ce877994e9122d89f842e358f38005eb980b632 Mon Sep 17 00:00:00 2001 From: Jan Zeleny <jzel...@redhat.com> Date: Tue, 15 Feb 2011 09:37:58 +0100 Subject: [PATCH] Don't load the LDAP schema during startup https://fedorahosted.org/freeipa/ticket/583 --- ipalib/encoder.py | 12 +++-- ipalib/plugins/baseldap.py | 21 ++++++++- ipalib/plugins/dns.py | 2 +- ipalib/plugins/host.py | 2 +- ipalib/plugins/permission.py | 4 +- ipalib/plugins/sudocmd.py | 2 +- ipaserver/install/dsinstance.py | 2 +- ipaserver/plugins/ldap2.py | 92 +++++++++++++++++++++++++++----------- 8 files changed, 97 insertions(+), 40 deletions(-) diff --git a/ipalib/encoder.py b/ipalib/encoder.py index f23e5659e848d37db1072ff59aa7e11796b0836c..762be9c096a4b7cbea03ecfcc82d03789951c0b5 100644 --- a/ipalib/encoder.py +++ b/ipalib/encoder.py @@ -56,9 +56,10 @@ class Encoder(object): self.encoder_settings = EncoderSettings() def _decode_dict_val(self, key, val): - f = self.encoder_settings.decode_dict_vals_table.get( - self.encoder_settings.decode_dict_vals_table_keygen(key, val) - ) + k = self.encoder_settings.decode_dict_vals_table_keygen(key, val) + if k is False: + return False + f = self.encoder_settings.decode_dict_vals_table.get(k) if f: return val return self.decode(val) @@ -154,7 +155,10 @@ class Encoder(object): tmp = self.encoder_settings.decode_postprocessor self.encoder_settings.decode_postprocessor = lambda x: x for (k, v) in dct.iteritems(): - dct[k] = self._decode_dict_val(k, v) + decoded_val = self._decode_dict_val(k, v) + if decoded_val is False: + return False + dct[k] = decoded_val if not self.encoder_settings.decode_dict_vals_postprocess: self.encoder_settings.decode_postprocessor = tmp return dct diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 3cb72d7b09cc8c8a77bd4e594660ee376d668013..78e329a33833b548dc70020ac0b1f4482520d5cb 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -189,6 +189,9 @@ def get_effective_rights(ldap, dn, attrs=None): if attrs is None: attrs = ['*', 'nsaccountlock', 'cospriority'] rights = ldap.get_effective_rights(dn, attrs) + if rights[1] in False: + return False + rdict = {} if 'attributelevelrights' in rights[1]: rights = rights[1]['attributelevelrights'] @@ -394,7 +397,11 @@ class LDAPObject(Object): objectclasses += self.possible_objectclasses # Get list of available attributes for this object for use # in the ACI UI. - attrs = self.api.Backend.ldap2.schema.attribute_types(objectclasses) + schema = self.api.Backend.ldap2.get_schema() + if not schema: + attrs = [] + else: + attrs = schema.attribute_types(objectclasses) attrlist = [] # Go through the MUST first for (oid, attr) in attrs[0].iteritems(): @@ -613,6 +620,10 @@ class LDAPCreate(CallbackInterface, crud.Create): except errors.NotFound: self.obj.handle_not_found(*keys) + if entry_attrs is False: + # FIXME: is there any other action needed (some cleanup?) + raise errors.InternalError() + for callback in self.POST_CALLBACKS: if hasattr(callback, 'im_self'): dn = callback(ldap, dn, entry_attrs, *keys, **options) @@ -725,7 +736,9 @@ class LDAPRetrieve(LDAPQuery): self.obj.handle_not_found(*keys) if options.get('rights', False) and options.get('all', False): - entry_attrs['attributelevelrights'] = get_effective_rights(ldap, dn) + rights = get_effective_rights(ldap, dn) + if rights is not False: + entry_attrs['attributelevelrights'] = rights for callback in self.POST_CALLBACKS: if hasattr(callback, 'im_self'): @@ -884,7 +897,9 @@ class LDAPUpdate(LDAPQuery, crud.Update): ) if options.get('rights', False) and options.get('all', False): - entry_attrs['attributelevelrights'] = get_effective_rights(ldap, dn) + rights = get_effective_rights(ldap, dn) + if rights is not False: + entry_attrs['attributelevelrights'] = rights for callback in self.POST_CALLBACKS: if hasattr(callback, 'im_self'): diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py index ed2f955c680d1603b7ba805ca82aaec50975d3cb..5a2c679f2c444d70ed095b6f0bb6c48268aa57f6 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -191,7 +191,7 @@ def add_forward_record(zone, name, str_address): def dns_container_exists(ldap): try: - ldap.get_entry(api.env.container_dns, []) + ldap.get_entry(api.env.container_dns, [], override=True) except errors.NotFound: return False return True diff --git a/ipalib/plugins/host.py b/ipalib/plugins/host.py index f5f5157b0ae8c5b6245b6cab3bcb21293b38d04d..321cc944816b640af3a05198ee446789f685b325 100644 --- a/ipalib/plugins/host.py +++ b/ipalib/plugins/host.py @@ -270,7 +270,7 @@ class host(LDAPObject): hostname = hostname[:-1] dn = super(host, self).get_dn(hostname, **options) try: - self.backend.get_entry(dn, ['']) + self.backend.get_entry(dn, [''], override=True) except errors.NotFound: try: (dn, entry_attrs) = self.backend.find_entry_by_attr( diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py index 784b4dae5c79e656ae6469fd2ac7797777d8123e..bf59b110adcf6e9b0fd7ba39f6734ebed4914146 100644 --- a/ipalib/plugins/permission.py +++ b/ipalib/plugins/permission.py @@ -265,7 +265,7 @@ class permission_mod(LDAPUpdate): # check if permission is in LDAP try: (dn, attrs) = ldap.get_entry( - dn, attrs_list, normalize=self.obj.normalize_dn + dn, attrs_list, normalize=self.obj.normalize_dn, override=True ) except errors.NotFound: self.obj.handle_not_found(*keys) @@ -276,7 +276,7 @@ class permission_mod(LDAPUpdate): try: new_dn = dn.replace(keys[-1], options['rename'], 1) (new_dn, attrs) = ldap.get_entry( - new_dn, attrs_list, normalize=self.obj.normalize_dn + new_dn, attrs_list, normalize=self.obj.normalize_dn, override=True ) raise errors.DuplicateEntry() except errors.NotFound: diff --git a/ipalib/plugins/sudocmd.py b/ipalib/plugins/sudocmd.py index 50da72254f2446592e1c283aa8d32a08f19ba4b6..f75f9e543b5324ffa8ff1bb89c6171c422fcd776 100644 --- a/ipalib/plugins/sudocmd.py +++ b/ipalib/plugins/sudocmd.py @@ -78,7 +78,7 @@ class sudocmd(LDAPObject): keys[-1] = keys[-1][:-1] dn = super(sudocmd, self).get_dn(*keys, **options) try: - self.backend.get_entry(dn, ['']) + self.backend.get_entry(dn, [''], override=True) except errors.NotFound: try: (dn, entry_attrs) = self.backend.find_entry_by_attr( diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py index 2544e167bdff28c13201c5371070ab729ca84b67..c2081700bb7348f4db7e3467f64d040effd07cbe 100644 --- a/ipaserver/install/dsinstance.py +++ b/ipaserver/install/dsinstance.py @@ -133,7 +133,7 @@ def has_managed_entries(host_name, dm_password): conn = ldap2(shared_instance=False, ldap_uri=ldapuri, base_dn='cn=config') conn.connect(bind_dn='cn=Directory Manager', bind_pw=dm_password) (dn, attrs) = conn.get_entry('cn=Managed Entries,cn=plugins', - ['*'], time_limit=2, size_limit=3000) + ['*'], time_limit=2, size_limit=3000, override=True) return True except errors.NotFound: return False diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py index d1e31f5e6eff20cd162c0a11eb4e4404b43ae4b2..d01ccef8b47d59c005a1a9b6bc386128bf4bfdbc 100644 --- a/ipaserver/plugins/ldap2.py +++ b/ipaserver/plugins/ldap2.py @@ -184,12 +184,6 @@ def get_schema(url, conn=None): return _ldap.schema.SubSchema(schema_entry[1]) -# cache schema when importing module -try: - _schema = get_schema(api.env.ldap_uri) -except AttributeError: - _schema = None - # The UPG setting will be cached the first time a module checks it _upg = None @@ -229,7 +223,6 @@ class ldap2(CrudBackend, Encoder): def __init__(self, shared_instance=True, ldap_uri=None, base_dn=None, schema=None): - global _schema CrudBackend.__init__(self, shared_instance=shared_instance) Encoder.__init__(self) self.encoder_settings.encode_dict_keys = True @@ -249,7 +242,7 @@ class ldap2(CrudBackend, Encoder): self.base_dn = api.env.basedn except AttributeError: self.base_dn = '' - self.schema = schema or _schema + self.schema = schema def __del__(self): if self.isconnected(): @@ -260,7 +253,11 @@ class ldap2(CrudBackend, Encoder): def get_syntax(self, attr, value): if not self.schema: - return None + schema = get_schema(self.ldap_uri, self.conn) + if not schema: + # we can't return None, since it could be mistaken for obj being None + return False + object.__setattr__(self, 'schema', schema) obj = self.schema.get_obj(_ldap.schema.AttributeType, attr) if obj is not None: return obj.syntax @@ -269,7 +266,11 @@ class ldap2(CrudBackend, Encoder): def get_allowed_attributes(self, objectclasses): if not self.schema: - return [] + schema = get_schema(self.ldap_uri, self.conn) + if not schema: + # we can't return None, since it could be mistaken for obj being None + return [] + object.__setattr__(self, 'schema', schema) allowed_attributes = [] for oc in objectclasses: obj = self.schema.get_obj(_ldap.schema.ObjectClass, oc) @@ -286,10 +287,16 @@ class ldap2(CrudBackend, Encoder): If there is a problem loading the schema or the attribute is not in the schema return None """ - if self.schema: - obj = self.schema.get_obj(_ldap.schema.AttributeType, attr) - return obj and obj.single_value - return None + if not self.schema: + schema = get_schema(self.ldap_uri, self.conn) + if not schema: + # if we cannot load the schema, it's safer to assume + # the attribute is single-valued + return True + object.__setattr__(self, 'schema', schema) + + obj = self.schema.get_obj(_ldap.schema.AttributeType, attr) + return obj and obj.single_value @encode_args(2, 3, 'bind_dn', 'bind_pw') def create_connection(self, ccache=None, bind_dn='', bind_pw='', @@ -310,7 +317,6 @@ class ldap2(CrudBackend, Encoder): Extends backend.Connectible.create_connection. """ - global _schema if tls_cacertfile is not None: _ldap.set_option(_ldap.OPT_X_TLS_CACERTFILE, tls_cacertfile) if tls_certfile is not None: @@ -335,10 +341,10 @@ class ldap2(CrudBackend, Encoder): except _ldap.LDAPError, e: _handle_errors(e, **{}) - if self.schema is None and _schema is None: - # explicitly use setattr here so the schema can be set after - # the object is finalized. - object.__setattr__(self, 'schema', get_schema(self.ldap_uri, conn)) + # For now let's say the schema is None (will be loaded later) + # - explicitly use setattr here so the schema can be set after + # the object is finalized. + object.__setattr__(self, 'schema', None) return conn def destroy_connection(self): @@ -616,17 +622,20 @@ class ldap2(CrudBackend, Encoder): return self.find_entries(filter, attrs_list, base_dn)[0][0] def get_entry(self, dn, attrs_list=None, time_limit=None, - size_limit=None, normalize=True): + size_limit=None, normalize=True, override=False): """ Get entry (dn, entry_attrs) by dn. Keyword arguments: attrs_list - list of attributes to return, all if None (default None) """ - return self.find_entries( + entries = self.find_entries( None, attrs_list, dn, self.SCOPE_BASE, time_limit=time_limit, size_limit=size_limit, normalize=normalize )[0][0] + if entries[1] is False and override == False: + raise errors.InternalError() + return entries config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]} def get_ipa_config(self, attrs_list=None): @@ -639,14 +648,27 @@ class ldap2(CrudBackend, Encoder): )[0][0] except errors.NotFound: config_entry = {} + + if config_entry is False: + raise errors.InternalError() + for a in self.config_defaults: if a not in config_entry: config_entry[a] = self.config_defaults[a] return (cdn, config_entry) - def get_schema(self): - """Returns a copy of the current LDAP schema.""" - return copy.deepcopy(self.schema) + def get_schema(self, deepcopy=False): + """ Returns either a reference to current schema or its deep copy """ + if not self.schema: + schema = get_schema(self.ldap_uri, self.conn) + if not schema: + return None + object.__setattr__(self, 'schema', schema) + + if (deepcopy): + return copy.deepcopy(self.schema) + else: + return self.schema def has_upg(self): """Returns True/False whether User-Private Groups are enabled. @@ -691,6 +713,9 @@ class ldap2(CrudBackend, Encoder): on the attribute. This only operates on a single attribute at a time. """ (dn, attrs) = self.get_effective_rights(dn, [attr]) + if attrs is False: + return False + if 'attributelevelrights' in attrs: attr_rights = attrs.get('attributelevelrights')[0].decode('UTF-8') (attr, rights) = attr_rights.split(':') @@ -705,6 +730,9 @@ class ldap2(CrudBackend, Encoder): on the attribute. This only operates on a single attribute at a time. """ (dn, attrs) = self.get_effective_rights(dn, [attr]) + if attrs is False: + return False + if 'attributelevelrights' in attrs: attr_rights = attrs.get('attributelevelrights')[0].decode('UTF-8') (attr, rights) = attr_rights.split(':') @@ -728,6 +756,9 @@ class ldap2(CrudBackend, Encoder): on the entry. """ (dn, attrs) = self.get_effective_rights(dn, ["*"]) + if attrs is False: + return False + if 'entrylevelrights' in attrs: entry_rights = attrs['entrylevelrights'][0].decode('UTF-8') if 'd' in entry_rights: @@ -741,6 +772,9 @@ class ldap2(CrudBackend, Encoder): on the entry. """ (dn, attrs) = self.get_effective_rights(dn, ["*"]) + if attrs is False: + return False + if 'entrylevelrights' in attrs: entry_rights = attrs['entrylevelrights'][0].decode('UTF-8') if 'a' in entry_rights: @@ -861,7 +895,7 @@ class ldap2(CrudBackend, Encoder): is True. """ # check if the entry exists - (dn, entry_attrs) = self.get_entry(dn, ['objectclass']) + (dn, entry_attrs) = self.get_entry(dn, ['objectclass'], override=True) # get group entry (group_dn, group_entry_attrs) = self.get_entry(group_dn, [member_attr]) @@ -931,6 +965,7 @@ class ldap2(CrudBackend, Encoder): size_limit=size_limit, normalize=normalize) except errors.NotFound: netresults = [] + results = results + netresults if membertype == MEMBERS_ALL: @@ -993,7 +1028,6 @@ class ldap2(CrudBackend, Encoder): size_limit=size_limit, normalize=normalize) except errors.NotFound: netresults = [] - results = results + netresults try: (pbacresults, truncated) = self.find_entries(searchfilter, attr_list, 'cn=pbac,%s' % api.env.basedn, @@ -1001,7 +1035,8 @@ class ldap2(CrudBackend, Encoder): normalize=normalize) except errors.NotFound: pbacresults = [] - results = results + pbacresults + + results = results + netresults + pbacresults direct = [] indirect = [] @@ -1134,7 +1169,10 @@ class ldap2(CrudBackend, Encoder): (entries, truncated) = self.find_entries( filter, attrs_list, base_dn, scope ) + for (dn, entry_attrs) in entries: + if entry_attrs is False: + continue entry_attrs['dn'] = [dn] output.append(entry_attrs) -- 1.7.4
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
