Skip to content

Commit

Permalink
ignore firewall packets reset error, check enable firewall config fla…
Browse files Browse the repository at this point in the history
…g and extend cgroup extension monitoring expiry time (#2502)
  • Loading branch information
nagworld9 authored Feb 11, 2022
1 parent a728b2e commit 902e183
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 59 deletions.
6 changes: 3 additions & 3 deletions azurelinuxagent/common/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def load_conf_from_file(conf_file_path, conf=__conf__):
"ResourceDisk.MountOptions": None,
"ResourceDisk.Filesystem": "ext3",
"AutoUpdate.GAFamily": "Prod",
"Debug.CgroupMonitorExpiryTime": "2022-01-31",
"Debug.CgroupMonitorExpiryTime": "2022-03-31",
"Debug.CgroupMonitorExtensionName": "Microsoft.Azure.Monitor.AzureMonitorLinuxAgent",
}

Expand Down Expand Up @@ -550,11 +550,11 @@ def get_agent_cpu_quota(conf=__conf__):

def get_cgroup_monitor_expiry_time (conf=__conf__):
"""
cgroups monitoring disabled after expiry time
cgroups monitoring for pilot extensions disabled after expiry time
NOTE: This option is experimental and may be removed in later versions of the Agent.
"""
return conf.get("Debug.CgroupMonitorExpiryTime", "2022-01-31")
return conf.get("Debug.CgroupMonitorExpiryTime", "2022-03-31")

def get_cgroup_monitor_extension_name (conf=__conf__):
"""
Expand Down
5 changes: 3 additions & 2 deletions azurelinuxagent/common/osutil/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,10 @@ def get_firewall_dropped_packets(self, dst_ip=None):
return int(m.group(1))

except Exception as e:
if isinstance(e, CommandError) and e.returncode == 3: # pylint: disable=E1101
# Transient error that we ignore. This code fires every loop
if isinstance(e, CommandError) and (e.returncode == 3 or e.returncode == 4): # pylint: disable=E1101
# Transient error that we ignore returncode 3. This code fires every loop
# of the daemon (60m), so we will get the value eventually.
# ignore returncode 4 as temporary fix (RULE_REPLACE failed (Invalid argument))
return 0
logger.warn("Failed to get firewall packets: {0}", ustr(e))
return -1
Expand Down
3 changes: 3 additions & 0 deletions azurelinuxagent/ga/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,9 @@ def _ensure_firewall_rules_persisted(dst_ip):

def _add_accept_tcp_firewall_rule_if_not_enabled(self, dst_ip):

if not conf.enable_firewall():
return

def _execute_run_command(command):
# Helper to execute a run command, returns True if no exception
# Here we primarily check if an iptable rule exist. True if it exits , false if not
Expand Down
7 changes: 7 additions & 0 deletions tests/common/osutil/test_default.py
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,13 @@ def test_get_firewall_dropped_packets_should_ignore_transient_errors(self):
mock_iptables.set_command(osutil.get_firewall_packets_command(mock_iptables.wait), exit_code=3, output="can't initialize iptables table `security': iptables who? (do you need to insmod?)")
self.assertEqual(0, osutil.DefaultOSUtil().get_firewall_dropped_packets())

def test_get_firewall_dropped_packets_should_ignore_returncode_4(self):

with TestOSUtil._mock_iptables() as mock_iptables:
with patch.object(osutil, '_enable_firewall', True):
mock_iptables.set_command(osutil.get_firewall_packets_command(mock_iptables.wait), exit_code=4, output="iptables v1.8.2 (nf_tables): RULE_REPLACE failed (Invalid argument): rule in chain OUTPUT")
self.assertEqual(0, osutil.DefaultOSUtil().get_firewall_dropped_packets())

def test_get_firewall_dropped_packets(self):

destination = '168.63.129.16'
Expand Down
109 changes: 56 additions & 53 deletions tests/ga/test_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -1699,73 +1699,76 @@ def test_it_should_set_dns_tcp_iptable_if_drop_available_accept_unavailable(self

with TestOSUtil._mock_iptables() as mock_iptables:
with _get_update_handler(test_data=DATA_FILE) as (update_handler, _):
with patch.object(osutil, '_enable_firewall', True):
# drop rule is present
mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0)
# non root tcp iptable rule is absent
mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1)
update_handler.run(debug=True)

drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination))

# Filtering the mock iptable command calls with only the once related to this test.
filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]]

self.assertEqual(len(filtered_mock_iptable_calls), 3, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls))
self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command,
"The first command should check the drop rule")
self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule,
"The second command should check the accept rule")
self.assertEqual(filtered_mock_iptable_calls[2], accept_tcp_insert_rule,
"The third command should add the accept rule")
with patch('azurelinuxagent.common.conf.enable_firewall', return_value=True):
with patch.object(osutil, '_enable_firewall', True):
# drop rule is present
mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0)
# non root tcp iptable rule is absent
mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1)
update_handler.run(debug=True)

drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination))

# Filtering the mock iptable command calls with only the once related to this test.
filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]]

self.assertEqual(len(filtered_mock_iptable_calls), 3, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls))
self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command,
"The first command should check the drop rule")
self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule,
"The second command should check the accept rule")
self.assertEqual(filtered_mock_iptable_calls[2], accept_tcp_insert_rule,
"The third command should add the accept rule")

def test_it_should_not_set_dns_tcp_iptable_if_drop_unavailable(self):

with TestOSUtil._mock_iptables() as mock_iptables:
with _get_update_handler(test_data=DATA_FILE) as (update_handler, _):
with patch.object(osutil, '_enable_firewall', True):
# drop rule is not available
mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1)
with patch('azurelinuxagent.common.conf.enable_firewall', return_value=True):
with patch.object(osutil, '_enable_firewall', True):
# drop rule is not available
mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=1)

update_handler.run(debug=True)
update_handler.run(debug=True)

drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination))
drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination))

# Filtering the mock iptable command calls with only the once related to this test.
filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]]
# Filtering the mock iptable command calls with only the once related to this test.
filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]]

self.assertEqual(len(filtered_mock_iptable_calls), 1, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls))
self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command,
"The first command should check the drop rule")
self.assertEqual(len(filtered_mock_iptable_calls), 1, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls))
self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command,
"The first command should check the drop rule")

def test_it_should_not_set_dns_tcp_iptable_if_drop_and_accept_available(self):

with TestOSUtil._mock_iptables() as mock_iptables:
with _get_update_handler(test_data=DATA_FILE) as (update_handler, _):
with patch.object(osutil, '_enable_firewall', True):
# drop rule is available
mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0)
# non root tcp iptable rule is available
mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0)

update_handler.run(debug=True)

drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination))

# Filtering the mock iptable command calls with only the once related to this test.
filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]]

self.assertEqual(len(filtered_mock_iptable_calls), 2, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls))
self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command,
"The first command should check the drop rule")
self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule,
"The second command should check the accept rule")
with patch('azurelinuxagent.common.conf.enable_firewall', return_value=True):
with patch.object(osutil, '_enable_firewall', True):
# drop rule is available
mock_iptables.set_command(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0)
# non root tcp iptable rule is available
mock_iptables.set_command(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination), exit_code=0)

update_handler.run(debug=True)

drop_check_command = TestOSUtil._command_to_string(osutil.get_firewall_drop_command(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_check_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.CHECK_COMMAND, mock_iptables.destination))
accept_tcp_insert_rule = TestOSUtil._command_to_string(osutil.get_accept_tcp_rule(mock_iptables.wait, AddFirewallRules.INSERT_COMMAND, mock_iptables.destination))

# Filtering the mock iptable command calls with only the once related to this test.
filtered_mock_iptable_calls = [cmd for cmd in mock_iptables.command_calls if cmd in [drop_check_command, accept_tcp_check_rule, accept_tcp_insert_rule]]

self.assertEqual(len(filtered_mock_iptable_calls), 2, "Incorrect number of calls to iptables: [{0}]".format(mock_iptables.command_calls))
self.assertEqual(filtered_mock_iptable_calls[0], drop_check_command,
"The first command should check the drop rule")
self.assertEqual(filtered_mock_iptable_calls[1], accept_tcp_check_rule,
"The second command should check the accept rule")

@contextlib.contextmanager
def _setup_test_for_ext_event_dirs_retention(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/test_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
Debug.CgroupDisableOnProcessCheckFailure = True
Debug.CgroupDisableOnQuotaCheckFailure = True
Debug.CgroupLogMetrics = False
Debug.CgroupMonitorExpiryTime = 2022-01-31
Debug.CgroupMonitorExpiryTime = 2022-03-31
Debug.CgroupMonitorExtensionName = Microsoft.Azure.Monitor.AzureMonitorLinuxAgent
Debug.EnableFastTrack = True
Debug.EnableGAVersioning = False
Expand Down

0 comments on commit 902e183

Please sign in to comment.