From e67f7160631e3e695356d59a59d0cf34a7b2494b Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Wed, 6 Dec 2023 17:00:00 +0100
Subject: [PATCH] Fix GET for conductors with a port or IPv6 address

Our validation does not expect a host-port pair, not having colons in
the hostname. We don't need to verify all possible cases: we will return
404 for a conductor that does not exist.

Change-Id: Iea65575f540a89a0de280fb730e430647c5733dc
---
 ironic/api/controllers/v1/conductor.py              |  8 ++++----
 ironic/common/args.py                               | 13 +++++++++++++
 .../tests/unit/api/controllers/v1/test_conductor.py | 13 +++++++++++++
 .../notes/port-in-conductor-a354a2665effca2e.yaml   |  7 +++++++
 4 files changed, 37 insertions(+), 4 deletions(-)
 create mode 100644 releasenotes/notes/port-in-conductor-a354a2665effca2e.yaml

diff --git a/ironic/api/controllers/v1/conductor.py b/ironic/api/controllers/v1/conductor.py
index d421e835e1..f5663f5cfb 100644
--- a/ironic/api/controllers/v1/conductor.py
+++ b/ironic/api/controllers/v1/conductor.py
@@ -102,9 +102,9 @@ class ConductorsController(rest.RestController):
 
     @METRICS.timer('ConductorsController.get_all')
     @method.expose()
-    @args.validate(marker=args.name, limit=args.integer, sort_key=args.string,
-                   sort_dir=args.string, fields=args.string_list,
-                   detail=args.boolean)
+    @args.validate(marker=args.host_port, limit=args.integer,
+                   sort_key=args.string, sort_dir=args.string,
+                   fields=args.string_list, detail=args.boolean)
     def get_all(self, marker=None, limit=None, sort_key='id', sort_dir='asc',
                 fields=None, detail=None):
         """Retrieve a list of conductors.
@@ -139,7 +139,7 @@ class ConductorsController(rest.RestController):
 
     @METRICS.timer('ConductorsController.get_one')
     @method.expose()
-    @args.validate(hostname=args.name, fields=args.string_list)
+    @args.validate(hostname=args.host_port, fields=args.string_list)
     def get_one(self, hostname, fields=None):
         """Retrieve information about the given conductor.
 
diff --git a/ironic/common/args.py b/ironic/common/args.py
index bd13e3eafd..5ad865d33f 100755
--- a/ironic/common/args.py
+++ b/ironic/common/args.py
@@ -14,6 +14,7 @@ import functools
 import inspect
 
 import jsonschema
+from oslo_utils import netutils
 from oslo_utils import strutils
 from oslo_utils import uuidutils
 
@@ -88,6 +89,18 @@ def name(name, value):
     return value
 
 
+def host_port(name, value):
+    if value is None:
+        return
+    try:
+        host, port = netutils.parse_host_port(value)
+    except (ValueError, TypeError) as exc:
+        raise exception.InvalidParameterValue(f'{name}: {exc}')
+    if not host:
+        raise exception.InvalidParameterValue(_('Missing host in %s') % name)
+    return value
+
+
 def uuid_or_name(name, value):
     """Validate that the value is a UUID or logical name
 
diff --git a/ironic/tests/unit/api/controllers/v1/test_conductor.py b/ironic/tests/unit/api/controllers/v1/test_conductor.py
index d5e54ee1b8..0429b6cb6a 100644
--- a/ironic/tests/unit/api/controllers/v1/test_conductor.py
+++ b/ironic/tests/unit/api/controllers/v1/test_conductor.py
@@ -85,6 +85,19 @@ class TestListConductors(test_api_base.BaseApiTest):
         self.assertEqual(data['hostname'], 'rocky.rocks')
         self.assertTrue(data['alive'])
 
+    def test_get_one_with_port_and_v6(self):
+        obj_utils.create_test_conductor(self.context, hostname='[::1]:8090')
+        data = self.get_json(
+            '/conductors/[::1]:8090',
+            headers={api_base.Version.string: str(api_v1.max_version())})
+        self.assertIn('hostname', data)
+        self.assertIn('drivers', data)
+        self.assertIn('conductor_group', data)
+        self.assertIn('alive', data)
+        self.assertIn('drivers', data)
+        self.assertEqual(data['hostname'], '[::1]:8090')
+        self.assertTrue(data['alive'])
+
     @mock.patch.object(timeutils, 'utcnow', autospec=True)
     def test_get_one_conductor_offline(self, mock_utcnow):
         self.config(heartbeat_timeout=10, group='conductor')
diff --git a/releasenotes/notes/port-in-conductor-a354a2665effca2e.yaml b/releasenotes/notes/port-in-conductor-a354a2665effca2e.yaml
new file mode 100644
index 0000000000..03d91911d2
--- /dev/null
+++ b/releasenotes/notes/port-in-conductor-a354a2665effca2e.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes getting details of a conductor if it uses a non-standard JSON RPC
+    port or an IPv6 address as the name, e.g.
+    ``GET /v1/conductors/[2001:db8::1]:8090``. Previously, it would result in
+    a HTTP error 400.