From 53c63888fb5e71d0ef672ad0acdcfc16e0714252 Mon Sep 17 00:00:00 2001
From: John Mulligan <jmulligan@redhat.com>
Date: Tue, 25 Jul 2023 14:08:52 -0400
Subject: [PATCH] cephadm: split DaemonSubIdentity class off from
 DaemonIdentity

Split DaemonSubIdentity into it's own subclass of DaemonIdentity
in order to better use the type system for validation and clarity.
DaemonIdentity loses the `subcomponent` field so that there's no
mistaking a DaemonIdentity with a DaemonSubIdentity.
This will make a lot more sense once there are more uses of
DaemonSubIdentity in the codebase.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
---
 src/cephadm/cephadm.py                    |  4 +-
 src/cephadm/cephadmlib/daemon_identity.py | 73 ++++++++++++++---------
 2 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/src/cephadm/cephadm.py b/src/cephadm/cephadm.py
index 492bce51438df..0aa28e1ff2fd3 100755
--- a/src/cephadm/cephadm.py
+++ b/src/cephadm/cephadm.py
@@ -135,7 +135,7 @@ from cephadmlib.net_utils import (
     wrap_ipv6,
 )
 from cephadmlib.locking import FileLock
-from cephadmlib.daemon_identity import DaemonIdentity
+from cephadmlib.daemon_identity import DaemonIdentity, DaemonSubIdentity
 from cephadmlib.packagers import create_packager, Packager
 from cephadmlib.logging import cephadm_init_logging
 
@@ -3969,7 +3969,7 @@ class InitContainer(BasicContainer):
             }
         return cls(
             ctx,
-            identity=primary.identity._replace(subcomponent='init'),
+            identity=DaemonSubIdentity.from_parent(primary.identity, 'init'),
             image=opts.get('image', primary.image),
             entrypoint=opts.get('entrypoint', primary.entrypoint),
             # note: args is not inherited from primary container
diff --git a/src/cephadm/cephadmlib/daemon_identity.py b/src/cephadm/cephadmlib/daemon_identity.py
index b6af4b50114b3..fad0c96561a4a 100644
--- a/src/cephadm/cephadmlib/daemon_identity.py
+++ b/src/cephadm/cephadmlib/daemon_identity.py
@@ -1,6 +1,8 @@
 # deamon_identity.py - classes for identifying daemons & services
 
-from typing import Union, Optional
+import re
+
+from typing import Union
 
 from .context import CephadmContext
 
@@ -11,12 +13,10 @@ class DaemonIdentity:
         fsid: str,
         daemon_type: str,
         daemon_id: Union[int, str],
-        subcomponent: str = '',
     ) -> None:
         self._fsid = fsid
         self._daemon_type = daemon_type
         self._daemon_id = str(daemon_id)
-        self._subcomponent = subcomponent
 
     @property
     def fsid(self) -> str:
@@ -30,10 +30,6 @@ class DaemonIdentity:
     def daemon_id(self) -> str:
         return self._daemon_id
 
-    @property
-    def subcomponent(self) -> str:
-        return self._subcomponent
-
     @property
     def legacy_container_name(self) -> str:
         return 'ceph-%s-%s.%s' % (self.fsid, self.daemon_type, self.daemon_id)
@@ -41,29 +37,8 @@ class DaemonIdentity:
     @property
     def container_name(self) -> str:
         name = f'ceph-{self.fsid}-{self.daemon_type}-{self.daemon_id}'
-        if self.subcomponent:
-            name = f'{name}-{self.subcomponent}'
         return name.replace('.', '-')
 
-    def _replace(
-        self,
-        *,
-        fsid: Optional[str] = None,
-        daemon_type: Optional[str] = None,
-        daemon_id: Union[None, int, str] = None,
-        subcomponent: Optional[str] = None,
-    ) -> 'DaemonIdentity':
-        return self.__class__(
-            fsid=self.fsid if fsid is None else fsid,
-            daemon_type=(
-                self.daemon_type if daemon_type is None else daemon_type
-            ),
-            daemon_id=self.daemon_id if daemon_id is None else daemon_id,
-            subcomponent=(
-                self.subcomponent if subcomponent is None else subcomponent
-            ),
-        )
-
     @classmethod
     def from_name(cls, fsid: str, name: str) -> 'DaemonIdentity':
         daemon_type, daemon_id = name.split('.', 1)
@@ -72,3 +47,45 @@ class DaemonIdentity:
     @classmethod
     def from_context(cls, ctx: 'CephadmContext') -> 'DaemonIdentity':
         return cls.from_name(ctx.fsid, ctx.name)
+
+
+class DaemonSubIdentity(DaemonIdentity):
+    def __init__(
+        self,
+        fsid: str,
+        daemon_type: str,
+        daemon_id: Union[int, str],
+        subcomponent: str = '',
+    ) -> None:
+        super().__init__(fsid, daemon_type, daemon_id)
+        self._subcomponent = subcomponent
+        if not re.match('^[a-zA-Z0-9]{1,15}$', self._subcomponent):
+            raise ValueError(
+                f'invalid subcomponent; invalid characters: {subcomponent!r}'
+            )
+
+    @property
+    def subcomponent(self) -> str:
+        return self._subcomponent
+
+    @property
+    def container_name(self) -> str:
+        name = f'ceph-{self.fsid}-{self.daemon_type}-{self.daemon_id}-{self.subcomponent}'
+        return name.replace('.', '-')
+
+    @property
+    def legacy_container_name(self) -> str:
+        raise ValueError(
+            'legacy_container_name not valid for DaemonSubIdentity'
+        )
+
+    @classmethod
+    def from_parent(
+        cls, parent: 'DaemonIdentity', subcomponent: str
+    ) -> 'DaemonSubIdentity':
+        return cls(
+            parent.fsid,
+            parent.daemon_type,
+            parent.daemon_id,
+            subcomponent,
+        )
-- 
2.39.5