diff --git a/e2e/15_set_ssh_config_path.tape b/e2e/15_set_ssh_config_path.tape new file mode 100644 index 0000000..74f2f30 --- /dev/null +++ b/e2e/15_set_ssh_config_path.tape @@ -0,0 +1,36 @@ +# Test that we can set ssh_config path and it will survive +# application run in normal mode and overriding state file. + +Set Shell bash + +# Create ssh_config +Type "touch /tmp/delete_me_goto_ssh_config_test_file" +Enter +Sleep 100ms +Type "gg -f temp --set-ssh-config-path /tmp/delete_me_goto_ssh_config_test_file" +Enter +Sleep 100ms + +# Run the app +Type "gg -f temp -l debug" +Enter +Sleep 100ms + +# Create a random host +Type "n" +Sleep 100ms +Type "network host" +Down +Ctrl+u +Type "127.0.0.1" +Down +Type "network host description" +# Place the host to "manual hosts" group +Down +Type "manual hosts" +Ctrl+S +# Should always give some time when save a host +Sleep 100ms + +Type "q" +Sleep 100ms diff --git a/e2e/expected/15_set_ssh_config_path_hosts.yaml b/e2e/expected/15_set_ssh_config_path_hosts.yaml new file mode 100644 index 0000000..b8f9eb5 --- /dev/null +++ b/e2e/expected/15_set_ssh_config_path_hosts.yaml @@ -0,0 +1,5 @@ +- host: + address: 127.0.0.1 + description: network host description + group: manual hosts + title: network host diff --git a/e2e/expected/15_set_ssh_config_path_state.yaml b/e2e/expected/15_set_ssh_config_path_state.yaml new file mode 100644 index 0000000..9560e82 --- /dev/null +++ b/e2e/expected/15_set_ssh_config_path_state.yaml @@ -0,0 +1,5 @@ +screen_layout: description +ssh_config_path: /tmp/delete_me_goto_ssh_config_test_file +selected: 1 +enable_ssh_config: true +theme: default diff --git a/internal/config/config.go b/internal/config/config.go index 5fa913e..3eec71c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -23,14 +23,18 @@ const ( // Configuration structs contains user-definable parameters. type Configuration struct { - AppMode constant.AppMode - AppName string - DisableFeature FeatureFlag - EnableFeature FeatureFlag - SetTheme string - AppHome string `env:"GG_HOME"` - LogLevel constant.LogLevel `env:"GG_LOG_LEVEL" envDefault:"info"` - SSHConfigFilePath string `env:"GG_SSH_CONFIG_FILE_PATH"` + AppMode constant.AppMode + AppName string + DisableFeature FeatureFlag + EnableFeature FeatureFlag + SetTheme string + AppHome string `env:"GG_HOME"` + LogLevel constant.LogLevel `env:"GG_LOG_LEVEL" envDefault:"info"` + SSHConfigPath string `env:"GG_SSH_CONFIG_FILE_PATH"` + // SetSSHConfigPath is not the same as SSHConfigPath, as when this is set, we must + // write the value to state file and exit. When SSHConfigPath is set, we just use it + // as the path to ssh config within the current application run. + SetSSHConfigPath string } func Initialize() (*Configuration, error) { @@ -81,9 +85,9 @@ func parseCommandLineFlags(envConfig *Configuration, args []string, exitOnError fs.StringVar(&cmdConfig.AppHome, "f", envConfig.AppHome, "Application home folder") fs.StringVar(&cmdConfig.LogLevel, "l", envConfig.LogLevel, "Log verbosity level: debug, info") fs.StringVar( - &cmdConfig.SSHConfigFilePath, + &cmdConfig.SSHConfigPath, "s", - envConfig.SSHConfigFilePath, + envConfig.SSHConfigPath, "Specifies an alternative per-user SSH configuration file path", ) fs.Var( @@ -97,6 +101,7 @@ func parseCommandLineFlags(envConfig *Configuration, args []string, exitOnError fmt.Sprintf("Disable feature. Supported values: %s", strings.Join(SupportedFeatures, "|")), ) fs.StringVar(&cmdConfig.SetTheme, "set-theme", "", "Set application theme") + fs.StringVar(&cmdConfig.SetSSHConfigPath, "set-ssh-config-path", "", "Set SSH configuration file path or URL.") err := fs.Parse(args[1:]) // args should not include program name, see docs if err != nil { @@ -115,6 +120,9 @@ func parseCommandLineFlags(envConfig *Configuration, args []string, exitOnError case cmdConfig.SetTheme != "": fmt.Printf("[CONFIG] Set theme to %q\n", cmdConfig.SetTheme) cmdConfig.AppMode = constant.AppModeType.HandleParam + case cmdConfig.SetSSHConfigPath != "": + fmt.Printf("[CONFIG] Set SSH config file path to %q\n", cmdConfig.SetSSHConfigPath) + cmdConfig.AppMode = constant.AppModeType.HandleParam } return &cmdConfig, nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 995f943..ab849b8 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -36,7 +36,7 @@ func Test_parseEnvironmentConfig(t *testing.T) { require.Empty(t, envConfig.DisableFeature) require.Empty(t, envConfig.EnableFeature) require.Empty(t, envConfig.SetTheme) - require.Empty(t, envConfig.SSHConfigFilePath) + require.Empty(t, envConfig.SSHConfigPath) require.Equal(t, "info", envConfig.LogLevel) }) @@ -52,16 +52,16 @@ func Test_parseEnvironmentConfig(t *testing.T) { require.Empty(t, envConfig.DisableFeature) require.Empty(t, envConfig.EnableFeature) require.Empty(t, envConfig.SetTheme) - require.Equal(t, "/tmp/custom_config", envConfig.SSHConfigFilePath) + require.Equal(t, "/tmp/custom_config", envConfig.SSHConfigPath) require.Equal(t, "debug", envConfig.LogLevel) }) } func Test_parseCommandLineFlags(t *testing.T) { envConfig := &Configuration{ - AppHome: "/tmp/home", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", + AppHome: "/tmp/home", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", } tests := []struct { @@ -74,116 +74,130 @@ func Test_parseCommandLineFlags(t *testing.T) { name: "No command line flags", args: []string{}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "", - DisableFeature: "", - EnableFeature: "", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", + AppHome: "/tmp/home", + AppMode: "", + DisableFeature: "", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", }, wantError: false, }, { name: "Display help", args: []string{"-h"}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "", - DisableFeature: "", - EnableFeature: "", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", - SetTheme: "", + AppHome: "/tmp/home", + AppMode: "", + DisableFeature: "", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", + SetTheme: "", }, wantError: true, // returns flag.ErrHelp, read the os.flag documentation for details }, { name: "Display version", args: []string{"-v"}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "DISPLAY_INFO", - DisableFeature: "", - EnableFeature: "", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", - SetTheme: "", + AppHome: "/tmp/home", + AppMode: "DISPLAY_INFO", + DisableFeature: "", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", + SetTheme: "", }, wantError: false, }, { name: "Set home app folder", args: []string{"-f", "/tmp/home2"}, wantConfig: &Configuration{ - AppHome: "/tmp/home2", - AppMode: "", - DisableFeature: "", - EnableFeature: "", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", - SetTheme: "", + AppHome: "/tmp/home2", + AppMode: "", + DisableFeature: "", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", + SetTheme: "", }, wantError: false, }, { name: "Set log level", args: []string{"-l", "debug"}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "", - DisableFeature: "", - EnableFeature: "", - LogLevel: "debug", - SSHConfigFilePath: "/tmp/custom_config", - SetTheme: "", + AppHome: "/tmp/home", + AppMode: "", + DisableFeature: "", + EnableFeature: "", + LogLevel: "debug", + SSHConfigPath: "/tmp/custom_config", + SetTheme: "", }, wantError: false, }, { name: "Set SSH config file path", args: []string{"-s", "/tmp/custom_config2"}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "", - DisableFeature: "", - EnableFeature: "", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config2", - SetTheme: "", + AppHome: "/tmp/home", + AppMode: "", + DisableFeature: "", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config2", + SetTheme: "", }, wantError: false, }, { name: "Set theme", args: []string{"--set-theme", "dark"}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "HANDLE_PARAM", - DisableFeature: "", - EnableFeature: "", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", - SetTheme: "dark", + AppHome: "/tmp/home", + AppMode: "HANDLE_PARAM", + DisableFeature: "", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", + SetTheme: "dark", }, wantError: false, }, { name: "Enable feature", args: []string{"-e", "ssh_config"}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "HANDLE_PARAM", - DisableFeature: "", - EnableFeature: "ssh_config", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", - SetTheme: "", + AppHome: "/tmp/home", + AppMode: "HANDLE_PARAM", + DisableFeature: "", + EnableFeature: "ssh_config", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", + SetTheme: "", }, wantError: false, }, { name: "Disable feature", args: []string{"-d", "ssh_config"}, wantConfig: &Configuration{ - AppHome: "/tmp/home", - AppMode: "HANDLE_PARAM", - DisableFeature: "ssh_config", - EnableFeature: "", - LogLevel: "info", - SSHConfigFilePath: "/tmp/custom_config", - SetTheme: "", + AppHome: "/tmp/home", + AppMode: "HANDLE_PARAM", + DisableFeature: "ssh_config", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", + SetTheme: "", + }, + wantError: false, + }, { + name: "Set ssh config path", + args: []string{"--set-ssh-config-path", "/tmp/custom_config2"}, + wantConfig: &Configuration{ + AppHome: "/tmp/home", + AppMode: "HANDLE_PARAM", + DisableFeature: "", + EnableFeature: "", + LogLevel: "info", + SSHConfigPath: "/tmp/custom_config", // this comes from env config, see above + SetSSHConfigPath: "/tmp/custom_config2", + SetTheme: "", }, wantError: false, }, @@ -207,7 +221,8 @@ func Test_parseCommandLineFlags(t *testing.T) { require.Equal(t, tt.wantConfig.DisableFeature, cfg.DisableFeature) require.Equal(t, tt.wantConfig.EnableFeature, cfg.EnableFeature) require.Equal(t, tt.wantConfig.LogLevel, cfg.LogLevel) - require.Equal(t, tt.wantConfig.SSHConfigFilePath, cfg.SSHConfigFilePath) + require.Equal(t, tt.wantConfig.SSHConfigPath, cfg.SSHConfigPath) + require.Equal(t, tt.wantConfig.SetSSHConfigPath, cfg.SetSSHConfigPath) require.Equal(t, tt.wantConfig.SetTheme, cfg.SetTheme) }) } diff --git a/internal/model/sshcommand/command.go b/internal/model/sshcommand/command.go index 8209b07..7558ec5 100644 --- a/internal/model/sshcommand/command.go +++ b/internal/model/sshcommand/command.go @@ -18,7 +18,7 @@ func ConnectCommand(options ...Option) string { } if sshconfig.IsUserDefinedPath() { - addOption(&sb, OptionConfigFilePath{Value: sshconfig.GetFilePath()}) + addOption(&sb, OptionConfigFilePath{Value: sshconfig.Path()}) } return sb.String() @@ -34,7 +34,7 @@ func LoadConfigCommand(options ...Option) string { } if sshconfig.IsUserDefinedPath() { - addOption(&sb, OptionConfigFilePath{Value: sshconfig.GetFilePath()}) + addOption(&sb, OptionConfigFilePath{Value: sshconfig.Path()}) } return sb.String() diff --git a/internal/model/sshcommand/option_test.go b/internal/model/sshcommand/option_test.go index 3155ba5..32a25ed 100644 --- a/internal/model/sshcommand/option_test.go +++ b/internal/model/sshcommand/option_test.go @@ -126,7 +126,7 @@ func Test_ConnectCommand(t *testing.T) { // Check that the command uses custom SSH config file path if defined state.Initialize(context.TODO(), - &config.Configuration{SSHConfigFilePath: "~/.ssh/custom_config"}, + &config.Configuration{SSHConfigPath: "~/.ssh/custom_config"}, &mocklogger.Logger{}) actual := ConnectCommand(OptionAddress{Value: "example.com"}) require.Contains(t, actual, `ssh example.com -F "~/.ssh/custom_config"`) @@ -161,7 +161,7 @@ func Test_LoadConfigCommand(t *testing.T) { // Repeat the first test with a custom SSH config file path mockLogger := mocklogger.Logger{} - state.Initialize(context.TODO(), &config.Configuration{SSHConfigFilePath: "~/.ssh/custom_config"}, &mockLogger) + state.Initialize(context.TODO(), &config.Configuration{SSHConfigPath: "~/.ssh/custom_config"}, &mockLogger) actual := LoadConfigCommand(tests[0].option) // Should use contains because on Windows version the command starts from 'cmd /c ...' require.Contains(t, actual, `ssh -G example.com -F "~/.ssh/custom_config"`) diff --git a/internal/model/sshconfig/config.go b/internal/model/sshconfig/config.go index bcfc494..58292dd 100644 --- a/internal/model/sshconfig/config.go +++ b/internal/model/sshconfig/config.go @@ -69,6 +69,11 @@ func getRegexFirstMatchingGroup(groups []string) string { return "" } +/* + SSHconfig paths below have nothing to do with model/config and + should be moved out of here! This is a good victim for refactoring. +*/ + // IsUserDefinedPath - checks if user re-defined SSH config file path. func IsUserDefinedPath() bool { if !state.IsInitialized() { @@ -79,7 +84,19 @@ func IsUserDefinedPath() bool { return state.Get().IsUserDefinedSSHConfigPath } -// GetFilePath - returns SSH config file path which is defined in application configuration. -func GetFilePath() string { - return state.Get().SSHConfigFilePath +var sshConfigPath *string + +// SetPath - set SSH config file path. This function does not validate or refine path. +func SetPath(path string) { + sshConfigPath = &path +} + +// Path - returns SSH config file path which is defined in application configuration. +func Path() string { + if sshConfigPath != nil { + return *sshConfigPath + } + + // Fallback to application state. + return state.Get().SSHConfigPath } diff --git a/internal/model/sshconfig/config_test.go b/internal/model/sshconfig/config_test.go index 0dbc4a8..8ea4136 100644 --- a/internal/model/sshconfig/config_test.go +++ b/internal/model/sshconfig/config_test.go @@ -1,9 +1,15 @@ package sshconfig import ( + "context" "testing" "github.com/stretchr/testify/require" + + "github.com/grafviktor/goto/internal/config" + "github.com/grafviktor/goto/internal/state" + "github.com/grafviktor/goto/internal/testutils/mocklogger" + "github.com/grafviktor/goto/internal/utils" ) func TestGetCurrentOSUser(t *testing.T) { @@ -39,3 +45,18 @@ func TestParseConfig(t *testing.T) { actual = Parse(unixMockSSHConfig) require.Equal(t, expected, actual) } + +func Test_Path(t *testing.T) { + _, err := state.Initialize( + context.TODO(), + &config.Configuration{SSHConfigPath: "/tmp/mock_config"}, + &mocklogger.Logger{}) + + require.NoError(t, err) + require.Nil(t, sshConfigPath) + expectedPath, _ := utils.SSHConfigPath("/tmp/mock_config") + require.Equal(t, expectedPath, Path()) + + SetPath("/custom/mock_config2") + require.Equal(t, "/custom/mock_config2", Path()) +} diff --git a/internal/state/state.go b/internal/state/state.go index 521172b..470d6cb 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -55,6 +55,10 @@ type loggerInterface interface { // State stores application state. type State struct { + // Note that the app load ssh_config_file_path from SSHConfigPath, + // but persists it to disk using SetSSHConfigPath. + // This is done to distinguish between --set-ssh-config-path flag usage and + // and setting the path via command line -s or env variable. AppHome string `yaml:"-"` AppMode constant.AppMode `yaml:"-"` Context context.Context `yaml:"-"` @@ -65,9 +69,10 @@ type State struct { Logger loggerInterface `yaml:"-"` LogLevel constant.LogLevel `yaml:"-"` ScreenLayout constant.ScreenLayout `yaml:"screen_layout,omitempty"` + SetSSHConfigPath string `yaml:"ssh_config_path,omitempty"` Selected int `yaml:"selected"` SSHConfigEnabled bool `yaml:"enable_ssh_config"` - SSHConfigFilePath string `yaml:"-"` + SSHConfigPath string `yaml:"-"` Theme string `yaml:"theme,omitempty"` Width int `yaml:"-"` } @@ -105,11 +110,11 @@ func Initialize(ctx context.Context, // Set default values st = &State{ - AppMode: cfg.AppMode, - AppHome: cfg.AppHome, - Context: ctx, - Logger: lg, - SSHConfigFilePath: defaultSSHConfigPath, + AppMode: cfg.AppMode, + AppHome: cfg.AppHome, + Context: ctx, + Logger: lg, + SSHConfigPath: defaultSSHConfigPath, } // Read state from file @@ -131,6 +136,7 @@ func (s *State) readFromFile() { Theme *string `yaml:"theme"` ScreenLayout *string `yaml:"screen_layout"` SSHConfigEnabled *bool `yaml:"enable_ssh_config"` + SSHConfigPath *string `yaml:"ssh_config_path"` } appStateFilePath := path.Join(s.AppHome, stateFile) @@ -168,9 +174,24 @@ func (s *State) readFromFile() { s.SSHConfigEnabled = *loadedState.SSHConfigEnabled } + if loadedState.SSHConfigPath != nil { + var sshConfigPath string + sshConfigPath, err = utils.SSHConfigPath(*loadedState.SSHConfigPath) + if err != nil { + s.Logger.Error("[APPSTATE] Cannot apply ssh_config file path from state file: %v", err) + } else { + s.SSHConfigPath = sshConfigPath + s.IsUserDefinedSSHConfigPath = true + // This value is only used when persist to disk, as + // we want to keep SSHConfigPath across app restarts. + s.SetSSHConfigPath = sshConfigPath + } + } + s.Logger.Debug("[APPSTATE] Screen layout: '%v'. Focused host id: '%v'", s.ScreenLayout, s.Selected) } +//nolint:gocognit // this function performs a single task and can't be splitted func (s *State) applyConfig(cfg *config.Configuration) error { if utils.StringEmpty(&cfg.AppMode) { s.AppMode = constant.AppModeType.StartUI @@ -200,12 +221,23 @@ func (s *State) applyConfig(cfg *config.Configuration) error { } } - if !utils.StringEmpty(&cfg.SSHConfigFilePath) { - userDefinedPath, err := utils.SSHConfigFilePath(cfg.SSHConfigFilePath) + // This will only trigger if user used --set-ssh-config-path flag. + // The app will close after persisting the new path. + // We do not need to assign the value to s.SSHConfigPath as the app will exit now. + if !utils.StringEmpty(&cfg.SetSSHConfigPath) { + userDefinedPath, err := utils.SSHConfigPath(cfg.SetSSHConfigPath) + if err != nil { + return fmt.Errorf("cannot set ssh config file path: %w", err) + } + s.SetSSHConfigPath = userDefinedPath + } + + if !utils.StringEmpty(&cfg.SSHConfigPath) { + userDefinedPath, err := utils.SSHConfigPath(cfg.SSHConfigPath) if err != nil { return fmt.Errorf("cannot set ssh config file path: %w", err) } - s.SSHConfigFilePath = userDefinedPath + s.SSHConfigPath = userDefinedPath s.IsUserDefinedSSHConfigPath = true } @@ -245,7 +277,7 @@ func (s *State) print() { fmt.Printf("Log level: %s\n", s.LogLevel) fmt.Printf("SSH config status: %s\n", lo.Ternary(s.SSHConfigEnabled, "enabled", "disabled")) if s.SSHConfigEnabled { - fmt.Printf("SSH config path: %s\n", s.SSHConfigFilePath) + fmt.Printf("SSH config path: %s\n", s.SSHConfigPath) } } @@ -261,6 +293,6 @@ func (s *State) LogDetails() { s.Logger.Info("[CONFIG] Application log level: %q\n", s.LogLevel) s.Logger.Info("[CONFIG] SSH config status: %q\n", lo.Ternary(s.SSHConfigEnabled, "enabled", "disabled")) if s.SSHConfigEnabled { - s.Logger.Info("[CONFIG] SSH config path: %q\n", s.SSHConfigFilePath) + s.Logger.Info("[CONFIG] SSH config path: %q\n", s.SSHConfigPath) } } diff --git a/internal/state/state_test.go b/internal/state/state_test.go index a30afcb..3a1cd68 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -18,6 +18,7 @@ import ( "github.com/grafviktor/goto/internal/config" "github.com/grafviktor/goto/internal/constant" + "github.com/grafviktor/goto/internal/utils" ) type MockLogger struct { @@ -178,6 +179,46 @@ screen_layout: compact Theme: "dark", Group: "default", }, + }, { + name: "Valid SSH config path should be picked up by state", + stateFileContent: ` +selected: 999 +enable_ssh_config: true +group: default +theme: dark +screen_layout: compact +ssh_config_path: /tmp/some_path +`, + expected: State{ + Selected: 999, + SSHConfigEnabled: true, + ScreenLayout: constant.ScreenLayoutCompact, + Theme: "dark", + Group: "default", + SSHConfigPath: "/tmp/some_path", + SetSSHConfigPath: "/tmp/some_path", + IsUserDefinedSSHConfigPath: true, + }, + }, { + name: "Valid remote SSH config path should be picked up by state", + stateFileContent: ` +selected: 999 +enable_ssh_config: true +group: default +theme: dark +screen_layout: compact +ssh_config_path: http://example.com/ssh_config +`, + expected: State{ + Selected: 999, + SSHConfigEnabled: true, + ScreenLayout: constant.ScreenLayoutCompact, + Theme: "dark", + Group: "default", + SSHConfigPath: "http://example.com/ssh_config", + SetSSHConfigPath: "http://example.com/ssh_config", + IsUserDefinedSSHConfigPath: true, + }, }, } @@ -193,19 +234,31 @@ screen_layout: compact test.readFromFile() - assert.Equal(t, tt.expected.Selected, test.Selected, "state.Selected value mismatch") - assert.Equal(t, tt.expected.SSHConfigEnabled, test.SSHConfigEnabled, "state.SSHConfigEnabled value mismatch") - assert.Equal(t, tt.expected.ScreenLayout, test.ScreenLayout, "state.ScreenLayout value mismatch") + // On Windows "/tmp/some_path" becomes "C:/tmp/some_path" + expectedSSHConfigPath := tt.expected.SSHConfigPath + if !utils.StringEmpty(&tt.expected.SSHConfigPath) { + expectedSSHConfigPath, _ = utils.SSHConfigPath(tt.expected.SSHConfigPath) + } + + // On Windows "/tmp/some_path" becomes "C:/tmp/some_path" + expectedSetSSHConfigPath := tt.expected.SetSSHConfigPath + if !utils.StringEmpty(&expectedSetSSHConfigPath) { + expectedSetSSHConfigPath, _ = utils.SSHConfigPath(tt.expected.SetSSHConfigPath) + } + assert.Equal(t, tt.expected.Theme, test.Theme, "state.Theme value mismatch") assert.Equal(t, tt.expected.Group, test.Group, "state.Group value mismatch") + assert.Equal(t, tt.expected.Selected, test.Selected, "state.Selected value mismatch") + assert.Equal(t, expectedSSHConfigPath, test.SSHConfigPath, "state.SSHConfigPath value mismatch") + assert.Equal(t, tt.expected.ScreenLayout, test.ScreenLayout, "state.ScreenLayout value mismatch") + assert.Equal(t, expectedSetSSHConfigPath, test.SetSSHConfigPath, "state.SetSSHConfigPath value mismatch") + assert.Equal(t, tt.expected.SSHConfigEnabled, test.SSHConfigEnabled, "state.SSHConfigEnabled value mismatch") + assert.Equal(t, tt.expected.IsUserDefinedSSHConfigPath, test.IsUserDefinedSSHConfigPath, "state.IsUserDefinedSSHConfigPath value mismatch") }) } } func Test_applyConfig(t *testing.T) { - // Use a mock to avoid sync.Once restrictions in tests - once = &mockOnce{} - tests := []struct { name string testCfg config.Configuration @@ -220,6 +273,17 @@ func Test_applyConfig(t *testing.T) { LogLevel: constant.LogLevelType.INFO, }, wantErr: false, + }, { + name: "Overwrite LogLevel and AppMode", + testCfg: config.Configuration{ + AppMode: constant.AppModeType.DisplayInfo, + LogLevel: constant.LogLevelType.DEBUG, + }, + expected: State{ + AppMode: constant.AppModeType.DisplayInfo, + LogLevel: constant.LogLevelType.DEBUG, + }, + wantErr: false, }, { name: "Supported feature enabled", testCfg: config.Configuration{EnableFeature: "ssh_config"}, @@ -249,21 +313,50 @@ func Test_applyConfig(t *testing.T) { expected: State{}, wantErr: true, }, { - name: "SSH config path set", - testCfg: config.Configuration{SSHConfigFilePath: "~/.ssh/custom_config"}, + name: "Set SSH config path for current session with '-s' parameter", + testCfg: config.Configuration{SSHConfigPath: "~/.ssh/custom_config"}, expected: State{ AppMode: constant.AppModeType.StartUI, LogLevel: constant.LogLevelType.INFO, - SSHConfigFilePath: "~/.ssh/custom_config", + SSHConfigPath: "~/.ssh/custom_config", IsUserDefinedSSHConfigPath: true, }, wantErr: false, + }, { + name: "Persist SSH config path with '--set-ssh-config-path' parameter", + testCfg: config.Configuration{SetSSHConfigPath: "~/.ssh/custom_config"}, + expected: State{ + AppMode: constant.AppModeType.StartUI, + LogLevel: constant.LogLevelType.INFO, + SSHConfigPath: "", + SetSSHConfigPath: "~/.ssh/custom_config", + }, + wantErr: false, + }, { + name: "Persist valid theme '--set-theme' parameter", + testCfg: config.Configuration{SetTheme: "nord"}, + expected: State{ + AppMode: constant.AppModeType.StartUI, + LogLevel: constant.LogLevelType.INFO, + Theme: "nord", + }, + wantErr: false, + }, { + name: "Persist invalid theme '--set-theme' parameter", + testCfg: config.Configuration{SetTheme: "no_such_theme"}, + expected: State{}, + wantErr: true, }, } + // Use a mock to avoid sync.Once restrictions in tests + once = &mockOnce{} + tmpHome := t.TempDir() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - actual := &State{} + tt.testCfg.AppHome = tmpHome // If remove, it'll extract themes into '.../internal/state/' folder + actual := &State{Logger: &MockLogger{}} err := actual.applyConfig(&tt.testCfg) if tt.wantErr { @@ -273,9 +366,10 @@ func Test_applyConfig(t *testing.T) { assert.Equal(t, tt.expected.AppMode, actual.AppMode, "AppMode mismatch") assert.Equal(t, tt.expected.LogLevel, actual.LogLevel, "LogLevel mismatch") assert.Equal(t, tt.expected.Theme, actual.Theme, "Theme mismatch") - assert.Equal(t, tt.expected.SSHConfigFilePath, actual.SSHConfigFilePath, "SSHConfigFilePath mismatch") + assert.Equal(t, tt.expected.SSHConfigPath, actual.SSHConfigPath, "SSHConfigPath mismatch") + assert.Equal(t, tt.expected.SetSSHConfigPath, actual.SetSSHConfigPath, "SetSSHConfigPath mismatch") assert.Equal(t, tt.expected.SSHConfigEnabled, actual.SSHConfigEnabled, "SSHConfigEnabled mismatch") - assert.Equal(t, tt.expected.IsUserDefinedSSHConfigPath, actual.IsUserDefinedSSHConfigPath, "IsSSHConfigFilePathDefinedByUser mismatch") + assert.Equal(t, tt.expected.IsUserDefinedSSHConfigPath, actual.IsUserDefinedSSHConfigPath, "IsUserDefinedSSHConfigPath mismatch") } }) } @@ -314,12 +408,15 @@ func Test_PersistApplicationState(t *testing.T) { // Test persisting app state. func Test_PersistApplicationStateError(t *testing.T) { - t.Skip() + // Create state file with read-only permissions + appHome := t.TempDir() + os.WriteFile(path.Join(appHome, "state.yaml"), []byte{}, 0o444) + // Create a mock logger for testing mockLogger := MockLogger{} // Call the Get function with the temporary directory and mock logger - underTest, _ := Initialize(context.TODO(), &config.Configuration{}, &mockLogger) + underTest, _ := Initialize(context.TODO(), &config.Configuration{AppHome: appHome}, &mockLogger) // Modify the application state underTest.Selected = 42 @@ -331,10 +428,10 @@ func Test_PersistApplicationStateError(t *testing.T) { func Test_PrintConfig(t *testing.T) { state := &State{ - AppHome: "/tmp/goto", - LogLevel: "debug", - SSHConfigEnabled: true, - SSHConfigFilePath: "/tmp/ssh_config", + AppHome: "/tmp/goto", + LogLevel: "debug", + SSHConfigEnabled: true, + SSHConfigPath: "/tmp/ssh_config", } actualOutput := captureOutput(state.PrintConfig) @@ -364,11 +461,11 @@ func captureOutput(f func()) string { func Test_LogDetails(t *testing.T) { logger := MockLogger{} state := &State{ - AppHome: "/tmp/goto", - LogLevel: "debug", - SSHConfigEnabled: true, - SSHConfigFilePath: "/tmp/ssh_config", - Logger: &logger, + AppHome: "/tmp/goto", + LogLevel: "debug", + SSHConfigEnabled: true, + SSHConfigPath: "/tmp/ssh_config", + Logger: &logger, } state.LogDetails() diff --git a/internal/storage/sshconfig/lexer.go b/internal/storage/sshconfig/lexer.go index 9c5a046..5ab024b 100644 --- a/internal/storage/sshconfig/lexer.go +++ b/internal/storage/sshconfig/lexer.go @@ -26,9 +26,9 @@ type Lexer struct { } // NewFileLexer creates a new instance of Lexer for the given SSH config file path. -func NewFileLexer(sshConfigFilePath string, log iLogger) *Lexer { +func NewFileLexer(sshConfigPath string, log iLogger) *Lexer { var pathType string - if utils.IsSupportedURL(sshConfigFilePath) { + if utils.IsSupportedURL(sshConfigPath) { pathType = pathTypeURL } else { pathType = pathTypeFile @@ -36,7 +36,7 @@ func NewFileLexer(sshConfigFilePath string, log iLogger) *Lexer { return &Lexer{ pathType: pathType, - currentPath: sshConfigFilePath, + currentPath: sshConfigPath, rawData: []byte{}, logger: log, } diff --git a/internal/storage/sshconfig_file.go b/internal/storage/sshconfig_file.go index b387579..f781240 100644 --- a/internal/storage/sshconfig_file.go +++ b/internal/storage/sshconfig_file.go @@ -10,6 +10,7 @@ import ( "github.com/grafviktor/goto/internal/constant" model "github.com/grafviktor/goto/internal/model/host" + sshConfigSettings "github.com/grafviktor/goto/internal/model/sshconfig" "github.com/grafviktor/goto/internal/state" "github.com/grafviktor/goto/internal/storage/sshconfig" ) @@ -44,7 +45,7 @@ func newSSHConfigStorage( st *state.State, logger iLogger, ) *SSHConfigFile { - lexer := sshconfig.NewFileLexer(st.SSHConfigFilePath, logger) + lexer := sshconfig.NewFileLexer(st.SSHConfigPath, logger) parser := sshconfig.NewParser(lexer, logger) return &SSHConfigFile{ fileLexer: lexer, @@ -55,23 +56,29 @@ func newSSHConfigStorage( // GetAll - returns all hosts. func (s *SSHConfigFile) GetAll() ([]model.Host, error) { - hosts, err := s.fileParser.Parse() - if err != nil { - return nil, err - } + // Optimization - this is a readonly storage. Therefore, it's pointless to reload + // hosts from the file if they are already loaded. That especially increases + // the performance when the app read hosts from a remote location. + if s.innerStorage == nil { + hosts, err := s.fileParser.Parse() + if err != nil { + return nil, err + } - err = s.createSSHConfigCopy() - if err != nil { - return nil, err - } + err = s.createSSHConfigCopy() + if err != nil { + return nil, err + } - s.updateApplicationState() - s.innerStorage = make(map[int]model.Host, len(hosts)) - for i := range hosts { - // Make sure that not assigning '0' as host id, because '0' is empty host identifier. - // Consider to use '-1' for all new hostnames. - hosts[i].ID = i + 1 - s.innerStorage[i+1] = hosts[i] + s.activateTempSSHConfig() + + s.innerStorage = make(map[int]model.Host, len(hosts)) + for i := range hosts { + // Make sure that not assigning '0' as host id, because '0' is empty host identifier. + // Consider to use '-1' for all new hostnames. + hosts[i].ID = i + 1 + s.innerStorage[i+1] = hosts[i] + } } values := lo.Values(s.innerStorage) @@ -121,8 +128,8 @@ func (s *SSHConfigFile) createSSHConfigCopy() error { return nil } -func (s *SSHConfigFile) updateApplicationState() { - s.appState.SSHConfigFilePath = s.sshConfigCopy.Name() +func (s *SSHConfigFile) activateTempSSHConfig() { + sshConfigSettings.SetPath(s.sshConfigCopy.Name()) } func (s *SSHConfigFile) Close() { diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 72471fe..de7955d 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -73,7 +73,7 @@ func getStorages( sshConfigEnabled := st.SSHConfigEnabled logger.Debug("[STORAGE] SSH config storage enable: '%t'", sshConfigEnabled) if sshConfigEnabled { - logger.Info("[STORAGE] Load ssh hosts from ssh config file: %q", st.SSHConfigFilePath) + logger.Info("[STORAGE] Load ssh hosts from ssh config file: %q", st.SSHConfigPath) sshConfigStorage := newSSHConfigStorage(ctx, st, logger) storageMap[sshConfigStorage.Type()] = sshConfigStorage } diff --git a/internal/ui/component/hostlist/hostlist.go b/internal/ui/component/hostlist/hostlist.go index d52beaf..297c987 100644 --- a/internal/ui/component/hostlist/hostlist.go +++ b/internal/ui/component/hostlist/hostlist.go @@ -485,10 +485,8 @@ func (m *ListModel) onGroupSelect(msg message.GroupSelect) tea.Cmd { // simplest way to implement it. cmds = append(cmds, m.loadHosts()) // Display selected group notification - if !utils.StringEmpty(&msg.Name) { - notificationMsg := fmt.Sprintf("group %q", msg.Name) - cmds = append(cmds, m.displayNotificationMsg(notificationMsg)) - } + message := lo.Ternary(utils.StringEmpty(&msg.Name), "display all hosts", fmt.Sprintf("group %q", msg.Name)) + cmds = append(cmds, m.displayNotificationMsg(message)) return tea.Sequence(cmds...) } diff --git a/internal/ui/model.go b/internal/ui/model.go index 2fb5187..4f5a940 100644 --- a/internal/ui/model.go +++ b/internal/ui/model.go @@ -283,7 +283,7 @@ func (m *MainModel) dispatchProcessSSHCopyID(msg message.RunProcessSSHCopyID) te m.logger.Debug("[EXEC] Copy ssh-key '%s.pub' to host '%s'", identityFile, hostname) if sshconfig.IsUserDefinedPath() { m.logger.Warn("[EXEC] copy ssh key when alternative ssh config file is used: %q. ssh config file is ignored.", - m.appState.SSHConfigFilePath) + m.appState.SSHConfigPath) } process := utils.BuildProcessInterceptStdAll(msg.Host.CmdSSHCopyID()) m.logger.Info("[EXEC] Run process: '%s'", process.String()) diff --git a/internal/utils/utils.go b/internal/utils/utils.go index d3e8934..f695804 100644 --- a/internal/utils/utils.go +++ b/internal/utils/utils.go @@ -187,8 +187,8 @@ func FetchFromURL(urlPath string) (io.ReadCloser, error) { return resp.Body, nil } -// SSHConfigFilePath - returns ssh_config path or error. -func SSHConfigFilePath(userDefinedPath string) (string, error) { +// SSHConfigPath - returns ssh_config path or error. +func SSHConfigPath(userDefinedPath string) (string, error) { if !StringEmpty(&userDefinedPath) { if IsSupportedURL(userDefinedPath) { return userDefinedPath, nil diff --git a/internal/utils/utils_test.go b/internal/utils/utils_test.go index dc252f2..2ea9adf 100644 --- a/internal/utils/utils_test.go +++ b/internal/utils/utils_test.go @@ -93,11 +93,11 @@ func Test_GetAppDir(t *testing.T) { require.Error(t, err, "App home folder should not be empty 2") } -func Test_SSHConfigFilePath(t *testing.T) { +func Test_SSHConfigPath(t *testing.T) { // Test case: SSH config file path userConfigDir, _ := os.UserHomeDir() expected := path.Join(userConfigDir, ".ssh", "config") - got, _ := SSHConfigFilePath("") + got, _ := SSHConfigPath("") require.Equal(t, expected, got, "Should return default ssh config file path") // Test case: custom file path @@ -105,12 +105,12 @@ func Test_SSHConfigFilePath(t *testing.T) { require.NoError(t, err, "Should create a temporary file for testing") defer tempFile.Close() // clean up customPath := tempFile.Name() - got, err = SSHConfigFilePath(customPath) + got, err = SSHConfigPath(customPath) require.NoError(t, err, "Should not return any errors because the path is valid") require.Equal(t, customPath, got, "Should return custom ssh config file path") // Test case: custom file path - unsupported URL - _, err = SSHConfigFilePath("http://127.0.0.1/ssh_config") + _, err = SSHConfigPath("http://127.0.0.1/ssh_config") require.NoError(t, err, "Should not return any errors because that's a valid URL") }