Skip to content
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

Dialer: add optional methods NetDialTLS and NetDialTLSContext #1

Merged
merged 2 commits into from
Dec 10, 2021

Conversation

lluiscampos
Copy link

Fixes issue: gorilla#745

With the previous interface, NetDial and NetDialContext were used for
both TLS and non-TLS TCP connections, and afterwards TLSClientConfig was
used to do the TLS handshake.

While this API works for most cases, it prevents from using more advance
authentication methods during the TLS handshake, as this is out of the
control of the user.

This commits introduces another pair of dial methods, NetDialTLS and
NetDialTLSContext which are used when dialing for TLS/TCP. The code then
assumes that the handshake is done there and TLSClientConfig is not
used.

This API change is fully backwards compatible and it better aligns with
net/http.Transport API, which has these four dial flavors. See:
https://pkg.go.dev/net/http#Transport

Fixes issue: gorilla#745

With the previous interface, NetDial and NetDialContext were used for
both TLS and non-TLS TCP connections, and afterwards TLSClientConfig was
used to do the TLS handshake.

While this API works for most cases, it prevents from using more advance
authentication methods during the TLS handshake, as this is out of the
control of the user.

This commits introduces another pair of dial methods, NetDialTLS and
NetDialTLSContext which are used when dialing for TLS/TCP. The code then
assumes that the handshake is done there and TLSClientConfig is not
used.

This API change is fully backwards compatible and it better aligns with
net/http.Transport API, which has these four dial flavors. See:
https://pkg.go.dev/net/http#Transport

Signed-off-by: Lluis Campos <[email protected]>
Copy link

@merlin-northern merlin-northern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm I have not reviewed the test.

@lluiscampos lluiscampos merged commit 8a45e5d into mendersoftware:master Dec 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants