sonic-net / sonic-mgmt

Configuration management examples for SONiC
Other
173 stars 691 forks source link

Enhance the test_ro_user test case to check for user permissions errors. #13411

Closed bmridul closed 4 weeks ago

bmridul commented 1 month ago

Description of PR

The PR fixes the test case test_ro_user.py to check if the command fails due to inadequate permissions. We see the following error .e.g. when the show command permission is changed to be for root user only..

root@m64-tor-0:/home/cisco# chmod 744 /usr/local/bin/show root@m64-tor-0:/home/cisco# ls -l /usr/local/bin/show -rwxr--r-- 1 root root 206 May 3 16:13 /usr/local/bin/show

Following is seen in the log, but the test case passes as it is looking for a different string.

21/05/2024 16:56:37 base._run L0104 DEBUG | /data/tests/tacacs/test_ro_user.py::ssh_remote_run#21: [localhost] AnsibleModule::shell Result => {"failed": true, "msg": "non-zero return code", "cmd": "sshpass -p 123456 ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null test_rouser@1.74.23.15 show version", "stdout": "", "stderr": "Warning: Permanently added '1.74.23.15' (RSA) to the list of known hosts.\r\nbash: line 1: /usr/local/bin/show: Permission denied", "rc": 126, "start": "2024-05-21 16:56:37.449193", "end": "2024-05-21 16:56:37.794768", "delta": "0:00:00.345575", "changed": true, "invocation": {"module_args": {"_raw_params": "sshpass -p 123456 ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null test_rouser@1.74.23.15 show version", "_uses_shell": true, "warn": true, "stdin_add_newline": true, "strip_empty_ends": true, "argv": null, "chdir": null, "executable": null, "creates": null, "removes": null, "stdin": null}}, "stdout_lines": [], "stderr_lines": ["Warning: Permanently added '1.74.23.15' (RSA) to the list of known hosts.", "bash: line 1: /usr/local/bin/show: Permission denied"], "_ansible_no_log": false} 21/05/2024 16:56:37 test_ro_user.ssh_remote_allow_run L0046 INFO | check command "show version" rc=126

Summary:

Changed the test case to look for a generic error string. Same test above fails and error is caught.

FAILED tacacs/test_ro_user.py::test_ro_user_allowed_command[m64-tor-0] - Failed: command 'show version' not authorized =============================================== 1 failed, 3 passed, 1 skipped, 1 warning in 206.51s (0:03:26) ================================================

Type of change

mssonicbld commented 1 month ago

The pre-commit check detected issues in the files touched by this pull request. The pre-commit check is a mandatory check, please fix detected issues.

Detailed pre-commit check results:

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check for added large files..............................................Passed
check python ast.........................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/tacacs/test_ro_user.py:48:13: E127 continuation line over-indented for visual indent

flake8...............................................(no files to check)Skipped
check conditional mark sort..........................(no files to check)Skipped

To run the pre-commit checks locally, you can follow below steps:

  1. Ensure that default python is python3. In sonic-mgmt docker container, default python is python2. You can run the check by activating the python3 virtual environment in sonic-mgmt docker container or outside of sonic-mgmt docker container.
  2. Ensure that the pre-commit package is installed:
    sudo pip install pre-commit
  3. Go to repository root folder
  4. Install the pre-commit hooks:
    pre-commit install
  5. Use pre-commit to check staged file:
    pre-commit
  6. Alternatively, you can check committed files using:
    pre-commit run --from-ref <commit_id> --to-ref <commit_id>
wsycqyz commented 4 weeks ago

LGTM

mssonicbld commented 4 weeks ago

Cherry-pick PR to 202405: https://github.com/sonic-net/sonic-mgmt/pull/13425

mssonicbld commented 4 weeks ago

Cherry-pick PR to 202311: https://github.com/sonic-net/sonic-mgmt/pull/13426

mssonicbld commented 4 weeks ago

Cherry-pick PR to 202305: https://github.com/sonic-net/sonic-mgmt/pull/13427