From b1de8a63d3ed7e3fe651b304e327e5167f939920 Mon Sep 17 00:00:00 2001
From: John Mulligan <jmulligan@redhat.com>
Date: Wed, 13 Aug 2025 12:58:06 -0400
Subject: [PATCH] mgr/smb: update port validation to use new smb constants

Make behavior consistent across the smb mgr module and the service spec
class by using the same set of constants.
Fix an issue supporting the customization of the `remote-control`
sidecar's port.

Signed-off-by: John Mulligan <jmulligan@redhat.com>
(cherry picked from commit c141eabafb823ac75b3d2b2e5381ccc00a3e322a)
---
 src/pybind/mgr/smb/tests/test_validation.py | 11 ++++++++++-
 src/pybind/mgr/smb/validation.py            | 17 +++++++++++++----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/src/pybind/mgr/smb/tests/test_validation.py b/src/pybind/mgr/smb/tests/test_validation.py
index bdf40a67448ee..3c30b6b02f69a 100644
--- a/src/pybind/mgr/smb/tests/test_validation.py
+++ b/src/pybind/mgr/smb/tests/test_validation.py
@@ -144,8 +144,17 @@ def test_check_access_name(value, ok, err_match):
         ({'smb': 4455, 'sbmetrics': 9009}, 'invalid service names'),
         (
             {'smb': 4455, 'smbmetrics': 9999, 'ctdb': 9999},
-            'must not be repeated',
+            'must be unique',
         ),
+        # can't use 445 it's the default smb port!
+        (
+            {'smbmetrics': 445},
+            'must be unique',
+        ),
+        # its valid to swap stuff around (don't do this please)
+        ({'smbmetrics': 4379, 'ctdb': 9922}, None),
+        # remote-control is a valid service name to modify
+        ({'remote-control': 65432}, None),
     ],
 )
 def test_check_custom_ports(value, err_match):
diff --git a/src/pybind/mgr/smb/validation.py b/src/pybind/mgr/smb/validation.py
index 8d83e24b4f97d..41d76b77d3f30 100644
--- a/src/pybind/mgr/smb/validation.py
+++ b/src/pybind/mgr/smb/validation.py
@@ -3,6 +3,8 @@ from typing import Dict, Optional
 import posixpath
 import re
 
+import ceph.smb.constants
+
 # Initially, this regex is pretty restrictive.  But I (JJM) find that
 # starting out more restricitive is better than not because it's generally
 # easier to relax strict rules then discover someone relies on lax rules
@@ -114,14 +116,14 @@ def check_access_name(name: str) -> None:
         raise ValueError('login name may not exceed 128 characters')
 
 
-PORT_SERVICES = {"smb", "ctdb", "smbmetrics"}
 _MAX_PORT = (1 << 16) - 1
 
 
 def check_custom_ports(ports: Optional[Dict[str, int]]) -> None:
     if ports is None:
         return
-    other = set(ports) - PORT_SERVICES
+    _defaults = ceph.smb.constants.DEFAULT_PORTS
+    other = set(ports) - set(_defaults)
     if other:
         raise ValueError(
             "invalid service names for custom ports:"
@@ -132,5 +134,12 @@ def check_custom_ports(ports: Optional[Dict[str, int]]) -> None:
         raise ValueError(
             f'invalid port number(s): {", ".join(sorted(invalid))}'
         )
-    if len(ports) != len(set(ports.values())):
-        raise ValueError('port numbers must not be repeated')
+    # make sure ports are unique per service
+    all_ports = _defaults | ports
+    for port in all_ports.values():
+        using_port = {s for s, p in all_ports.items() if p == port}
+        if len(using_port) > 1:
+            raise ValueError(
+                'port numbers must be unique:'
+                f' {port} used for {", ".join(sorted(using_port))}'
+            )
-- 
2.39.5