From f5d6d11f1d285ab8f6d512f79038d5aa27bac8c8 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Thu, 21 Dec 2023 19:04:52 +0100
Subject: [PATCH] osbuild: error when {Device,Mount} is modified after creation

This is a drive-by change after spending some quality time with the
mount code. The `id` field of `Mount` is calculated only once and
only when creating a `Mount`. This seems slightly dangerous as
any change to an attribute after creation will not update the
id. This means two options:
1. dynamically update the `id` on changes
2. forbid changes after the `id` is calculcated

I went with (2) but happy to discuss of course but it seems more
the spirit of the class.

It also does the same change for "devices.Device"
---
 osbuild/devices.py      |  3 ++-
 osbuild/inputs.py       |  1 -
 osbuild/mixins.py       | 15 +++++++++++++++
 osbuild/mounts.py       |  3 ++-
 test/mod/test_mixins.py | 32 ++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 osbuild/mixins.py
 create mode 100644 test/mod/test_mixins.py

diff --git a/osbuild/devices.py b/osbuild/devices.py
index 44541a72..8490f1e4 100644
--- a/osbuild/devices.py
+++ b/osbuild/devices.py
@@ -21,10 +21,11 @@ import stat
 from typing import Any, Dict, Optional
 
 from osbuild import host
+from osbuild.mixins import MixinImmutableID
 from osbuild.util import ctx
 
 
-class Device:
+class Device(MixinImmutableID):
     """
     A single device with its corresponding options
     """
diff --git a/osbuild/inputs.py b/osbuild/inputs.py
index 5e845df7..229c1264 100644
--- a/osbuild/inputs.py
+++ b/osbuild/inputs.py
@@ -48,7 +48,6 @@ class Input:
         self.id = self.calc_id()
 
     def calc_id(self):
-
         # NB: The input `name` is not included here on purpose since it
         # is either prescribed by the stage itself and thus not actual
         # parameter or arbitrary and chosen by the manifest generator
diff --git a/osbuild/mixins.py b/osbuild/mixins.py
new file mode 100644
index 00000000..24f09d35
--- /dev/null
+++ b/osbuild/mixins.py
@@ -0,0 +1,15 @@
+"""
+Mixin helper classes
+"""
+
+
+class MixinImmutableID:
+    """
+    Mixin to ensure that "self.id" attributes are immutable after id is set
+    """
+
+    def __setattr__(self, name, val):
+        if hasattr(self, "id"):
+            class_name = self.__class__.__name__
+            raise ValueError(f"cannot set '{name}': {class_name} cannot be changed after creation")
+        super().__setattr__(name, val)
diff --git a/osbuild/mounts.py b/osbuild/mounts.py
index 86576ad0..b938d21d 100644
--- a/osbuild/mounts.py
+++ b/osbuild/mounts.py
@@ -16,9 +16,10 @@ from typing import Dict, List
 
 from osbuild import host
 from osbuild.devices import DeviceManager
+from osbuild.mixins import MixinImmutableID
 
 
-class Mount:
+class Mount(MixinImmutableID):
     """
     A single mount with its corresponding options
     """
diff --git a/test/mod/test_mixins.py b/test/mod/test_mixins.py
new file mode 100644
index 00000000..e793ad7e
--- /dev/null
+++ b/test/mod/test_mixins.py
@@ -0,0 +1,32 @@
+from unittest.mock import Mock
+
+import pytest
+
+from osbuild.devices import Device
+from osbuild.meta import ModuleInfo
+from osbuild.mounts import Mount
+
+
+def test_mount_immutable_mixin():
+    info = Mock(spec=ModuleInfo)
+    info.name = "some-name"
+    device = Mock(spec=ModuleInfo)
+    device.id = "some-id"
+    partition = 1
+    target = "/"
+    opts = {"opt1": 1}
+    mnt = Mount("name", info, device, partition, target, opts)
+    with pytest.raises(ValueError) as e:
+        mnt.name = "new-name"
+    assert str(e.value) == "cannot set 'name': Mount cannot be changed after creation"
+
+
+def test_device_immutable_mixins():
+    info = Mock(spec=ModuleInfo)
+    info.name = "some-name"
+    parent = None
+    opts = {"opt1": 1}
+    dev = Device("name", info, parent, opts)
+    with pytest.raises(ValueError) as e:
+        dev.name = "new-name"
+    assert str(e.value) == "cannot set 'name': Device cannot be changed after creation"