Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[generic-config-updater] Add caclrule validator #2103

Merged
merged 5 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion generic_config_updater/change_applier.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def _services_validate(self, old_cfg, upd_cfg, keys):

for cmd in lst_cmds:
ret = self._invoke_cmd(cmd, old_cfg, upd_cfg, keys)
if ret:
if not ret:
log_error("service invoked: {} failed with ret={}".format(cmd, ret))
return ret
log_debug("service invoked: {}".format(cmd))
Expand Down
6 changes: 6 additions & 0 deletions generic_config_updater/generic_config_updater.conf.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@
},
"VLAN": {
"services_to_validate": [ "vlan-service" ]
},
"ACL_RULE": {
"services_to_validate": [ "caclmgrd-service" ]
}
},
"services": {
Expand All @@ -59,6 +62,9 @@
},
"vlan-service": {
"validate_commands": [ "generic_config_updater.services_validator.vlan_validator" ]
},
"caclmgrd-service": {
"validate_commands": [ "generic_config_updater.services_validator.caclmgrd_validator" ]
}
}
}
Expand Down
27 changes: 27 additions & 0 deletions generic_config_updater/services_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,30 @@ def vlan_validator(old_config, upd_config, keys):
# No update to DHCP servers.
return True

def caclmgrd_validator(old_config, upd_config, keys):
old_acltable = old_config.get("ACL_TABLE", {})
upd_acltable = upd_config.get("ACL_TABLE", {})

old_cacltable = [table for table, fields in old_acltable.items()
if fields.get("type", "") == "CTRLPLANE"]
upd_cacltable = [table for table, fields in upd_acltable.items()
if fields.get("type", "") == "CTRLPLANE"]

old_aclrule = old_config.get("ACL_RULE", {})
upd_aclrule = upd_config.get("ACL_RULE", {})

old_caclrule = [rule for rule in old_aclrule
if rule.split("|")[0] in old_cacltable]
upd_caclrule = [rule for rule in upd_aclrule
if rule.split("|")[0] in upd_cacltable]

# Only sleep when cacl rule is changed as this will update iptable.
for key in set(old_caclrule).union(set(upd_caclrule)):
if (old_aclrule.get(key, {}) != upd_aclrule.get(key, {})):
wen587 marked this conversation as resolved.
Show resolved Hide resolved
# caclmgrd will update in 0.5 sec when configuration stops,
# we sleep 1 sec to make sure it does update.
time.sleep(1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this is a blind sleep. If caclmgrd is changed in future to wait 1sec, we may have to revisit here. I don't have a better suggestion though.

return True
# No update to ACL_RULE.
return True

3 changes: 3 additions & 0 deletions tests/generic_config_updater/change_applier_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def system_health(old_cfg, new_cfg, keys):
if svcs != None:
assert svc_name in svcs
svcs.remove(svc_name)
return True


def _validate_keys(keys):
Expand Down Expand Up @@ -201,11 +202,13 @@ def _validate_svc(svc_name, old_cfg, new_cfg, keys):
def acl_validate(old_cfg, new_cfg, keys):
debug_print("acl_validate called")
_validate_svc("acl_validate", old_cfg, new_cfg, keys)
return True


def vlan_validate(old_cfg, new_cfg, keys):
debug_print("vlan_validate called")
_validate_svc("vlan_validate", old_cfg, new_cfg, keys)
return True


class TestChangeApplier(unittest.TestCase):
Expand Down
103 changes: 98 additions & 5 deletions tests/generic_config_updater/service_validator_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@
from collections import defaultdict
from unittest.mock import patch

from generic_config_updater.services_validator import vlan_validator, rsyslog_validator
from generic_config_updater.services_validator import vlan_validator, rsyslog_validator, caclmgrd_validator
import generic_config_updater.gu_common


# Mimics os.system call
#
os_system_calls = []
os_system_call_index = 0
time_sleep_calls = []
time_sleep_call_index = 0
msg = ""

def mock_os_system_call(cmd):
Expand All @@ -26,6 +28,15 @@ def mock_os_system_call(cmd):
assert cmd == entry["cmd"], msg
return entry["rc"]

def mock_time_sleep_call(sleep_time):
global time_sleep_calls, time_sleep_call_index

assert time_sleep_call_index < len(time_sleep_calls)
entry = time_sleep_calls[time_sleep_call_index]
time_sleep_call_index += 1

assert sleep_time == entry["sleep_time"], msg


test_data = [
{ "old": {}, "upd": {}, "cmd": "" },
Expand Down Expand Up @@ -60,6 +71,77 @@ def mock_os_system_call(cmd):
}
]

test_caclrule = [
{ "old": {}, "upd": {}, "sleep_time": 0 },
{
"old": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } }
},
"upd": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } }
},
"sleep_time": 0
},
{
"old": {
"ACL_TABLE": {
"XXX": { "type": "CTRLPLANE" },
"YYY": { "type": "L3" }
},
"ACL_RULE": {
"XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" },
"YYY|RULE_1": { "SRC_IP": "192.168.1.10/32" }
}
},
"upd": {
"ACL_TABLE": {
"XXX": { "type": "CTRLPLANE" }
},
"ACL_RULE": {
"XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" }
}
},
"sleep_time": 0
},
{
"old": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } }
},
"upd": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "11.11.11.11/16" } }
},
"sleep_time": 1
},
{
"old": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": {
"XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" }
}
},
"upd": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": {
"XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" },
"XXX|RULE_2": { "SRC_IP": "12.12.12.12/16" }
}
},
"sleep_time": 1
},
{
"old": {
"ACL_TABLE": { "XXX": { "type": "CTRLPLANE" } },
"ACL_RULE": { "XXX|RULE_1": { "SRC_IP": "10.10.10.10/16" } }
},
"upd": {},
"sleep_time": 1
},
]

test_rsyslog_fail = [
# Fail the calls, to get the entire fail path calls invoked
#
Expand All @@ -70,17 +152,14 @@ def mock_os_system_call(cmd):
{ "cmd": "systemctl restart rsyslog", "rc": 1 }, # restart again; fails
]


class TestServiceValidator(unittest.TestCase):

@patch("generic_config_updater.change_applier.os.system")
def test_change_apply(self, mock_os_sys):
global os_system_expected_cmd
def test_change_apply_os_system(self, mock_os_sys):
global os_system_calls, os_system_call_index

mock_os_sys.side_effect = mock_os_system_call

i = 0
for entry in test_data:
if entry["cmd"]:
os_system_calls.append({"cmd": entry["cmd"], "rc": 0 })
Expand All @@ -89,6 +168,7 @@ def test_change_apply(self, mock_os_sys):
vlan_validator(entry["old"], entry["upd"], None)



# Test failure case
#
os_system_calls = test_rsyslog_fail
Expand All @@ -97,3 +177,16 @@ def test_change_apply(self, mock_os_sys):
rc = rsyslog_validator("", "", "")
assert not rc, "rsyslog_validator expected to fail"

@patch("generic_config_updater.services_validator.time.sleep")
def test_change_apply_time_sleep(self, mock_time_sleep):
global time_sleep_calls, time_sleep_call_index

mock_time_sleep.side_effect = mock_time_sleep_call

for entry in test_caclrule:
if entry["sleep_time"]:
time_sleep_calls.append({"sleep_time": entry["sleep_time"]})
msg = "case failed: {}".format(str(entry))

caclmgrd_validator(entry["old"], entry["upd"], None)