-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
RFD 144: Client tools auto updates #30317
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall makes sense and I like the reduction in scope, just left a bunch of suggestions/clarifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial questions, thank you!
- More details on binary execution - More security details
|
||
Ideally, we'd like this feature to be integrated seamlessly. Users of the Teleport client tools should not need to search for documentation and spend time figuring out how to enable auto updates for their cleint tools. | ||
|
||
After client tools auto updates is deployed and the first time the client tools are executed, the user will be asked two questions: 1) if they would like to update to the latest version (default yes) and 2) if they would like to enroll in auto updates for client tools (default yes). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to turn this on by default when Cloud clusters are targeted, and require an opt-out command to disable it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on either way. I think most users would opt-in to auto updates so I think it'd be fine to turn this on by default.
It could also be nice to prompt the user when auto updates are first available to let the user know that auto updates is configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should ask whether they want to enroll or not. Let's just enable it by default.
Sorry for the back and forth. After some more discussion we're leaning more towards just using the system package manager to update the client tools. This might end up being tedious to add support for all our installation methods, but the solution is a lot simpler and straight forward. |
|
||
Another downside to this approach would be that upgrades will require sudo/admin privileges. | ||
|
||
### Caching (alternative) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I would move this section way down below, maybe under Appendix/Alternatives or something so the reader does not lose context of how things will actually work with the selected approach.
|
||
### Config | ||
|
||
Automatic updates for client tools can be configured through tsh configuration. If `DISABLE_AUTO_UPDATE=true`, then auto updates for client tools will be disabled. If `DISABLE_UPDATE_PROMPT=true`, then the user will not be prompted to confirm the update. Both values will default to false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are DISABLE_AUTO_UPDATE
and DISABLE_UPDATE_PROMPT
environment variables? It's not super clear what "tsh configuration" means in this case. If these are env. vars, let's say so explicitly.
I think it would also make sense to add these options to tsh.yaml config and give an example of what tsh config will look like.
|
||
Ideally, we'd like this feature to be integrated seamlessly. Users of the Teleport client tools should not need to search for documentation and spend time figuring out how to enable auto updates for their cleint tools. | ||
|
||
After client tools auto updates is deployed and the first time the client tools are executed, the user will be asked two questions: 1) if they would like to update to the latest version (default yes) and 2) if they would like to enroll in auto updates for client tools (default yes). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should ask whether they want to enroll or not. Let's just enable it by default.
|
||
We should provide some observability into the download status. Whenever a new cloud-stable version is available for download, the progress of the update will be output to stdout. e.g. `New cloud-stable version of Teleport detected`, `Downloading latest version of tsh/tctl... `, `Updated tsh/tctl to version v13.2.3!`. | ||
|
||
To avoid breaking existing scripts, a `--[no-]auto-update` flag will be included. Without the flag given, when a tty is detected and an update is available, the user will be prompted to update. If the flag is not provided and a tty is not detected, updates will be skipped to avoid breaking scripts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need the flag at all? Why not just skip updates when there's no tty?
|
||
### Package update | ||
|
||
The client tools will be updated using the package manager available on the system. Teleport supports multiple package repositories and installation methods so we'll need to handle the update differently on each system. This will probably require a decent amount of testing and maintenance work, but we think this is the simplest and most straight forward solution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The client tools will be updated using the package manager available on the system.
This needs a little bit more clarification. Say, I'm on Ubuntu but installed teleport binary directly. Will auto-update still try to attempt to use apt
in this case? There may not be apt
repository configured.
I think we need to flesh out how detection/installation logic will work a bit more, for each supported package type (direct download, Debian/Ubuntu deb, RHEL/CentOS rpm, macOS pkg, etc.) and include this in the RFD.
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
Let's move the discussion to #39805. |
Contributes to https://github.com/gravitational/cloud/issues/4880
Supersedes #28337
For the initial implementation we've decided to limit scope to Cloud users only. We also will not be supporting multiple versions as all Cloud tenants are expected to be running the same Teleport version.