Skip to content
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.

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

[v9] Backport: tsh: ignore empty or non-existing config files (#11495) #11571

Merged
merged 1 commit into from
Mar 30, 2022

Conversation

marcoandredinis
Copy link
Contributor

backport of #11495

If the newly created config.yaml didnt exist we would load the default
values and continue the flow
However, we were not resetting the `err`'s value and would have an ERROR
message at the end and an invalid exit code

Most of the commands would reset that variable to the output of the
command's execution
One of them was not: `tsh version`
The version command has no return value, so the program would execute
as expected until the last statement: `trace.Wrap(error)` which would
re-use the `err` variable whose value is the result of the `loadConfig`
method.

We could either reset the `err`s value inside the `PrintVersion` switch
case block or reset it right after we check for `IsNotFound`.

We ended up picking the first option as it seems cleaner

```
 # before
$ make full > /dev/null; build/tsh version
Teleport v10.0.0-dev git:v8.0.0-alpha.1-899-g335adf1f4 go1.18
ERROR: open /home/marco/.tsh/config/config.yaml: no such file or directory

 # after
$ make full > /dev/null; build/tsh version
Teleport v10.0.0-dev git:v8.0.0-alpha.1-899-g335adf1f4 go1.18
```
@github-actions github-actions bot added the tsh tsh - Teleport's command line tool for logging into nodes running Teleport. label Mar 30, 2022
@github-actions github-actions bot requested review from lxea, r0mant and russjones March 30, 2022 16:03
@r0mant r0mant enabled auto-merge (squash) March 30, 2022 17:25

func TestLoadConfigEmptyFile(t *testing.T) {
file, err := os.CreateTemp("", "test-telelport")
file.Write([]byte(" "))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here you are writing to the file before you've checked that it was successfully created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for picking this up
I've opened a follow up PR here
#11603

@r0mant r0mant merged commit 7253530 into branch/v9 Mar 30, 2022
@r0mant r0mant deleted the marco/v9/tsh_fix_configfile_notfound branch March 30, 2022 18:22
@webvictim webvictim mentioned this pull request Apr 19, 2022
@webvictim webvictim mentioned this pull request Jun 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tsh tsh - Teleport's command line tool for logging into nodes running Teleport.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants