Fix test timing issue + minor code cleanup

azurelinuxagent/ga/firewall_manager.py

@@ -72,20 +72,27 @@ def create(wire_server_address):
         """
         try:
             manager = IpTables(wire_server_address)
-            logger.info("Using iptables to manage firewall rules")
+            event.info(WALAEventOperation.Firewall, "Using iptables [version {0}] to manage firewall rules", manager.version)
             return manager
         except FirewallManagerNotAvailableError:
             pass
 
         try:
             manager = NfTables(wire_server_address)
-            logger.info("Using nftables to manage firewall rules")
+            event.info(WALAEventOperation.Firewall, "Using nft [version {0}] to manage firewall rules", manager.version)
             return manager
         except FirewallManagerNotAvailableError:
             pass
 
         raise FirewallManagerNotAvailableError("Cannot create a firewall manager; no known command-line tools are available")
 
+    @property
+    def version(self):
+        """
+        Returns the version of the underlying command-line tool.
+        """
+        raise NotImplementedError()
+
     def setup(self):
         """
         Sets up the firewall rules for the WireServer.
@@ -277,8 +284,8 @@ def __init__(self, wire_server_address):
             match = re.match(r"^[^\d.]*([\d.]+).*$", output)
             if match is None:
                 raise Exception('output of "--version": {0}'.format(output))
-            version = FlexibleVersion(match.group(1))
-            use_wait_option = version >= FlexibleVersion('1.4.21')
+            self._version = FlexibleVersion(match.group(1))
+            use_wait_option = self._version >= FlexibleVersion('1.4.21')
 
         except Exception as exception:
             if isinstance(exception, OSError) and exception.errno == errno.ENOENT:  # pylint: disable=no-member
@@ -291,6 +298,10 @@ def __init__(self, wire_server_address):
         else:
             self._base_command = ["iptables", "-t", "security"]
 
+    @property
+    def version(self):
+        return self._version
+
     def _execute_delete_command(self, command):
         """
         Continually execute the delete operation until the return code is non-zero or the limit has been reached.
@@ -346,6 +357,10 @@ def __init__(self, wire_server_address):
                 raise FirewallManagerNotAvailableError("nft is not available")
             self._version = "unknown"
 
+    @property
+    def version(self):
+        return self._version
+
     def _get_state_command(self):
         return ["firewall-cmd", "--permanent", "--direct", "--get-all-passthroughs"]
 

azurelinuxagent/ga/persist_firewall_rules.py

@@ -130,6 +130,7 @@ def setup(self):
         # because on system reboot, all iptables rules are reset by firewalld.service, so it would be a no-op.
         # setup permanent firewalld rules
         firewall_manager = FirewallCmd(self._dst_ip)
+        event.info(WALAEventOperation.Firewall, "Using firewall-cmd [version {0}] to manage the persistent firewall rules", firewall_manager.version)
 
         try:
             firewall_manager.remove_legacy_rule()

tests/ga/test_env.py

@@ -167,9 +167,10 @@ def test_it_should_restore_missing_firewall_rules(self):
             enable_firewall = EnableFirewall('168.63.129.16')
 
             test_cases = [  # Exit codes for the "-C" (check) command
-                {"accept_dns": 1, "accept": 0, "drop": 0, "legacy": 0},
-                {"accept_dns": 0, "accept": 1, "drop": 0, "legacy": 0},
-                {"accept_dns": 0, "accept": 0, "drop": 1, "legacy": 0},
+                # {"accept_dns": 1, "accept": 0, "drop": 0, "legacy": 0},
+                # {"accept_dns": 0, "accept": 1, "drop": 0, "legacy": 0},
+                # {"accept_dns": 0, "accept": 1, "drop": 0, "legacy": 0},
+                {"accept_dns": 1, "accept": 1, "drop": 1, "legacy": 0},
             ]
 
             for test_case in test_cases:

tests_e2e/tests/agent_firewall/agent_firewall.py

@@ -33,9 +33,7 @@ def __init__(self, context: AgentVmTestContext):
         self._ssh_client = self._context.create_ssh_client()
 
     def run(self):
-        log.info("Checking firewall rules added by the agent")
         self._run_remote_test(self._ssh_client, f"agent_firewall-verify_all_firewall_rules.py --user {self._context.username}", use_sudo=True)
-        log.info("Successfully verified all rules present and working as expected.")
 
     def get_ignore_error_rules(self) -> List[Dict[str, Any]]:
         return [

tests_e2e/tests/lib/agent_log.py

@@ -369,18 +369,6 @@ def get_errors(self) -> List[AgentLogRecord]:
                 'if': lambda r: r.prefix == 'ExtHandler' and r.thread == 'ExtHandler'
             },
             #
-            # TODO: Currently Ubuntu minimal does not include the 'iptables' command. Remove this rule once this has been addressed.
-            #
-            # We don't have an easy way to distinguish Ubuntu minimal, so this rule suppresses for any Ubuntu. This is OK; if 'iptables' was missing from the regular Ubuntu images, the firewall tests would fail.
-            #
-            # 2024-03-27T16:12:35.666460Z ERROR ExtHandler ExtHandler Unable to setup the persistent firewall rules: Unable to determine version of iptables: [Errno 2] No such file or directory: 'iptables'
-            # 2024-03-27T16:12:35.667253Z WARNING ExtHandler ExtHandler Unable to determine version of iptables: [Errno 2] No such file or directory: 'iptables'
-            #
-            {
-                'message': r"Unable to determine version of iptables: \[Errno 2\] No such file or directory: 'iptables'",
-                'if': lambda r: DISTRO_NAME == 'ubuntu'
-            },
-            #
             # Some distros are running older agents, which do not add the DNS rule
             #
             # 2024-08-02T21:44:44.330727Z WARNING ExtHandler ExtHandler The firewall rules for Azure Fabric are not setup correctly (the environment thread will fix it): The following rules are missing: ['ACCEPT DNS']

tests_e2e/tests/lib/firewall_manager.py

@@ -264,17 +264,19 @@ def get_state(self) -> str:
     }
 
     def get_missing_rules(self) -> List[str]:
-        missing = []
+        if "table ip walinuxagent" not in shellutil.run_command(["sudo", "nft", "list", "tables"]):
+            return [FirewallManager.ACCEPT_DNS, FirewallManager.ACCEPT, FirewallManager.DROP]
 
         try:
+            missing = []
+
             wireserver_rule = self._get_wireserver_rule()
             for rule, regexp in NfTables._rule_regexp.items():
                 if re.search(regexp, wireserver_rule) is None:
                     missing.append(rule)
+            return missing
         except FirewallConfigurationError:
-            missing = [FirewallManager.ACCEPT_DNS, FirewallManager.ACCEPT, FirewallManager.DROP]
-
-        return missing
+            return [FirewallManager.ACCEPT_DNS, FirewallManager.ACCEPT, FirewallManager.DROP]
 
     def check_rule(self, rule_name: str) -> bool:
         try: