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. -- Thank you Jan Zeleny Red Hat Software Engineer Brno, Czech Republic
From bdab2d3b9b8a7397e13acdad9d2809ae355cd5a7 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 | 11 +++-- 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 | 90 ++++++++++++++++++++++++++++----------- 8 files changed, 96 insertions(+), 38 deletions(-) diff --git a/ipalib/encoder.py b/ipalib/encoder.py index f23e5659e848d37db1072ff59aa7e11796b0836c..fbcae1739ee6541bdb989d6d01f8a4c9fb614b62 100644 --- a/ipalib/encoder.py +++ b/ipalib/encoder.py @@ -56,11 +56,12 @@ 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(key) if f: - return val + return f(val) return self.decode(val) def encode(self, var): @@ -155,6 +156,8 @@ class Encoder(object): self.encoder_settings.decode_postprocessor = lambda x: x for (k, v) in dct.iteritems(): dct[k] = self._decode_dict_val(k, v) + if dct[k] is False: + return False 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..adc19fec8ff96eef2750ad08715e9324c1c536c6 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 None: + return None + 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 None: + # 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 None: + 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 None: + 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..1fc6c101159d59354558411a78708f7c8da4bccd 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,9 @@ class ldap2(CrudBackend, Encoder): def get_allowed_attributes(self, objectclasses): if not self.schema: - return [] + self.schema = get_schema(self.ldap_uri, self.conn) + if not self.schema: + return [] allowed_attributes = [] for oc in objectclasses: obj = self.schema.get_obj(_ldap.schema.ObjectClass, oc) @@ -286,10 +285,15 @@ 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: + self.schema = get_schema(self.ldap_uri, self.conn) + if not self.schema: + # if we cannot load the schema, it's safer to assume + # the attribute is single-valued + return True + + 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 +314,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 +338,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 +619,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 None and override == False: + raise errors.InternalError() + return entries config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]} def get_ipa_config(self, attrs_list=None): @@ -639,14 +645,24 @@ class ldap2(CrudBackend, Encoder): )[0][0] except errors.NotFound: config_entry = {} + + if config_entry is None: + 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: + self.schema = get_schema(self.ldap_uri, self.conn) + + 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 +707,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 None: + return False + if 'attributelevelrights' in attrs: attr_rights = attrs.get('attributelevelrights')[0].decode('UTF-8') (attr, rights) = attr_rights.split(':') @@ -705,6 +724,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 None: + return False + if 'attributelevelrights' in attrs: attr_rights = attrs.get('attributelevelrights')[0].decode('UTF-8') (attr, rights) = attr_rights.split(':') @@ -728,6 +750,9 @@ class ldap2(CrudBackend, Encoder): on the entry. """ (dn, attrs) = self.get_effective_rights(dn, ["*"]) + if attrs is None: + return False + if 'entrylevelrights' in attrs: entry_rights = attrs['entrylevelrights'][0].decode('UTF-8') if 'd' in entry_rights: @@ -741,6 +766,9 @@ class ldap2(CrudBackend, Encoder): on the entry. """ (dn, attrs) = self.get_effective_rights(dn, ["*"]) + if attrs is None: + return False + if 'entrylevelrights' in attrs: entry_rights = attrs['entrylevelrights'][0].decode('UTF-8') if 'a' in entry_rights: @@ -861,7 +889,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 +959,9 @@ class ldap2(CrudBackend, Encoder): size_limit=size_limit, normalize=normalize) except errors.NotFound: netresults = [] + + if results is None: + results = [] results = results + netresults if membertype == MEMBERS_ALL: @@ -993,6 +1024,9 @@ class ldap2(CrudBackend, Encoder): size_limit=size_limit, normalize=normalize) except errors.NotFound: netresults = [] + + if results is None: + results = [] results = results + netresults try: (pbacresults, truncated) = self.find_entries(searchfilter, @@ -1001,6 +1035,9 @@ class ldap2(CrudBackend, Encoder): normalize=normalize) except errors.NotFound: pbacresults = [] + + if pbacresults is None: + pbacresults = [] results = results + pbacresults direct = [] @@ -1134,6 +1171,9 @@ class ldap2(CrudBackend, Encoder): (entries, truncated) = self.find_entries( filter, attrs_list, base_dn, scope ) + if entries is None: + return (len(output), output) + for (dn, entry_attrs) in entries: 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
