From 03289bec541c1f644b15e505d4e8f1987426775f Mon Sep 17 00:00:00 2001 From: Benjamin Bertrand <benjamin.bertrand@esss.se> Date: Fri, 6 Apr 2018 16:26:18 +0200 Subject: [PATCH] Add machine_type table "type" field on host replaced with foreign key "machine_type_id" JIRA INFRA-281 --- app/api/network.py | 4 +- app/models.py | 28 +++++++++- app/network/forms.py | 3 +- app/network/views.py | 8 +-- app/static/js/hosts.js | 18 +++++-- app/templates/network/create_host.html | 2 +- app/templates/network/edit_host.html | 4 +- app/templates/network/view_host.html | 6 +-- .../ac6b3c416b07_add_machine_type_table.py | 52 +++++++++++++++++++ tests/functional/conftest.py | 1 + tests/functional/factories.py | 10 ++++ tests/functional/test_api.py | 21 +++++--- 12 files changed, 132 insertions(+), 25 deletions(-) create mode 100644 migrations/versions/ac6b3c416b07_add_machine_type_table.py diff --git a/app/api/network.py b/app/api/network.py index e5f689d..d47f628 100644 --- a/app/api/network.py +++ b/app/api/network.py @@ -141,11 +141,11 @@ def create_host(): .. :quickref: Network; Create new host :jsonparam name: hostname - :jsonparam type: Physical|Virtual + :jsonparam machine_type: Physical|Virtual|... :jsonparam description: (optional) description :jsonparam item_id: (optional) linked item primary key """ - return create_generic_model(models.Host, mandatory_fields=('name',)) + return create_generic_model(models.Host, mandatory_fields=('name', 'machine_type')) @bp.route('/macs') diff --git a/app/models.py b/app/models.py index d726733..2dce67c 100644 --- a/app/models.py +++ b/app/models.py @@ -520,14 +520,38 @@ class Tag(QRCodeMixin, db.Model): admin_only = db.Column(db.Boolean, nullable=False, default=False) +class MachineType(db.Model): + __tablename__ = 'machine_type' + id = db.Column(db.Integer, primary_key=True) + name = db.Column(CIText, nullable=False, unique=True) + + hosts = db.relationship('Host', backref='machine_type') + + def __str__(self): + return self.name + + def to_dict(self): + return { + 'id': self.id, + 'name': self.name, + 'hosts': [str(host) for host in self.hosts] + } + + class Host(CreatedMixin, db.Model): name = db.Column(db.Text, nullable=False, unique=True) - type = db.Column(db.Text) description = db.Column(db.Text) + machine_type_id = db.Column(db.Integer, db.ForeignKey('machine_type.id'), nullable=False) item_id = db.Column(db.Integer, db.ForeignKey('item.id')) interfaces = db.relationship('Interface', backref='host') + def __init__(self, **kwargs): + # Automatically convert machine_type as an instance of its class if passed as a string + if 'machine_type' in kwargs: + kwargs['machine_type'] = utils.convert_to_model(kwargs['machine_type'], MachineType) + super().__init__(**kwargs) + def __str__(self): return str(self.name) @@ -546,7 +570,7 @@ class Host(CreatedMixin, db.Model): d = super().to_dict() d.update({ 'name': self.name, - 'type': self.type, + 'machine_type': str(self.machine_type), 'description': self.description, 'item': utils.format_field(self.item), 'interfaces': [str(interface) for interface in self.interfaces], diff --git a/app/network/forms.py b/app/network/forms.py index 268bbb3..21d1763 100644 --- a/app/network/forms.py +++ b/app/network/forms.py @@ -124,12 +124,13 @@ class HostForm(CSEntryForm): validators.Regexp(HOST_NAME_RE), Unique(models.Host)], filters=[utils.lowercase_field]) - type = SelectField('Type', choices=utils.get_choices(('Virtual', 'Physical'))) description = TextAreaField('Description') + machine_type_id = SelectField('Machine Type') item_id = SelectField('Item', coerce=utils.coerce_to_str_or_none) def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + self.machine_type_id.choices = utils.get_model_choices(models.MachineType) self.item_id.choices = utils.get_model_choices(models.Item, allow_none=True, attr='ics_id') diff --git a/app/network/views.py b/app/network/views.py index a4e8da9..10f16e5 100644 --- a/app/network/views.py +++ b/app/network/views.py @@ -48,7 +48,7 @@ def create_host(): if form.validate_on_submit(): network_id = form.network_id.data host = models.Host(name=form.name.data, - type=form.type.data, + machine_type_id=form.machine_type_id.data, description=form.description.data or None, item_id=form.item_id.data) # The total number of tags will always be quite small @@ -93,7 +93,7 @@ def edit_host(name): form = HostForm(request.form, obj=host) if form.validate_on_submit(): host.name = form.name.data - host.type = form.type.data + host.machine_type_id = form.machine_type_id.data host.item_id = form.item_id.data host.description = form.description.data or None current_app.logger.debug(f'Trying to update: {host!r}') @@ -113,7 +113,7 @@ def edit_host(name): @login_groups_accepted('admin', 'create') def create_interface(hostname): host = models.Host.query.filter_by(name=hostname).first_or_404() - random_mac = host.type == 'Virtual' + random_mac = host.machine_type.name == 'Virtual' form = InterfaceForm(request.form, host_id=host.id, interface_name=host.name, random_mac=random_mac) if form.validate_on_submit(): @@ -280,7 +280,7 @@ def create_scope(): @login_required def retrieve_hosts(): data = [(host.name, - host.type, + str(host.machine_type), host.description, interface.name, interface.ip, diff --git a/app/static/js/hosts.js b/app/static/js/hosts.js index 62f5a59..3574fff 100644 --- a/app/static/js/hosts.js +++ b/app/static/js/hosts.js @@ -43,12 +43,22 @@ $(document).ready(function() { set_default_ip(); }); - // Enable / disable item_id field depending on type + // Enable / disable item_id on edit host first page load + if( $("#editHostForm").length ) { + var machine_type = $("#machine_type_id option:selected").text(); + if( machine_type == "Physical" ) { + $("#item_id").prop("disabled", false); + } else { + $("#item_id").prop("disabled", true); + } + } + + // Enable / disable item_id field depending on machine_type // Item can only be assigned for physical hosts // And check / uncheck random_mac checkbox - $("#type").on('change', function() { - var host_type = $(this).val(); - if( host_type == "Physical" ) { + $("#machine_type_id").on('change', function() { + var machine_type = $("#machine_type_id option:selected").text(); + if( machine_type == "Physical" ) { $("#item_id").prop("disabled", false); $("#random_mac").prop("checked", false).change(); } else { diff --git a/app/templates/network/create_host.html b/app/templates/network/create_host.html index 1a2f35f..515296e 100644 --- a/app/templates/network/create_host.html +++ b/app/templates/network/create_host.html @@ -7,7 +7,7 @@ <form id="hostForm" method="POST"> {{ form.hidden_tag() }} {{ render_field(form.name, class_="text-lowercase") }} - {{ render_field(form.type) }} + {{ render_field(form.machine_type_id) }} {{ render_field(form.description) }} {{ render_field(form.item_id, disabled=True) }} {{ render_field(form.network_id) }} diff --git a/app/templates/network/edit_host.html b/app/templates/network/edit_host.html index fedffe3..6de7213 100644 --- a/app/templates/network/edit_host.html +++ b/app/templates/network/edit_host.html @@ -19,9 +19,9 @@ <form id="editHostForm" method="POST"> {{ form.hidden_tag() }} {{ render_field(form.name, class_="text-lowercase") }} - {{ render_field(form.type) }} + {{ render_field(form.machine_type_id) }} {{ render_field(form.description) }} - {{ render_field(form.item_id, disabled=True) }} + {{ render_field(form.item_id) }} <div class="form-group row"> <div class="col-sm-10"> <button type="submit" class="btn btn-primary">Submit</button> diff --git a/app/templates/network/view_host.html b/app/templates/network/view_host.html index 50ae3f9..bb38187 100644 --- a/app/templates/network/view_host.html +++ b/app/templates/network/view_host.html @@ -19,9 +19,9 @@ <dl class="row"> <dt class="col-sm-3">Hostname</dt> <dd class="col-sm-9">{{ host.name }}</dd> - <dt class="col-sm-3">Type</dt> - <dd class="col-sm-9">{{ host.type }}</dd> - {% if host.type == 'Physical' %} + <dt class="col-sm-3">Machine Type</dt> + <dd class="col-sm-9">{{ host.machine_type }}</dd> + {% if host.machine_type.name == 'Physical' %} <dt class="col-sm-3">Item</dt> <dd class="col-sm-9">{{ link_to_item(host.item) }}</dd> {% endif %} diff --git a/migrations/versions/ac6b3c416b07_add_machine_type_table.py b/migrations/versions/ac6b3c416b07_add_machine_type_table.py new file mode 100644 index 0000000..a2ff1ca --- /dev/null +++ b/migrations/versions/ac6b3c416b07_add_machine_type_table.py @@ -0,0 +1,52 @@ +"""Add machine_type table + +Revision ID: ac6b3c416b07 +Revises: dfd4eae61224 +Create Date: 2018-04-06 12:17:16.469046 + +""" +from alembic import op +import sqlalchemy as sa +import citext + + +# revision identifiers, used by Alembic. +revision = 'ac6b3c416b07' +down_revision = 'dfd4eae61224' +branch_labels = None +depends_on = None + + +def upgrade(): + machine_type = op.create_table( + 'machine_type', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('name', citext.CIText(), nullable=False), + sa.PrimaryKeyConstraint('id', name=op.f('pk_machine_type')), + sa.UniqueConstraint('name', name=op.f('uq_machine_type_name')) + ) + # WARNING! If the database is not emppty, we can't set the machine_type_id to nullable=False before adding a value! + op.add_column('host', sa.Column('machine_type_id', sa.Integer(), nullable=True)) + op.create_foreign_key(op.f('fk_host_machine_type_id_machine_type'), 'host', 'machine_type', ['machine_type_id'], ['id']) + # Create the Physical and Virtual machine types + op.execute(machine_type.insert().values([ + {'id': 1, 'name': 'Physical'}, + {'id': 2, 'name': 'Virtual'}, + ])) + # Fill the host machine_type_id based on the value from the type column + host = sa.sql.table('host', sa.sql.column('machine_type_id'), sa.sql.column('type')) + op.execute(host.update().where(host.c.type == 'Physical').values(machine_type_id=1)) + op.execute(host.update().where(host.c.type == 'Virtual').values(machine_type_id=2)) + op.drop_column('host', 'type') + # Add the nullable=False constraint + op.alter_column('host', 'machine_type_id', nullable=False) + + +def downgrade(): + op.add_column('host', sa.Column('type', sa.TEXT(), autoincrement=False, nullable=True)) + host = sa.sql.table('host', sa.sql.column('machine_type_id'), sa.sql.column('type')) + op.execute(host.update().where(host.c.machine_type_id == 1).values(type='Physical')) + op.execute(host.update().where(host.c.machine_type_id == 2).values(type='Virtual')) + op.drop_constraint(op.f('fk_host_machine_type_id_machine_type'), 'host', type_='foreignkey') + op.drop_column('host', 'machine_type_id') + op.drop_table('machine_type') diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 05575ff..597f36e 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -27,6 +27,7 @@ register(factories.ItemFactory) register(factories.NetworkScopeFactory) register(factories.NetworkFactory) register(factories.InterfaceFactory) +register(factories.MachineTypeFactory) register(factories.HostFactory) register(factories.MacFactory) register(factories.DomainFactory) diff --git a/tests/functional/factories.py b/tests/functional/factories.py index a5fc4d3..b101dc6 100644 --- a/tests/functional/factories.py +++ b/tests/functional/factories.py @@ -151,6 +151,15 @@ class InterfaceFactory(factory.alchemy.SQLAlchemyModelFactory): user = factory.SubFactory(UserFactory) +class MachineTypeFactory(factory.alchemy.SQLAlchemyModelFactory): + class Meta: + model = models.MachineType + sqlalchemy_session = common.Session + sqlalchemy_session_persistence = 'commit' + + name = factory.Sequence(lambda n: f'Type{n}') + + class HostFactory(factory.alchemy.SQLAlchemyModelFactory): class Meta: model = models.Host @@ -159,6 +168,7 @@ class HostFactory(factory.alchemy.SQLAlchemyModelFactory): name = factory.Sequence(lambda n: f'host{n}') user = factory.SubFactory(UserFactory) + machine_type = factory.SubFactory(MachineTypeFactory) class MacFactory(factory.alchemy.SQLAlchemyModelFactory): diff --git a/tests/functional/test_api.py b/tests/functional/test_api.py index 7faab03..50646c9 100644 --- a/tests/functional/test_api.py +++ b/tests/functional/test_api.py @@ -758,16 +758,22 @@ def test_get_hosts(client, host_factory, readonly_token): check_input_is_subset_of_response(response, (host1.to_dict(), host2.to_dict())) -def test_create_host(client, item_factory, user_token): +def test_create_host(client, item_factory, machine_type_factory, user_token): item = item_factory() - # check that name is mandatory + machine_type = machine_type_factory(name='Virtual') + # check that name and machine_type are mandatory response = post(client, f'{API_URL}/network/hosts', data={}, token=user_token) check_response_message(response, "Missing mandatory field 'name'", 422) + response = post(client, f'{API_URL}/network/hosts', data={'name': 'myhost'}, token=user_token) + check_response_message(response, "Missing mandatory field 'machine_type'", 422) + response = post(client, f'{API_URL}/network/hosts', data={'machine_type': 'Physical'}, token=user_token) + check_response_message(response, "Missing mandatory field 'name'", 422) - data = {'name': 'my-hostname'} + data = {'name': 'my-hostname', + 'machine_type': machine_type.name} response = post(client, f'{API_URL}/network/hosts', data=data, token=user_token) assert response.status_code == 201 - assert {'id', 'name', 'type', 'description', + assert {'id', 'name', 'machine_type', 'description', 'item', 'interfaces', 'created_at', 'updated_at', 'user'} == set(response.json.keys()) assert response.json['name'] == data['name'] @@ -778,6 +784,7 @@ def test_create_host(client, item_factory, user_token): # Check that we can pass an item_id data2 = {'name': 'another-hostname', + 'machine_type': machine_type.name, 'item_id': item.id} response = post(client, f'{API_URL}/network/hosts', data=data2, token=user_token) assert response.status_code == 201 @@ -786,8 +793,10 @@ def test_create_host(client, item_factory, user_token): assert models.Host.query.count() == 2 -def test_create_host_as_consultant(client, item_factory, consultant_token): - data = {'name': 'my-hostname'} +def test_create_host_as_consultant(client, item_factory, machine_type_factory, consultant_token): + machine_type = machine_type_factory() + data = {'name': 'my-hostname', + 'machine_type': machine_type.name} response = post(client, f'{API_URL}/network/hosts', data=data, token=consultant_token) assert response.status_code == 201 -- GitLab