Jan Zelený <jzel...@redhat.com> wrote: > 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
Rebased against master Jan
From 7ff1e505395c401a163ac479220910cb39e4182d 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, 96 insertions(+), 41 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 4441e79602e2f8622e2638148db5fea22181e3f0..f1229169706ddde4b27d0773aa786ef976c29341 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -183,6 +183,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'] @@ -388,7 +391,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(): @@ -607,6 +614,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) @@ -719,7 +730,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'): @@ -878,7 +891,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 a18940b3ac6268b587c691c440a81db492eead86..59b39b5e1bd440067b4c587c583f3abb284e93b0 100644 --- a/ipalib/plugins/dns.py +++ b/ipalib/plugins/dns.py @@ -217,7 +217,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 db063334fddbe20ca3b5f7b2a57655da3a988896..164bf82d1a24c64684c4440e4eacbbd341d523cd 100644 --- a/ipalib/plugins/permission.py +++ b/ipalib/plugins/permission.py @@ -266,7 +266,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) @@ -277,7 +277,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 528d79079bfb563254700e0b1f9b4148a4ccb6cb..d166652eeada933932848b243fa32ca359b95f77 100644 --- a/ipalib/plugins/sudocmd.py +++ b/ipalib/plugins/sudocmd.py @@ -81,7 +81,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 bf631a67fb16b890e2e1933f59392e443de2cf0e..68ca45f9562cda442a6c5c4c49b91f2134d6f6c8 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 568792d1b28f9e8152ad34251b2f303770601dae..ea310ac33fea24544dcd2ed8d568da0fbf7900fb 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,6 @@ class ldap2(CrudBackend, Encoder): normalize=normalize) except errors.NotFound: pbacresults = [] - results = results + pbacresults try: (sudoresults, truncated) = self.find_entries(searchfilter, attr_list, 'cn=sudo,%s' % api.env.basedn, @@ -1009,7 +1042,7 @@ class ldap2(CrudBackend, Encoder): normalize=normalize) except errors.NotFound: sudoresults = [] - results = results + sudoresults + results = results + netresults + pbacresults + sudoresults direct = [] indirect = [] @@ -1142,7 +1175,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
