From c8059bb231848672e70fefbe9bb12dc2d02c4079 Mon Sep 17 00:00:00 2001 From: Sapan Bhatia Date: Tue, 27 Jul 2010 23:13:29 -0400 Subject: [PATCH] Bug fixes --- PLC/Methods/AddPersonToSlice.py | 2 +- PLC/Methods/UpdateSlice.py | 65 ++++-------------------------- PLC/SlicesPolicy.py | 70 ++++++++++++++++++++++++++++----- PLC/Table.py | 21 +++++++--- 4 files changed, 83 insertions(+), 75 deletions(-) diff --git a/PLC/Methods/AddPersonToSlice.py b/PLC/Methods/AddPersonToSlice.py index 80383d6a..878a2d80 100644 --- a/PLC/Methods/AddPersonToSlice.py +++ b/PLC/Methods/AddPersonToSlice.py @@ -44,7 +44,7 @@ class AddPersonToSlice(Method): raise PLCInvalidArgument, "No such slice or access denied to requested slice" slice = slices[0] - slices.update_rows(['key':'slice_person','type':Person,'value':person) + slices.add_to_rows({'slice_person':(Person,person)}) # Logging variables self.event_objects = {'Person': [person['person_id']], diff --git a/PLC/Methods/UpdateSlice.py b/PLC/Methods/UpdateSlice.py index 90341025..19f63a01 100644 --- a/PLC/Methods/UpdateSlice.py +++ b/PLC/Methods/UpdateSlice.py @@ -50,75 +50,23 @@ class UpdateSlice(Method): returns = Parameter(int, '1 if successful') def call(self, auth, slice_id_or_name, slice_fields): - + # SSS: To figure out and refactor # split provided fields [native,related,tags,rejected] = Row.split_fields(slice_fields,[Slice.fields,Slice.related_fields,Slice.tags]) - # type checking - native = Row.check_fields (native, self.accepted_fields) - if rejected: - raise PLCInvalidArgument, "Cannot update Slice column(s) %r"%rejected - - slices = Slices(self.api, [slice_id_or_name]) + slices = Slices(self.api, self.caller, [slice_id_or_name]) if not slices: - raise PLCInvalidArgument, "No such slice %r"%slice_id_or_name - slice = slices[0] - - if slice['peer_id'] is not None: - raise PLCInvalidArgument, "Not a local slice" - - # Authenticated function - assert self.caller is not None - - if 'admin' not in self.caller['roles']: - if self.caller['person_id'] in slice['person_ids']: - pass - elif 'pi' not in self.caller['roles']: - raise PLCPermissionDenied, "Not a member of the specified slice" - elif slice['site_id'] not in self.caller['site_ids']: - raise PLCPermissionDenied, "Specified slice not associated with any of your sites" - - # Renewing - renewing=False - if 'expires' in slice_fields and slice_fields['expires'] > slice['expires']: - sites = Sites(self.api, [slice['site_id']]) - assert sites - site = sites[0] - - if site['max_slices'] <= 0: - raise PLCInvalidArgument, "Slice creation and renewal have been disabled for the site" - - # Maximum expiration date is 8 weeks from now - # XXX Make this configurable - max_expires = time.time() + (8 * 7 * 24 * 60 * 60) - - if 'admin' not in self.caller['roles'] and slice_fields['expires'] > max_expires: - raise PLCInvalidArgument, "Cannot renew a slice beyond 8 weeks from now" - - # XXX Make this a configurable policy - if slice['description'] is None or not slice['description'].strip(): - if 'description' not in slice_fields or slice_fields['description'] is None or \ - not slice_fields['description'].strip(): - raise PLCInvalidArgument, "Cannot renew a slice with an empty description or URL" - - if slice['url'] is None or not slice['url'].strip(): - if 'url' not in slice_fields or slice_fields['url'] is None or \ - not slice_fields['url'].strip(): - raise PLCInvalidArgument, "Cannot renew a slice with an empty description or URL" - renewing=True - - if 'max_nodes' in slice_fields and slice_fields['max_nodes'] != slice['max_nodes']: - if 'admin' not in self.caller['roles'] and \ - 'pi' not in self.caller['roles']: - raise PLCInvalidArgument, "Only admins and PIs may update max_nodes" + raise PLCInvalidArgument, "No such slice %r or access denied"%slice_id_or_name # Make requested associations for (k,v) in related.iteritems(): slice.associate(auth,k,v) - slice.update(slice_fields) + slice.update_rows(slice_fields) slice.sync(commit=True) + + # SSS To refactor code below for (tagname,value) in tags.iteritems(): # the tagtype instance is assumed to exist, just check that if not TagTypes(self.api,{'tagname':tagname}): @@ -134,6 +82,7 @@ class UpdateSlice(Method): self.message='Slice %s updated'%slice['name'] else: self.message='Slice %d updated'%slice['slice_id'] + if renewing: # it appears that slice['expires'] may be either an int, or a formatted string try: diff --git a/PLC/SlicesPolicy.py b/PLC/SlicesPolicy.py index e5eb4d99..e204e5e7 100644 --- a/PLC/SlicesPolicy.py +++ b/PLC/SlicesPolicy.py @@ -14,26 +14,76 @@ from PLC.SliceTags import SliceTag from PLC.Timestamp import Timestamp class SlicesPolicy(Policy): + updatable_fields = set(['instantiation', 'url', 'description', 'max_nodes', 'expires', 'persons', 'nodes']) + def __init__(self, api, caller, states): slice_states = ['incoming','outgoing'] return Policy(api, caller, slice_states) - def modify_ok(self, slice_filter, column_filter, value_filters): - for value_filter in value_filters: + """ + All update calls to the Slice Table go via this method + """ + def modify_ok(self, column_values): + # Requested columns must be modifiable + columns = set([]) + columns = set(column_values.keys()) + if (not self.updatable_fields > columns): + raise PLCInvalidArgument, "Cannot update Slice column(s) %r"%rejected + + columns = columns & self.updatable_fields + + for slice in self: + # Cannot modify non-local slices + if slice['peer_id'] is not None: + raise PLCInvalidArgument, "Not a local slice" + # N.B. Allow foreign users to be added to local slices and # local users to be added to foreign slices (and, of course, # local users to be added to local slices). - if (value_filter['key']=='slice_person'): - person = value_filter['value'] + if ('slice_person' in columns): + person = column_values['slice_person'] if person['peer_id'] is not None and self['peer_id'] is not None: raise PLCInvalidArgument, "Cannot add foreign users to foreign slices" - # If we are not admin, make sure the caller is a PI - # of the site associated with the slice - # XXX no PI check around here, which is suggested by the preceding comment - if 'admin' not in self.caller['roles']: - if self['site_id'] not in self.caller['site_ids']: - raise PLCPermissionDenied, "Not allowed to add users to this slice" + # If we are not admin, make sure the caller is a PI + # of the site associated with the slice + # XXX no PI check around here, which is suggested by the preceding comment - Sapan + if 'admin' not in self.caller['roles']: + if slice['site_id'] not in self.caller['site_ids']: + raise PLCPermissionDenied, "Not allowed to add users to this slice" + + renewing=False + if ('expires' in columns and column_values['expires'] > slice['expires']: + sites = Sites(self.api, [slice['site_id']]) + assert sites + site = sites[0] + + if site['max_slices'] <= 0: + raise PLCInvalidArgument, "Slice creation and renewal have been disabled for the site" + + # Maximum expiration date is 8 weeks from now + # XXX Make this configurable + max_expires = time.time() + (8 * 7 * 24 * 60 * 60) + + if 'admin' not in self.caller['roles'] and column_values['expires'] > max_expires: + raise PLCInvalidArgument, "Cannot renew a slice beyond 8 weeks from now" + + # XXX Make this a configurable policy + if slice['description'] is None or not slice['description'].strip(): + if 'description' not in column_values or column_values['description'] is None or \ + not column_values['description'].strip(): + raise PLCInvalidArgument, "Cannot renew a slice with an empty description or URL" + + if slice['url'] is None or not slice['url'].strip(): + if 'url' not in column_values or column_values['url'] is None or \ + not column_values['url'].strip(): + raise PLCInvalidArgument, "Cannot renew a slice with an empty description or URL" + renewing=True + + if 'max_nodes' in column_values and column_values['max_nodes'] != slice['max_nodes']: + if 'admin' not in self.caller['roles'] and \ + 'pi' not in self.caller['roles']: + raise PLCInvalidArgument, "Only admins and PIs may update max_nodes" return True diff --git a/PLC/Table.py b/PLC/Table.py index 86f60cb0..e1911010 100644 --- a/PLC/Table.py +++ b/PLC/Table.py @@ -437,15 +437,24 @@ class Table(list): return dict([(obj[key_field], obj) for obj in self]) - def update_rows(self, update_spec): + def add_to_rows(self, update_spec): """ Dispatch requested change to individual rows, if write policy permits it """ - if (self.modify_ok(values)): + if (self.modify_ok(update_spec)): for row in self: - for update in update_spec: - key = update['key'] - type = update['type'] - value = update['value'] + for key in update_spec.keys(): + type,value = update_spec[key] add_fn = row.add_object(type, key) add_fn(value) + + def update_rows(self, update_spec): + """ + Dispatch requested change to individual rows, if write policy permits it + """ + if (self.modify_ok(update_spec)): + for row in self: + fields = {} + for key in update_spec.keys(): + type,value = update_spec[key] + row.update(fields) -- 2.47.0