Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix interactive sessions always exiting with code 0 #8081

Merged
merged 3 commits into from
Sep 13, 2021

Conversation

rosstimothy
Copy link
Contributor

@rosstimothy rosstimothy commented Aug 27, 2021

Purpose

Correctly propagate error codes from an interactive tsh ssh session - fixes #3202

Implementation

  • Explicitly call Wait on the ssh.Session for interactive sessions
  • Set TeleportClient.ExitStatus if there was an ssh.ExitError or ssh.ExitMissingError returned by runShell

@rosstimothy rosstimothy force-pushed the tross/fix-ssh-exit-code branch 2 times, most recently from a5c7cb3 to 58f0f09 Compare August 30, 2021 18:07
integration/integration_test.go Show resolved Hide resolved
integration/integration_test.go Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
lib/client/api.go Outdated Show resolved Hide resolved
@andrejtokarcik
Copy link
Contributor

The issue linked to in the PR description seems to be incorrect BTW.

@rosstimothy rosstimothy force-pushed the tross/fix-ssh-exit-code branch 2 times, most recently from 26b7826 to a1c5b99 Compare August 31, 2021 15:14
@rosstimothy rosstimothy force-pushed the tross/fix-ssh-exit-code branch from a1c5b99 to 1fb30b4 Compare September 1, 2021 14:24
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
Comment on lines +1939 to +1960
case *ssh.ExitMissingError:
tc.ExitStatus = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this is semantically correct. Must ExitMissingError imply a failed/non-zero ExitStatus?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lib/client/api.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
integration/integration_test.go Outdated Show resolved Hide resolved
@rosstimothy rosstimothy force-pushed the tross/fix-ssh-exit-code branch 6 times, most recently from 2e1f82e to d4bbb01 Compare September 7, 2021 14:20
@rosstimothy rosstimothy force-pushed the tross/fix-ssh-exit-code branch from d4bbb01 to 00e9299 Compare September 13, 2021 15:36
@rosstimothy rosstimothy merged commit 7c327ee into master Sep 13, 2021
@rosstimothy rosstimothy deleted the tross/fix-ssh-exit-code branch September 13, 2021 17:42
zmb3 pushed a commit that referenced this pull request Sep 23, 2021
* propogate error codes from interactive ssh sessions correctly (#3202)
kimlisa added a commit that referenced this pull request Nov 2, 2021
Removes the call for wait for ssh.Session end to check for errors
in web terminal which fixes a regression bug where typing "exit" 
in web terminal does not return session end event. 
PR #8081 removed the need to check for errors as it correctly 
returns exit errors whereas before it returned nil.
kimlisa added a commit that referenced this pull request Nov 2, 2021
Removes the call for wait for ssh.Session end to check for errors
in web terminal which fixes a regression bug where typing "exit" 
in web terminal does not return session end event. 
PR #8081 removed the need to check for errors as it correctly 
returns exit errors whereas before it returned nil.
kimlisa added a commit that referenced this pull request Nov 2, 2021
Removes the call for wait for ssh.Session end to check for errors
in web terminal which fixes a regression bug where typing "exit" 
in web terminal does not return session end event. 
PR #8081 removed the need to check for errors as it correctly 
returns exit errors whereas before it returned nil.
kimlisa added a commit that referenced this pull request Nov 2, 2021
…8818)

Removes the call for wait for ssh.Session end to check for errors
in web terminal which fixes a regression bug where typing "exit" 
in web terminal does not return session end event. 
PR #8081 removed the need to check for errors as it correctly 
returns exit errors whereas before it returned nil.
kimlisa added a commit that referenced this pull request Nov 2, 2021
…8816)

Removes the call for wait for ssh.Session end to check for errors
in web terminal which fixes a regression bug where typing "exit" 
in web terminal does not return session end event. 
PR #8081 removed the need to check for errors as it correctly 
returns exit errors whereas before it returned nil.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Interactive sessions do not exit with correct exit code.
4 participants