-
Notifications
You must be signed in to change notification settings - Fork 0
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
impl: ssh config improvements #41
Merged
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
- useful to make differentiate between Gateway plugin and Toolbox plugin
matifali
reviewed
Mar 24, 2025
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 have a concern about --usage-app=toolbox
, as we will not ba able to track it before https://github.com/coder/coder/issues/16957.
@f0ssel, @bpmct curious for your thoughts here.
src/test/resources/fixtures/inputs/existing-end-no-newline.conf
Outdated
Show resolved
Hide resolved
src/test/resources/fixtures/inputs/existing-end-no-newline.conf
Outdated
Show resolved
Hide resolved
- instead of the raw interfaces - this prepares the plugin for the next stage where some of the settings are read by the URI handling component.
… enabled - uri handling always tried to start the workspace if it was stopped. - this commit honors the `disable autostart` setting which should stop the starting of the workspace automatically when connecting via SSH. - instead user is warned via a pop-up dialog that manual starting is needed
- this patch makes the window visible in uri handling flows when an error dialog pops-up.
- catches the exception, logs the errors and pops-up an error dialog during URI handling - previously the call failed silently (for example when the workspace is stopped and out of date)
- to pop-up Toolbox window when it's not showing.
- too much confusion around CoderSettings, CoderSettingsState, CoderSettingsService - lots of properties and some models were introduced only to inject data during tests - test related properties were leaking in the business code and there was no clear definition between the readable and writable interface - with this commit we have CoderSettingsStore exposing read+write, CoderSettings exposing reads + PluginSettingsStore the underlying persistable store - the tests control data via an instance of PluginSettingsStore just like the business code - two settings are now also exposed and configurable in the UI (previously only from tests): - ssh log dir - ssh extra arguments to the proxy command - resolves #40 - some of the options that did not make sense to be configurable and were only used in the tests were removed: - setupCommand - ignoreSetupFailures - these two are Gateway specific
- resolves #46 - it is configurable in the Coder's plugin setting page
- from Coder Toolbox to Coder
- back to jetbrains because there is no tracking support for toolbox, yet.
- use Java's Path to do platform independent path comparison
matifali
approved these changes
Mar 27, 2025
- slf4j api is now bundled and exposed by Toolbox
matifali
reviewed
Mar 27, 2025
matifali
reviewed
Mar 27, 2025
- Toolbox calls the Coder environment to get SSH connection information. In this particular case for wildcard SSH connections the hostname is a bit different. - this patch computes the correct hostname depending on whether wildcard ssh config is enabled or not.
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.
different header blocks to differentiate between Gateway and Toolbox ssh config
different key for Toolbox for workspace usage
honors the
disable autostart
setting in the URI handlingplugin's display name was changed from Coder Toolbox to Coder
adds support for ssh wildcard config, configurable from plugin's Settings page.
the settings models were reworked because there was too much confusion around CoderSettings, CoderSettingsState, CoderSettingsService. Lots of properties and some models were introduced only to inject data during tests.
among other issues, test related properties were leaking in the business code and there was no clear definition
between the readable and writable interface. With this PR we have CoderSettingsStore exposing read+write, CoderSettings exposing reads + PluginSettingsStore the underlying persistable store.
the tests control data via an instance of PluginSettingsStore just like the business code
some of the options that did not make sense to be configurable and were only used in the tests were removed:
two settings are now also exposed and configurable in the UI (previously only from tests):
resolves Coder settings are not taken into account #40
resolves convert
.ssh/config
to use wildcard entries #46