Fixed ssh_password_login_disallowed test for Juniper#5253
Fixed ssh_password_login_disallowed test for Juniper#5253ram-mac merged 2 commits intoopenconfig:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a test failure observed on Juniper devices related to SSH credential handling. The changes involve adjusting the format of public keys sent to Juniper devices to include the full OpenSSH key string, as well as increasing SSH connection timeouts to accommodate potential delays in the device's SSH daemon responsiveness. These modifications aim to stabilize the ssh_password_login_disallowed test for Juniper. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
Pull Request Functional Test Report for #5253 / f3a9318Virtual Devices
Hardware Devices
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a failing test on Juniper devices by switching to RSA keys, providing the full OpenSSH public key string, and increasing timeouts. The changes are logical and directly address the issue described. My review includes a few critical suggestions to prevent potential panics from malformed key files and to handle unknown key types more robustly, justifying the use of t.Fatalf for these critical test failures. I've also pointed out some code duplication that could be refactored for better maintainability and a minor issue with an oversimplified key type mapping, aligning with the rule for keeping constants general for future reusability.
3 participants
Juniper device's SSH daemon was probably not responding when the test applies the CA configurations hence failing the test. Following fixes were made to the test to pass this test.
-In RotateTrustedUserCA and RotateAuthorizedKey, the test extracts the public key from the generated .pub files and sends only the Base64 section (AAAA...), stripping the prefix (ssh-ed25519 or ssh-rsa). So with this change we now pass the Full OpenSSH public key string (e.g., ssh-rsa AAAA...) to the device instead of just the Base64 portion.
-Increased maxSSHRetryTime to 120 seconds, and updated the context timeouts in ssh_password_login_disallowed_test.go from 60 to 120 seconds. This is just to make sure that there is enough time given to Juniper in case there was some issue with the SSH daemon.