[v9] Improve error handling in tbot start (#11756)
#12012
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Improve error handling in `tbot start` This attempts to improve a number of error handling issues while loading the bot identity from storage in `tbot start`: 1. Identity loading errors are silently ignored and the bot always attempts to generate a new identity from token. This isn't always correct and is impossible to debug as the true error is never logged. We now debug log these errors. 2. `LoadIdentity()` doesn't properly account for existing-but-empty identity files and happily tries to load empty identities from `tbot init`. This isn't hugely harmful, but produces nonsensical error logs once #1 is fixed. * Use `O_RDWR` instead of `O_WRONLY` in `botfs.openStandard()` This behaves the same as the fs_linux secure implementation in all cases, and moves the open mode to a shared constant for good measure. * Add a small unit test for symlinks mode read/write. * Fail on non-NotFound errors while reading an Identity. * Add small unit test for empty identities. * Remove outdated TODO comment * Apply suggestions from code review Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com> * Address review feedback Co-authored-by: Zac Bergquist <zmb3@users.noreply.github.com>
zmb3
approved these changes
Apr 15, 2022
probakowski
approved these changes
Apr 15, 2022
r0mant
approved these changes
Apr 18, 2022
timothyb89
deleted the
timothyb89/v9/machineid-improve-identity-loading-errors
branch
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #11756 for branch/v9.
tbot startThis attempts to improve a number of error handling issues while
loading the bot identity from storage in
tbot start:always attempts to generate a new identity from token. This isn't
always correct and is impossible to debug as the true error is
never logged. We now debug log these errors.
LoadIdentity()doesn't properly account for existing-but-emptyidentity files and happily tries to load empty identities from
tbot init. This isn't hugely harmful, but produces nonsensicalerror logs once Implement a prototype for a proxying SSH server that implements concepts expressed in readme #1 is fixed.
O_RDWRinstead ofO_WRONLYinbotfs.openStandard()This behaves the same as the fs_linux secure implementation in
all cases, and moves the open mode to a shared constant for good
measure.
Add a small unit test for symlinks mode read/write.
Fail on non-NotFound errors while reading an Identity.
Add small unit test for empty identities.
Remove outdated TODO comment
Apply suggestions from code review
Co-authored-by: Zac Bergquist zmb3@users.noreply.github.com
Co-authored-by: Zac Bergquist zmb3@users.noreply.github.com