Merge pull request #267 from patrickdillon/cvo-proxy

Bug 1766907: Add proxy support for CVO.

Author: openshift-merge-robot

README.md

@@ -3,7 +3,7 @@
 ## Building and Publishing CVO
 
 ```sh
-./hack/build-image.sh && REPO=<your personal repo (quay.io/ahbinavdahiya | docker.io/abhinavdahiya)> ./hack/push-image.go
+./hack/build-image.sh && REPO=<your personal repo (quay.io/ahbinavdahiya | docker.io/abhinavdahiya)> ./hack/push-image.sh
 ```
 
 1. This builds image locally and then pushes `${VERSION}` and `latest` tags to `${REPO}/origin-cluster-version-operator`.
@@ -17,7 +17,7 @@
 2. Run the following command to create release-image at `docker.io/abhinavdahiya/origin-release:latest`:
 
 ```sh
-oc adm release new -n openshift --server https://api.ci.openshift.org \
+oc adm release new -n origin \
     --from-image-stream=origin-v4.0 \
     --to-image-base=docker.io/abhinavdahiya/origin-cluster-version-operator:latest \
     --to-image docker.io/abhinavdahiya/origin-release:latest

pkg/cvo/availableupdates.go

@@ -24,7 +24,6 @@ import (
 // object. It will set the RetrievedUpdates condition. Updates are only checked if it has been more than
 // the minimumUpdateCheckInterval since the last check.
 func (optr *Operator) syncAvailableUpdates(config *configv1.ClusterVersion) error {
-	var tlsConfig *tls.Config
 	usedDefaultUpstream := false
 	upstream := string(config.Spec.Upstream)
 	if len(upstream) == 0 {
@@ -41,16 +40,10 @@ func (optr *Operator) syncAvailableUpdates(config *configv1.ClusterVersion) erro
 		return nil
 	}
 
-	proxyURL, cmNameRef, err := optr.getHTTPSProxyURL()
+	proxyURL, tlsConfig, err := optr.getTransportOpts()
 	if err != nil {
 		return err
 	}
-	if cmNameRef != "" {
-		tlsConfig, err = optr.getTLSConfig(cmNameRef)
-		if err != nil {
-			return err
-		}
-	}
 
 	updates, condition := calculateAvailableUpdatesStatus(string(config.Spec.ClusterID), proxyURL, tlsConfig, upstream, arch, channel, optr.releaseVersion)
 

pkg/cvo/cvo.go

@@ -2,7 +2,10 @@ package cvo
 
 import (
 	"context"
+	"crypto/tls"
 	"fmt"
+	"net/http"
+	"net/url"
 	"strconv"
 	"sync"
 	"time"
@@ -22,6 +25,7 @@ import (
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/client-go/transport"
 	"k8s.io/client-go/util/workqueue"
 	"k8s.io/klog"
 
@@ -212,6 +216,16 @@ func New(
 	return optr
 }
 
+// verifyClientBuilder is a wrapper around the operator's HTTPClient method.
+// It is used by the releaseVerifier to get an up-to-date http client.
+type verifyClientBuilder struct {
+	builder func() (*http.Client, error)
+}
+
+func (vcb *verifyClientBuilder) HTTPClient() (*http.Client, error) {
+	return vcb.builder()
+}
+
 // InitializeFromPayload retrieves the payload contents and verifies the initial state, then configures the
 // controller that loads and applies content to the cluster. It returns an error if the payload appears to
 // be in error rather than continuing.
@@ -228,8 +242,11 @@ func (optr *Operator) InitializeFromPayload(restConfig *rest.Config, burstRestCo
 	optr.releaseCreated = update.ImageRef.CreationTimestamp.Time
 	optr.releaseVersion = update.ImageRef.Name
 
+	// Wraps operator's HTTPClient method to allow releaseVerifier to create http client with up-to-date config.
+	clientBuilder := &verifyClientBuilder{builder: optr.HTTPClient}
+
 	// attempt to load a verifier as defined in the payload
-	verifier, err := verify.LoadFromPayload(update)
+	verifier, err := verify.LoadFromPayload(update, clientBuilder)
 	if err != nil {
 		return err
 	}
@@ -660,3 +677,42 @@ func (optr *Operator) defaultPreconditionChecks() precondition.List {
 		preconditioncv.NewUpgradeable(optr.cvLister),
 	}
 }
+
+// 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()
+	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 {
+		return nil, err
+	}
+	return &http.Client{
+		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, cmNameRef, err := optr.getHTTPSProxyURL()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	var tlsConfig *tls.Config
+	if cmNameRef != "" {
+		tlsConfig, err = optr.getTLSConfig(cmNameRef)
+		if err != nil {
+			return nil, nil, err
+		}
+	}
+	return proxyURL, tlsConfig, nil
+}

pkg/verify/verify.go

@@ -18,13 +18,11 @@ import (
 	"strings"
 	"time"
 
-	"k8s.io/klog"
 	"github.com/pkg/errors"
 	"golang.org/x/crypto/openpgp"
-
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/client-go/transport"
+	"k8s.io/klog"
 
 	"github.com/openshift/cluster-version-operator/pkg/payload"
 )
@@ -74,7 +72,7 @@ var Reject Interface = rejectVerifier{}
 // if each provided public key has signed the release image digest. The signature may be in any
 // store and the lookup order is internally defined.
 //
-func LoadFromPayload(update *payload.Update) (Interface, error) {
+func LoadFromPayload(update *payload.Update, clientBuilder ClientBuilder) (Interface, error) {
 	configMapGVK := corev1.SchemeGroupVersion.WithKind("ConfigMap")
 	for _, manifest := range update.Manifests {
 		if manifest.GVK != configMapGVK {
@@ -117,16 +115,10 @@ func LoadFromPayload(update *payload.Update) (Interface, error) {
 			return nil, fmt.Errorf("%s did not provide any GPG public keys to verify signatures from and cannot be used", src)
 		}
 
-		transport, err := transport.New(&transport.Config{})
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to create HTTP client for verifying signatures: %v", err)
-		}
 		return &releaseVerifier{
-			verifiers: verifiers,
-			stores:    stores,
-			client: &http.Client{
-				Transport: transport,
-			},
+			verifiers:     verifiers,
+			stores:        stores,
+			clientBuilder: clientBuilder,
 		}, nil
 	}
 	return nil, nil
@@ -148,9 +140,23 @@ var validReleaseDigest = regexp.MustCompile(`^[a-zA-Z0-9:]+$`)
 // digest - all verifiers must have at least one valid signature attesting the release
 // digest. If any failure occurs the caller should assume the content is unverified.
 type releaseVerifier struct {
-	verifiers map[string]openpgp.EntityList
-	stores    []*url.URL
-	client    *http.Client
+	verifiers     map[string]openpgp.EntityList
+	stores        []*url.URL
+	clientBuilder ClientBuilder
+}
+
+// ClientBuilder provides a method for generating an HTTP Client configured
+// with cluster proxy settings, if they exist.
+type ClientBuilder interface {
+	HTTPClient() (*http.Client, error)
+}
+
+// simpleClientBuilder implements the ClientBuilder interface and should be used for testing.
+type simpleClientBuilder struct{}
+
+// HTTPClient from simpleClientBuilder creates an httpClient with no configuration.
+func (s *simpleClientBuilder) HTTPClient() (*http.Client, error) {
+	return &http.Client{}, nil
 }
 
 // String summarizes the verifier for human consumption
@@ -243,6 +249,11 @@ func (v *releaseVerifier) Verify(ctx context.Context, releaseDigest string) erro
 		return len(remaining) > 0, nil
 	}
 
+	client, err := v.clientBuilder.HTTPClient()
+	if err != nil {
+		return err
+	}
+
 	for _, store := range v.stores {
 		if len(remaining) == 0 {
 			break
@@ -256,7 +267,7 @@ func (v *releaseVerifier) Verify(ctx context.Context, releaseDigest string) erro
 		case "http", "https":
 			copied := *store
 			copied.Path = path.Join(store.Path, name)
-			if err := checkHTTPSignatures(ctx, v.client, copied, maxSignatureSearch, verifier); err != nil {
+			if err := checkHTTPSignatures(ctx, client, copied, maxSignatureSearch, verifier); err != nil {
 				return err
 			}
 		default:
@@ -325,7 +336,6 @@ func checkHTTPSignatures(ctx context.Context, client *http.Client, u url.URL, ma
 			return fmt.Errorf("could not build request to check signature: %v", err)
 		}
 		req = req.WithContext(ctx)
-
 		// load the body, being careful not to allow unbounded reads
 		resp, err := client.Do(req)
 		if err != nil {

pkg/verify/verify_test.go

@@ -10,8 +10,6 @@ import (
 	"path/filepath"
 	"testing"
 
-	"k8s.io/client-go/transport"
-
 	"github.com/openshift/cluster-version-operator/lib"
 	"github.com/openshift/cluster-version-operator/pkg/payload"
 	"golang.org/x/crypto/openpgp"
@@ -161,16 +159,13 @@ func Test_releaseVerifier_Verify(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			transport, err := transport.New(&transport.Config{})
 			if err != nil {
 				t.Fatal(err)
 			}
 			v := &releaseVerifier{
-				verifiers: tt.verifiers,
-				stores:    tt.stores,
-				client: &http.Client{
-					Transport: transport,
-				},
+				verifiers:     tt.verifiers,
+				stores:        tt.stores,
+				clientBuilder: &simpleClientBuilder{},
 			}
 			if err := v.Verify(context.Background(), tt.releaseDigest); (err != nil) != tt.wantErr {
 				t.Errorf("releaseVerifier.Verify() error = %v, wantErr %v", err, tt.wantErr)
@@ -344,7 +339,7 @@ func Test_loadReleaseVerifierFromPayload(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := LoadFromPayload(tt.update)
+			got, err := LoadFromPayload(tt.update, &simpleClientBuilder{})
 			if (err != nil) != tt.wantErr {
 				t.Fatalf("loadReleaseVerifierFromPayload() error = %v, wantErr %v", err, tt.wantErr)
 				return