From 00091984fbca7f6002bd3bf21c4c27a7179f3176 Mon Sep 17 00:00:00 2001
From: Robert Kukura <kukura@noironetworks.com>
Date: Mon, 11 Jun 2018 13:44:18 -0400
Subject: [PATCH] [AIM] Fix validation of security groups

Add a unit test for validation of security groups, and ensure port
numbers are converted to strings.

Change-Id: I59b201bda32445b678137355763e6e4de81ce269
---
 .../drivers/cisco/apic/aim_validation.py      |  9 +++--
 .../grouppolicy/test_aim_validation.py        | 33 +++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py
index 77f442099..64b5c7571 100644
--- a/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py
+++ b/gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/aim_validation.py
@@ -134,10 +134,13 @@ class ValidationManager(object):
             print("resource %s already expected" % resource)
             raise InternalValidationError()
         for attr_name, attr_type in resource.other_attributes.items():
-            # REVISIT: May also need to dedup arrays of types other
-            # than string.
-            if (attr_type['type'] == 'array' and
+            attr_type_type = attr_type['type']
+            if attr_type_type == 'string':
+                setattr(resource, attr_name, str(getattr(resource, attr_name)))
+            elif (attr_type_type == 'array' and
                 attr_type['items']['type'] == 'string'):
+                # REVISIT: May also need to dedup arrays of types
+                # other than string.
                 value = list(set(getattr(resource, attr_name)))
                 setattr(resource, attr_name, value)
         expected_resources[key] = resource
diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py
index c0ecb8285..98c6380e8 100644
--- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py
+++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_validation.py
@@ -642,6 +642,39 @@ class TestNeutronMapping(AimValidationTestCase):
         self._test_routed_subnet(subnet1_id, '10.0.1.1')
         self._test_unscoped_vrf(router_id)
 
+    def test_security_group(self):
+        # Create security group with a rule.
+        sg = self._make_security_group(
+            self.fmt, 'sg1', 'security group 1')['security_group']
+        rule1 = self._build_security_group_rule(
+            sg['id'], 'ingress', 'tcp', '22', '23')
+        rules = {'security_group_rules': [rule1['security_group_rule']]}
+        sg_rule = self._make_security_group_rule(
+            self.fmt, rules)['security_group_rules'][0]
+        self._validate()
+
+        # Test the AIM SecurityGroup.
+        tenant_name = self.driver.aim_mech_driver.name_mapper.project(
+            None, sg['project_id'])
+        sg_name = sg['id']
+        aim_sg = aim_resource.SecurityGroup(
+            name=sg_name, tenant_name=tenant_name)
+        self._test_aim_resource(aim_sg)
+
+        # Test the AIM SecurityGroupSubject.
+        aim_subject = aim_resource.SecurityGroupSubject(
+            name='default', security_group_name=sg_name,
+            tenant_name=tenant_name)
+        self._test_aim_resource(aim_subject)
+
+        # Test the AIM SecurityGroupRule.
+        aim_rule = aim_resource.SecurityGroupRule(
+            name=sg_rule['id'],
+            security_group_subject_name='default',
+            security_group_name=sg_name,
+            tenant_name=tenant_name)
+        self._test_aim_resource(aim_rule)
+
 
 class TestGbpMapping(AimValidationTestCase):