diff --git a/acl_loader/main.py b/acl_loader/main.py index e81e05d9b7..f73f3eb039 100644 --- a/acl_loader/main.py +++ b/acl_loader/main.py @@ -413,7 +413,7 @@ def parse_acl_json(filename): raise AclLoaderException("Invalid input file %s" % filename) return yang_acl - def load_rules_from_file(self, filename): + def load_rules_from_file(self, filename, skip_action_validation=False): """ Load file with ACL rules configuration in openconfig ACL format. Convert rules to Config DB schema. @@ -421,9 +421,9 @@ def load_rules_from_file(self, filename): :return: """ self.yang_acl = AclLoader.parse_acl_json(filename) - self.convert_rules() + self.convert_rules(skip_action_validation) - def convert_action(self, table_name, rule_idx, rule): + def convert_action(self, table_name, rule_idx, rule, skip_validation=False): rule_props = {} if rule.actions.config.forwarding_action == "ACCEPT": @@ -452,13 +452,13 @@ def convert_action(self, table_name, rule_idx, rule): raise AclLoaderException("Unknown rule action {} in table {}, rule {}".format( rule.actions.config.forwarding_action, table_name, rule_idx)) - if not self.validate_actions(table_name, rule_props): + if not self.validate_actions(table_name, rule_props, skip_validation): raise AclLoaderException("Rule action {} is not supported in table {}, rule {}".format( rule.actions.config.forwarding_action, table_name, rule_idx)) return rule_props - def validate_actions(self, table_name, action_props): + def validate_actions(self, table_name, action_props, skip_validation=False): if self.is_table_control_plane(table_name): return True @@ -481,6 +481,11 @@ def validate_actions(self, table_name, action_props): else: aclcapability = self.statedb.get_all(self.statedb.STATE_DB, "{}|{}".format(self.ACL_STAGE_CAPABILITY_TABLE, stage.upper())) switchcapability = self.statedb.get_all(self.statedb.STATE_DB, "{}|switch".format(self.SWITCH_CAPABILITY_TABLE)) + # In the load_minigraph path, it's possible that the STATE_DB entry haven't pop up because orchagent is stopped + # before loading acl.json. So we skip the validation if any table is empty + if skip_validation and (not aclcapability or not switchcapability): + warning("Skipped action validation as capability table is not present in STATE_DB") + return True for action_key in dict(action_props): action_list_key = self.ACL_ACTIONS_CAPABILITY_FIELD if action_list_key not in aclcapability: @@ -699,7 +704,7 @@ def validate_rule_fields(self, rule_props): if ("ICMPV6_TYPE" in rule_props or "ICMPV6_CODE" in rule_props) and protocol != 58: raise AclLoaderException("IP_PROTOCOL={} is not ICMPV6, but ICMPV6 fields were provided".format(protocol)) - def convert_rule_to_db_schema(self, table_name, rule): + def convert_rule_to_db_schema(self, table_name, rule, skip_action_validation=False): """ Convert rules format from openconfig ACL to Config DB schema :param table_name: ACL table name to which rule belong @@ -729,7 +734,7 @@ def convert_rule_to_db_schema(self, table_name, rule): elif self.is_table_l3(table_name): rule_props["ETHER_TYPE"] = str(self.ethertype_map["ETHERTYPE_IPV4"]) - deep_update(rule_props, self.convert_action(table_name, rule_idx, rule)) + deep_update(rule_props, self.convert_action(table_name, rule_idx, rule, skip_action_validation)) deep_update(rule_props, self.convert_l2(table_name, rule_idx, rule)) deep_update(rule_props, self.convert_ip(table_name, rule_idx, rule)) deep_update(rule_props, self.convert_icmp(table_name, rule_idx, rule)) @@ -761,7 +766,7 @@ def deny_rule(self, table_name): return {} # Don't add default deny rule if table is not [L3, L3V6] return rule_data - def convert_rules(self): + def convert_rules(self, skip_aciton_validation=False): """ Convert rules in openconfig ACL format to Config DB schema :return: @@ -780,7 +785,7 @@ def convert_rules(self): for acl_entry_name in acl_set.acl_entries.acl_entry: acl_entry = acl_set.acl_entries.acl_entry[acl_entry_name] try: - rule = self.convert_rule_to_db_schema(table_name, acl_entry) + rule = self.convert_rule_to_db_schema(table_name, acl_entry, skip_aciton_validation) deep_update(self.rules_info, rule) except AclLoaderException as ex: error("Error processing rule %s: %s. Skipped." % (acl_entry_name, ex)) @@ -1149,8 +1154,9 @@ def update(ctx): @click.option('--session_name', type=click.STRING, required=False) @click.option('--mirror_stage', type=click.Choice(["ingress", "egress"]), default="ingress") @click.option('--max_priority', type=click.INT, required=False) +@click.option('--skip_action_validation', is_flag=True, default=False, help="Skip action validation") @click.pass_context -def full(ctx, filename, table_name, session_name, mirror_stage, max_priority): +def full(ctx, filename, table_name, session_name, mirror_stage, max_priority, skip_action_validation): """ Full update of ACL rules configuration. If a table_name is provided, the operation will be restricted in the specified table. @@ -1168,7 +1174,7 @@ def full(ctx, filename, table_name, session_name, mirror_stage, max_priority): if max_priority: acl_loader.set_max_priority(max_priority) - acl_loader.load_rules_from_file(filename) + acl_loader.load_rules_from_file(filename, skip_action_validation) acl_loader.full_update() diff --git a/config/main.py b/config/main.py index 14f34f1318..bea5016fdb 100644 --- a/config/main.py +++ b/config/main.py @@ -1745,7 +1745,7 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config, update_sonic_environment() if os.path.isfile('/etc/sonic/acl.json'): - clicommon.run_command(['acl-loader', 'update', 'full', '/etc/sonic/acl.json'], display_cmd=True) + clicommon.run_command(['acl-loader', 'update', 'full', '/etc/sonic/acl.json', '--skip_action_validation'], display_cmd=True) # Load port_config.json try: diff --git a/tests/acl_loader_test.py b/tests/acl_loader_test.py index 599e47461a..01dc8602d5 100644 --- a/tests/acl_loader_test.py +++ b/tests/acl_loader_test.py @@ -56,6 +56,36 @@ def test_validate_mirror_action(self, acl_loader): assert acl_loader.validate_actions("DATAACL", forward_packet_action) assert not acl_loader.validate_actions("DATAACL", drop_packet_action) + def test_load_rules_when_capability_table_is_empty(self, acl_loader): + """ + Test case to verify that acl_loader can still load dataplane acl rules when skip_action_validation + is true, and capability table in state_db is absent + """ + # Backup and empty the capability table from state_db + SWITCH_CAPABILITY = "SWITCH_CAPABILITY|switch" + if acl_loader.per_npu_statedb: + statedb = list(acl_loader.per_npu_statedb.values())[0] + else: + statedb = acl_loader.statedb + switchcapability = statedb.get_all("STATE_DB", SWITCH_CAPABILITY) + statedb.delete("STATE_DB", SWITCH_CAPABILITY) + try: + acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json'), skip_action_validation=True) + assert acl_loader.rules_info[("DATAACL", "RULE_2")] + assert acl_loader.rules_info[("DATAACL", "RULE_2")] == { + "VLAN_ID": 369, + "ETHER_TYPE": "2048", + "IP_PROTOCOL": 6, + "SRC_IP": "20.0.0.2/32", + "DST_IP": "30.0.0.3/32", + "PACKET_ACTION": "FORWARD", + "PRIORITY": "9998" + } + finally: + # Restore the capability table in state_db + for key, value in switchcapability.items(): + statedb.set("STATE_DB", SWITCH_CAPABILITY, key, value) + def test_vlan_id_translation(self, acl_loader): acl_loader.rules_info = {} acl_loader.load_rules_from_file(os.path.join(test_path, 'acl_input/acl1.json'))