Bug 1978749: pkg/cvo: Respect noProxy

go.mod

@@ -6,6 +6,7 @@ require (
 	github.com/blang/semver/v4 v4.0.0
 	github.com/davecgh/go-spew v1.1.1
 	github.com/ghodss/yaml v1.0.0
+	github.com/google/go-cmp v0.5.2
 	github.com/google/uuid v1.1.2
 	github.com/hashicorp/golang-lru v0.5.3 // indirect
 	github.com/imdario/mergo v0.3.8 // indirect
@@ -16,6 +17,7 @@ require (
 	github.com/prometheus/client_golang v1.7.1
 	github.com/prometheus/client_model v0.2.0
 	github.com/spf13/cobra v1.1.1
+	golang.org/x/net v0.0.0-20210224082022-3d97a244fca7
 	golang.org/x/time v0.0.0-20210220033141-f8bda1e9f3ba
 	k8s.io/api v0.21.1
 	k8s.io/apiextensions-apiserver v0.21.1

pkg/cincinnati/cincinnati.go

@@ -2,7 +2,6 @@ package cincinnati
 
 import (
 	"context"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -28,13 +27,12 @@ const (
 // an upstream Cincinnati stack.
 type Client struct {
 	id        uuid.UUID
-	proxyURL  *url.URL
-	tlsConfig *tls.Config
+	transport *http.Transport
 }
 
 // NewClient creates a new Cincinnati client with the given client identifier.
-func NewClient(id uuid.UUID, proxyURL *url.URL, tlsConfig *tls.Config) Client {
-	return Client{id: id, proxyURL: proxyURL, tlsConfig: tlsConfig}
+func NewClient(id uuid.UUID, transport *http.Transport) Client {
+	return Client{id: id, transport: transport}
 }
 
 // Update is a single node from the update graph.
@@ -66,7 +64,6 @@ func (err *Error) Error() string {
 // image can be downloaded.
 func (c Client) GetUpdates(ctx context.Context, uri *url.URL, arch string, channel string, version semver.Version) (Update, []Update, error) {
 	var current Update
-	transport := http.Transport{}
 	// Prepare parametrized cincinnati query.
 	queryParams := uri.Query()
 	queryParams.Add("arch", arch)
@@ -81,21 +78,25 @@ func (c Client) GetUpdates(ctx context.Context, uri *url.URL, arch string, chann
 		return current, nil, &Error{Reason: "InvalidRequest", Message: err.Error(), cause: err}
 	}
 	req.Header.Add("Accept", GraphMediaType)
-	if c.tlsConfig != nil {
-		if c.tlsConfig.ClientCAs == nil {
+	if c.transport != nil && c.transport.TLSClientConfig != nil {
+		if c.transport.TLSClientConfig.ClientCAs == nil {
 			klog.V(5).Infof("Using a root CA pool with 0 root CA subjects to request updates from %s", uri)
 		} else {
-			klog.V(5).Infof("Using a root CA pool with %n root CA subjects to request updates from %s", len(c.tlsConfig.RootCAs.Subjects()), uri)
+			klog.V(5).Infof("Using a root CA pool with %n root CA subjects to request updates from %s", len(c.transport.TLSClientConfig.RootCAs.Subjects()), uri)
 		}
-		transport.TLSClientConfig = c.tlsConfig
 	}
 
-	if c.proxyURL != nil {
-		klog.V(5).Infof("Using proxy %s to request updates from %s", c.proxyURL.Host, uri)
-		transport.Proxy = http.ProxyURL(c.proxyURL)
+	if c.transport != nil && c.transport.Proxy != nil {
+		proxy, err := c.transport.Proxy(req)
+		if err == nil && proxy != nil {
+			klog.V(5).Infof("Using proxy %s to request updates from %s", proxy.Host, uri)
+		}
 	}
 
-	client := http.Client{Transport: &transport}
+	client := http.Client{}
+	if c.transport != nil {
+		client.Transport = c.transport
+	}
 	timeoutCtx, cancel := context.WithTimeout(ctx, getUpdatesTimeout)
 	defer cancel()
 	resp, err := client.Do(req.WithContext(timeoutCtx))

pkg/cincinnati/cincinnati_test.go

@@ -2,7 +2,6 @@ package cincinnati
 
 import (
 	"context"
-	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"net/http"
@@ -120,10 +119,8 @@ func TestGetUpdates(t *testing.T) {
 
 			ts := httptest.NewServer(http.HandlerFunc(handler))
 			defer ts.Close()
-			var proxyURL *url.URL
-			var tlsConfig *tls.Config
 
-			c := NewClient(clientID, proxyURL, tlsConfig)
+			c := NewClient(clientID, nil)
 
 			uri, err := url.Parse(ts.URL)
 			if err != nil {

pkg/cvo/availableupdates.go

@@ -2,8 +2,8 @@ package cvo
 
 import (
 	"context"
-	"crypto/tls"
 	"fmt"
+	"net/http"
 	"net/url"
 	"runtime"
 	"sort"
@@ -43,12 +43,12 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
 		return nil
 	}
 
-	proxyURL, tlsConfig, err := optr.getTransportOpts()
+	transport, err := optr.getTransport()
 	if err != nil {
 		return err
 	}
 
-	current, updates, condition := calculateAvailableUpdatesStatus(ctx, string(config.Spec.ClusterID), proxyURL, tlsConfig, upstream, arch, channel, optr.release.Version)
+	current, updates, condition := calculateAvailableUpdatesStatus(ctx, string(config.Spec.ClusterID), transport, upstream, arch, channel, optr.release.Version)
 
 	if usedDefaultUpstream {
 		upstream = ""
@@ -144,7 +144,7 @@ func (optr *Operator) getAvailableUpdates() *availableUpdates {
 	return optr.availableUpdates
 }
 
-func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, proxyURL *url.URL, tlsConfig *tls.Config, upstream, arch, channel, version string) (configv1.Release, []configv1.Release, configv1.ClusterOperatorStatusCondition) {
+func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, transport *http.Transport, upstream, arch, channel, version string) (configv1.Release, []configv1.Release, configv1.ClusterOperatorStatusCondition) {
 	var cvoCurrent configv1.Release
 	if len(upstream) == 0 {
 		return cvoCurrent, nil, configv1.ClusterOperatorStatusCondition{
@@ -199,7 +199,7 @@ func calculateAvailableUpdatesStatus(ctx context.Context, clusterID string, prox
 		}
 	}
 
-	current, updates, err := cincinnati.NewClient(uuid, proxyURL, tlsConfig).GetUpdates(ctx, upstreamURI, arch, channel, currentVersion)
+	current, updates, err := cincinnati.NewClient(uuid, transport).GetUpdates(ctx, upstreamURI, arch, channel, currentVersion)
 	if err != nil {
 		klog.V(2).Infof("Upstream server %s could not return available updates: %v", upstream, err)
 		if updateError, ok := err.(*cincinnati.Error); ok {

pkg/cvo/cvo.go

@@ -2,10 +2,8 @@ package cvo
 
 import (
 	"context"
-	"crypto/tls"
 	"fmt"
 	"net/http"
-	"net/url"
 	"strconv"
 	"sync"
 	"time"
@@ -865,14 +863,10 @@ func (optr *Operator) defaultPreconditionChecks() precondition.List {
 // HTTPClient provides a method for generating an HTTP client
 // with the proxy and trust settings, if set in the cluster.
 func (optr *Operator) HTTPClient() (*http.Client, error) {
-	proxyURL, tlsConfig, err := optr.getTransportOpts()
+	transportOption, err := optr.getTransport()
 	if err != nil {
 		return nil, err
 	}
-	transportOption := &http.Transport{
-		Proxy:           http.ProxyURL(proxyURL),
-		TLSClientConfig: tlsConfig,
-	}
 	transportConfig := &transport.Config{Transport: transportOption}
 	transport, err := transport.New(transportConfig)
 	if err != nil {
@@ -882,19 +876,3 @@ func (optr *Operator) HTTPClient() (*http.Client, error) {
 		Transport: transport,
 	}, nil
 }
-
-// getTransportOpts retrieves the URL of the cluster proxy and the CA
-// trust, if they exist.
-func (optr *Operator) getTransportOpts() (*url.URL, *tls.Config, error) {
-	proxyURL, err := optr.getHTTPSProxyURL()
-	if err != nil {
-		return nil, nil, err
-	}
-
-	var tlsConfig *tls.Config
-	tlsConfig, err = optr.getTLSConfig()
-	if err != nil {
-		return nil, nil, err
-	}
-	return proxyURL, tlsConfig, nil
-}

pkg/cvo/egress.go

@@ -3,37 +3,65 @@ package cvo
 import (
 	"crypto/tls"
 	"crypto/x509"
+	"errors"
 	"fmt"
+	"net/http"
 	"net/url"
 
-	"k8s.io/apimachinery/pkg/api/errors"
+	"golang.org/x/net/http/httpproxy"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 )
 
-// getHTTPSProxyURL returns a url.URL object for the configured
-// https proxy only. It can be nil if does not exist or there is an error.
-func (optr *Operator) getHTTPSProxyURL() (*url.URL, error) {
+// getTransport constructs an HTTP transport configuration, including
+// any custom proxy configuration.
+func (optr *Operator) getTransport() (*http.Transport, error) {
+	transport := &http.Transport{}
+
+	proxyConfig, err := optr.getProxyConfig()
+	if err != nil {
+		return transport, err
+	} else if proxyConfig != nil {
+		proxyFunc := proxyConfig.ProxyFunc()
+		transport.Proxy = func(req *http.Request) (*url.URL, error) {
+			if req == nil {
+				return nil, errors.New("cannot calculate proxy URI for nil request")
+			}
+			return proxyFunc(req.URL)
+		}
+	}
+
+	tlsConfig, err := optr.getTLSConfig()
+	if err != nil {
+		return transport, err
+	} else if tlsConfig != nil {
+		transport.TLSClientConfig = tlsConfig
+	}
+
+	return transport, err
+}
+
+// getProxyConfig returns a proxy configuration.  It can be nil if
+// does not exist or there is an error.
+func (optr *Operator) getProxyConfig() (*httpproxy.Config, error) {
 	proxy, err := optr.proxyLister.Get("cluster")
 
-	if errors.IsNotFound(err) {
+	if apierrors.IsNotFound(err) {
 		return nil, nil
 	}
 	if err != nil {
 		return nil, err
 	}
 
-	if proxy.Status.HTTPSProxy != "" {
-		proxyURL, err := url.Parse(proxy.Status.HTTPSProxy)
-		if err != nil {
-			return nil, err
-		}
-		return proxyURL, nil
-	}
-	return nil, nil
+	return &httpproxy.Config{
+		HTTPProxy:  proxy.Status.HTTPProxy,
+		HTTPSProxy: proxy.Status.HTTPSProxy,
+		NoProxy:    proxy.Status.NoProxy,
+	}, nil
 }
 
 func (optr *Operator) getTLSConfig() (*tls.Config, error) {
 	cm, err := optr.cmConfigManagedLister.Get("trusted-ca-bundle")
-	if errors.IsNotFound(err) {
+	if apierrors.IsNotFound(err) {
 		return nil, nil
 	}
 	if err != nil {