diff --git a/app/api/network.py b/app/api/network.py index 584eff344b15877745160c96103d673352bc491a..ff90b8748f9eb983b055fe489f09420040fa25ec 100644 --- a/app/api/network.py +++ b/app/api/network.py @@ -129,7 +129,7 @@ def create_interface(): :jsonparam network: network name :jsonparam ip: interface IP :jsonparam name: interface name - :jsonparam mac_id: (optional) linked MAC address primary key + :jsonparam mac: (optional) MAC address :jsonparam host_id: (optional) linked host primary key """ # The validate_interfaces method from the Network class is called when diff --git a/app/helpers.py b/app/helpers.py index eff5bcabff89ad61a9ee0d6c6b5adc38167d1613..c40e557ba60da7a9bb393337eef3bdea96dcd5f2 100644 --- a/app/helpers.py +++ b/app/helpers.py @@ -9,10 +9,7 @@ This module implements helpers functions for the models. :license: BSD 2-Clause, see LICENSE for more details. """ -import sqlalchemy as sa from flask_wtf import FlaskForm -from .extensions import db -from . import models class CSEntryForm(FlaskForm): @@ -29,24 +26,6 @@ class CSEntryForm(FlaskForm): super().__init__(formdata=formdata, obj=obj, **kwargs) -def associate_mac_to_interface(address, interface): - """Associate the given address to an interface - - The Mac is retrieved if it exists or created otherwise - - :param address: Mac address string - :param interface: Interface instance - """ - if not address: - return - try: - mac = models.Mac.query.filter_by(address=address).one() - except sa.orm.exc.NoResultFound: - mac = models.Mac(address=address) - db.session.add(mac) - mac.interfaces.append(interface) - - def get_model(class_, id_): if id_ is None: return None diff --git a/app/models.py b/app/models.py index cc9e2dae5ddc4a9abc99998c561979fcce43fedf..7795899e0ba3015e049288218d3ff362893726c8 100644 --- a/app/models.py +++ b/app/models.py @@ -1144,7 +1144,7 @@ class Interface(CreatedMixin, db.Model): network_id = db.Column(db.Integer, db.ForeignKey("network.id"), nullable=False) ip = db.Column(postgresql.INET, nullable=False, unique=True) name = db.Column(db.Text, nullable=False, unique=True) - mac_id = db.Column(db.Integer, db.ForeignKey("mac.id")) + mac = db.Column(postgresql.MACADDR, nullable=True, unique=True) host_id = db.Column(db.Integer, db.ForeignKey("host.id")) # Add delete and delete-orphan options to automatically delete cnames when: @@ -1195,6 +1195,15 @@ class Interface(CreatedMixin, db.Model): raise ValidationError("Interface name shall match [a-z0-9\-]{2,20}") return lower_string + @validates("mac") + def validate_mac(self, key, string): + """Ensure the mac is a valid MAC address""" + if not string: + return None + if MAC_ADDRESS_RE.fullmatch(string) is None: + raise ValidationError(f"'{string}' does not appear to be a MAC address") + return string + @property def address(self): return ipaddress.ip_address(self.ip) @@ -1240,10 +1249,6 @@ class Mac(db.Model): address = db.Column(postgresql.MACADDR, nullable=False, unique=True) item_id = db.Column(db.Integer, db.ForeignKey("item.id")) - interfaces = db.relationship( - "Interface", backref=db.backref("mac", lazy="joined"), lazy=True - ) - def __str__(self): return str(self.address) @@ -1261,7 +1266,6 @@ class Mac(db.Model): "id": self.id, "address": self.address, "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 aa66f2340e3820d17951b83893f1a16ac0ffd41d..83d478cc6089806394ea716557ad4d90ebae40c5 100644 --- a/app/network/forms.py +++ b/app/network/forms.py @@ -183,11 +183,12 @@ class InterfaceForm(CSEntryForm): filters=[utils.lowercase_field], ) random_mac = BooleanField("Random MAC", default=False) - mac_address = StringField( + mac = StringField( "MAC", validators=[ validators.Optional(), validators.Regexp(MAC_ADDRESS_RE, message="Invalid MAC address"), + Unique(models.Interface, column="mac"), ], ) cnames_string = StringField( diff --git a/app/network/views.py b/app/network/views.py index a6a4a451e1d0b86e82aff69c558fb04501b2c315..555c094d5ec7dacd6b541de2866029e94212d61a 100644 --- a/app/network/views.py +++ b/app/network/views.py @@ -36,7 +36,7 @@ from .forms import ( ) from ..extensions import db from ..decorators import login_groups_accepted -from .. import models, utils, helpers, tasks +from .. import models, utils, tasks bp = Blueprint("network", __name__) @@ -89,13 +89,13 @@ def create_host(): interface = models.Interface( name=form.interface_name.data, ip=form.ip.data, + mac=form.mac.data, network=models.Network.query.get(network_id), tags=tags, ) interface.cnames = [ models.Cname(name=name) for name in form.cnames_string.data.split() ] - helpers.associate_mac_to_interface(form.mac_address.data, interface) host.interfaces = [interface] current_app.logger.debug(f"Trying to create: {host!r}") db.session.add(host) @@ -230,13 +230,13 @@ def create_interface(hostname): interface = models.Interface( name=form.interface_name.data, ip=form.ip.data, + mac=form.mac.data, network=models.Network.query.get(form.network_id.data), tags=tags, ) interface.cnames = [ models.Cname(name=name) for name in form.cnames_string.data.split() ] - helpers.associate_mac_to_interface(form.mac_address.data, interface) # Make sure to update host.interfaces instead of using interface.host_id # to force the host to be added to the session for indexing host.interfaces.append(interface) @@ -261,15 +261,10 @@ def create_interface(hostname): def edit_interface(name): interface = models.Interface.query.filter_by(name=name).first_or_404() cnames_string = " ".join([str(cname) for cname in interface.cnames]) - try: - mac_address = interface.mac.address - except AttributeError: - mac_address = "" form = InterfaceForm( request.form, obj=interface, interface_name=interface.name, - mac_address=mac_address, cnames_string=cnames_string, ) # Remove the random_mac field (not used when editing) @@ -286,18 +281,10 @@ def edit_interface(name): if form.validate_on_submit(): interface.name = form.interface_name.data interface.ip = form.ip.data + interface.mac = form.mac.data # Setting directly network_id doesn't update the relationship and bypass the checks # performed on the model interface.network = models.Network.query.get(form.network_id.data) - if form.mac_address.data: - if form.mac_address.data != mac_address: - # The MAC changed - add the new one to the interface - # that will remove the association to the previous one - helpers.associate_mac_to_interface(form.mac_address.data, interface) - # else: nothing to do (address didn't change) - else: - # No MAC associated - interface.mac = None # Delete the cnames that have been removed new_cnames_string = form.cnames_string.data.split() for (index, cname) in enumerate(interface.cnames): diff --git a/app/templates/network/create_host.html b/app/templates/network/create_host.html index 85cb94c6b2d7720de5583d23f2f41a49cf5e76ec..92a33ea23f63cad8b66b18f273959d2f6964198b 100644 --- a/app/templates/network/create_host.html +++ b/app/templates/network/create_host.html @@ -13,7 +13,7 @@ {{ render_field(form.network_id) }} {{ render_field(form.ip) }} {{ render_field(form.random_mac) }} - {{ render_field(form.mac_address) }} + {{ render_field(form.mac) }} {{ render_field(form.cnames_string) }} {{ render_field(form.tags, class_="selectpicker") }} {{ render_field(form.ansible_vars) }} diff --git a/app/templates/network/create_interface.html b/app/templates/network/create_interface.html index afeae4047f60b68818caea9c13083acd0c5697a5..a090a6ccab8b38d2b5de6d3b58a6a9acd0af3aaf 100644 --- a/app/templates/network/create_interface.html +++ b/app/templates/network/create_interface.html @@ -24,7 +24,7 @@ {{ render_field(form.network_id) }} {{ render_field(form.ip) }} {{ render_field(form.random_mac) }} - {{ render_field(form.mac_address) }} + {{ render_field(form.mac) }} {{ render_field(form.cnames_string) }} {{ render_field(form.tags, class_="selectpicker") }} <div class="form-group row"> diff --git a/app/templates/network/edit_interface.html b/app/templates/network/edit_interface.html index 7f6ae115fb9ed1cba3063affdb8d80f108e63a5a..82a122f160ccdec89783a156b09fdfd2a8b0b08f 100644 --- a/app/templates/network/edit_interface.html +++ b/app/templates/network/edit_interface.html @@ -26,7 +26,7 @@ {{ render_field(form.interface_name, class_="text-lowercase") }} {{ render_field(form.network_id) }} {{ render_field(form.ip) }} - {{ render_field(form.mac_address) }} + {{ render_field(form.mac) }} {{ render_field(form.cnames_string) }} {{ render_field(form.tags, class_="selectpicker") }} <div class="form-group row"> diff --git a/migrations/versions/9184cc675b4e_move_mac_address_to_interface_table.py b/migrations/versions/9184cc675b4e_move_mac_address_to_interface_table.py new file mode 100644 index 0000000000000000000000000000000000000000..dbf116f37e86b00839c3b19e416757a04dceac2d --- /dev/null +++ b/migrations/versions/9184cc675b4e_move_mac_address_to_interface_table.py @@ -0,0 +1,62 @@ +"""Move mac address to interface table + +Revision ID: 9184cc675b4e +Revises: 8f9b5c5ed49f +Create Date: 2018-10-24 15:23:14.750157 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "9184cc675b4e" +down_revision = "8f9b5c5ed49f" +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column("interface", sa.Column("mac", postgresql.MACADDR(), nullable=True)) + op.create_unique_constraint(op.f("uq_interface_mac"), "interface", ["mac"]) + op.drop_constraint("fk_interface_mac_id_mac", "interface", type_="foreignkey") + # Fill the new mac column based on mac_id + conn = op.get_bind() + res = conn.execute( + "SELECT interface.id, mac.address FROM interface INNER JOIN mac ON interface.mac_id = mac.id" + ) + results = res.fetchall() + interface = sa.sql.table("interface", sa.sql.column("id"), sa.sql.column("mac")) + for result in results: + op.execute( + interface.update().where(interface.c.id == result[0]).values(mac=result[1]) + ) + op.drop_column("interface", "mac_id") + + +def downgrade(): + op.add_column( + "interface", + sa.Column("mac_id", sa.INTEGER(), autoincrement=False, nullable=True), + ) + op.create_foreign_key( + "fk_interface_mac_id_mac", "interface", "mac", ["mac_id"], ["id"] + ) + op.drop_constraint(op.f("uq_interface_mac"), "interface", type_="unique") + # Fill the mac_id column based on mac + conn = op.get_bind() + # Get the id of existing addresses in the mac table + res = conn.execute( + "SELECT interface.id, mac.id FROM interface INNER JOIN mac ON interface.mac = mac.address" + ) + results = res.fetchall() + interface = sa.sql.table("interface", sa.sql.column("id"), sa.sql.column("mac_id")) + for result in results: + op.execute( + interface.update() + .where(interface.c.id == result[0]) + .values(mac_id=result[1]) + ) + # WARNING: the above operation only associate a mac to existing addresses in the mac table! + # If new addresses were created after the migration, they will be lost + op.drop_column("interface", "mac") diff --git a/tests/functional/factories.py b/tests/functional/factories.py index 2631e8712b9a8675fdd05894b4573d21c6e7b2b6..aa7b35d18712ee1233658ecdc1f76292ef4d2c5b 100644 --- a/tests/functional/factories.py +++ b/tests/functional/factories.py @@ -156,6 +156,7 @@ class InterfaceFactory(factory.alchemy.SQLAlchemyModelFactory): ip = factory.LazyAttributeSequence( lambda o, n: str(ipaddress.ip_address(o.network.first_ip) + n) ) + mac = factory.Faker("mac_address") user = factory.SubFactory(UserFactory) diff --git a/tests/functional/test_api.py b/tests/functional/test_api.py index 4360338e5d1aadda4c8870eab199f47be85add8a..45f2fe107b4fb1704dd1a308caef6ea8f7de98ba 100644 --- a/tests/functional/test_api.py +++ b/tests/functional/test_api.py @@ -1025,7 +1025,12 @@ def test_create_interface(client, network_factory, user_token): ) # Check that all parameters can be passed - data2 = {"network": network.vlan_name, "ip": "192.168.1.21", "name": "myhostname"} + data2 = { + "network": network.vlan_name, + "ip": "192.168.1.21", + "name": "myhostname", + "mac": "7c:e2:ca:64:d0:68", + } response = post( client, f"{API_URL}/network/interfaces", data=data2, token=user_token ) @@ -1142,7 +1147,7 @@ def test_create_mac(client, item_factory, user_token): data = {"address": "b5:4b:7d:a4:23:43"} response = post(client, f"{API_URL}/network/macs", data=data, token=user_token) assert response.status_code == 201 - assert {"id", "address", "item", "interfaces"} == set(response.get_json().keys()) + assert {"id", "address", "item"} == set(response.get_json().keys()) assert response.get_json()["address"] == data["address"] # Check that address shall be unique diff --git a/tests/functional/test_web.py b/tests/functional/test_web.py index cd655baa6c2f1cb8607bffc926aa684aab687ab3..a03ebd5c45172c9b05b5f5566b7b8cd9653cd273 100644 --- a/tests/functional/test_web.py +++ b/tests/functional/test_web.py @@ -246,3 +246,33 @@ def test_edit_item_comment_in_index( # And in the index instances, nb = models.Item.search("world") assert list(instances) == [item1] + + +def test_create_host(logged_rw_client, network_factory, device_type): + network = network_factory( + address="192.168.1.0/24", first_ip="192.168.1.10", last_ip="192.168.1.250" + ) + name = "myhost" + ip = "192.168.1.11" + mac = "02:42:42:45:3c:89" + form = { + "network_id": network.id, + "name": name, + "device_type_id": device_type.id, + "ip": ip, + "mac": mac, + "description": "test", + "ansible_vars": "", + "ansible_groups": [], + "tags": [], + "random_mac": False, + "cnames_string": "", + "interface_name": "myhost", + } + response = logged_rw_client.post(f"/network/hosts/create", data=form) + assert response.status_code == 302 + # The host was created + host = models.Host.query.filter_by(name=name).first() + assert host is not None + assert host.interfaces[0].ip == ip + assert host.interfaces[0].mac == mac