From aa75f8d31601ee9d2accfc7d80db6728c4a345f5 Mon Sep 17 00:00:00 2001 From: junchao Date: Fri, 19 Aug 2022 18:25:21 +0800 Subject: [PATCH 1/5] Support host side rate limit configuration --- scripts/hostcfgd | 93 ++++++++++++++++++++++++-- tests/hostcfgd/hostcfgd_test.py | 50 +++++++++++++- tests/hostcfgd/mock_empty_rsyslog.conf | 0 tests/hostcfgd/mock_rsyslog.conf | 29 ++++++++ 4 files changed, 164 insertions(+), 8 deletions(-) create mode 100644 tests/hostcfgd/mock_empty_rsyslog.conf create mode 100644 tests/hostcfgd/mock_rsyslog.conf diff --git a/scripts/hostcfgd b/scripts/hostcfgd index d549d560..f64bf24c 100755 --- a/scripts/hostcfgd +++ b/scripts/hostcfgd @@ -243,7 +243,7 @@ class FeatureHandler(object): if not feature_name: syslog.syslog(syslog.LOG_WARNING, "Feature is None") continue - + device_config = {} device_config.update(self._device_config) device_config.update(self._device_running_config) @@ -301,7 +301,7 @@ class FeatureHandler(object): def sync_feature_asic_scope(self, feature_config): """Updates the has_per_asic_scope field in the FEATURE|* tables as the field might have to be rendered based on DEVICE_METADATA table or Device Running configuration. - Disable the ASIC instance service unit file it the render value is False and update config + Disable the ASIC instance service unit file it the render value is False and update config Args: feature: An object represents a feature's configuration in `FEATURE` @@ -334,7 +334,7 @@ class FeatureHandler(object): self.set_feature_state(feature, self.FEATURE_STATE_FAILED) return self._config_db.mod_entry('FEATURE', feature_config.name, {'has_per_asic_scope': str(feature_config.has_per_asic_scope)}) - + def update_systemd_config(self, feature_config): """Updates `Restart=` field in feature's systemd configuration file according to the value of `auto_restart` field in `FEATURE` table of `CONFIG_DB`. @@ -935,7 +935,7 @@ class PasswHardening(object): def __init__(self): self.passw_policies_default = {} self.passw_policies = {} - + self.debug = False self.trace = False @@ -1109,7 +1109,7 @@ class PasswHardening(object): def modify_passw_conf_file(self): passw_policies = self.passw_policies_default.copy() passw_policies.update(self.passw_policies) - + # set new Password Hardening policies. self.set_passw_hardening_policies(passw_policies) @@ -1303,6 +1303,7 @@ class PamLimitsCfg(object): "modify pam_limits config file failed with exception: {}" .format(e)) + class DeviceMetaCfg(object): """ DeviceMetaCfg Config Daemon @@ -1440,6 +1441,71 @@ class MgmtIfaceCfg(object): self.mgmt_vrf_enabled = enabled +class SyslogCfg: + SYSLOG_RATE_LIMIT_INTERVAL = 'rate_limit_interval' + SYSLOG_RATE_LIMIT_BURST = 'rate_limit_burst' + HOST_KEY = 'GLOBAL' + + # syslog conf file path in docker + SYSLOG_CONF_PATH = '/etc/rsyslog.conf' + + # Regular expressions to extract value from rsyslog.conf + INTERVAL_PATTERN = '.*SystemLogRateLimitInterval\s+(\d+).*' + BURST_PATTERN = '.*SystemLogRateLimitBurst\s+(\d+).*' + + def __init__(self): + self.current_interval, self.current_burst = self.parse_syslog_conf() + + def syslog_update(self, data): + """Update syslog related configuration + + Args: + data (dict): CONFIG DB data: {: } + """ + new_interval = '0' + new_burst = '0' + if data: + new_interval = data.get(self.SYSLOG_RATE_LIMIT_INTERVAL, '0') + new_burst = data.get(self.SYSLOG_RATE_LIMIT_BURST, '0') + + if new_interval == self.current_interval and new_burst == self.current_burst: + syslog.syslog(syslog.LOG_INFO, 'Syslog rate limit configuration does not change, ignore it') + return + + syslog.syslog(syslog.LOG_INFO, f'Configure syslog rate limit interval={new_interval}, burst={new_burst}') + + run_cmd('systemctl reset-failed rsyslog-config rsyslog') + run_cmd('systemctl restart rsyslog-config') + self.current_interval = new_interval + self.current_burst = new_burst + + def load(self, data): + if self.HOST_KEY in data: + self.syslog_update(data[self.HOST_KEY]) + + def parse_syslog_conf(self): + """Passe existing syslog conf and extract config values + + Returns: + tuple: interval,burst,target_ip + """ + interval = '0' + burst = '0' + + with open(self.SYSLOG_CONF_PATH, 'r') as f: + content = f.read() + pattern = re.compile(self.INTERVAL_PATTERN) + for match in pattern.finditer(content): + interval = match.group(1) + break + + pattern = re.compile(self.BURST_PATTERN) + for match in pattern.finditer(content): + burst = match.group(1) + break + return interval, burst + + class HostConfigDaemon: def __init__(self): # Just a sanity check to verify if the CONFIG_DB has been initialized @@ -1486,6 +1552,9 @@ class HostConfigDaemon: # Initialize MgmtIfaceCfg self.mgmtifacecfg = MgmtIfaceCfg() + # Initialize SyslogCfg + self.syslogcfg = SyslogCfg() + def load(self, init_data): features = init_data['FEATURE'] aaa = init_data['AAA'] @@ -1501,6 +1570,7 @@ class HostConfigDaemon: dev_meta = init_data.get(swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, {}) mgmt_ifc = init_data.get(swsscommon.CFG_MGMT_INTERFACE_TABLE_NAME, {}) mgmt_vrf = init_data.get(swsscommon.CFG_MGMT_VRF_CONFIG_TABLE_NAME, {}) + syslog = init_data['SYSLOG_CONFIG'] self.feature_handler.sync_state_field(features) self.aaacfg.load(aaa, tacacs_global, tacacs_server, radius_global, radius_server) @@ -1510,6 +1580,12 @@ class HostConfigDaemon: self.passwcfg.load(passwh) self.devmetacfg.load(dev_meta) self.mgmtifacecfg.load(mgmt_ifc, mgmt_vrf) + self.syslogcfg.load(syslog) + + dev_meta = self.config_db.get_table('DEVICE_METADATA') + if 'localhost' in dev_meta: + if 'hostname' in dev_meta['localhost']: + self.hostname_cache = dev_meta['localhost']['hostname'] # Update AAA with the hostname self.aaacfg.hostname_update(self.devmetacfg.hostname) @@ -1609,6 +1685,9 @@ class HostConfigDaemon: syslog.syslog(syslog.LOG_INFO, 'DeviceMeta handler...') self.devmetacfg.hostname_update(data) + def syslog_handler(self, key, op, data): + self.syslogcfg.syslog_update(data) + def wait_till_system_init_done(self): # No need to print the output in the log file so using the "--quiet" # flag @@ -1647,7 +1726,7 @@ class HostConfigDaemon: self.config_db.subscribe('VLAN_SUB_INTERFACE', make_callback(self.vlan_sub_intf_handler)) self.config_db.subscribe('PORTCHANNEL_INTERFACE', make_callback(self.portchannel_intf_handler)) self.config_db.subscribe('INTERFACE', make_callback(self.phy_intf_handler)) - + # Handle DEVICE_MEATADATA changes self.config_db.subscribe(swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, make_callback(self.device_metadata_handler)) @@ -1656,6 +1735,8 @@ class HostConfigDaemon: self.config_db.subscribe(swsscommon.CFG_MGMT_VRF_CONFIG_TABLE_NAME, make_callback(self.mgmt_vrf_handler)) + self.config_db.subscribe('SYSLOG_CONFIG', make_callback(self.syslog_handler)) + syslog.syslog(syslog.LOG_INFO, "Waiting for systemctl to finish initialization") self.wait_till_system_init_done() diff --git a/tests/hostcfgd/hostcfgd_test.py b/tests/hostcfgd/hostcfgd_test.py index 93ba7b96..663a6ec9 100644 --- a/tests/hostcfgd/hostcfgd_test.py +++ b/tests/hostcfgd/hostcfgd_test.py @@ -30,7 +30,7 @@ class TestFeatureHandler(TestCase): - """Test methods of `FeatureHandler` class. + """Test methods of `FeatureHandler` class. """ def checks_config_table(self, feature_table, expected_table): """Compares `FEATURE` table in `CONFIG_DB` with expected output table. @@ -417,7 +417,7 @@ def test_devicemeta_event(self): def test_mgmtiface_event(self): """ Test handling mgmt events. - 1) Management interface setup + 1) Management interface setup 2) Management vrf setup """ MockConfigDb.set_config_db(HOSTCFG_DAEMON_CFG_DB) @@ -455,3 +455,49 @@ def test_mgmtiface_event(self): ] mocked_subprocess.check_call.assert_has_calls(expected, any_order=True) + +class TestSyslogHandler: + @mock.patch('hostcfgd.run_cmd') + def test_syslog_update(self, mock_run_cmd): + syslog_cfg = hostcfgd.SyslogCfg() + syslog_cfg.parse_syslog_conf = mock.MagicMock(return_value=('100', '200')) + data = { + 'rate_limit_interval': '100', + 'rate_limit_burst': '200' + } + syslog_cfg.syslog_update(data) + mock_run_cmd.assert_not_called() + + data = { + 'rate_limit_interval': '200', + 'rate_limit_burst': '200' + } + syslog_cfg.syslog_update(data) + expected = [call('systemctl reset-failed rsyslog-config rsyslog'), + call('systemctl restart rsyslog-config')] + mock_run_cmd.assert_has_calls(expected) + + def test_load(self): + syslog_cfg = hostcfgd.SyslogCfg() + syslog_cfg.syslog_update = mock.MagicMock() + + data = {} + syslog_cfg.load(data) + syslog_cfg.syslog_update.assert_not_called() + + data = {syslog_cfg.HOST_KEY: {}} + syslog_cfg.load(data) + syslog_cfg.syslog_update.assert_called_once() + + def test_parse_syslog_conf(self): + syslog_cfg = hostcfgd.SyslogCfg() + + syslog_cfg.SYSLOG_CONF_PATH = os.path.join(test_path, 'hostcfgd', 'mock_rsyslog.conf') + interval, burst = syslog_cfg.parse_syslog_conf() + assert interval == '50' + assert burst == '10002' + + syslog_cfg.SYSLOG_CONF_PATH = os.path.join(test_path, 'hostcfgd', 'mock_empty_rsyslog.conf') + interval, burst = syslog_cfg.parse_syslog_conf() + assert interval is '0' + assert burst is '0' diff --git a/tests/hostcfgd/mock_empty_rsyslog.conf b/tests/hostcfgd/mock_empty_rsyslog.conf new file mode 100644 index 00000000..e69de29b diff --git a/tests/hostcfgd/mock_rsyslog.conf b/tests/hostcfgd/mock_rsyslog.conf new file mode 100644 index 00000000..68e917b9 --- /dev/null +++ b/tests/hostcfgd/mock_rsyslog.conf @@ -0,0 +1,29 @@ +$ModLoad imuxsock # provides support for local system logging + +# +# Set a rate limit on messages from the container +# + + +$SystemLogRateLimitInterval 50 +$SystemLogRateLimitBurst 10002 + +#$ModLoad imklog # provides kernel logging support +#$ModLoad immark # provides --MARK-- message capability + +# provides UDP syslog reception +#$ModLoad imudp +#$UDPServerRun 514 + +# provides TCP syslog reception +#$ModLoad imtcp +#$InputTCPServerRun 514 + + +########################### +#### GLOBAL DIRECTIVES #### +########################### + +# Set remote syslog server +template (name="ForwardFormatInContainer" type="string" string="<%PRI%>%TIMESTAMP:::date-rfc3339% %HOSTNAME% pmon#%syslogtag%%msg:::sp-if-no-1st-sp%%msg%") +*.* action(type="omfwd" target="127.0.0.1" port="514" protocol="udp" Template="ForwardFormatInContainer") From 7c07508ba742798d02e2f451a58424046b625cdf Mon Sep 17 00:00:00 2001 From: junchao Date: Tue, 25 Oct 2022 21:45:27 +0800 Subject: [PATCH 2/5] Fix unit test issue caused by rebase --- scripts/hostcfgd | 26 +++++++++++++++----------- tests/hostcfgd/hostcfgd_test.py | 2 +- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/scripts/hostcfgd b/scripts/hostcfgd index f64bf24c..aa24e70c 100755 --- a/scripts/hostcfgd +++ b/scripts/hostcfgd @@ -1492,17 +1492,21 @@ class SyslogCfg: interval = '0' burst = '0' - with open(self.SYSLOG_CONF_PATH, 'r') as f: - content = f.read() - pattern = re.compile(self.INTERVAL_PATTERN) - for match in pattern.finditer(content): - interval = match.group(1) - break + try: + with open(self.SYSLOG_CONF_PATH, 'r') as f: + content = f.read() + pattern = re.compile(self.INTERVAL_PATTERN) + for match in pattern.finditer(content): + interval = match.group(1) + break - pattern = re.compile(self.BURST_PATTERN) - for match in pattern.finditer(content): - burst = match.group(1) - break + pattern = re.compile(self.BURST_PATTERN) + for match in pattern.finditer(content): + burst = match.group(1) + break + except OSError: + syslog.syslog(syslog.LOG_ERR, f'Failed to read file {self.SYSLOG_CONF_PATH}') + return None, None return interval, burst @@ -1570,7 +1574,7 @@ class HostConfigDaemon: dev_meta = init_data.get(swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, {}) mgmt_ifc = init_data.get(swsscommon.CFG_MGMT_INTERFACE_TABLE_NAME, {}) mgmt_vrf = init_data.get(swsscommon.CFG_MGMT_VRF_CONFIG_TABLE_NAME, {}) - syslog = init_data['SYSLOG_CONFIG'] + syslog = init_data.get('SYSLOG_CONFIG', {}) self.feature_handler.sync_state_field(features) self.aaacfg.load(aaa, tacacs_global, tacacs_server, radius_global, radius_server) diff --git a/tests/hostcfgd/hostcfgd_test.py b/tests/hostcfgd/hostcfgd_test.py index 663a6ec9..14bee327 100644 --- a/tests/hostcfgd/hostcfgd_test.py +++ b/tests/hostcfgd/hostcfgd_test.py @@ -458,9 +458,9 @@ def test_mgmtiface_event(self): class TestSyslogHandler: @mock.patch('hostcfgd.run_cmd') + @mock.patch('hostcfgd.SyslogCfg.parse_syslog_conf', mock.MagicMock(return_value=('100', '200'))) def test_syslog_update(self, mock_run_cmd): syslog_cfg = hostcfgd.SyslogCfg() - syslog_cfg.parse_syslog_conf = mock.MagicMock(return_value=('100', '200')) data = { 'rate_limit_interval': '100', 'rate_limit_burst': '200' From 72c7556d3d958628093521558c488fe2bad33112 Mon Sep 17 00:00:00 2001 From: junchao Date: Thu, 10 Nov 2022 14:04:26 +0800 Subject: [PATCH 3/5] Fix review comment --- scripts/hostcfgd | 23 ++++++++++------------- tests/hostcfgd/hostcfgd_test.py | 14 ++++++++++++-- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/scripts/hostcfgd b/scripts/hostcfgd index aa24e70c..ea9a5d8a 100755 --- a/scripts/hostcfgd +++ b/scripts/hostcfgd @@ -1469,22 +1469,24 @@ class SyslogCfg: new_burst = data.get(self.SYSLOG_RATE_LIMIT_BURST, '0') if new_interval == self.current_interval and new_burst == self.current_burst: - syslog.syslog(syslog.LOG_INFO, 'Syslog rate limit configuration does not change, ignore it') return - syslog.syslog(syslog.LOG_INFO, f'Configure syslog rate limit interval={new_interval}, burst={new_burst}') + syslog.syslog(syslog.LOG_INFO, f'Configure syslog rate limit interval={new_interval} (old:{self.current_interval}), burst={new_burst} (old:{self.current_burst})') - run_cmd('systemctl reset-failed rsyslog-config rsyslog') - run_cmd('systemctl restart rsyslog-config') - self.current_interval = new_interval - self.current_burst = new_burst + try: + run_cmd('systemctl reset-failed rsyslog-config rsyslog', raise_exception=True) + run_cmd('systemctl restart rsyslog-config', raise_exception=True) + self.current_interval = new_interval + self.current_burst = new_burst + except Exception as e: + syslog.syslog(syslog.LOG_ERR, f'Failed to configure syslog rate limit for host - {e}') def load(self, data): if self.HOST_KEY in data: self.syslog_update(data[self.HOST_KEY]) def parse_syslog_conf(self): - """Passe existing syslog conf and extract config values + """Parse existing syslog conf and extract config values Returns: tuple: interval,burst,target_ip @@ -1506,7 +1508,7 @@ class SyslogCfg: break except OSError: syslog.syslog(syslog.LOG_ERR, f'Failed to read file {self.SYSLOG_CONF_PATH}') - return None, None + return interval, burst return interval, burst @@ -1586,11 +1588,6 @@ class HostConfigDaemon: self.mgmtifacecfg.load(mgmt_ifc, mgmt_vrf) self.syslogcfg.load(syslog) - dev_meta = self.config_db.get_table('DEVICE_METADATA') - if 'localhost' in dev_meta: - if 'hostname' in dev_meta['localhost']: - self.hostname_cache = dev_meta['localhost']['hostname'] - # Update AAA with the hostname self.aaacfg.hostname_update(self.devmetacfg.hostname) diff --git a/tests/hostcfgd/hostcfgd_test.py b/tests/hostcfgd/hostcfgd_test.py index 14bee327..a5a868aa 100644 --- a/tests/hostcfgd/hostcfgd_test.py +++ b/tests/hostcfgd/hostcfgd_test.py @@ -477,6 +477,16 @@ def test_syslog_update(self, mock_run_cmd): call('systemctl restart rsyslog-config')] mock_run_cmd.assert_has_calls(expected) + data = { + 'rate_limit_interval': '100', + 'rate_limit_burst': '100' + } + mock_run_cmd.side_effect = Exception() + syslog_cfg.syslog_update(data) + # when exception occurs, interval and burst should not be updated + assert syslog_cfg.current_interval == '200' + assert syslog_cfg.current_burst == '200' + def test_load(self): syslog_cfg = hostcfgd.SyslogCfg() syslog_cfg.syslog_update = mock.MagicMock() @@ -499,5 +509,5 @@ def test_parse_syslog_conf(self): syslog_cfg.SYSLOG_CONF_PATH = os.path.join(test_path, 'hostcfgd', 'mock_empty_rsyslog.conf') interval, burst = syslog_cfg.parse_syslog_conf() - assert interval is '0' - assert burst is '0' + assert interval == '0' + assert burst == '0' From 91537fb43a1260c87c51aa1633bf6e82251907e2 Mon Sep 17 00:00:00 2001 From: Junchao-Mellanox <57339448+Junchao-Mellanox@users.noreply.github.com> Date: Fri, 11 Nov 2022 15:13:25 +0800 Subject: [PATCH 4/5] Update hostcfgd_test.py --- tests/hostcfgd/hostcfgd_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/hostcfgd/hostcfgd_test.py b/tests/hostcfgd/hostcfgd_test.py index 542c4f4e..967ec30c 100644 --- a/tests/hostcfgd/hostcfgd_test.py +++ b/tests/hostcfgd/hostcfgd_test.py @@ -479,8 +479,8 @@ def test_syslog_update(self, mock_run_cmd): 'rate_limit_burst': '200' } syslog_cfg.syslog_update(data) - expected = [call('systemctl reset-failed rsyslog-config rsyslog'), - call('systemctl restart rsyslog-config')] + expected = [call('systemctl reset-failed rsyslog-config rsyslog', raise_exception=True), + call('systemctl restart rsyslog-config', raise_exception=True)] mock_run_cmd.assert_has_calls(expected) data = { From ab4c060eae45180563352c0171a5c52a054fc513 Mon Sep 17 00:00:00 2001 From: junchao Date: Wed, 30 Nov 2022 11:30:09 +0800 Subject: [PATCH 5/5] Fix review comment --- scripts/hostcfgd | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 scripts/hostcfgd diff --git a/scripts/hostcfgd b/scripts/hostcfgd old mode 100644 new mode 100755