From a8bce0bfee81218cd1c0ddcf3e2b86b96659933e Mon Sep 17 00:00:00 2001
From: Felipe Reyes <felipe.reyes@canonical.com>
Date: Thu, 5 Oct 2023 18:45:55 -0300
Subject: [PATCH] Add validator for fixed_subnet

Validate the existance of the subnet referenced by fixed_subnet. It's
not checked if the subnet is associated to the fixed_network.

Closes-Bug: #2038109
Change-Id: Ia75f0ae525b768ad5b965d22b522cca6f80dcab2
---
 magnum/api/attr_validator.py                 | 35 +++++++--
 magnum/tests/unit/api/test_attr_validator.py | 81 ++++++++++++++++++++
 2 files changed, 110 insertions(+), 6 deletions(-)

diff --git a/magnum/api/attr_validator.py b/magnum/api/attr_validator.py
index 03278859a3..cc93043c85 100644
--- a/magnum/api/attr_validator.py
+++ b/magnum/api/attr_validator.py
@@ -118,6 +118,28 @@ def validate_fixed_network(cli, fixed_network):
     return network_id
 
 
+def validate_fixed_subnet(cli, fixed_subnet):
+    """Validate fixed subnet"""
+
+    count = 0
+    subnet_id = None
+    subnets = cli.neutron().list_subnets()
+    for subnet in subnets.get('subnets'):
+        if fixed_subnet in [subnet.get('name'), subnet.get('id')]:
+            count += 1
+            subnet_id = subnet.get('id')
+
+    if count == 0:
+        # Unable to find the configured fixed_subnet.
+        raise exception.FixedSubnetNotFound(subnet=fixed_subnet)
+    elif count > 1:
+        msg = _("Multiple subnets exist with same name '%s'. "
+                "Please use the subnet ID instead.")
+        raise exception.Conflict(msg % fixed_subnet)
+    else:
+        return subnet_id
+
+
 def validate_labels(labels):
     """"Validate labels"""
 
@@ -148,15 +170,15 @@ def validate_os_resources(context, cluster_template, cluster=None):
 
     for attr, validate_method in validators.items():
         if cluster and attr in cluster and cluster[attr]:
-            if attr != 'labels':
-                validate_method(cli, cluster[attr])
-            else:
+            if attr == 'labels':
                 validate_method(cluster[attr])
-        elif attr in cluster_template and cluster_template[attr] is not None:
-            if attr != 'labels':
-                validate_method(cli, cluster_template[attr])
             else:
+                validate_method(cli, cluster[attr])
+        elif attr in cluster_template and cluster_template[attr] is not None:
+            if attr == 'labels':
                 validate_method(cluster_template[attr])
+            else:
+                validate_method(cli, cluster_template[attr])
 
     if cluster:
         validate_keypair(cli, cluster['keypair'])
@@ -202,6 +224,7 @@ validators = {'image_id': validate_image,
               'master_flavor_id': validate_flavor,
               'external_network_id': validate_external_network,
               'fixed_network': validate_fixed_network,
+              'fixed_subnet': validate_fixed_subnet,
               'labels': validate_labels}
 
 labels_validators = {'swarm_strategy': validate_labels_strategy}
diff --git a/magnum/tests/unit/api/test_attr_validator.py b/magnum/tests/unit/api/test_attr_validator.py
index d5b7c05a7a..de8a1c8636 100644
--- a/magnum/tests/unit/api/test_attr_validator.py
+++ b/magnum/tests/unit/api/test_attr_validator.py
@@ -132,6 +132,46 @@ class TestAttrValidator(base.BaseTestCase):
                           attr_validator.validate_fixed_network,
                           mock_os_cli, 'test_net')
 
