From 2490964386f12b2733f02962631bbb50b791140d Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 19 Nov 2024 16:30:08 +0100 Subject: [PATCH] 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 | 41 ++++++++++++++++------------ pkg/ostree/ostree_test.go | 56 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 18 deletions(-) diff --git a/pkg/ostree/ostree.go b/pkg/ostree/ostree.go index 1b7cec0b65..cbc6118b12 100644 --- a/pkg/ostree/ostree.go +++ b/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,12 +184,12 @@ 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) { @@ -206,9 +197,25 @@ func resolveRef(ss SourceSpec) (string, error) { } } - 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) diff --git a/pkg/ostree/ostree_test.go b/pkg/ostree/ostree_test.go index a9f3b5f984..539668a5a1 100644 --- a/pkg/ostree/ostree_test.go +++ b/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)) +}