Changes to support external transport implementations

[mdarocha]

Will help with #540

After discussion, this PR implements changes that will allow external packages to extend
DockerClientConfiguration and implement their own transport methods, including SSH.

[dnfadmin]

All CLA requirements met.

[WojciechNagorski]

Great work!
Are you going to create a PR to https://github.com/sshnet/SSH.NET with changes from your fork https://github.com/TrapTech/SSH.NET?

[mdarocha]

Great work!
Are you going to create a PR to https://github.com/sshnet/SSH.NET with changes from your fork https://github.com/TrapTech/SSH.NET?

I dont think thats needed , since my fork just merges changes that already have a PR made there.

[WojciechNagorski]

Do you mean sshnet/SSH.NET#144 ? I didn't know about this PR. Maybe It should be updated because it stucks in the 2020 year.

[mdarocha]

Do you mean sshnet/SSH.NET#144 ? I didn't know about this PR. Maybe It should be updated because it stucks in the 2020 year.

Yes, thats the one. Maybe I'll see about making a new PR

[WojciechNagorski]

@mdarocha any update?

[mdarocha]

@mdarocha any update?

Sorry, didn't have time to work on this.

I made sshnet/SSH.NET#1052, and will wait for a response. You can consider this PR dependant on that one.

Unless someone figures out some other way than SSH.NET to communicate over SSH.

For the time being, if anyone needs this functionality, you can use the forked TrapTech.Docker.DotNet nuget, which includes this.

[mdarocha]

Unless we refactor this PR to contain only the changes in the main Docker.DotNet project, and keep the Docker.DotNet.SSH out of this repo, as a completely separate, custom nuget? Then everybody could use the upstream library, and install SSH support as a separate nuget, not necessarily built from this repo.

I'm not sure however how to solve the assembly linking problems that can result from this, since currently all Docker.DotNet.* assemblies have to have the same version.

[HofmeisterAn]

I'm not sure however how to solve the assembly linking problems that can result from this, since currently all Docker.DotNet.* assemblies have to have the same version.

Can't you use the version ranges to pin it to a specific version?

[galvesribeiro]

Yeah, I agree. We should keep things separated...

[mdarocha]

Ok, I've reverted the changes that add the SSH project and dependencies on the forked SSH library.

Now that remains in this PR is enough scaffolding for anyone to hook up with their own SSH library. I plan on publishing the code that was here as a separate nuget if we merge this PR.

There are some additional diffs in this PR due to some problems with line endings on the master branch.

[mdarocha]

Does the PR look ok to merge in its current state?

[mdarocha]

@WojciechNagorski @HofmeisterAn I've done a bit of a refactor according to your suggestions

[WojciechNagorski]

LGTM. I have two questions but overall it looks great.

This PR also contains many whitespace changes in files that have not been changed in this PR. I always try to avoid this situation because it makes the reviewer's job more difficult and causes merge conflicts in other PRs. Some reviewers might reject such PR.

[mdarocha]

As per the diff: yeah, I normally avoid such situations, but it appears the master branch has some files checked in with CRLF line endings, which caused those problems.

[WojciechNagorski]

Looks even better 😄

[mdarocha]

Any updates? @HofmeisterAn is this ok to merge? Note that it no longer depends on the SSH.NET changes.

[HofmeisterAn]

Any updates? @HofmeisterAn is this ok to merge? Note that it no longer depends on the SSH.NET changes.

In general, I am okay with the PR, but I cannot merge it. The repository's owner, @galvesribeiro, has the necessary permission to do so. One thing that has already been discussed are the white space changes. It would be helpful if we could remove them from the PR, as they make up the majority of the changes (and are unnecessary).

[galvesribeiro]

Can you please remove all the spacing/formatting changes? It is kinda hard to see what are the actual changes.

One thing that I'm struggling to understand is the usefulness of this change and wether or not does it fit to the library. Perhaps after you clean up the space changes it will be easier to understand/review.

Please let me know when it is ready and I'll take a look.

[mdarocha]

@galvesribeiro @HofmeisterAn ok, I think I managed to fix the whitespace changes.
This should really be fixed on the main branch, since it makes it a bit annoying to work on the repo :D

One thing that I'm struggling to understand is the usefulness of this change and wether or not does it fit to the library. Perhaps after you clean up the space changes it will be easier to understand/review.

Keep in mind that previously this PR was focused on implementing SSH transport support for this library (something that's needed, since it's the only non-deprecated way left for remote access), but since SSH.NET is missing the required functionality (sshnet/SSH.NET#1052) this PR has transformed into a way of allowing any external library to implement custom transports for the Docker api. This allows to ie. implement the SSH logic in a completely separate library (and if Docker introduces any future ways of connecting, they can be implemented without waiting for upstream Docker.DotNet to be updated).

[galvesribeiro]

Keep in mind that previously this PR was focused on implementing SSH transport support for this library

We can potentially implement this as a new NuGet package for sure.

(something that's needed, since it's the only non-deprecated way left for remote access)

@mdarocha, can you point me to any official docs that state that? I may have missed the notes on this.

[mdarocha]

(something that's needed, since it's the only non-deprecated way left for remote access)

@mdarocha, can you point me to any official docs that state that? I may have missed the notes on this.

https://docs.docker.com/engine/security/protect-access/#use-tls-https-to-protect-the-docker-daemon-socket well guess I overshot it with the 'deprecated' claim, the docs just suggest that it's hard :D

[WojciechNagorski]

The new 2024.0.0 SSH.NET version support input stream.

Currently, we can send some string (data) via inputStream that can be open command.CreateInputStream():

var inputByteArray = Encoding.UTF8.GetBytes("Hello world!");

using (var command = _sshClient.CreateCommand("docker system dial-stdio"))
{
    var asyncResult = command.BeginExecute();

    var inputStream = command.CreateInputStream())
    {
        inputStream.Write(inputByteArray, 0, inputByteArray.Length);
    }

    command.EndExecute(asyncResult);
}

It should be possible to use JoinedReadWriteStream from the previous version of this PR.

var result = new JoinedReadWriteStream(cmd.OutputStream, inputStream);

[PascalSenn]

Is there any chance this Pull Request can be revived?

[PascalSenn]

This would be fantastic for libraries like https://github.com/SwissLife-OSS/squadron or https://github.com/testcontainers/testcontainers-dotnet because this would allow to run all containers in testing on a remote machine and keep the local device free of any pressure.

[galvesribeiro]

Hello! We can get this in. Doing a wave of merges on older PRs with improvements/fixes before I start adding the new code.

@mdarocha do you mind rebasing on the latest main and solve the conflicts so we can merge it?

[mdarocha]

Hello! We can get this in. Doing a wave of merges on older PRs with improvements/fixes before I start adding the new code.

@mdarocha do you mind rebasing on the latest main and solve the conflicts so we can merge it?

Sure, I'll try to do it over the weekend

[mdarocha]

Ok, updated.

Seems like SSH.NET has since added the missing library features, so SSH transport can now be implemented by overriding DockerClientConfiguration using the latest version of SSH.NET. I'm leaving this up to some other contributor, either as a new nuget in this repository, or a completely new project.

[campersau]

Should this be virtual so others can override it?