Skip to content

Fix printing empty usage and terminate CLI for parsing global flags#57401

Merged
vapopov merged 3 commits intomasterfrom
vapopov/fix-cli-usage-print
Aug 1, 2025
Merged

Fix printing empty usage and terminate CLI for parsing global flags#57401
vapopov merged 3 commits intomasterfrom
vapopov/fix-cli-usage-print

Conversation

@vapopov
Copy link
Copy Markdown
Contributor

@vapopov vapopov commented Jul 31, 2025

In this PR added hidden initiation of kingpin.Application which prevents termination and print usage information

Related: #54563
Related: https://github.com/gravitational/cloud/issues/13207
Thread: https://gravitational.slack.com/archives/C01TYKHFVTQ/p1753828345916079

changelog: Fixed usage print for global --help flag

@vapopov vapopov requested review from Tener and hugoShaka July 31, 2025 21:24
@github-actions github-actions bot added size/sm tctl tctl - Teleport admin tool tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Jul 31, 2025
@vapopov vapopov enabled auto-merge August 1, 2025 02:09
Copy link
Copy Markdown
Contributor

@Tener Tener left a comment

Choose a reason for hiding this comment

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

The fix works, thank you!

I'd really like to see some test coverage here; kingpin can behave in some rather surprising ways and it should help prevent a regression in that area. Seeing as individual commands are separate we'd likely need multiple copies of such a test.

Comment thread lib/utils/cli.go
…ge print is not empty and both identical.

Add godoc clarification
Comment thread tool/tsh/common/tsh_test.go
@vapopov vapopov added this pull request to the merge queue Aug 1, 2025
Merged via the queue into master with commit 71e84c6 Aug 1, 2025
40 checks passed
@vapopov vapopov deleted the vapopov/fix-cli-usage-print branch August 1, 2025 16:55
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@vapopov See the table below for backport results.

Branch Result
branch/v17 Create PR
branch/v18 Create PR

vapopov added a commit that referenced this pull request Aug 1, 2025
…57401)

* Fix printing empty usage and terminate CLI for parsing global flags

* Add test with check of both `--help` flag and `help` command that usage print is not empty and both identical.
Add godoc clarification

* Disable managed update check for version help command test
vapopov added a commit that referenced this pull request Aug 1, 2025
…57401)

* Fix printing empty usage and terminate CLI for parsing global flags

* Add test with check of both `--help` flag and `help` command that usage print is not empty and both identical.
Add godoc clarification

* Disable managed update check for version help command test
github-merge-queue bot pushed a commit that referenced this pull request Aug 7, 2025
* Client-tools managed updates version caching (#54563)

* Add profile integration to disable update and re-execution for specific cluster

* Complete integration for the tctl and tsh

* Add commands for tsh

* Fix linter warnings

* Add config file with version and disabling status

* Move check out from helper

* Fixed re-execution ignore if versions is identical

* Move logic out from client

* Remove helper package and profile integration

* Fix argument parsing by filtering

* Use same Darwin platform approach of package extraction for Linux
Add client tools cleanup for V1 directories

* Fix packaging unit test

* Add cleanup for last recently used tools

* Add migration from v1 for better support
Show error log message about failed update/re-execution instead of failing command execution in case if updated binary was broken, modified or not able to validate signature

* Add ignore the version check fail, add more debug information

* Check update for commands `tsh ssh`, `tsh proxy ssh`
Fixed creating `.tsh` subdirectory when TELEPORT_HOME is set
Fix `tsh --proxy` flag parsing

* Wraps client init function to check client tools managed update only when it requested for `tsh ssh` and `tsh proxy ssh`

* Move filesystem lock to configuration library
Configuration modification protected by lock, other process must wait until it is released

* Rename command to `tsh update`, `tsh update --clear`

* Add test for argument filtering

* Update RFD
Make max tools installed to be configurable and set to 3 by default

* Replace "automatic updates" to "managed updates"

* Updated comments to reflect the latest changes

* Fix migration for older versions with two packages

* CR changes

* Prevent failing tools execution if configuration file is corrupted

* Remove lock file as part of cleanup command

* Added context to arguments

* Use a separate Kingpin application for tctl, as is already done for tsh. Double parsing may cause issues since it is not stateless.

* CTMU no longer uses a static path, any re-execution from the tools directory must disable further re-execution

* Gracefully fail when cannot do client tools updates (#57142)

* Gracefully fail when cannot do client tool update

* Gracefully fail when cannot check the version

* Fix printing empty usage and terminate CLI for parsing global flags (#57401)

* Fix printing empty usage and terminate CLI for parsing global flags

* Add test with check of both `--help` flag and `help` command that usage print is not empty and both identical.
Add godoc clarification

* Disable managed update check for version help command test

---------

Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
github-merge-queue bot pushed a commit that referenced this pull request Aug 7, 2025
* Client-tools managed updates version caching (#54563)

* Add profile integration to disable update and re-execution for specific cluster

* Complete integration for the tctl and tsh

* Add commands for tsh

* Fix linter warnings

* Add config file with version and disabling status

* Move check out from helper

* Fixed re-execution ignore if versions is identical

* Move logic out from client

* Remove helper package and profile integration

* Fix argument parsing by filtering

* Use same Darwin platform approach of package extraction for Linux
Add client tools cleanup for V1 directories

* Fix packaging unit test

* Add cleanup for last recently used tools

* Add migration from v1 for better support
Show error log message about failed update/re-execution instead of failing command execution in case if updated binary was broken, modified or not able to validate signature

* Add ignore the version check fail, add more debug information

* Check update for commands `tsh ssh`, `tsh proxy ssh`
Fixed creating `.tsh` subdirectory when TELEPORT_HOME is set
Fix `tsh --proxy` flag parsing

* Wraps client init function to check client tools managed update only when it requested for `tsh ssh` and `tsh proxy ssh`

* Move filesystem lock to configuration library
Configuration modification protected by lock, other process must wait until it is released

* Rename command to `tsh update`, `tsh update --clear`

* Add test for argument filtering

* Update RFD
Make max tools installed to be configurable and set to 3 by default

* Replace "automatic updates" to "managed updates"

* Updated comments to reflect the latest changes

* Fix migration for older versions with two packages

* CR changes

* Prevent failing tools execution if configuration file is corrupted

* Remove lock file as part of cleanup command

* Added context to arguments

* Use a separate Kingpin application for tctl, as is already done for tsh. Double parsing may cause issues since it is not stateless.

* CTMU no longer uses a static path, any re-execution from the tools directory must disable further re-execution

* Gracefully fail when cannot do client tools updates (#57142)

* Gracefully fail when cannot do client tool update

* Gracefully fail when cannot check the version

* Fix printing empty usage and terminate CLI for parsing global flags (#57401)

* Fix printing empty usage and terminate CLI for parsing global flags

* Add test with check of both `--help` flag and `help` command that usage print is not empty and both identical.
Add godoc clarification

* Disable managed update check for version help command test

---------

Co-authored-by: Hugo Shaka <hugo.hervieux@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 backport/branch/v18 size/sm tctl tctl - Teleport admin tool 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.

3 participants