From 58ff63b595a188c6184ac3b713b5168e7ab4b8f1 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Wed, 25 Apr 2018 17:40:17 +0200 Subject: [PATCH 01/15] Fix querying relations with Any() --- serveradmin/serverdb/sql_generator.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/serveradmin/serverdb/sql_generator.py b/serveradmin/serverdb/sql_generator.py index 607623708..7ef38a7c7 100644 --- a/serveradmin/serverdb/sql_generator.py +++ b/serveradmin/serverdb/sql_generator.py @@ -89,6 +89,10 @@ def _get_sql_condition(servertypes, attribute, filt): else: template = '{0} = ' + _raw_sql_escape(filt.value) + return _covered_sql_condition(servertypes, attribute, template, negate) + + +def _covered_sql_condition(servertypes, attribute, template, negate=False): if attribute.type in ('hostname', 'reverse_hostname', 'supernet'): template = ( '{{0}} IN (' @@ -133,7 +137,7 @@ def _logical_filter_sql_condition(servertypes, attribute, filt): servertypes, attribute, simple_values[0] ) else: - template = _condition_sql( + template = _covered_sql_condition( servertypes, attribute, '{{0}} IN ({0})'.format(', '.join( _raw_sql_escape(v.value) for v in simple_values )) From 6f425c1d00eb8b5da687396bf275db45738b96ef Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Tue, 6 Mar 2018 17:57:55 +0100 Subject: [PATCH 02/15] Simplify access control groups --- .../access_control/migrations/0001_initial.py | 13 ++-- serveradmin/access_control/models.py | 22 +++--- serveradmin/serverdb/query_committer.py | 72 ++++++++----------- 3 files changed, 42 insertions(+), 65 deletions(-) diff --git a/serveradmin/access_control/migrations/0001_initial.py b/serveradmin/access_control/migrations/0001_initial.py index dd66f879c..6a7b94255 100644 --- a/serveradmin/access_control/migrations/0001_initial.py +++ b/serveradmin/access_control/migrations/0001_initial.py @@ -9,21 +9,18 @@ class Migration(migrations.Migration): dependencies = [ migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ('apps', '__first__'), + ('apps', '0001_initial'), ] operations = [ migrations.CreateModel( name='AccessControlGroup', fields=[ - ('id', models.AutoField(primary_key=True, auto_created=True, serialize=False, verbose_name='ID')), + ('id', models.AutoField(auto_created=True, primary_key=True, verbose_name='ID', serialize=False)), ('name', models.CharField(max_length=80, unique=True)), - ('create_server_query', models.CharField(max_length=1000)), - ('edit_server_query', models.CharField(max_length=1000)), - ('commit_server_query', models.CharField(max_length=1000)), - ('delete_server_query', models.CharField(max_length=1000)), - ('applications', models.ManyToManyField(blank=True, to='apps.Application', related_name='access_control_groups')), - ('members', models.ManyToManyField(blank=True, to=settings.AUTH_USER_MODEL, related_name='access_control_groups')), + ('query', models.CharField(max_length=1000)), + ('applications', models.ManyToManyField(related_name='access_control_groups', blank=True, to='apps.Application')), + ('members', models.ManyToManyField(related_name='access_control_groups', blank=True, to=settings.AUTH_USER_MODEL)), ], options={ 'db_table': 'access_control_group', diff --git a/serveradmin/access_control/models.py b/serveradmin/access_control/models.py index 0ce079f75..634400f62 100644 --- a/serveradmin/access_control/models.py +++ b/serveradmin/access_control/models.py @@ -7,10 +7,7 @@ class AccessControlGroup(Model): name = CharField(max_length=80, unique=True) - create_server_query = CharField(max_length=1000) - edit_server_query = CharField(max_length=1000) - commit_server_query = CharField(max_length=1000) - delete_server_query = CharField(max_length=1000) + query = CharField(max_length=1000) members = ManyToManyField( User, blank=True, @@ -29,18 +26,15 @@ class Meta: def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self._filters = {} + self._filters = None def __str__(self): return self.name - def get_filters(self, action): - if action not in self._filters: - query = getattr(self, action + '_server_query') - self._filters[action] = parse_query(query) - return self._filters[action] + def get_filters(self): + if self._filters is None: + self._filters = parse_query(self.query) + return self._filters - def match_server(self, action, server): - return all( - f.matches(server[a]) for a, f in self.get_filters(action).items() - ) + def match_server(self, server): + return all(f.matches(server[a]) for a, f in self.get_filters().items()) diff --git a/serveradmin/serverdb/query_committer.py b/serveradmin/serverdb/query_committer.py index b3117eb7e..098f7b602 100644 --- a/serveradmin/serverdb/query_committer.py +++ b/serveradmin/serverdb/query_committer.py @@ -1,5 +1,6 @@ import json from ipaddress import ip_network, ip_interface +from itertools import chain from django.core.exceptions import PermissionDenied, ValidationError from django.db import IntegrityError, transaction @@ -56,6 +57,7 @@ def __call__(self): self._fetch() self._validate() self._apply() + self._access_control() self._log_changes() if self.warnings: @@ -80,7 +82,6 @@ def _fetch(self): ) } self._changed_objects = _materialize_servers(self._changed_servers) - access_control('edit', self._changed_objects, self.user, self.app) self._deleted_servers = { s.server_id: s for s in ( @@ -89,7 +90,6 @@ def _fetch(self): ) } self._deleted_objects = _materialize_servers(self._deleted_servers) - access_control('delete', self._deleted_objects, self.user, self.app) def _validate(self): servertype_attributes = _get_servertype_attributes( @@ -178,7 +178,6 @@ def _apply(self): # Re-fetch servers before invoking hook, otherwise the hook will # receive incomplete data. self._changed_objects = _materialize_servers(self._changed_servers) - access_control('commit', self._changed_objects, self.user, self.app) try: on_server_attribute_changed.invoke( commit=self, @@ -270,12 +269,6 @@ def _create_servers(self): created_server['project'] = project.pk created_server['intern_ip'] = intern_ip - access_control( - 'create', - {server.server_id: created_server}, - self.user, - self.app, - ) self._created_servers[server.server_id] = server self._created_objects = _materialize_servers(self._created_servers) @@ -331,6 +324,33 @@ def _upsert_attributes(self): else: server_attribute.save_value(change['new']) + def _access_control(self): + entities = list() + if not self.user.is_superuser: + entities.append(( + 'user', + self.user, + list(self.user.access_control_groups.all()), + )) + if self.app and not self.app.superuser: + entities.append(( + 'application', + self.app, + list(self.app.access_control_groups.all()), + )) + + for server in chain( + self._created_objects.values(), + self._changed_objects.values(), + self._deleted_objects.values(), + ): + for entity_class, entity_name, groups in entities: + if not any(g.match_server(server) for g in groups): + raise PermissionDenied( + 'Insufficient access rights to server "{}" for {} "{}"' + .format(server['hostname'], entity_class, entity_name) + ) + class CommitError(ValidationError): pass @@ -395,40 +415,6 @@ def filtered_fn(servers, changed, **kwargs): ) -def access_control(action, servers, user, app): - entities = list() - if not user.is_superuser: - entities.append(( - 'user', - user, - list(user.access_control_groups.all()), - )) - if app and not app.superuser: - entities.append(( - 'application', - app, - list(app.access_control_groups.all()), - )) - - for server in servers.values(): - matched_groups = set() - for entity_class, entity_name, entity_groups in entities: - for group in entity_groups: - if group in matched_groups: - break - if group.match_server(action, server): - matched_groups.add(group) - break - else: - raise PermissionDenied( - 'Insufficient access rights on {} of server "{}" ' - 'for {} "{}"' - .format( - action, server['hostname'], entity_class, entity_name - ) - ) - - def _materialize_servers(servers): return { o['object_id']: o From 3a7271e4f92013551b100721a5ebd682804dc48c Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Wed, 14 Mar 2018 18:29:07 +0100 Subject: [PATCH 03/15] Support attribute based ACLs --- .../0002_accesscontrolgroup_attributes.py | 21 +++++++ serveradmin/access_control/models.py | 10 +++- serveradmin/serverdb/query_committer.py | 59 ++++++++++++++----- 3 files changed, 71 insertions(+), 19 deletions(-) create mode 100644 serveradmin/access_control/migrations/0002_accesscontrolgroup_attributes.py diff --git a/serveradmin/access_control/migrations/0002_accesscontrolgroup_attributes.py b/serveradmin/access_control/migrations/0002_accesscontrolgroup_attributes.py new file mode 100644 index 000000000..fb222b428 --- /dev/null +++ b/serveradmin/access_control/migrations/0002_accesscontrolgroup_attributes.py @@ -0,0 +1,21 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.7 on 2018-04-25 15:58 +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('serverdb', '0001_initial'), + ('access_control', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='accesscontrolgroup', + name='attributes', + field=models.ManyToManyField(blank=True, related_name='access_control_groups', to='serverdb.Attribute'), + ), + ] diff --git a/serveradmin/access_control/models.py b/serveradmin/access_control/models.py index 634400f62..d88fef9e6 100644 --- a/serveradmin/access_control/models.py +++ b/serveradmin/access_control/models.py @@ -3,6 +3,7 @@ from adminapi.parse import parse_query from serveradmin.apps.models import Application +from serveradmin.serverdb.models import Attribute class AccessControlGroup(Model): @@ -20,6 +21,12 @@ class AccessControlGroup(Model): limit_choices_to={'disabled': False, 'superuser': False}, related_name='access_control_groups', ) + attributes = ManyToManyField( + Attribute, + blank=True, + limit_choices_to={'readonly': False}, + related_name='access_control_groups', + ) class Meta: db_table = 'access_control_group' @@ -35,6 +42,3 @@ def get_filters(self): if self._filters is None: self._filters = parse_query(self.query) return self._filters - - def match_server(self, server): - return all(f.matches(server[a]) for a, f in self.get_filters().items()) diff --git a/serveradmin/serverdb/query_committer.py b/serveradmin/serverdb/query_committer.py index 098f7b602..e07b3ad29 100644 --- a/serveradmin/serverdb/query_committer.py +++ b/serveradmin/serverdb/query_committer.py @@ -21,7 +21,10 @@ ChangeDelete, Project, ) -from serveradmin.serverdb.query_materializer import QueryMaterializer +from serveradmin.serverdb.query_materializer import ( + QueryMaterializer, + get_default_attribute_values, +) pre_commit = Signal() @@ -56,9 +59,9 @@ def __call__(self): with transaction.atomic(): self._fetch() self._validate() - self._apply() - self._access_control() - self._log_changes() + created_objects, changed_objects = self._apply() + self._access_control(created_objects, changed_objects) + self._log_changes(created_objects) if self.warnings: warnings = '\n'.join(self.warnings) @@ -67,8 +70,8 @@ def __call__(self): ) return DatasetCommit( - list(self._created_objects.values()), - list(self._changed_objects.values()), + list(created_objects.values()), + list(changed_objects.values()), list(self._deleted_objects.values()), ) @@ -139,7 +142,7 @@ def _validate(self): if newer: raise CommitNewerData('Newer data available', newer) - def _log_changes(self): + def _log_changes(self, created_objects): commit = ChangeCommit.objects.create(app=self.app, user=self.user) for updates in self.changed: @@ -157,7 +160,7 @@ def _log_changes(self): attributes_json=attributes_json, ) - for obj in self._created_objects.values(): + for obj in created_objects.values(): attributes_json = json.dumps( obj, default=json_encode_extra ) @@ -171,17 +174,17 @@ def _apply(self): # Changes should be applied in order to prevent integrity errors. self._delete_attributes() self._delete_servers() - self._create_servers() + created_objects = self._create_servers() self._update_servers() self._upsert_attributes() # Re-fetch servers before invoking hook, otherwise the hook will # receive incomplete data. - self._changed_objects = _materialize_servers(self._changed_servers) + changed_objects = _materialize_servers(self._changed_servers) try: on_server_attribute_changed.invoke( commit=self, - servers=list(self._changed_objects.values()), + servers=list(changed_objects.values()), changed=self.changed, ) except CommitIncomplete as error: @@ -189,6 +192,8 @@ def _apply(self): 'Commit hook failed:\n{}'.format(' '.join(error.messages)) ) + return created_objects, changed_objects + def _delete_attributes(self): # We first have to delete all of the hostname attributes # to avoid integrity errors. Other attributes will just go away @@ -271,7 +276,7 @@ def _create_servers(self): self._created_servers[server.server_id] = server - self._created_objects = _materialize_servers(self._created_servers) + return _materialize_servers(self._created_servers) def _update_servers(self): changed = set() @@ -324,7 +329,7 @@ def _upsert_attributes(self): else: server_attribute.save_value(change['new']) - def _access_control(self): + def _access_control(self, created_objects, changed_objects): entities = list() if not self.user.is_superuser: entities.append(( @@ -340,17 +345,39 @@ def _access_control(self): )) for server in chain( - self._created_objects.values(), - self._changed_objects.values(), + created_objects.values(), + changed_objects.values(), self._deleted_objects.values(), ): for entity_class, entity_name, groups in entities: - if not any(g.match_server(server) for g in groups): + if not any(self._can_access_server(server, g) for g in groups): raise PermissionDenied( 'Insufficient access rights to server "{}" for {} "{}"' .format(server['hostname'], entity_class, entity_name) ) + def _can_access_server(self, new_object, acl): + if not all( + f.matches(new_object[a]) + for a, f in acl.get_filters().items() + ): + return False + + if new_object['object_id'] in self._changed_objects: + old_object = self._changed_objects[new_object['object_id']] + else: + old_object = get_default_attribute_values(new_object['servertype']) + + attribute_ids = {a.pk for a in acl.attributes.all()} + if not all( + a in attribute_ids or v == old_object[a] or + Attribute.objects.get(pk=a).readonly + for a, v in new_object.items() + ): + return False + + return True + class CommitError(ValidationError): pass From 493459933343e0110ee0f3a5ce243ce5de1936a5 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Fri, 27 Apr 2018 16:00:04 +0200 Subject: [PATCH 04/15] Split the complex function on sql_generator --- serveradmin/serverdb/sql_generator.py | 38 ++++++++++++++++----------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/serveradmin/serverdb/sql_generator.py b/serveradmin/serverdb/sql_generator.py index 7ef38a7c7..7bfb02bd7 100644 --- a/serveradmin/serverdb/sql_generator.py +++ b/serveradmin/serverdb/sql_generator.py @@ -1,6 +1,9 @@ # XXX: It is terrible to generate SQL this way. We should make this use # parameterized queries at least. +# XXX: The code in this module is almost randomly split into functions. Do +# not try to guess what they would do. + from adminapi.filters import ( All, Any, @@ -20,7 +23,11 @@ StartsWith, Not, ) -from serveradmin.serverdb.models import ServerAttribute +from serveradmin.serverdb.models import ( + Server, + ServerAttribute, + ServerHostnameAttribute, +) def get_server_query(servertypes, attribute_filters): @@ -205,7 +212,7 @@ def _containment_filter_template(attribute, filt): return template.format(_raw_sql_escape(value)) -def _condition_sql(servertypes, attribute, template): # NOQA: C901 +def _condition_sql(servertypes, attribute, template): if attribute.special: field = attribute.special.field if field.startswith('_'): @@ -214,7 +221,7 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 return template.format('server.' + field) if attribute.type == 'supernet': - return _exists_sql('server', 'sub', ( + return _exists_sql(Server, 'sub', ( "sub.servertype_id = '{0}'".format( attribute.target_servertype.pk ), @@ -222,10 +229,8 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 template.format('sub.server_id'), )) - rel_table = ServerAttribute.get_model('hostname')._meta.db_table - if attribute.type == 'reverse_hostname': - return _exists_sql(rel_table, 'sub', ( + return _exists_sql(ServerHostnameAttribute, 'sub', ( "sub.attribute_id = '{0}'".format( attribute.reversed_attribute.pk ), @@ -233,8 +238,12 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 template.format('sub.server_id'), )) - # We must have handled the virtual attribute types. - assert attribute.can_be_materialized() + return _real_condition_sql(servertypes, attribute, template) + + +def _real_condition_sql(servertypes, attribute, template): + model = ServerAttribute.get_model(attribute.type) + assert model is not None # We start with the condition for the attributes the server has on # its own. Then, add the conditions for all possible relations. They @@ -258,7 +267,7 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 ) assert related_via_servertypes if related_via_attribute.type == 'supernet': - relation_condition = _exists_sql('server', 'rel1', ( + relation_condition = _exists_sql(Server, 'rel1', ( "rel1.servertype_id = '{0}'".format( related_via_attribute.target_servertype.pk ), @@ -266,7 +275,7 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 'rel1.server_id = sub.server_id', )) elif related_via_attribute.type == 'reverse_hostname': - relation_condition = _exists_sql(rel_table, 'rel1', ( + relation_condition = _exists_sql(ServerHostnameAttribute, 'rel1', ( "rel1.attribute_id = '{0}'".format( related_via_attribute.reversed_attribute.pk ), @@ -275,7 +284,7 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 )) else: assert related_via_attribute.type == 'hostname' - relation_condition = _exists_sql(rel_table, 'rel1', ( + relation_condition = _exists_sql(ServerHostnameAttribute, 'rel1', ( "rel1.attribute_id = '{0}'".format( related_via_attribute.pk ), @@ -291,7 +300,6 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 ) assert relation_conditions - table = ServerAttribute.get_model(attribute.type)._meta.db_table if len(relation_conditions) == 1: mixed_relation_condition = relation_conditions[0][0] else: @@ -303,16 +311,16 @@ def _condition_sql(servertypes, attribute, template): # NOQA: C901 for relation_condition, servertypes in relation_conditions )) - return _exists_sql(table, 'sub', ( + return _exists_sql(model, 'sub', ( mixed_relation_condition, "sub.attribute_id = '{0}'".format(attribute.pk), template.format('sub.value'), )) -def _exists_sql(table, alias, conditions): +def _exists_sql(model, alias, conditions): return 'EXISTS (SELECT 1 FROM {0} AS {1} WHERE {2})'.format( - table, alias, ' AND '.join(c for c in conditions if c) + model._meta.db_table, alias, ' AND '.join(c for c in conditions if c) ) From 2366f58ca852f4a501331a518602f4aaf0815bec Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Fri, 27 Apr 2018 17:35:11 +0200 Subject: [PATCH 05/15] Fix Django 1.9 compatibility on clone --- serveradmin/servershell/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/serveradmin/servershell/views.py b/serveradmin/servershell/views.py index f25a16aa6..04caaa090 100644 --- a/serveradmin/servershell/views.py +++ b/serveradmin/servershell/views.py @@ -335,10 +335,10 @@ def get_values(request): @login_required # NOQA: C901 def new_server(request): - if 'clone_from' in request.REQUEST: + if 'clone_from' in request.GET: try: clone_from = Query( - {'hostname': request.REQUEST['clone_from']}, + {'hostname': request.GET['clone_from']}, [ a.pk for a in Attribute.objects.all() if a.special or a.can_be_materialized() From 77f860bbefb487f98f23df3170e6497b38e48688 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Fri, 27 Apr 2018 17:26:48 +0200 Subject: [PATCH 06/15] Mark attributes to be cloned explicitly --- .../migrations/0005_attribute_clone.py | 30 +++++++++++++++++++ serveradmin/serverdb/models.py | 4 +-- serveradmin/servershell/views.py | 2 +- 3 files changed, 32 insertions(+), 4 deletions(-) create mode 100644 serveradmin/serverdb/migrations/0005_attribute_clone.py diff --git a/serveradmin/serverdb/migrations/0005_attribute_clone.py b/serveradmin/serverdb/migrations/0005_attribute_clone.py new file mode 100644 index 000000000..75a484af3 --- /dev/null +++ b/serveradmin/serverdb/migrations/0005_attribute_clone.py @@ -0,0 +1,30 @@ +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('serverdb', '0004_attribute_value_constraints'), + ] + + operations = [ + migrations.AddField( + model_name='attribute', + name='clone', + field=models.BooleanField(default=False), + ), + migrations.RunSQL( + 'UPDATE attribute ' + 'SET clone = true ' + "WHERE type NOT IN ('reverse_hostname', 'supernet')" + ), + migrations.RunSQL( + 'ALTER TABLE attribute ' + 'ADD CONSTRAINT attribute_clone_check ' + ' CHECK (' + " NOT clone OR type NOT IN ('reverse_hostname', 'supernet')" + ' )' + ), + ] diff --git a/serveradmin/serverdb/models.py b/serveradmin/serverdb/models.py index 57f3fe44f..abb359444 100644 --- a/serveradmin/serverdb/models.py +++ b/serveradmin/serverdb/models.py @@ -279,6 +279,7 @@ def __init__(self, *args, **kwargs): '_reversed_attribute_id' ) target_servertype = Servertype.foreign_key_lookup('_target_servertype_id') + clone = models.BooleanField(null=False, default=False) class Meta: app_label = 'serverdb' @@ -291,9 +292,6 @@ def related_servertype_attributes(self): _related_via_attribute__isnull=False ) - def can_be_materialized(self): - return bool(ServerAttribute.get_model(self.type)) - def initializer(self): if self.multi: return set diff --git a/serveradmin/servershell/views.py b/serveradmin/servershell/views.py index 04caaa090..113780a9a 100644 --- a/serveradmin/servershell/views.py +++ b/serveradmin/servershell/views.py @@ -341,7 +341,7 @@ def new_server(request): {'hostname': request.GET['clone_from']}, [ a.pk for a in Attribute.objects.all() - if a.special or a.can_be_materialized() + if a.special or a.clone ] ).get() except ValidationError: From b2350888e8630a5a5ae9e879c4753a0cf956d8f7 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Sat, 28 Apr 2018 11:48:05 +0200 Subject: [PATCH 07/15] Do not query any attributes by default --- adminapi/dataset/__init__.py | 4 ++-- serveradmin/dataset/__init__.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/adminapi/dataset/__init__.py b/adminapi/dataset/__init__.py index 3000a57cb..6d0bf3a8f 100644 --- a/adminapi/dataset/__init__.py +++ b/adminapi/dataset/__init__.py @@ -18,7 +18,7 @@ class DatasetError(Exception): class BaseQuery(object): - def __init__(self, filters=None, restrict=None, order_by=None): + def __init__(self, filters=None, restrict=[], order_by=None): if filters is None: self._filters = None self._results = [] @@ -502,7 +502,7 @@ def _handle_exception(result): # XXX: Deprecated, use Query() instead def query(**kwargs): - return Query(kwargs) + return Query(kwargs, None) # XXX: Deprecated, use Query().new_object() instead diff --git a/serveradmin/dataset/__init__.py b/serveradmin/dataset/__init__.py index 8c31ed36c..28f4067c4 100644 --- a/serveradmin/dataset/__init__.py +++ b/serveradmin/dataset/__init__.py @@ -24,4 +24,4 @@ def _fetch_results(self): # XXX: Deprecated def query(**kwargs): - return Query(kwargs) + return Query(kwargs, None) From 4e5dbaf247f2629285ad240e75469943c96a98e9 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Sat, 28 Apr 2018 11:52:22 +0200 Subject: [PATCH 08/15] api: Do not return all attributes for empty --- serveradmin/api/views.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/serveradmin/api/views.py b/serveradmin/api/views.py index cd6e73d73..635ebd678 100644 --- a/serveradmin/api/views.py +++ b/serveradmin/api/views.py @@ -71,13 +71,7 @@ def dataset_query(request, app, data): for attr, filter_obj in data['filters'].items(): filters[attr] = filter_from_obj(filter_obj) - # Empty list means query all attributes to the older versions of - # the adminapi. - if not data.get('restrict'): - restrict = None - else: - restrict = data['restrict'] - + restrict = data.get('restrict') order_by = data.get('order_by') filterer = QueryFilterer(filters) From 4344a4e317a5194c9d0f3bd7489431ae88b53ed7 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Sat, 28 Apr 2018 11:57:07 +0200 Subject: [PATCH 09/15] adminapi: Ask for object_id explicitly --- adminapi/dataset/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/adminapi/dataset/__init__.py b/adminapi/dataset/__init__.py index 6d0bf3a8f..10b8130a1 100644 --- a/adminapi/dataset/__init__.py +++ b/adminapi/dataset/__init__.py @@ -217,7 +217,7 @@ def commit(self): def _fetch_results(self): request_data = {'filters': self._filters} if self._restrict is not None: - request_data['restrict'] = self._restrict + request_data['restrict'] = list(self._restrict) + ['object_id'] if self._order_by is not None: request_data['order_by'] = self._order_by From 4776ef98c0f114603188c1770fc9a30581526bbd Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Sat, 28 Apr 2018 17:24:30 +0200 Subject: [PATCH 10/15] Remove JavaScript debugging leftover --- serveradmin/servershell/static/servershell/servershell.js | 1 - 1 file changed, 1 deletion(-) diff --git a/serveradmin/servershell/static/servershell/servershell.js b/serveradmin/servershell/static/servershell/servershell.js index 9f1796c94..3532ced47 100644 --- a/serveradmin/servershell/static/servershell/servershell.js +++ b/serveradmin/servershell/static/servershell/servershell.js @@ -1030,7 +1030,6 @@ $(function() { $('#shell_servers').empty(); search['page'] = 1; ev.stopPropagation(); - console.log(search); search['term'] = $('#shell_search').val(); refresh_servers(function() { render_server_table(); From e60347a7e1a03142c8da219489f81cf054985bf9 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Fri, 27 Apr 2018 18:31:25 +0200 Subject: [PATCH 11/15] Use serializable database transactions for query --- adminapi/dataset/__init__.py | 4 +- serveradmin/api/views.py | 10 ++-- serveradmin/dataset/__init__.py | 6 +-- serveradmin/serverdb/query_executer.py | 65 ++++++++++++++++++++++++++ serveradmin/serverdb/query_filterer.py | 59 ----------------------- 5 files changed, 72 insertions(+), 72 deletions(-) create mode 100644 serveradmin/serverdb/query_executer.py delete mode 100644 serveradmin/serverdb/query_filterer.py diff --git a/adminapi/dataset/__init__.py b/adminapi/dataset/__init__.py index 10b8130a1..7a6558c1b 100644 --- a/adminapi/dataset/__init__.py +++ b/adminapi/dataset/__init__.py @@ -52,7 +52,7 @@ def __repr__(self): def _get_results(self): if self._results is None: - self._results = list(self._fetch_results()) + self._results = self._fetch_results() return self._results def _fetch_results(self): @@ -224,7 +224,7 @@ def _fetch_results(self): response = send_request(QUERY_ENDPOINT, post_params=request_data) if response['status'] == 'error': _handle_exception(response) - return (_format_obj(s) for s in response['result']) + return [_format_obj(s) for s in response['result']] class DatasetObject(dict): diff --git a/serveradmin/api/views.py b/serveradmin/api/views.py index 635ebd678..58a5fe9af 100644 --- a/serveradmin/api/views.py +++ b/serveradmin/api/views.py @@ -14,10 +14,9 @@ from serveradmin.api.decorators import api_view from serveradmin.api.utils import build_function_description from serveradmin.serverdb.query_committer import QueryCommitter -from serveradmin.serverdb.query_filterer import QueryFilterer +from serveradmin.serverdb.query_executer import execute_query from serveradmin.serverdb.query_materializer import ( - QueryMaterializer, - get_default_attribute_values, + get_default_attribute_values ) @@ -74,12 +73,9 @@ def dataset_query(request, app, data): restrict = data.get('restrict') order_by = data.get('order_by') - filterer = QueryFilterer(filters) - materializer = QueryMaterializer(filterer, restrict, order_by) - return { 'status': 'success', - 'result': list(materializer), + 'result': execute_query(filters, restrict, order_by), } except (FilterValueError, ValidationError) as error: return { diff --git a/serveradmin/dataset/__init__.py b/serveradmin/dataset/__init__.py index 28f4067c4..575aa107e 100644 --- a/serveradmin/dataset/__init__.py +++ b/serveradmin/dataset/__init__.py @@ -1,8 +1,7 @@ from adminapi.dataset import BaseQuery, DatasetObject from serveradmin.serverdb.query_committer import QueryCommitter -from serveradmin.serverdb.query_filterer import QueryFilterer +from serveradmin.serverdb.query_executer import execute_query from serveradmin.serverdb.query_materializer import ( - QueryMaterializer, get_default_attribute_values, ) @@ -18,8 +17,7 @@ def commit(self, app=None, user=None): self._confirm_changes() def _fetch_results(self): - filterer = QueryFilterer(self._filters) - return QueryMaterializer(filterer, self._restrict, self._order_by) + return execute_query(self._filters, self._restrict, self._order_by) # XXX: Deprecated diff --git a/serveradmin/serverdb/query_executer.py b/serveradmin/serverdb/query_executer.py new file mode 100644 index 000000000..03412f476 --- /dev/null +++ b/serveradmin/serverdb/query_executer.py @@ -0,0 +1,65 @@ +from collections import defaultdict + +from django.db import connection, transaction + +from serveradmin.serverdb.models import ( + Attribute, + Servertype, + ServertypeAttribute, + Server, +) +from serveradmin.serverdb.sql_generator import get_server_query +from serveradmin.serverdb.query_materializer import QueryMaterializer + + +@transaction.atomic +def execute_query(filters, restrict, order_by): + connection.cursor().execute( + 'SET TRANSACTION ISOLATION LEVEL REPEATABLE READ READ ONLY' + ) + servers = _get_servers(filters) + materializer = QueryMaterializer(servers, restrict, order_by) + + return list(materializer) + + +def _get_servers(filters): + # We can just deal with the servertype filter ourself. + if 'servertype' in filters: + servertype_filt = filters.pop('servertype') + else: + servertype_filt = None + + attribute_filters = [] + real_attributes = [] + for attribute_id, filt in filters.items(): + attribute = Attribute.objects.get(pk=attribute_id) + attribute_filters.append((attribute, filt)) + if not attribute.special: + real_attributes.append(attribute) + + servertypes = _get_possible_servertypes(real_attributes) + if servertype_filt: + servertypes = list(filter( + lambda s: servertype_filt.matches(s.pk), + servertypes, + )) + + if not servertypes: + return [] + + return Server.objects.raw(get_server_query(servertypes, attribute_filters)) + + +def _get_possible_servertypes(attributes): + servertypes = set(Servertype.objects.all()) + + if attributes: + attribute_servertypes = defaultdict(set) + for sa in ServertypeAttribute.query(attributes=attributes).all(): + attribute_servertypes[sa.attribute].add(sa.servertype) + + for new in attribute_servertypes.values(): + servertypes = servertypes.intersection(new) + + return servertypes diff --git a/serveradmin/serverdb/query_filterer.py b/serveradmin/serverdb/query_filterer.py deleted file mode 100644 index b366a5955..000000000 --- a/serveradmin/serverdb/query_filterer.py +++ /dev/null @@ -1,59 +0,0 @@ -from collections import defaultdict - -from serveradmin.serverdb.models import ( - Attribute, - Servertype, - ServertypeAttribute, - Server, -) -from serveradmin.serverdb.sql_generator import get_server_query - - -class QueryFilterer(object): - def __init__(self, filters): - # We can just deal with the servertype filter ourself. - if 'servertype' in filters: - servertype_filt = filters.pop('servertype') - else: - servertype_filt = None - - self._attribute_filters = [] - real_attributes = [] - for attribute_id, filt in filters.items(): - attribute = Attribute.objects.get(pk=attribute_id) - self._attribute_filters.append((attribute, filt)) - if not attribute.special: - real_attributes.append(attribute) - - self._possible_servertypes = _get_possible_servertypes(real_attributes) - if servertype_filt: - self._possible_servertypes = list(filter( - lambda s: servertype_filt.matches(s.pk), - self._possible_servertypes, - )) - - def __iter__(self): - # If there are no possible matches, there is no need to make - # a database query. - if self._possible_servertypes: - result = Server.objects.raw(get_server_query( - self._possible_servertypes, self._attribute_filters - )) - else: - result = [] - - return iter(result) - - -def _get_possible_servertypes(attributes): - servertypes = set(Servertype.objects.all()) - - if attributes: - attribute_servertypes = defaultdict(set) - for sa in ServertypeAttribute.query(attributes=attributes).all(): - attribute_servertypes[sa.attribute].add(sa.servertype) - - for new in attribute_servertypes.values(): - servertypes = servertypes.intersection(new) - - return servertypes From b7038f36ce938fd8f5e8c6834cb96c9e4e6e1d04 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Mon, 12 Feb 2018 16:51:08 +0100 Subject: [PATCH 12/15] Remove unused Change model --- .../serverdb/migrations/0001_initial.py | 10 -------- serveradmin/serverdb/models.py | 24 ------------------- 2 files changed, 34 deletions(-) diff --git a/serveradmin/serverdb/migrations/0001_initial.py b/serveradmin/serverdb/migrations/0001_initial.py index 1cbb41a0d..d53c11fa1 100644 --- a/serveradmin/serverdb/migrations/0001_initial.py +++ b/serveradmin/serverdb/migrations/0001_initial.py @@ -34,16 +34,6 @@ class Migration(migrations.Migration): 'db_table': 'attribute', }, ), - migrations.CreateModel( - name='Change', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('change_on', models.DateTimeField(db_index=True, default=django.utils.timezone.now)), - ('changes_json', models.TextField()), - ('app', models.ForeignKey(to='apps.Application', on_delete=django.db.models.deletion.PROTECT, null=True)), - ('user', models.ForeignKey(to=settings.AUTH_USER_MODEL, on_delete=django.db.models.deletion.PROTECT, null=True)), - ], - ), migrations.CreateModel( name='ChangeAdd', fields=[ diff --git a/serveradmin/serverdb/models.py b/serveradmin/serverdb/models.py index abb359444..091b5ac78 100644 --- a/serveradmin/serverdb/models.py +++ b/serveradmin/serverdb/models.py @@ -843,30 +843,6 @@ class Meta: # Change Log Models # -class Change(models.Model): - change_on = models.DateTimeField(default=now, db_index=True) - user = models.ForeignKey( - User, - null=True, - on_delete=models.PROTECT, - ) - app = models.ForeignKey( - Application, - null=True, - on_delete=models.PROTECT, - ) - changes_json = models.TextField() - - class Meta: - app_label = 'serverdb' - - @property - def changes(self): - return json.loads(self.changes_json) - - def __str__(self): - return str(self.change_on) - class ChangeCommit(models.Model): change_on = models.DateTimeField(default=now, db_index=True) From 286132e6419e3746088fdbd259f7b2600ceeac37 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Mon, 12 Feb 2018 17:48:25 +0100 Subject: [PATCH 13/15] Make changes a separate module --- .../changes/migrations/0001_initial.py | 70 +++++++++++++++ serveradmin/changes/migrations/__init__.py | 0 serveradmin/changes/models.py | 72 +++++++++++++++ .../templates/changes/commits.html} | 24 ++--- .../templates/changes}/history.html | 2 +- serveradmin/changes/urls.py | 13 +++ serveradmin/{serverdb => changes}/views.py | 25 +++--- serveradmin/serverdb/admin.py | 2 - .../serverdb/migrations/0001_initial.py | 52 ----------- serveradmin/serverdb/models.py | 88 ------------------- serveradmin/serverdb/query_committer.py | 13 ++- serveradmin/serverdb/urls.py | 13 --- .../static/servershell/servershell.js | 2 +- .../templates/servershell/index.html | 4 +- serveradmin/settings.example.py | 1 + 15 files changed, 187 insertions(+), 194 deletions(-) create mode 100644 serveradmin/changes/migrations/0001_initial.py create mode 100644 serveradmin/changes/migrations/__init__.py create mode 100644 serveradmin/changes/models.py rename serveradmin/{serverdb/templates/serverdb/changes.html => changes/templates/changes/commits.html} (63%) rename serveradmin/{serverdb/templates/serverdb => changes/templates/changes}/history.html (95%) create mode 100644 serveradmin/changes/urls.py rename serveradmin/{serverdb => changes}/views.py (79%) delete mode 100644 serveradmin/serverdb/urls.py diff --git a/serveradmin/changes/migrations/0001_initial.py b/serveradmin/changes/migrations/0001_initial.py new file mode 100644 index 000000000..8eb09e92c --- /dev/null +++ b/serveradmin/changes/migrations/0001_initial.py @@ -0,0 +1,70 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations +from django.conf import settings +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ('apps', '0001_initial'), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='Addition', + fields=[ + ('id', models.AutoField(serialize=False, auto_created=True, verbose_name='ID', primary_key=True)), + ('server_id', models.IntegerField(db_index=True)), + ('attributes_json', models.TextField()), + ], + ), + migrations.CreateModel( + name='Commit', + fields=[ + ('id', models.AutoField(serialize=False, auto_created=True, verbose_name='ID', primary_key=True)), + ('change_on', models.DateTimeField(default=django.utils.timezone.now, db_index=True)), + ('app', models.ForeignKey(to='apps.Application', on_delete=django.db.models.deletion.PROTECT, null=True)), + ('user', models.ForeignKey(to=settings.AUTH_USER_MODEL, on_delete=django.db.models.deletion.PROTECT, null=True)), + ], + ), + migrations.CreateModel( + name='Deletion', + fields=[ + ('id', models.AutoField(serialize=False, auto_created=True, verbose_name='ID', primary_key=True)), + ('server_id', models.IntegerField(db_index=True)), + ('attributes_json', models.TextField()), + ('commit', models.ForeignKey(to='changes.Commit')), + ], + ), + migrations.CreateModel( + name='Modification', + fields=[ + ('id', models.AutoField(serialize=False, auto_created=True, verbose_name='ID', primary_key=True)), + ('server_id', models.IntegerField(db_index=True)), + ('updates_json', models.TextField()), + ('commit', models.ForeignKey(to='changes.Commit')), + ], + ), + migrations.AddField( + model_name='addition', + name='commit', + field=models.ForeignKey(to='changes.Commit'), + ), + migrations.AlterUniqueTogether( + name='modification', + unique_together=set([('commit', 'server_id')]), + ), + migrations.AlterUniqueTogether( + name='deletion', + unique_together=set([('commit', 'server_id')]), + ), + migrations.AlterUniqueTogether( + name='addition', + unique_together=set([('commit', 'server_id')]), + ), + ] diff --git a/serveradmin/changes/migrations/__init__.py b/serveradmin/changes/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/serveradmin/changes/models.py b/serveradmin/changes/models.py new file mode 100644 index 000000000..5f97ce64a --- /dev/null +++ b/serveradmin/changes/models.py @@ -0,0 +1,72 @@ +import json + +from django.db import models +from django.utils.timezone import now +from django.contrib.auth.models import User + +from serveradmin.apps.models import Application + + +class Commit(models.Model): + change_on = models.DateTimeField(default=now, db_index=True) + user = models.ForeignKey( + User, + null=True, + on_delete=models.PROTECT, + ) + app = models.ForeignKey( + Application, + null=True, + on_delete=models.PROTECT, + ) + + def __str__(self): + return str(self.change_on) + + +class Addition(models.Model): + commit = models.ForeignKey(Commit, on_delete=models.CASCADE) + server_id = models.IntegerField(db_index=True) + attributes_json = models.TextField() + + class Meta: + unique_together = [['commit', 'server_id']] + + @property + def attributes(self): + return json.loads(self.attributes_json) + + def __str__(self): + return '{0}: {1}'.format(str(self.commit), self.server_id) + + +class Modification(models.Model): + commit = models.ForeignKey(Commit, on_delete=models.CASCADE) + server_id = models.IntegerField(db_index=True) + updates_json = models.TextField() + + class Meta: + unique_together = [['commit', 'server_id']] + + @property + def updates(self): + return json.loads(self.updates_json) + + def __str__(self): + return '{0}: {1}'.format(str(self.commit), self.server_id) + + +class Deletion(models.Model): + commit = models.ForeignKey(Commit, on_delete=models.CASCADE) + server_id = models.IntegerField(db_index=True) + attributes_json = models.TextField() + + class Meta: + unique_together = [['commit', 'server_id']] + + @property + def attributes(self): + return json.loads(self.attributes_json) + + def __str__(self): + return '{0}: {1}'.format(str(self.commit), self.server_id) diff --git a/serveradmin/serverdb/templates/serverdb/changes.html b/serveradmin/changes/templates/changes/commits.html similarity index 63% rename from serveradmin/serverdb/templates/serverdb/changes.html rename to serveradmin/changes/templates/changes/commits.html index 68f3cede3..1d3ab7e27 100644 --- a/serveradmin/serverdb/templates/serverdb/changes.html +++ b/serveradmin/changes/templates/changes/commits.html @@ -26,33 +26,33 @@

Changes

- {% if commit.changedelete_set.count %} - {% for change_del in commit.changedelete_set.all %} + {% if commit.deletion_set.count %} + {% for deletion in commit.deletions_set.all %} {% endif %} - {% if commit.changeadd_set.count %} + {% if commit.addition_set.count %} {% endif %} - {% if commit.changeupdate_set.count %} + {% if commit.modification_set.count %} {% endif %} diff --git a/serveradmin/serverdb/templates/serverdb/history.html b/serveradmin/changes/templates/changes/history.html similarity index 95% rename from serveradmin/serverdb/templates/serverdb/history.html rename to serveradmin/changes/templates/changes/history.html index 36b6e9169..61d351b26 100644 --- a/serveradmin/serverdb/templates/serverdb/history.html +++ b/serveradmin/changes/templates/changes/history.html @@ -97,7 +97,7 @@

Deleted server on {{ change.commit.change_on|date:"DATETIME_FORMAT" }} {% endfor %} {% if commit_id %} -

Show complete history

+

Show complete history

{% endif %} {% endblock %} diff --git a/serveradmin/changes/urls.py b/serveradmin/changes/urls.py new file mode 100644 index 000000000..94ca441bf --- /dev/null +++ b/serveradmin/changes/urls.py @@ -0,0 +1,13 @@ +from django.conf.urls import url + +from serveradmin.changes.views import commits, history, restore_deleted + +urlpatterns = [ + url(r'^changes$', commits, name='changes_commits'), + url( + r'^changes_restore/(\d+)$', + restore_deleted, + name='serverdb_restore_deleted', + ), + url(r'^history$', history, name='changes_history'), +] diff --git a/serveradmin/serverdb/views.py b/serveradmin/changes/views.py similarity index 79% rename from serveradmin/serverdb/views.py rename to serveradmin/changes/views.py index eb7c0ba05..ce6a09c6e 100644 --- a/serveradmin/serverdb/views.py +++ b/serveradmin/changes/views.py @@ -6,25 +6,20 @@ from django.contrib import messages from django.core.urlresolvers import reverse -from serveradmin.serverdb.models import ( - ChangeCommit, - ChangeAdd, - ChangeUpdate, - ChangeDelete, -) +from serveradmin.changes.models import Commit, Addition, Modification, Deletion from serveradmin.serverdb.query_committer import CommitError, QueryCommitter @login_required -def changes(request): - commits = ChangeCommit.objects.order_by('-change_on').prefetch_related() +def commits(request): + commits = Commit.objects.order_by('-change_on').prefetch_related() paginator = Paginator(commits, 100) try: page = paginator.page(request.GET.get('page', 1)) except (PageNotAnInteger, EmptyPage): page = paginator.page(1) - return TemplateResponse(request, 'serverdb/changes.html', { + return TemplateResponse(request, 'changes/commits.html', { 'commits': page }) @@ -40,9 +35,9 @@ def history(request): except (KeyError, ValueError): commit_id = None - adds = ChangeAdd.objects.filter(server_id=server_id).select_related() - updates = ChangeUpdate.objects.filter(server_id=server_id).select_related() - deletes = ChangeDelete.objects.filter(server_id=server_id).select_related() + adds = Addition.objects.filter(server_id=server_id).select_related() + updates = Modification.objects.filter(server_id=server_id).select_related() + deletes = Deletion.objects.filter(server_id=server_id).select_related() if commit_id: adds = adds.filter(commit__pk=commit_id) @@ -63,7 +58,7 @@ def history(request): else: commit.commit_by = 'unknown' - return TemplateResponse(request, 'serverdb/history.html', { + return TemplateResponse(request, 'changes/history.html', { 'change_list': change_list, 'commit_id': commit_id, 'server_id': server_id, @@ -76,7 +71,7 @@ def history(request): @login_required def restore_deleted(request, change_commit): deleted = get_object_or_404( - ChangeDelete, + Deletion, server_id=request.POST.get('server_id'), commit__pk=change_commit, ) @@ -90,6 +85,6 @@ def restore_deleted(request, change_commit): messages.success(request, 'Server restored.') return redirect( - reverse('serverdb_history') + + reverse('changes_history') + '?server_id=' + str(server_obj['object_id']) ) diff --git a/serveradmin/serverdb/admin.py b/serveradmin/serverdb/admin.py index 2b3ff2573..f3520b049 100644 --- a/serveradmin/serverdb/admin.py +++ b/serveradmin/serverdb/admin.py @@ -8,7 +8,6 @@ Server, ServerHostnameAttribute, ServerStringAttribute, - ChangeDelete, ) @@ -42,4 +41,3 @@ class ServerAdmin(admin.ModelAdmin): admin.site.register(Servertype, ServertypeAdmin) admin.site.register(Attribute) admin.site.register(Server, ServerAdmin) -admin.site.register(ChangeDelete) diff --git a/serveradmin/serverdb/migrations/0001_initial.py b/serveradmin/serverdb/migrations/0001_initial.py index d53c11fa1..70e5a6edb 100644 --- a/serveradmin/serverdb/migrations/0001_initial.py +++ b/serveradmin/serverdb/migrations/0001_initial.py @@ -34,41 +34,6 @@ class Migration(migrations.Migration): 'db_table': 'attribute', }, ), - migrations.CreateModel( - name='ChangeAdd', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('server_id', models.IntegerField(db_index=True)), - ('attributes_json', models.TextField()), - ], - ), - migrations.CreateModel( - name='ChangeCommit', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('change_on', models.DateTimeField(db_index=True, default=django.utils.timezone.now)), - ('app', models.ForeignKey(to='apps.Application', on_delete=django.db.models.deletion.PROTECT, null=True)), - ('user', models.ForeignKey(to=settings.AUTH_USER_MODEL, on_delete=django.db.models.deletion.PROTECT, null=True)), - ], - ), - migrations.CreateModel( - name='ChangeDelete', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('server_id', models.IntegerField(db_index=True)), - ('attributes_json', models.TextField()), - ('commit', models.ForeignKey(to='serverdb.ChangeCommit')), - ], - ), - migrations.CreateModel( - name='ChangeUpdate', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('server_id', models.IntegerField(db_index=True)), - ('updates_json', models.TextField()), - ('commit', models.ForeignKey(to='serverdb.ChangeCommit')), - ], - ), migrations.CreateModel( name='Project', fields=[ @@ -210,11 +175,6 @@ class Migration(migrations.Migration): name='_servertype', field=models.ForeignKey(db_column='servertype_id', to='serverdb.Servertype', on_delete=django.db.models.deletion.PROTECT), ), - migrations.AddField( - model_name='changeadd', - name='commit', - field=models.ForeignKey(to='serverdb.ChangeCommit'), - ), migrations.AddField( model_name='attribute', name='_target_servertype', @@ -280,16 +240,4 @@ class Migration(migrations.Migration): name='serverbooleanattribute', index_together=set([('_attribute',)]), ), - migrations.AlterUniqueTogether( - name='changeupdate', - unique_together=set([('commit', 'server_id')]), - ), - migrations.AlterUniqueTogether( - name='changedelete', - unique_together=set([('commit', 'server_id')]), - ), - migrations.AlterUniqueTogether( - name='changeadd', - unique_together=set([('commit', 'server_id')]), - ), ] diff --git a/serveradmin/serverdb/models.py b/serveradmin/serverdb/models.py index 091b5ac78..22c191350 100644 --- a/serveradmin/serverdb/models.py +++ b/serveradmin/serverdb/models.py @@ -1,5 +1,4 @@ import re -import json from collections import OrderedDict from distutils.util import strtobool @@ -12,13 +11,11 @@ from django.core.exceptions import ValidationError from django.core.signals import request_started from django.core.validators import RegexValidator -from django.utils.timezone import now from django.contrib.auth.models import User import netfields from adminapi.datatype import STR_BASED_DATATYPES -from serveradmin.apps.models import Application # @@ -837,88 +834,3 @@ class Meta: db_table = 'server_date_attribute' unique_together = (('server', '_attribute', 'value'), ) index_together = (('_attribute', 'value'), ) - - -# -# Change Log Models -# - - -class ChangeCommit(models.Model): - change_on = models.DateTimeField(default=now, db_index=True) - user = models.ForeignKey( - User, - null=True, - on_delete=models.PROTECT, - ) - app = models.ForeignKey( - Application, - null=True, - on_delete=models.PROTECT, - ) - - class Meta: - app_label = 'serverdb' - - def __str__(self): - return str(self.change_on) - - -class ChangeDelete(models.Model): - commit = models.ForeignKey( - ChangeCommit, - on_delete=models.CASCADE, - ) - server_id = models.IntegerField(db_index=True) - attributes_json = models.TextField() - - class Meta: - app_label = 'serverdb' - unique_together = [['commit', 'server_id']] - - @property - def attributes(self): - return json.loads(self.attributes_json) - - def __str__(self): - return '{0}: {1}'.format(str(self.commit), self.server_id) - - -class ChangeUpdate(models.Model): - commit = models.ForeignKey( - ChangeCommit, - on_delete=models.CASCADE, - ) - server_id = models.IntegerField(db_index=True) - updates_json = models.TextField() - - class Meta: - app_label = 'serverdb' - unique_together = [['commit', 'server_id']] - - @property - def updates(self): - return json.loads(self.updates_json) - - def __str__(self): - return '{0}: {1}'.format(str(self.commit), self.server_id) - - -class ChangeAdd(models.Model): - commit = models.ForeignKey( - ChangeCommit, - on_delete=models.CASCADE, - ) - server_id = models.IntegerField(db_index=True) - attributes_json = models.TextField() - - class Meta: - app_label = 'serverdb' - unique_together = [['commit', 'server_id']] - - @property - def attributes(self): - return json.loads(self.attributes_json) - - def __str__(self): - return '{0}: {1}'.format(str(self.commit), self.server_id) diff --git a/serveradmin/serverdb/query_committer.py b/serveradmin/serverdb/query_committer.py index e07b3ad29..17fc393f7 100644 --- a/serveradmin/serverdb/query_committer.py +++ b/serveradmin/serverdb/query_committer.py @@ -8,6 +8,7 @@ from adminapi.dataset import DatasetCommit from adminapi.request import json_encode_extra +from serveradmin.changes.models import Addition, Commit, Modification, Deletion from serveradmin.hooks.slots import HookSlot from serveradmin.serverdb.models import ( Servertype, @@ -15,10 +16,6 @@ Server, ServerAttribute, ServerHostnameAttribute, - ChangeAdd, - ChangeCommit, - ChangeUpdate, - ChangeDelete, Project, ) from serveradmin.serverdb.query_materializer import ( @@ -143,10 +140,10 @@ def _validate(self): raise CommitNewerData('Newer data available', newer) def _log_changes(self, created_objects): - commit = ChangeCommit.objects.create(app=self.app, user=self.user) + commit = Commit.objects.create(app=self.app, user=self.user) for updates in self.changed: - ChangeUpdate.objects.create( + Modification.objects.create( commit=commit, server_id=updates['object_id'], updates_json=json.dumps(updates, default=json_encode_extra), @@ -154,7 +151,7 @@ def _log_changes(self, created_objects): for attributes in self._deleted_objects.values(): attributes_json = json.dumps(attributes, default=json_encode_extra) - ChangeDelete.objects.create( + Deletion.objects.create( commit=commit, server_id=attributes['object_id'], attributes_json=attributes_json, @@ -164,7 +161,7 @@ def _log_changes(self, created_objects): attributes_json = json.dumps( obj, default=json_encode_extra ) - ChangeAdd.objects.create( + Addition.objects.create( commit=commit, server_id=obj['object_id'], attributes_json=attributes_json, diff --git a/serveradmin/serverdb/urls.py b/serveradmin/serverdb/urls.py deleted file mode 100644 index 936f3517d..000000000 --- a/serveradmin/serverdb/urls.py +++ /dev/null @@ -1,13 +0,0 @@ -from django.conf.urls import url - -from serveradmin.serverdb.views import changes, restore_deleted, history - -urlpatterns = [ - url(r'^changes$', changes, name='serverdb_changes'), - url( - r'^changes_restore/(\d+)$', - restore_deleted, - name='serverdb_restore_deleted', - ), - url(r'^history$', history, name='serverdb_history'), -] diff --git a/serveradmin/servershell/static/servershell/servershell.js b/serveradmin/servershell/static/servershell/servershell.js index 3532ced47..d89b9ce3c 100644 --- a/serveradmin/servershell/static/servershell/servershell.js +++ b/serveradmin/servershell/static/servershell/servershell.js @@ -669,7 +669,7 @@ function handle_command_changes() { function handle_command_history() { function show_history(server) { var query_str = '?' + $.param({'server_id': server['object_id']}); - $.get(serverdb_history_url + query_str, function(data) { + $.get(changes_history_url + query_str, function(data) { var dialog = $('
'); dialog.append(data); dialog.dialog({ diff --git a/serveradmin/servershell/templates/servershell/index.html b/serveradmin/servershell/templates/servershell/index.html index 081505f2d..dec14eba0 100644 --- a/serveradmin/servershell/templates/servershell/index.html +++ b/serveradmin/servershell/templates/servershell/index.html @@ -313,9 +313,9 @@

Available commands

var shell_values_url = "{% url 'servershell_values' %}"; var shell_new_url = "{% url 'servershell_new' %}"; var shell_graph_url = "{% url 'graphite_graph_table' %}"; -var shell_changes_url = "{% url 'serverdb_changes' %}"; +var shell_changes_url = "{% url 'changes_commits' %}"; var shell_store_command_url = "{% url 'servershell_store_command' %}"; -var serverdb_history_url = "{% url 'serverdb_history' %}"; +var changes_history_url = "{% url 'changes_history' %}"; //]]> diff --git a/serveradmin/settings.example.py b/serveradmin/settings.example.py index df9106fba..2dfab59b4 100644 --- a/serveradmin/settings.example.py +++ b/serveradmin/settings.example.py @@ -157,6 +157,7 @@ 'serveradmin.common', 'serveradmin.servershell', 'serveradmin.apps', + 'serveradmin.changes', 'serveradmin.serverdb', 'serveradmin.api', 'serveradmin.graphite', From 4bf7663d4681fb33f42ccd44f1f3449370e53ec2 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Mon, 12 Feb 2018 17:56:34 +0100 Subject: [PATCH 14/15] Add changes to Django Admin --- serveradmin/changes/admin.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 serveradmin/changes/admin.py diff --git a/serveradmin/changes/admin.py b/serveradmin/changes/admin.py new file mode 100644 index 000000000..fa4a99f62 --- /dev/null +++ b/serveradmin/changes/admin.py @@ -0,0 +1,12 @@ +from django.contrib.admin import site, ModelAdmin, TabularInline + +from serveradmin.changes.models import Commit, Addition, Modification, Deletion + + +site.register(Commit, type('CommitAdmin', (ModelAdmin, ), { + 'inlines': [ + type('AdditionInline', (TabularInline, ), {'model': Addition}), + type('ModificationInline', (TabularInline, ), {'model': Modification}), + type('DeletionInline', (TabularInline, ), {'model': Deletion}), + ] +})) From dd36c173c6d9db22ee1f96315cf2340b0d4d4882 Mon Sep 17 00:00:00 2001 From: Emre Hasegeli Date: Mon, 12 Feb 2018 17:57:02 +0100 Subject: [PATCH 15/15] Allow subscribing to changes --- adminapi/dataset/__init__.py | 30 +++++++++++++++++++++++++++ serveradmin/api/urls.py | 2 ++ serveradmin/api/views.py | 40 ++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/adminapi/dataset/__init__.py b/adminapi/dataset/__init__.py index 7a6558c1b..fc0dfee88 100644 --- a/adminapi/dataset/__init__.py +++ b/adminapi/dataset/__init__.py @@ -1,6 +1,7 @@ from distutils.util import strtobool from ipaddress import IPv4Address, IPv4Network, IPv6Address, IPv6Network from itertools import chain +from time import sleep from types import GeneratorType from adminapi.datatype import validate_value, json_to_datatype @@ -11,6 +12,7 @@ COMMIT_ENDPOINT = '/dataset/commit' QUERY_ENDPOINT = '/dataset/query' CREATE_ENDPOINT = '/dataset/create' +GET_COMMITS_ENDPOINT = '/dataset/get_commits' class DatasetError(Exception): @@ -194,6 +196,17 @@ def get_free_ip_addrs(self): if host not in used_hosts: yield host + def subscribe_changes(self, callback, newer_than_mins=None, slot=None): + self._results = self._fetch_results() + while True: + for commit in self._fetch_commits(newer_than_mins, slot): + callback(commit) + self._save_changes_slot(commit) + sleep(1) + + def _apply_commit(self, commit): + pass + class Query(BaseQuery): @@ -226,6 +239,23 @@ def _fetch_results(self): _handle_exception(response) return [_format_obj(s) for s in response['result']] + def _fetch_commits(self, newer_than_mins=None, slot=None): + request_data = {'filters': self._filters} + if self._restrict is not None: + request_data['restrict'] = self._restrict + if self._order_by is not None: + request_data['order_by'] = self._order_by + if newer_than_mins is not None: + request_data['newer_than_mins'] = newer_than_mins + if slot is not None: + request_data['slot'] = slot + + response = send_request(GET_COMMITS_ENDPOINT, post_params=request_data) + for result in response['result']: + print(result) + if False: + yield result + class DatasetObject(dict): """This class must redefine all mutable methods of the dict class diff --git a/serveradmin/api/urls.py b/serveradmin/api/urls.py index 145de8a78..cffbecf03 100644 --- a/serveradmin/api/urls.py +++ b/serveradmin/api/urls.py @@ -6,6 +6,7 @@ dataset_commit, dataset_new_object, dataset_create, + dataset_get_commits, api_call, ) @@ -15,5 +16,6 @@ url('^dataset/commit$', dataset_commit), url('^dataset/new_object$', dataset_new_object), url('^dataset/create$', dataset_create), + url('^dataset/get_commits$', dataset_get_commits), url('^call$', api_call), ] diff --git a/serveradmin/api/views.py b/serveradmin/api/views.py index 58a5fe9af..635783e82 100644 --- a/serveradmin/api/views.py +++ b/serveradmin/api/views.py @@ -1,3 +1,4 @@ +from datetime import datetime, timedelta from operator import itemgetter from django.core.exceptions import ( @@ -13,6 +14,8 @@ from serveradmin.api import ApiError, AVAILABLE_API_FUNCTIONS from serveradmin.api.decorators import api_view from serveradmin.api.utils import build_function_description +from serveradmin.apps.models import ApplicationSlot +from serveradmin.changes.models import Commit from serveradmin.serverdb.query_committer import QueryCommitter from serveradmin.serverdb.query_executer import execute_query from serveradmin.serverdb.query_materializer import ( @@ -211,6 +214,43 @@ def dataset_create(request, app, data): } +@api_view +def dataset_get_commits(request, app, data): + if request.method != 'POST': + raise SuspiciousOperation('Method not allowed') + if not isinstance(data, dict): + raise SuspiciousOperation('Invalid payload') + + if 'filters' not in data or not isinstance(data['filters'], dict): + raise SuspiciousOperation('"filters" must be a dictionary') + + queryset = Commit.objects + + if 'newer_than_mins' in data: + if not isinstance(data['newer_than_mins'], int): + raise SuspiciousOperation('"newer_than_mins" must be an int') + queryset = queryset.filter(change_on__gt=( + datetime.now() - timedelta(minutes=data['newer_than_mins']) + )) + + if 'slot' in data: + if not isinstance(data['slot'], str): + raise SuspiciousOperation('"newer_than_mins" must be a string') + try: + slot = ApplicationSlot.objects.get( + application=app, name=data['slot'] + ) + except ApplicationSlot.DoesNotExist: + pass + else: + queryset = queryset.filter(id__gt=slot.commit_id) + + return [{ + 'user': c.user.username, + 'application': c.app.application_id, + } for c in queryset] + + @api_view def api_call(request, app, data): try: