ostree: extract httpClientForRef from resolveRef

This commit extracts a `httpClientForRef` helper from the `resolveRef()`
function. This helps with readability and testability. It also adds
a test for the proxy setting, CA and MTLS settings.

pkg/ostree/ostree.go

@@ -151,18 +151,9 @@ func verifyChecksum(commit string) bool {
 	return len(commit) > 0 && ostreeCommitRE.MatchString(commit)
 }
 
-// resolveRef resolves the URL path specified by the location and ref
-// (location+"refs/heads/"+ref) and returns the commit ID for the named ref. If
-// there is an error, it will be of type ResolveRefError.
-func resolveRef(ss SourceSpec) (string, error) {
-	u, err := url.Parse(ss.URL)
-	if err != nil {
-		return "", NewResolveRefError("error parsing ostree repository location: %v", err)
-	}
-	u.Path = path.Join(u.Path, "refs", "heads", ss.Ref)
-
+func httpClientForRef(scheme string, ss SourceSpec) (*http.Client, error) {
 	transport := http.DefaultTransport.(*http.Transport).Clone()
-	if u.Scheme == "https" {
+	if scheme == "https" {
 		tlsConf := &tls.Config{
 			MinVersion: tls.VersionTLS12,
 		}
@@ -171,18 +162,18 @@ func resolveRef(ss SourceSpec) (string, error) {
 		if ss.MTLS != nil && ss.MTLS.CA != "" {
 			caCertPEM, err := os.ReadFile(ss.MTLS.CA)
 			if err != nil {
-				return "", NewResolveRefError("error adding ca certificate when resolving ref: %s", err)
+				return nil, NewResolveRefError("error adding ca certificate when resolving ref: %s", err)
 			}
 			tlsConf.RootCAs = x509.NewCertPool()
 			if ok := tlsConf.RootCAs.AppendCertsFromPEM(caCertPEM); !ok {
-				return "", NewResolveRefError("error adding ca certificate when resolving ref")
+				return nil, NewResolveRefError("error adding ca certificate when resolving ref")
 			}
 		}
 
 		if ss.MTLS != nil && ss.MTLS.ClientCert != "" && ss.MTLS.ClientKey != "" {
 			cert, err := tls.LoadX509KeyPair(ss.MTLS.ClientCert, ss.MTLS.ClientKey)
 			if err != nil {
-				return "", NewResolveRefError("error adding client certificate when resolving ref: %s", err)
+				return nil, NewResolveRefError("error adding client certificate when resolving ref: %s", err)
 			}
 			tlsConf.Certificates = []tls.Certificate{cert}
 		}
@@ -193,22 +184,38 @@ func resolveRef(ss SourceSpec) (string, error) {
 	if ss.Proxy != "" {
 		host, port, err := net.SplitHostPort(ss.Proxy)
 		if err != nil {
-			return "", NewResolveRefError("error parsing MTLS proxy URL '%s': %v", ss.URL, err)
+			return nil, NewResolveRefError("error parsing MTLS proxy URL '%s': %v", ss.URL, err)
 		}
 
 		proxyURL, err := url.Parse("http://" + host + ":" + port)
 		if err != nil {
-			return "", NewResolveRefError("error parsing MTLS proxy URL '%s': %v", ss.URL, err)
+			return nil, NewResolveRefError("error parsing MTLS proxy URL '%s': %v", ss.URL, err)
 		}
 
 		transport.Proxy = func(request *http.Request) (*url.URL, error) {
 			return proxyURL, nil
 		}
 	}
 
-	client := &http.Client{
+	return &http.Client{
 		Transport: transport,
 		Timeout:   300 * time.Second,
+	}, nil
+}
+
+// resolveRef resolves the URL path specified by the location and ref
+// (location+"refs/heads/"+ref) and returns the commit ID for the named ref. If
+// there is an error, it will be of type ResolveRefError.
+func resolveRef(ss SourceSpec) (string, error) {
+	u, err := url.Parse(ss.URL)
+	if err != nil {
+		return "", NewResolveRefError("error parsing ostree repository location: %v", err)
+	}
+	u.Path = path.Join(u.Path, "refs", "heads", ss.Ref)
+
+	client, err := httpClientForRef(u.Scheme, ss)
+	if err != nil {
+		return "", err
 	}
 
 	req, err := http.NewRequest(http.MethodGet, u.String(), nil)

pkg/ostree/ostree_test.go

@@ -1,6 +1,7 @@
 package ostree
 
 import (
+	"crypto/tls"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -85,7 +86,7 @@ func TestOstreeResolveRef(t *testing.T) {
 				&MTLS{mTLSSrv.CAPath, mTLSSrv.ClientCrtPath, mTLSSrv.ClientKeyPath},
 				"",
 			})
-			assert.NoError(t, err)
+			require.NoError(t, err)
 			assert.Equal(t, expOut, out)
 		}
 
@@ -226,3 +227,56 @@ func TestValidate(t *testing.T) {
 	}
 
 }
+
+func TestClientForRefProxy(t *testing.T) {
+	ss := SourceSpec{
+		Proxy: "foo:1234",
+	}
+	client, err := httpClientForRef("https", ss)
+	assert.NoError(t, err)
+
+	proxy, err := client.Transport.(*http.Transport).Proxy(&http.Request{})
+	assert.NoError(t, err)
+	assert.Equal(t, "foo", proxy.Hostname())
+	assert.Equal(t, "1234", proxy.Port())
+}
+
+func TestClientForRefClientCertKey(t *testing.T) {
+	ss := SourceSpec{
+		MTLS: &MTLS{
+			ClientCert: "test_mtls_server/client.crt",
+			ClientKey:  "test_mtls_server/client.key",
+		},
+	}
+	client, err := httpClientForRef("https", ss)
+	assert.NoError(t, err)
+
+	tlsConf := client.Transport.(*http.Transport).TLSClientConfig
+	assert.NoError(t, err)
+	expectedCert, err := tls.LoadX509KeyPair("test_mtls_server/client.crt", "test_mtls_server/client.key")
+	assert.NoError(t, err)
+	assert.Equal(t, 1, len(tlsConf.Certificates))
+	assert.Equal(t, expectedCert, tlsConf.Certificates[0])
+	// no RootCAs got added
+	assert.Nil(t, tlsConf.RootCAs)
+}
+
+func TestClientForRefCA(t *testing.T) {
+	ss := SourceSpec{
+		MTLS: &MTLS{
+			CA: "test_mtls_server/ca.crt",
+		},
+	}
+	client, err := httpClientForRef("https", ss)
+	assert.NoError(t, err)
+
+	tlsConf := client.Transport.(*http.Transport).TLSClientConfig
+	assert.NoError(t, err)
+	// the RootCAs is a x590.CertPool which provides almost no
+	// introspection (only Subjects() which is deprecated). So
+	// we do just chech that *something* got added and hope for the
+	// best here.
+	assert.NotNil(t, tlsConf.RootCAs)
+	// no certificates got added
+	assert.Equal(t, 0, len(tlsConf.Certificates))
+}