Exposing TLS Configuration via Connection

[nigurr]

Why?

1. Currently the AzDev Client hides http.client implementation which makes using custom http.client implementations. By exposing this users can use custom http.client implementations which can be used to access AzDev API

Proposed Solution:

1. Client is already exposed struct and exposing struct by hiding it's members seems anti-pattern. By exposing them, an implementation can use the custom http client.

Alternative Solution:

1. Adding another NewClient function which accepts custom http.client, however method overriding is anti-pattern. Exposing altogether separate function with httpclient argument didn't make sense as we still need another params.

[tedchamb]

Can you share how you plan to use each of these properties? Once they are set on the connection they are not supposed to be changed. Clients are cached on the connection, so the properties need to match. Resource locations are cached on the client, so changing the base url would invalidate it.

[nigurr]

client := &http.Client{Transport: tr}
if connection.Timeout != nil {
	client.Timeout = *connection.Timeout
}

c :=  &azuredevops.Client{
	baseUrl:                 connection.BaseUrl,
	client:                  client,
	authorization:           connection.AuthorizationString,
	suppressFedAuthRedirect: connection.SuppressFedAuthRedirect,
	forceMsaPassThrough:     connection.ForceMsaPassThrough,
	userAgent:               connection.UserAgent,
}

currently this is not feasible. as these are not exposed variables.

[nigurr]

I agree to the point changing properties is not the way intended. Creating Client with own configurable options should be exposed.

[tedchamb]

The intention is to create a new connection if you want to change these. Most apps should never need more than one connection unless you are trying to connect to multiple organizations or trying to sign in with multiple identities.

[nigurr]

Agree. Current issue is, http client is internally created, this limits the customisation of standard http client creation (specially TLS Client).

As mentioned two approaches, we can add NewTLSClient(c Connection, baseUrl string, tls tls.Config)
or the above approach. I am kinda favouring the NewTLSClient the more I think about it. What's your suggestion!

[tedchamb]

Can we create a new constructor for the Connection to take a delegate that will be used to construct the client? This way we ensure a consistent http client is used for each http client created from the Connection.

[nigurr]

Can you please review the changes now?