From a336f335f6377ea06f36505fd2c513e4c21044f5 Mon Sep 17 00:00:00 2001 From: Rob Brockbank Date: Tue, 9 Aug 2016 17:36:33 -0700 Subject: [PATCH] Remove ID field from Rules object --- calico_containers/pycalico/datastore.py | 6 +-- .../pycalico/datastore_datatypes.py | 9 ++-- .../tests/unit/test_datastore.py | 41 +++++++++++++------ 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/calico_containers/pycalico/datastore.py b/calico_containers/pycalico/datastore.py index 0214aa05..90d56626 100644 --- a/calico_containers/pycalico/datastore.py +++ b/calico_containers/pycalico/datastore.py @@ -879,8 +879,7 @@ def create_policy(self, tier_name, policy_name, selector, policy_path = POLICY_PATH % {"tier_name": tier_name, "policy_name": policy_name} default_allow = Rule(action="allow") - rules = rules or Rules(id=policy_name, - inbound_rules=[default_allow], + rules = rules or Rules(inbound_rules=[default_allow], outbound_rules=[default_allow]) order = order or 100 @@ -974,8 +973,7 @@ def create_profile(self, name, rules=None, labels=None): # be accepted by another profile on the endpoint. accept_self = Rule(action="allow", src_tag=name) default_allow = Rule(action="allow") - rules = rules or Rules(id=name, - inbound_rules=[accept_self], + rules = rules or Rules(inbound_rules=[accept_self], outbound_rules=[default_allow]) self.etcd_client.write(profile_path + "rules", rules.to_json()) diff --git a/calico_containers/pycalico/datastore_datatypes.py b/calico_containers/pycalico/datastore_datatypes.py index 722c8d4f..6c9f0663 100644 --- a/calico_containers/pycalico/datastore_datatypes.py +++ b/calico_containers/pycalico/datastore_datatypes.py @@ -35,7 +35,7 @@ """ -class Rules(namedtuple("Rules", ["id", "inbound_rules", "outbound_rules"])): +class Rules(namedtuple("Rules", ["inbound_rules", "outbound_rules"])): """ A set of Calico rules describing inbound and outbound network traffic policy. @@ -79,8 +79,7 @@ def from_json(cls, json_str): outbound_rules = [] for rule in json_dict["outbound_rules"]: outbound_rules.append(Rule(**rule)) - rules = cls(id=json_dict["id"], - inbound_rules=inbound_rules, + rules = cls(inbound_rules=inbound_rules, outbound_rules=outbound_rules) return rules @@ -389,7 +388,7 @@ def __init__(self, name): self.tags = set() # Default to empty lists of rules. - self.rules = Rules(name, [], []) + self.rules = Rules([], []) class Policy(object): @@ -400,7 +399,7 @@ def __init__(self, tier_name, policy_name): self.order = 0 # Default to empty lists of rules. - self.rules = Rules(policy_name, [], []) + self.rules = Rules([], []) # Default to empty selector. self.selector = "" diff --git a/calico_containers/tests/unit/test_datastore.py b/calico_containers/tests/unit/test_datastore.py index 4ddb124c..cf010ac5 100644 --- a/calico_containers/tests/unit/test_datastore.py +++ b/calico_containers/tests/unit/test_datastore.py @@ -106,10 +106,9 @@ } # A complicated set of Rules JSON for testing serialization / deserialization. -RULES_JSON = """ +RULES_JSON_FMT = """ { - "id": "PROF_GROUP1", - "inbound_rules": [ + %s"inbound_rules": [ { "action": "allow", "src_tag": "PROF_GROUP1" @@ -141,6 +140,14 @@ ] }""" +# A complicated set of rules adhering to the correct etcd data model +RULES_JSON = RULES_JSON_FMT % "" + +# An identical set of rules which includes an additional "id" field that was +# added by older versions of libcalico. We test that we can still read profiles +# of older versions. +RULES_JSON_WITH_ID = RULES_JSON_FMT % ' "id": "PROFILE1",\n' + class TestRule(unittest.TestCase): @@ -256,12 +263,26 @@ def test_rules(self): # Convert a JSON blob into a Rules object. rules = Rules.from_json(RULES_JSON) + # Call common check code. + self.check_rules(rules) + + def test_rules_with_id(self): + """ + Create a detailed set of rules that include an id field. This was included + in older versions of libcalico. We check here that we can read it in ok. + """ + # Convert a JSON blob into a Rules object. + rules = Rules.from_json(RULES_JSON_WITH_ID) + + # Call common check code. + self.check_rules(rules) + + def check_rules(self, rules): # Convert the Rules object to JSON and then back again. new_json = rules.to_json() new_rules = Rules.from_json(new_json) # Compare the two rules objects. - assert_equal(rules.id, new_rules.id) assert_equal(rules.inbound_rules, new_rules.inbound_rules) assert_equal(rules.outbound_rules, @@ -723,8 +744,7 @@ def test_profile_update_tags(self): profile = Profile("TEST") profile.tags = {"TAG4", "TAG5"} - profile.rules = Rules(id="TEST", - inbound_rules=[ + profile.rules = Rules(inbound_rules=[ Rule(action="allow", dst_ports=[12]), Rule(action="allow", protocol="udp"), Rule(action="deny") @@ -747,8 +767,7 @@ def test_profile_update_rules(self): profile = Profile("TEST") profile.tags = {"TAG4", "TAG5"} - profile.rules = Rules(id="TEST", - inbound_rules=[ + profile.rules = Rules(inbound_rules=[ Rule(action="allow", dst_ports=[12]), Rule(action="allow", protocol="udp"), Rule(action="deny") @@ -1165,8 +1184,7 @@ def test_create_profile(self): Test create_profile() """ self.datastore.create_profile("TEST") - rules = Rules(id="TEST", - inbound_rules=[Rule(action="allow", + rules = Rules(inbound_rules=[Rule(action="allow", src_tag="TEST")], outbound_rules=[Rule(action="allow")]) expected_calls = [call(TEST_PROFILE_PATH + "tags", '["TEST"]'), @@ -1177,8 +1195,7 @@ def test_create_profile_with_rules(self): """ Test create_profile() with rules specified """ - rules = Rules(id="TEST", - inbound_rules=[Rule(action="deny")], + rules = Rules(inbound_rules=[Rule(action="deny")], outbound_rules=[Rule(action="allow")]) self.datastore.create_profile("TEST", rules) expected_calls = [call(TEST_PROFILE_PATH + "tags", '["TEST"]'),