[#456] #EXTEND 'assemblyName: DotNet.Testcontainers; function: WithDo… #457
Conversation
…nction: WithDockerEndpoint ' {Extended method with Docker.DotNet.Credentials parameter and made required changes to related code to pass credentials to Docker.DotNet.DockerClientConfiguration ctor }
…ts; class: TestcontainersAccessInformationTest' {Fixed tests after changes added in previous commit}
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Thanks for your contribution. I can do the review at the beginning of next month. Right now, I have limit access to a proper device — sorry for the delay. I think this relates to #370 too.
| { | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using Docker.DotNet; |
There was a problem hiding this comment.
The library (builder) does not expose models from this namespace. Please see the discussions in #370 too. I think it makes sense to combine the endpoint and credential in a single interface and offer different implementations specific for the authentication.
| /// <inheritdoc cref="IAbstractBuilder{TBuilderEntity}" /> | ||
| public TBuilderEntity WithDockerEndpoint(string endpoint, Credentials credentials) | ||
| { | ||
| return this.MergeNewConfiguration(new DockerResourceConfiguration(endpoint: new Uri(endpoint), credentials: credentials)); |
There was a problem hiding this comment.
We should use our own interface. The implementation should consider the common env variables (DOCKER_HOST, DOCKER_TLS_VERIFY, DOCKER_CERT_PATH) too (at least in the future). Then we can set and apply the default credentials from the constructor based on the machine configuration.
There was a problem hiding this comment.
If I implement our own interface and class for certificate credentials will it be acceptable for now?
I know that perfect soultion would be connecting to dockerd secured with tls automatically as it is done in java lib but it sounds like a lot of work and as we can see in #370 last comment was in the end of 2k21 ;)
I'd like to use this lib in my projects but without credentials support is useless now.
We could treat this solution as first step to handle connection to secured dockerd - I don't exclude my further contribution on that
There was a problem hiding this comment.
We could treat this solution as first step to handle connection to secured dockerd
I can publish your changes in its own package as soon as I’m back. For develop, I would prefer the mentioned things above.
There was a problem hiding this comment.
Ok, if I want to try to implement solution with docker env variables, should I do that in this PR or create new one?
There was a problem hiding this comment.
Yes, a new pull request would be nice. Smaller increments would be appreciated too (first the interface). I'll publish your package later that day.
There was a problem hiding this comment.
Your changes are published in 1.6.0-beta.GitHub-457.
|
I'll close this in favor of #370 (comment). You can now use the latest version to configure / authenticate the client. |
…ckerEndpoint ' {Extended method with Docker.DotNet.Credentials parameter and made required changes to related code to pass credentials to Docker.DotNet.DockerClientConfiguration ctor }