Fix the issue in test_ssh_limit.py due to password rotation#12255
Fix the issue in test_ssh_limit.py due to password rotation#12255wangxin merged 11 commits intosonic-net:masterfrom
test_ssh_limit.py due to password rotation#12255Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
tests/tacacs/test_authorization.py
Outdated
| remote_ip, username=remote_username, password=remote_password, allow_agent=False, | ||
| look_for_keys=False, auth_timeout=TIMEOUT_LIMIT) | ||
| return ssh | ||
| def ssh_connect_remote(duthost, remote_ip, remote_username, remote_passwords): |
There was a problem hiding this comment.
This function is shared by tests in different folders. Why not move it to the common folder?
There was a problem hiding this comment.
Another thing is that the duthost_ssh function can be more generic. The duthost argument is not mandatory for it. The function name can be more generic. Arguments do not need to have "sonic_" prefix.
tests/common/utilities.py
Outdated
| ssh = paramiko.SSHClient() | ||
| ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy()) | ||
| ssh.connect(ipaddr, username=username, password=password, | ||
| allow_agent=False, look_for_keys=False, timeout=10) |
There was a problem hiding this comment.
There is code repetition here. It would be easier to workaround it with code like below:
if isinstance(passwords, str):
candidate_passwords = [passwords]
elif isinstance(passwords, list):
candidate_passwords = passwords
else:
raise Exception("The passwords argument must be either a string or a list of string.")
# Then loop through the candidate_passwords to try connection.There was a problem hiding this comment.
Good idea!
tests/common/utilities.py
Outdated
|
|
||
| def duthost_ssh(duthost, sonic_username, sonic_passwords, sonic_ip): | ||
| for password in sonic_passwords: | ||
| def paramiko_ssh(username, passwords, ipaddr): |
There was a problem hiding this comment.
This would be a common function to be used by lots of other tests. Add docstring as a document is important. Can you add docstring for it?
There was a problem hiding this comment.
Name ipaddr may be conflicting with a library also named ipaddr.
The convention is to have ip address as the first argument. Then username and password.
4 participants
Description of PR
Fix the issue in
test_ssh_limit.pydue to password rotationIn this PR, we provide a public function connecting to device through paramiko. It can try either several potential passwords or a single password. We will use this new function to connect device.
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
Fix the issue in
test_ssh_limit.pydue to password rotationIn this PR, we provide a public function connecting to device through paramiko. It can try either several potential passwords or a single password. We will use this new function to connect device.
How did you do it?
How did you verify/test it?
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation