Add support of using lab tacacs server by default

ansible/config_sonic_basedon_testbed.yml

@@ -560,6 +560,18 @@
         become: true
         shell: config bgp startup all
 
+      - name: Configure TACACS
+        become: true
+        shell: "{{ tacacs_config_cmd }}"
+        loop:
+          - config tacacs passkey {{ tacacs_passkey }}
+          - config tacacs authtype pap
+          - config aaa authentication login tacacs+
+        loop_control:
+          loop_var: tacacs_config_cmd
+        ignore_errors: true
+        when: tacacs_enabled_by_default is defined and tacacs_enabled_by_default|bool == true
+
       - name: execute configlet application script, which applies configlets in strict order.
         become: true
         shell: bash "/etc/sonic/apply_clet.sh"

ansible/group_vars/lab/lab.yml

@@ -12,7 +12,7 @@ syslog_servers: ['10.0.0.5', '10.0.0.6']
 dns_servers: ['10.0.0.5', '10.0.0.6']
 
 # forced_mgmt_routes
-forced_mgmt_routes: ['172.17.0.1']
+forced_mgmt_routes: ['172.17.0.1/24']
 
 # ErspanDestinationIpv4
 erspan_dest: ['10.0.0.7']
@@ -21,13 +21,17 @@ radius_servers: []
 
 radius_passkey: testing123
 
-tacacs_servers: ['10.0.20.9', '10.0.20.8']
-
+# It can be a real lab tacacs server.
+tacacs_servers: ['172.17.0.6']
 tacacs_passkey: testing123
 
 # tacacs grous
 tacacs_group: 'testlab'
 
+# Determine whether enable tacacs authentication during deploy-minigraph. If false, use local authentication.
+# If yes, authenticate using servers configured in `tacacs_servers`
+tacacs_enabled_by_default: false
+
 # snmp servers
 snmp_servers: ['10.0.0.9']
 

tests/common/utilities.py

@@ -320,8 +320,10 @@ def get_host_visible_vars(inv_files, hostname):
     The variable could be defined in host_vars or in group_vars that the host belongs to.
 
     Args:
-        inv_files (list or string): List of inventory file pathes, or string of a single inventory file path. In tests,
+        inv_files (list or string): List of inventory file paths, or string of a single inventory file path. In tests,
             it can be get from request.config.getoption("ansible_inventory").
+            MUST use the inventory file under the ansible folder, otherwise host_vars and group_vars would not be
+            visible.
         hostname (string): Hostname
 
     Returns:

tests/tacacs/conftest.py

@@ -5,7 +5,7 @@
 from .utils import setup_tacacs_client, setup_tacacs_server, cleanup_tacacs, restore_tacacs_servers
 
 logger = logging.getLogger(__name__)
-TACACS_CREDS_FILE='tacacs_creds.yaml'
+TACACS_CREDS_FILE = 'tacacs_creds.yaml'
 
 
 @pytest.fixture(scope="module")
@@ -15,18 +15,20 @@ def tacacs_creds(creds_all_duts):
     creds_all_duts.update(hardcoded_creds)
     return creds_all_duts
 
+
 @pytest.fixture(scope="module")
 def check_tacacs(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds):
     logger.info('tacacs_creds: {}'.format(str(tacacs_creds)))
     duthost = duthosts[enum_rand_one_per_hwsku_hostname]
     tacacs_server_ip = ptfhost.mgmt_ip
-    default_tacacs_servers = setup_tacacs_client(duthost, tacacs_creds, tacacs_server_ip)
+    setup_tacacs_client(duthost, tacacs_creds, tacacs_server_ip)
     setup_tacacs_server(ptfhost, tacacs_creds, duthost)
 
     yield
 
     cleanup_tacacs(ptfhost, tacacs_creds, duthost)
-    restore_tacacs_servers(duthost, default_tacacs_servers, tacacs_server_ip)
+    restore_tacacs_servers(duthost)
+
 
 @pytest.fixture(scope="module")
 def check_tacacs_v6(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_creds):
