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
Feature/configurable ssh multiplexing #1172
Feature/configurable ssh multiplexing #1172
Conversation
|
||
public function addSshOption(string $option, $value) : Host | ||
{ | ||
$this->sshArguments = $this->sshArguments->withOption($option, $value); |
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 like this approach (btw i like immutability). Why dot just call $this->sshArguments->addOption($option, $value);
?
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.
My idea behind this was to keep the arguments for the host from being altered accidentally outside of the class. E.g if a task called getSshArguments then added more flags / options without immutability this would change the sshArguments on the host object.
If you don't think this is something we need to be concerned with it's a relatively simple change I can do.
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.
What usage of this? Can you show an example?
Example of my current usage host('xxx')
->port(xxxx)
->stage('dev')
->user('www-data')
->set('branch', 'develop')
->set('keep_releases', 1)
->set('deploy_path', 'xxxxx')
->multiplexing()
->addSshFlag('-q')
->addSshOption('UserKnownHostsFile', '/dev/null')
->addSshOption('StrictHostKeyChecking', 'no')
->addSshOption('ControlPersist', '600'); It's also possible to do the options as an array if the user would want to. |
I wanted more structure to the data and to be able to configure not just the options but all areas of the ssh flags/options as this really helps with debugging problems (and how I found the issue with #1155 )
To do this I created an immutable ssh arguments class that handles pretty much everything including the multiplex arguments. This was very important to me because these arguments may need to be used elsewhere and not just in the client, for example the
rsync
recipe will currently not be able to use the multiplexing unless the code is duplicated into that recipe.I also updated the tests where applicable including the yaml fixture with the new format I'm proposing.
Hopefully it makes sense, I'm open to feedback and I have some changes waiting to update the
rsync
recipe if this gets merged.As a side note, IMO multiplexing should be enabled by default, but this is a separate discussion we can have.