Skip to content

[ssh] Bug Fix: Change how ssh vm reads the ssh client logs #5366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zhoxing-ms merged 10 commits into from
Oct 21, 2022
Merged

[ssh] Bug Fix: Change how ssh vm reads the ssh client logs #5366

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
46bcf95
Read stderr during ssh execution. Print only error messages as they a…
vthiebaut10 Aug 16, 2022
9f8813c
filter out known messages that are printed but not needed
vthiebaut10 Aug 16, 2022
8c12176
fix tests and style
vthiebaut10 Aug 16, 2022
f991d56
banner changes
vthiebaut10 Aug 26, 2022
7f10b83
handle stderr differently depending on os and resource type
vthiebaut10 Sep 8, 2022
d8351f3
Mitigate weird issues on linux
vthiebaut10 Sep 14, 2022
dea6b87
No longer redirect stderr if user expecting ssh logs. When stderr not…
vthiebaut10 Sep 19, 2022
73decda
remove warning
vthiebaut10 Sep 19, 2022
0634828
Merge branch 'main' into 08_16_22_banner_bug
vthiebaut10 Oct 18, 2022
a93c6f8
Resolve conflicts, fix unit tests
vthiebaut10 Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/ssh/HISTORY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
Release History
===============
1.1.3
-----
* Fix bug where extension prints ssh banners after the connection is closed.
Copy link
Contributor

@zhoxing-ms zhoxing-ms Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to release a new extension version, please also upgrade the version defined in setup.py

Copy link
Contributor Author

@vthiebaut10 vthiebaut10 Oct 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zhoxing-ms I was planning on upgrading the version only after all the open PRs for the next release are merged and I have the chance to run some final tests. For now I'll just remove the changes in the HISTORY.md file for this PR.

Copy link
Contributor

@zhoxing-ms zhoxing-ms Oct 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vthiebaut10 OK. Or I can merge this PR first, and you can add new content to the history notes of subsequent PRs.

Copy link
Contributor

@zhoxing-ms zhoxing-ms Oct 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future, if you do not want to release a new version immediately, I suggest you add comments in PR to tell us


1.1.2
-----
* Remove dependency to cryptography (Az CLI core alredy has cryptography)
Expand Down
2 changes: 1 addition & 1 deletion src/ssh/azext_ssh/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def _do_ssh_op(cmd, op_info, op_call):
op_info.private_key_file + ', ' if delete_keys else "",
op_info.public_key_file + ', ' if delete_keys else "",
op_info.cert_file if delete_cert else "")
ssh_utils.do_cleanup(delete_keys, delete_cert, op_info.cert_file,
ssh_utils.do_cleanup(delete_keys, delete_cert, op_info.delete_credentials, op_info.cert_file,
op_info.private_key_file, op_info.public_key_file)
raise e

Expand Down
8 changes: 4 additions & 4 deletions src/ssh/azext_ssh/rdp_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ def start_rdp_connection(ssh_info, delete_keys, delete_cert):
ssh_process, print_ssh_logs = start_ssh_tunnel(ssh_info)
ssh_connection_t0 = time.time()
ssh_success, log_list = wait_for_ssh_connection(ssh_process, print_ssh_logs)
ssh_utils.do_cleanup(delete_keys, delete_cert, ssh_info.cert_file, ssh_info.private_key_file,
ssh_info.public_key_file)
ssh_utils.do_cleanup(delete_keys, delete_cert, ssh_info.delete_credentials, ssh_info.cert_file,
ssh_info.private_key_file, ssh_info.public_key_file)
if ssh_success and ssh_process.poll() is None:
call_rdp(local_port)

Expand All @@ -56,8 +56,8 @@ def start_rdp_connection(ssh_info, delete_keys, delete_cert):
telemetry.add_extension_event('ssh', ssh_connection_data)

terminate_ssh(ssh_process, log_list, print_ssh_logs)
ssh_utils.do_cleanup(delete_keys, delete_cert, ssh_info.cert_file, ssh_info.private_key_file,
ssh_info.public_key_file)
ssh_utils.do_cleanup(delete_keys, delete_cert, ssh_info.delete_credentials, ssh_info.cert_file,
ssh_info.private_key_file, ssh_info.public_key_file)
if delete_keys:
# This is only true if keys were generated, so they must be in a temp folder.
temp_dir = os.path.dirname(ssh_info.cert_file)
Expand Down
Loading