From 91d6426b06376dd6c3b77640e3513ee0b56738cb Mon Sep 17 00:00:00 2001
From: Kaifeng Wang <kaifeng.w@gmail.com>
Date: Sat, 24 Oct 2020 22:05:36 +0800
Subject: [PATCH] Fixes empty physical_network is not guarded

An empty physical_network can be set to port and make the port
unusable.

Change-Id: I58cf04839f40922cf0c7ddffc08b843cb3c50e06
Story: 2008279
Task: 41153
---
 ironic/api/controllers/v1/port.py             | 11 +++++++++
 .../unit/api/controllers/v1/test_port.py      | 23 +++++++++++++++++++
 ...pty-physical-network-2248a4adef210289.yaml |  5 ++++
 3 files changed, 39 insertions(+)
 create mode 100644 releasenotes/notes/empty-physical-network-2248a4adef210289.yaml

diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py
index 3f0ae6b47a..331adb0af3 100644
--- a/ironic/api/controllers/v1/port.py
+++ b/ironic/api/controllers/v1/port.py
@@ -627,6 +627,11 @@ class PortsController(rest.RestController):
                 "Smart NIC port must have port_id "
                 "and hostname in local_link_connection")
 
+        physical_network = pdict.get('physical_network')
+        if physical_network is not None and not physical_network:
+            raise exception.Invalid('A non-empty value is required when '
+                                    'setting physical_network')
+
         create_remotely = api.request.rpcapi.can_send_create_port()
         if (not create_remotely and pdict.get('portgroup_uuid')):
             # NOTE(mgoddard): In RPC API v1.41, port creation was moved to the
@@ -749,6 +754,12 @@ class PortsController(rest.RestController):
             raise exception.ClientSideError(msg,
                                             status_code=http_client.CONFLICT)
 
+        if (api_utils.is_path_updated(patch, '/physical_network')
+            and rpc_port['physical_network'] is not None
+                and not rpc_port['physical_network']):
+            raise exception.Invalid('A non-empty value is required when '
+                                    'setting physical_network')
+
         notify_extra = {'node_uuid': rpc_node.uuid,
                         'portgroup_uuid': port.portgroup_uuid}
         notify.emit_start_notification(context, rpc_port, 'update',
diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py
index b5e145b9fc..b93a554045 100644
--- a/ironic/tests/unit/api/controllers/v1/test_port.py
+++ b/ironic/tests/unit/api/controllers/v1/test_port.py
@@ -1779,6 +1779,19 @@ class TestPatch(test_api_base.BaseApiTest):
         self.assertEqual(http_client.BAD_REQUEST, response.status_int)
         self.assertIn('maximum character', response.json['error_message'])
 
+    def test_invalid_physnet_empty_string(self, mock_upd):
+        physnet = ''
+        headers = {api_base.Version.string: versions.max_version_string()}
+        response = self.patch_json('/ports/%s' % self.port.uuid,
+                                   [{'path': '/physical_network',
+                                     'value': physnet,
+                                     'op': 'replace'}],
+                                   expect_errors=True,
+                                   headers=headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+        self.assertIn('non-empty value', response.json['error_message'])
+
     def test_portgroups_subresource_patch(self, mock_upd):
         portgroup = obj_utils.create_test_portgroup(self.context,
                                                     node_id=self.node.id)
@@ -2600,6 +2613,16 @@ class TestPost(test_api_base.BaseApiTest):
         self.assertIn('maximum character', response.json['error_message'])
         self.assertFalse(mock_create.called)
 
+    def test_create_port_invalid_physnet_empty_string(self, mock_create):
+        physnet = ''
+        pdict = post_get_test_port(physical_network=physnet)
+        response = self.post_json('/ports', pdict, expect_errors=True,
+                                  headers=self.headers)
+        self.assertEqual('application/json', response.content_type)
+        self.assertEqual(http_client.BAD_REQUEST, response.status_int)
+        self.assertIn('non-empty value', response.json['error_message'])
+        self.assertFalse(mock_create.called)
+
     def test_create_port_with_is_smartnic(self, mock_create):
         llc = {'hostname': 'host1', 'port_id': 'rep0-0'}
         pdict = post_get_test_port(is_smartnic=True, node_uuid=self.node.uuid,
diff --git a/releasenotes/notes/empty-physical-network-2248a4adef210289.yaml b/releasenotes/notes/empty-physical-network-2248a4adef210289.yaml
new file mode 100644
index 0000000000..d554b907d8
--- /dev/null
+++ b/releasenotes/notes/empty-physical-network-2248a4adef210289.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    fixes an issue that physical_network could be set to an empty string,
+    which makes the port unusable.