From 58369a1f6da1502a030c153aa22b60f8642e3bf3 Mon Sep 17 00:00:00 2001 From: Sudipta Biswas Date: Mon, 13 Jul 2015 23:39:35 +0530 Subject: [PATCH] Cleanup NovaObjectDictCompat from security_group_rule Cleanup subclassing on NovaObjectDictCompat and fix subsequent tests and code associated with nova/objects/security_group_rule.py Change-Id: Idffd15a6d4ce043d97f9e8ca4ac0f5abe51e5f2c --- nova/objects/security_group_rule.py | 9 +++-- .../unit/objects/test_security_group_rule.py | 5 +-- nova/tests/unit/virt/libvirt/test_firewall.py | 35 ++++++++++--------- nova/virt/firewall.py | 32 ++++++++--------- nova/virt/xenapi/firewall.py | 28 +++++++-------- 5 files changed, 56 insertions(+), 53 deletions(-) diff --git a/nova/objects/security_group_rule.py b/nova/objects/security_group_rule.py index f224025b7676..eea7019fab26 100644 --- a/nova/objects/security_group_rule.py +++ b/nova/objects/security_group_rule.py @@ -21,10 +21,8 @@ from nova.objects import fields OPTIONAL_ATTRS = ['parent_group', 'grantee_group'] -# TODO(berrange): Remove NovaObjectDictCompat @base.NovaObjectRegistry.register -class SecurityGroupRule(base.NovaPersistentObject, base.NovaObject, - base.NovaObjectDictCompat): +class SecurityGroupRule(base.NovaPersistentObject, base.NovaObject): # Version 1.0: Initial version # Version 1.1: Added create() and set id as read_only VERSION = '1.1' @@ -52,9 +50,10 @@ class SecurityGroupRule(base.NovaPersistentObject, base.NovaObject, expected_attrs = [] for field in rule.fields: if field in expected_attrs: - rule[field] = rule._from_db_subgroup(context, db_rule[field]) + setattr(rule, field, + rule._from_db_subgroup(context, db_rule[field])) elif field not in OPTIONAL_ATTRS: - rule[field] = db_rule[field] + setattr(rule, field, db_rule[field]) rule._context = context rule.obj_reset_changes() return rule diff --git a/nova/tests/unit/objects/test_security_group_rule.py b/nova/tests/unit/objects/test_security_group_rule.py index c6e5af1984f7..5fcd37b65824 100644 --- a/nova/tests/unit/objects/test_security_group_rule.py +++ b/nova/tests/unit/objects/test_security_group_rule.py @@ -42,9 +42,10 @@ class _TestSecurityGroupRuleObject(object): self.context, 1) for field in fake_rule: if field == 'cidr': - self.assertEqual(fake_rule[field], str(rule[field])) + self.assertEqual(fake_rule[field], str(getattr(rule, + field))) else: - self.assertEqual(fake_rule[field], rule[field]) + self.assertEqual(fake_rule[field], getattr(rule, field)) sgrg.assert_called_with(self.context, 1) def test_get_by_security_group(self): diff --git a/nova/tests/unit/virt/libvirt/test_firewall.py b/nova/tests/unit/virt/libvirt/test_firewall.py index 420a6e354194..a30dfdf6eeb9 100644 --- a/nova/tests/unit/virt/libvirt/test_firewall.py +++ b/nova/tests/unit/virt/libvirt/test_firewall.py @@ -453,24 +453,26 @@ class IptablesFirewallTestCase(test.NoDBTestCase): self.assertEqual(0, len(rules)) # add a rule angd send the update message, check for 1 rule - mock_fwrules.return_value = [{'protocol': 'tcp', - 'cidr': '10.99.99.99/32', - 'from_port': 1, - 'to_port': 65535}] + sec_grp_rule_obj = objects.SecurityGroupRule(protocol='tcp', + cidr='10.99.99.99/32', + from_port=1, + to_port=65535) + mock_fwrules.return_value = [sec_grp_rule_obj] self.fw.refresh_provider_fw_rules() rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules if rule.chain == 'provider'] self.assertEqual(1, len(rules)) # Add another, refresh, and make sure number of rules goes to two - mock_fwrules.return_value = [{'protocol': 'tcp', - 'cidr': '10.99.99.99/32', - 'from_port': 1, - 'to_port': 65535}, - {'protocol': 'udp', - 'cidr': '10.99.99.99/32', - 'from_port': 1, - 'to_port': 65535}] + sec_grp_rule_obj1 = objects.SecurityGroupRule(protocol='tcp', + cidr='10.99.99.99/32', + from_port=1, + to_port=65535) + sec_grp_rule_obj2 = objects.SecurityGroupRule(protocol='udp', + cidr='10.99.99.99/32', + from_port=1, + to_port=65535) + mock_fwrules.return_value = [sec_grp_rule_obj1, sec_grp_rule_obj2] self.fw.refresh_provider_fw_rules() rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules if rule.chain == 'provider'] @@ -490,10 +492,11 @@ class IptablesFirewallTestCase(test.NoDBTestCase): self.assertEqual(1, len(provjump_rules)) # remove a rule from the db, cast to compute to refresh rule - mock_fwrules.return_value = [{'protocol': 'udp', - 'cidr': '10.99.99.99/32', - 'from_port': 1, - 'to_port': 65535}] + sec_grp_rule_obj = objects.SecurityGroupRule(protocol='udp', + cidr='10.99.99.99/32', + from_port=1, + to_port=65535) + mock_fwrules.return_value = [sec_grp_rule_obj] self.fw.refresh_provider_fw_rules() rules = [rule for rule in self.fw.iptables.ipv4['filter'].rules if rule.chain == 'provider'] diff --git a/nova/virt/firewall.py b/nova/virt/firewall.py index 239b53f4cb8c..8fc2110a6092 100644 --- a/nova/virt/firewall.py +++ b/nova/virt/firewall.py @@ -293,8 +293,8 @@ class IptablesFirewallDriver(FirewallDriver): '-s %s/128 -p icmpv6 -j ACCEPT' % (gateway_v6,)) def _build_icmp_rule(self, rule, version): - icmp_type = rule['from_port'] - icmp_code = rule['to_port'] + icmp_type = rule.from_port + icmp_code = rule.to_port if icmp_type == -1: icmp_type_arg = None @@ -312,12 +312,12 @@ class IptablesFirewallDriver(FirewallDriver): return [] def _build_tcp_udp_rule(self, rule, version): - if rule['from_port'] == rule['to_port']: - return ['--dport', '%s' % (rule['from_port'],)] + if rule.from_port == rule.to_port: + return ['--dport', '%s' % (rule.from_port,)] else: return ['-m', 'multiport', - '--dports', '%s:%s' % (rule['from_port'], - rule['to_port'])] + '--dports', '%s:%s' % (rule.from_port, + rule.to_port)] def instance_rules(self, instance, network_info): ctxt = context.get_admin_context() @@ -487,29 +487,29 @@ class IptablesFirewallDriver(FirewallDriver): ipv6_rules = [] rules = self._virtapi.provider_fw_rule_get_all(ctxt) for rule in rules: - LOG.debug('Adding provider rule: %s', rule['cidr']) - version = netutils.get_ip_version(rule['cidr']) + LOG.debug('Adding provider rule: %s', rule.cidr) + version = netutils.get_ip_version(rule.cidr) if version == 4: fw_rules = ipv4_rules else: fw_rules = ipv6_rules - protocol = rule['protocol'] + protocol = rule.protocol if version == 6 and protocol == 'icmp': protocol = 'icmpv6' - args = ['-p', protocol, '-s', rule['cidr']] + args = ['-p', protocol, '-s', str(rule.cidr)] if protocol in ['udp', 'tcp']: - if rule['from_port'] == rule['to_port']: - args += ['--dport', '%s' % (rule['from_port'],)] + if rule.from_port == rule.to_port: + args += ['--dport', '%s' % (rule.from_port,)] else: args += ['-m', 'multiport', - '--dports', '%s:%s' % (rule['from_port'], - rule['to_port'])] + '--dports', '%s:%s' % (rule.from_port, + rule.to_port)] elif protocol == 'icmp': - icmp_type = rule['from_port'] - icmp_code = rule['to_port'] + icmp_type = rule.from_port + icmp_code = rule.to_port if icmp_type == -1: icmp_type_arg = None diff --git a/nova/virt/xenapi/firewall.py b/nova/virt/xenapi/firewall.py index ebebf3d9c10b..27fb168a7f5a 100644 --- a/nova/virt/xenapi/firewall.py +++ b/nova/virt/xenapi/firewall.py @@ -53,12 +53,12 @@ class Dom0IptablesFirewallDriver(firewall.IptablesFirewallDriver): self.iptables.ipv6['filter'].add_rule('sg-fallback', '-j DROP') def _build_tcp_udp_rule(self, rule, version): - if rule['from_port'] == rule['to_port']: - return ['--dport', '%s' % (rule['from_port'],)] + if rule.from_port == rule.to_port: + return ['--dport', '%s' % (rule.from_port,)] else: # No multiport needed for XS! - return ['--dport', '%s:%s' % (rule['from_port'], - rule['to_port'])] + return ['--dport', '%s:%s' % (rule.from_port, + rule.to_port)] def _provider_rules(self): """Generate a list of rules from provider for IP4 & IP6. @@ -72,28 +72,28 @@ class Dom0IptablesFirewallDriver(firewall.IptablesFirewallDriver): ipv6_rules = [] rules = self._virtapi.provider_fw_rule_get_all(ctxt) for rule in rules: - LOG.debug('Adding provider rule: %s', rule['cidr']) - version = netutils.get_ip_version(rule['cidr']) + LOG.debug('Adding provider rule: %s', rule.cidr) + version = netutils.get_ip_version(rule.cidr) if version == 4: fw_rules = ipv4_rules else: fw_rules = ipv6_rules - protocol = rule['protocol'] + protocol = rule.protocol if version == 6 and protocol == 'icmp': protocol = 'icmpv6' - args = ['-p', protocol, '-s', rule['cidr']] + args = ['-p', protocol, '-s', rule.cidr] if protocol in ['udp', 'tcp']: - if rule['from_port'] == rule['to_port']: - args += ['--dport', '%s' % (rule['from_port'],)] + if rule.from_port == rule.to_port: + args += ['--dport', '%s' % (rule.from_port,)] else: - args += ['--dport', '%s:%s' % (rule['from_port'], - rule['to_port'])] + args += ['--dport', '%s:%s' % (rule.from_port, + rule.to_port)] elif protocol == 'icmp': - icmp_type = rule['from_port'] - icmp_code = rule['to_port'] + icmp_type = rule.from_port + icmp_code = rule.to_port if icmp_type == -1: icmp_type_arg = None