@@ -41,3 +43,4 @@ def check_tacacs_v6(ptfhost, duthosts, enum_rand_one_per_hwsku_hostname, tacacs_
     yield
 
     cleanup_tacacs(ptfhost, tacacs_creds, duthost)
+    restore_tacacs_servers(duthost)

tests/tacacs/utils.py

@@ -5,20 +5,22 @@
 from tests.common.errors import RunAnsibleModuleFail
 from tests.common.utilities import wait_until, check_skip_release
 from tests.common.helpers.assertions import pytest_assert
-from tests.common.errors import RunAnsibleModuleFail
 
 logger = logging.getLogger(__name__)
 
+
 # per-command authorization and accounting feature not avaliable in following versions
 per_command_check_skip_versions = ["201811", "201911", "202012", "202106"]
 
+
 def check_output(output, exp_val1, exp_val2):
     pytest_assert(not output['failed'], output['stderr'])
-    for l in output['stdout_lines']:
-        fds = l.split(':')
+    for line in output['stdout_lines']:
+        fds = line.split(':')
         if fds[0] == exp_val1:
             pytest_assert(fds[4] == exp_val2)
 
+
 def check_all_services_status(ptfhost):
     res = ptfhost.command("service --status-all")
     logger.info(res["stdout_lines"])
@@ -28,18 +30,21 @@ def start_tacacs_server(ptfhost):
     ptfhost.command("service tacacs_plus restart", module_ignore_errors=True)
     return "tacacs+ running" in ptfhost.command("service tacacs_plus status", module_ignore_errors=True)["stdout_lines"]
 
+
 def stop_tacacs_server(ptfhost):
     ptfhost.service(name="tacacs_plus", state="stopped")
     check_all_services_status(ptfhost)
 
+
 def setup_local_user(duthost, tacacs_creds):
     try:
         duthost.shell("sudo deluser {}".format(tacacs_creds['local_user']))
     except RunAnsibleModuleFail:
         logger.info("local user not exist")
 
     duthost.shell("sudo useradd {}".format(tacacs_creds['local_user']))
-    duthost.shell('sudo echo "{}:{}" | chpasswd'.format(tacacs_creds['local_user'],tacacs_creds['local_user_passwd']))
+    duthost.shell('sudo echo "{}:{}" | chpasswd'.format(tacacs_creds['local_user'], tacacs_creds['local_user_passwd']))
+
 
 def setup_tacacs_client(duthost, tacacs_creds, tacacs_server_ip):
     """setup tacacs client"""
@@ -68,35 +73,84 @@ def setup_tacacs_client(duthost, tacacs_creds, tacacs_server_ip):
     setup_local_user(duthost, tacacs_creds)
     return default_tacacs_servers
 
-def restore_tacacs_servers(duthost, default_tacacs_servers, tacacs_server_ip):
-    duthost.shell("sudo config tacacs delete %s" % tacacs_server_ip)
-    for tacacs_server in default_tacacs_servers:
+
+def restore_tacacs_servers(duthost):
+    # Restore the TACACS plus server in config_db.json
+    config_facts = duthost.config_facts(host=duthost.hostname, source="persistent")["ansible_facts"]
+    for tacacs_server in config_facts.get("TACPLUS_SERVER", {}):
         duthost.shell("sudo config tacacs add %s" % tacacs_server)
 
-def fix_symbolic_link_in_config(duthost, ptfhost, symbolic_link_path, path_to_be_fix = None):
+    cmds = []
+    aaa_config = config_facts.get("AAA", {})
+    if aaa_config:
+        cfg = aaa_config.get("authentication", {}).get("login", "")
+        if cfg:
+            cmds.append("config aaa authentication login %s" % cfg)
+
+        cfg = aaa_config.get("authentication", {}).get("failthrough", "")
+        if cfg.lower() == "true":
+            cmds.append("config aaa authentication failthrough enable")
+        elif cfg.lower() == "false":
+            cmds.append("config aaa authentication failthrough disable")
+
+        cfg = aaa_config.get("authorization", {}).get("login", "")
+        if cfg:
+            cmds.append("config aaa authorization %s" % cfg)
+
+        cfg = aaa_config.get("accounting", {}).get("login", "")
+        if cfg:
+            cmds.append("config aaa accounting %s" % cfg)
+
+    tacplus_config = config_facts.get("TACPLUS", {})
+    if tacplus_config:
+        cfg = tacplus_config.get("global", {}).get("auth_type", "")
+        if cfg:
+            cmds.append("config tacacs authtype %s" % cfg)
+
+        cfg = tacplus_config.get("global", {}).get("passkey", "")
+        if cfg:
+            cmds.append("config tacacs passkey %s" % cfg)
+
+        cfg = tacplus_config.get("global", {}).get("timeout", "")
+        if cfg:
+            cmds.append("config tacacs timeout %s" % cfg)
+
+    # Cleanup AAA and TACPLUS config
+    duthost.copy(src="./tacacs/templates/del_tacacs_config.json", dest='/tmp/del_tacacs_config.json')
+    duthost.shell("configlet -d -j {}".format("/tmp/del_tacacs_config.json"))
+
+    # Restore AAA and TACPLUS config
+    duthost.shell_cmds(cmds=cmds)
+
+
+def fix_symbolic_link_in_config(duthost, ptfhost, symbolic_link_path, path_to_be_fix=None):
     """
         Fix symbolic link in tacacs config
-        Because tac_plus server not support regex in command name, and SONiC will send full path to tacacs server side for authorization, so the 'python' and 'ld' path in tac_plus config file need fix.
+        Because tac_plus server not support regex in command name, and SONiC will send full path to tacacs server side
+        for authorization, so the 'python' and 'ld' path in tac_plus config file need fix.
     """
     read_link_command = "readlink -f {0}".format(symbolic_link_path)
     target_path = duthost.shell(read_link_command)['stdout']
     # Escape path string, will use it as regex in sed command.
-    
+
     link_path_regex = re.escape(symbolic_link_path)
-    if path_to_be_fix != None:
+    if path_to_be_fix is not None:
         link_path_regex = re.escape(path_to_be_fix)
 
     target_path_regex = re.escape(target_path)
     ptfhost.shell("sed -i 's|{0}|{1}|g' /etc/tacacs+/tac_plus.conf".format(link_path_regex, target_path_regex))
 
+
 def get_ld_path(duthost):
     """
         Fix symbolic link in tacacs config
-        Because tac_plus server not support regex in command name, and SONiC will send full path to tacacs server side for authorization, so the 'python' and 'ld' path in tac_plus config file need fix.
+        Because tac_plus server not support regex in command name, and SONiC will send full path to tacacs server side
+        for authorization, so the 'python' and 'ld' path in tac_plus config file need fix.
     """
-    find_ld_command = "find /lib/ -type f,l -regex '\/lib\/.*-linux-.*/ld-linux-.*\.so\.[0-9]*'"
+    find_ld_command = "find /lib/ -type f,l -regex '\/lib\/.*-linux-.*/ld-linux-.*\.so\.[0-9]*'"   # noqa W605
     return duthost.shell(find_ld_command)['stdout']
 
+
 def fix_ld_path_in_config(duthost, ptfhost):
     """
         Fix ld path in tacacs config
@@ -105,6 +159,7 @@ def fix_ld_path_in_config(duthost, ptfhost):
     if not ld_symbolic_link_path:
         fix_symbolic_link_in_config(duthost, ptfhost, ld_symbolic_link_path, "/lib/arch-linux-abi/ld-linux-arch.so")
 
+
 def setup_tacacs_server(ptfhost, tacacs_creds, duthost):
     """setup tacacs server"""
 
@@ -115,7 +170,9 @@ def setup_tacacs_server(ptfhost, tacacs_creds, duthost):
                   'tacacs_ro_user': tacacs_creds['tacacs_ro_user'],
                   'tacacs_ro_user_passwd': crypt.crypt(tacacs_creds['tacacs_ro_user_passwd'], 'abc'),
                   'tacacs_authorization_user': tacacs_creds['tacacs_authorization_user'],
-                  'tacacs_authorization_user_passwd': crypt.crypt(tacacs_creds['tacacs_authorization_user_passwd'], 'abc'),
+                  'tacacs_authorization_user_passwd': crypt.crypt(
+                        tacacs_creds['tacacs_authorization_user_passwd'],
+                        'abc'),
                   'tacacs_jit_user': tacacs_creds['tacacs_jit_user'],
                   'tacacs_jit_user_passwd': crypt.crypt(tacacs_creds['tacacs_jit_user_passwd'], 'abc'),
                   'tacacs_jit_user_membership': tacacs_creds['tacacs_jit_user_membership']}
@@ -129,7 +186,11 @@ def setup_tacacs_server(ptfhost, tacacs_creds, duthost):
     # Find ld lib symbolic link target, and fix the tac_plus config file
     fix_ld_path_in_config(duthost, ptfhost)
 
-    ptfhost.lineinfile(path="/etc/default/tacacs+", line="DAEMON_OPTS=\"-d 10 -l /var/log/tac_plus.log -C /etc/tacacs+/tac_plus.conf\"", regexp='^DAEMON_OPTS=.*')
+    ptfhost.lineinfile(
+        path="/etc/default/tacacs+",
+        line="DAEMON_OPTS=\"-d 10 -l /var/log/tac_plus.log -C /etc/tacacs+/tac_plus.conf\"",
+        regexp='^DAEMON_OPTS=.*'
+    )
     check_all_services_status(ptfhost)
 
     # FIXME: This is a short term mitigation, we need to figure out why \nthe tacacs+ server does not start
@@ -144,26 +205,33 @@ def cleanup_tacacs(ptfhost, tacacs_creds, duthost):
 
     # reset tacacs client configuration
     remove_all_tacacs_server(duthost)
-    duthost.shell("sudo config tacacs default passkey")
-    duthost.shell("sudo config aaa authentication login default")
-    duthost.shell("sudo config aaa authentication failthrough default")
+    cmds = [
+        "config tacacs default passkey",
+        "config aaa authentication login default",
+        "config aaa authentication failthrough default"
+    ]
+    duthost.shell_cmds(cmds=cmds)
 
     (skip, _) = check_skip_release(duthost, per_command_check_skip_versions)
     if not skip:
         duthost.shell("sudo config aaa authorization local")
         duthost.shell("sudo config aaa accounting disable")
 
-    duthost.user(name=tacacs_creds['tacacs_ro_user'], state='absent', remove='yes', force='yes', module_ignore_errors=True)
-    duthost.user(name=tacacs_creds['tacacs_rw_user'], state='absent', remove='yes', force='yes', module_ignore_errors=True)
-    duthost.user(name=tacacs_creds['tacacs_jit_user'], state='absent', remove='yes', force='yes', module_ignore_errors=True)
+    duthost.user(
+        name=tacacs_creds['tacacs_ro_user'], state='absent', remove='yes', force='yes', module_ignore_errors=True
+    )
+    duthost.user(
+        name=tacacs_creds['tacacs_rw_user'], state='absent', remove='yes', force='yes', module_ignore_errors=True
+    )
+    duthost.user(
+        name=tacacs_creds['tacacs_jit_user'], state='absent', remove='yes', force='yes', module_ignore_errors=True
+    )
 
-    duthost.copy(src="./tacacs/templates/del_tacacs_keys.json", dest='/tmp/del_tacacs_keys.json')
-    duthost.shell("configlet -d -j {}".format("/tmp/del_tacacs_keys.json"))
 
 def remove_all_tacacs_server(duthost):
     # use grep command to extract tacacs server address from tacacs config
-    find_server_command = 'show tacacs | grep -Po "TACPLUS_SERVER address \K.*"'
-    server_list = duthost.shell(find_server_command, module_ignore_errors=True)['stdout']
+    find_server_command = 'show tacacs | grep -Po "TACPLUS_SERVER address \K.*"'    # noqa W605
+    server_list = duthost.shell(find_server_command, module_ignore_errors=True)['stdout_lines']
     for tacacs_server in server_list:
         tacacs_server = tacacs_server.rstrip()
         if tacacs_server: