From 16ac327222100c0d2feaeb6d61e5bf4c6901ec83 Mon Sep 17 00:00:00 2001
From: Benjamin Bertrand <benjamin.bertrand@esss.se>
Date: Wed, 24 Oct 2018 22:14:47 +0200
Subject: [PATCH] Make mac addresses unique by interface
- mac is now a column in the interface table (no link to the mac table)
- mac column is unique (can't have 2 interfaces with the same mac)
- the mac table is kept for the items
JIRA INFRA-639
---
app/api/network.py | 2 +-
app/helpers.py | 21 -------
app/models.py | 16 +++--
app/network/forms.py | 3 +-
app/network/views.py | 21 ++-----
app/templates/network/create_host.html | 2 +-
app/templates/network/create_interface.html | 2 +-
app/templates/network/edit_interface.html | 2 +-
...b4e_move_mac_address_to_interface_table.py | 62 +++++++++++++++++++
tests/functional/factories.py | 1 +
tests/functional/test_api.py | 9 ++-
tests/functional/test_web.py | 30 +++++++++
12 files changed, 120 insertions(+), 51 deletions(-)
create mode 100644 migrations/versions/9184cc675b4e_move_mac_address_to_interface_table.py
diff --git a/app/api/network.py b/app/api/network.py
index 584eff3..ff90b87 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 eff5bca..c40e557 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 cc9e2da..7795899 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 aa66f23..83d478c 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 a6a4a45..555c094 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 85cb94c..92a33ea 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 afeae40..a090a6c 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 7f6ae11..82a122f 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 0000000..dbf116f
--- /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 2631e87..aa7b35d 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 4360338..45f2fe1 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 cd655ba..a03ebd5 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
--
GitLab