Close connection on broken tunnel, private key is not mandatory #159
Conversation
jindrichskupa
commented
Aug 26, 2021
•
- Skip private key in case of error (encrypted without passphrase, wrong format, ...)
- Close open reader/writer on ssh channel when finish or error occurs
- Don't fail but create new empty config when no config (empty string) file was used
|
@davrodpin please let me know your opinion |
tunnel/config.go
Outdated
| @@ -21,6 +21,9 @@ type SSHConfigFile struct { | |||
| // NewSSHConfigFile creates a new instance of SSHConfigFile based on the | |||
| // ssh config file from configPath | |||
| func NewSSHConfigFile(configPath string) (*SSHConfigFile, error) { | |||
| if configPath == "" { | |||
There was a problem hiding this comment.
How about returning a specific error message explaining what the error is about?
There was a problem hiding this comment.
Empty config path is not an error, I don't want to use existing config, I want to use the empty one. Maybe, teh better solution is update the code above to skip reading of existing config not here with error.
There was a problem hiding this comment.
Please see my previous comment on how to allow cfgPath to be empty.
There was a problem hiding this comment.
Sure, thanks for your review. I will update the code later this week.
tunnel/tunnel.go
Outdated
| c = NewEmptySSHConfigStruct() | ||
| } else { | ||
| c, err := NewSSHConfigFile(cfgPath) |
There was a problem hiding this comment.
By using :=, the code is creating a new c variable rather than using the one defined outside of the else scope. Please use =. This is also causing the app to fail when building.
There was a problem hiding this comment.
:= must be used, because err is not defined, so I defined it.
There was a problem hiding this comment.
You can do something like:
} else {
var err error
c, err = NewSSHConfigFile(cfgPath)
...There was a problem hiding this comment.
I defined var err error above. Should I move it here?
|
Hi @jindrichskupa, changes are all good but the fix you made to close the I've provided a fix and it is merged to master. Could you please update your branch with the latest master? Thanks! |
This changes adds a new command to show the runtime configuration of one or all running instances of mole running on the system. It leverages the internal rpc server to call a remote procedure that returns the runtime configuration.
This change makes the runtime information to return the addresses used by the ssh channels instead of the input.
This change makes the connection to the ssh server and channels setup to happen in a goroutine when mole is trying to reconnect. The reason for this change is to allow the tunnel to be closed while the attemp to reconnect is still in progress.
|
Should be done. Thanks for you patience. |
|
@jindrichskupa, thank you very much for your contribution! |