+    def test_validate_fixed_subnet_with_valid_subnet(self):
+        mock_neutron = mock.MagicMock()
+        mock_subnets = {'subnets': [{'name': 'test_subnet',
+                                     'id': 'test_subnet_id',
+                                     'network_id': 'test_net_id'}]}
+        mock_neutron.list_subnets.return_value = mock_subnets
+        mock_os_cli = mock.MagicMock()
+        mock_os_cli.neutron.return_value = mock_neutron
+        self.assertEqual('test_subnet_id',
+                         attr_validator.validate_fixed_subnet(mock_os_cli,
+                                                              'test_subnet'))
+        mock_neutron.list_subnets.assert_called_with()
+
+    def test_validate_fixed_subnet_with_invalid_subnet(self):
+        mock_neutron = mock.MagicMock()
+        mock_subnets = {'subnets': [{'name': 'test_subnet',
+                                     'id': 'test_subnet_id',
+                                     'network_id': 'test_net_id'}]}
+        mock_neutron.list_subnets.return_value = mock_subnets
+        mock_os_cli = mock.MagicMock()
+        mock_os_cli.neutron.return_value = mock_neutron
+        self.assertRaises(exception.FixedSubnetNotFound,
+                          attr_validator.validate_fixed_subnet,
+                          mock_os_cli, 'test_subnet_not_found')
+
+    def test_validate_fixed_subnet_with_multiple_valid_subnet(self):
+        mock_neutron = mock.MagicMock()
+        mock_subnets = {'subnets': [{'name': 'test_subnet',
+                                     'id': 'test_subnet_id',
+                                     'network_id': 'test_net_id'},
+                                    {'name': 'test_subnet',
+                                     'id': 'test_subnet_id2',
+                                     'network_id': 'test_net_id2'}]}
+        mock_neutron.list_subnets.return_value = mock_subnets
+        mock_os_cli = mock.MagicMock()
+        mock_os_cli.neutron.return_value = mock_neutron
+        self.assertRaises(exception.Conflict,
+                          attr_validator.validate_fixed_subnet,
+                          mock_os_cli, 'test_subnet')
+
     def test_validate_keypair_with_no_keypair(self):
         mock_keypair = mock.MagicMock()
         mock_keypair.id = None
@@ -289,6 +329,47 @@ class TestAttrValidator(base.BaseTestCase):
         attr_validator.validate_os_resources(mock_context,
                                              mock_cluster_template)
 
+    @mock.patch('magnum.common.clients.OpenStackClients')
+    def test_validate_os_resources_with_valid_fixed_subnet(self,
+                                                           os_clients_klass):
+        mock_cluster_template = {'fixed_network': 'test_net',
+                                 'fixed_subnet': 'test_subnet'}
+        mock_context = mock.MagicMock()
+        mock_os_cli = mock.MagicMock()
+        os_clients_klass.return_value = mock_os_cli
+        mock_neutron = mock.MagicMock()
+        mock_networks = {'networks': [{'name': 'test_net',
+                                       'id': 'test_net_id'}]}
+        mock_neutron.list_networks.return_value = mock_networks
+        mock_subnets = {'subnets': [{'name': 'test_subnet',
+                                     'id': 'test_subnet_id',
+                                     'network_id': 'test_net_id'}]}
+        mock_neutron.list_subnets.return_value = mock_subnets
+        mock_os_cli.neutron.return_value = mock_neutron
+        attr_validator.validate_os_resources(mock_context,
+                                             mock_cluster_template)
+
+    @mock.patch('magnum.common.clients.OpenStackClients')
+    def test_validate_os_resources_with_invalid_fixed_subnet(self,
+                                                             os_clients_klass):
+        mock_cluster_template = {'fixed_network': 'test_net',
+                                 'fixed_subnet': 'test_subnet2'}
+        mock_context = mock.MagicMock()
+        mock_os_cli = mock.MagicMock()
+        os_clients_klass.return_value = mock_os_cli
+        mock_neutron = mock.MagicMock()
+        mock_networks = {'networks': [{'name': 'test_net',
+                                       'id': 'test_net_id'}]}
+        mock_neutron.list_networks.return_value = mock_networks
+        mock_subnets = {'subnets': [{'name': 'test_subnet',
+                                     'id': 'test_subnet_id',
+                                     'network_id': 'test_net_id'}]}
+        mock_neutron.list_subnets.return_value = mock_subnets
+        mock_os_cli.neutron.return_value = mock_neutron
+        self.assertRaises(exception.FixedSubnetNotFound,
+                          attr_validator.validate_os_resources, mock_context,
+                          mock_cluster_template)
+
     @mock.patch('magnum.common.clients.OpenStackClients')
     def test_validate_os_resources_with_cluster(self, mock_os_cli):
         mock_cluster_template = {}