Merge pull request #1052 from petr-muller/revert-1030-custom-signature-stores

OCPBUGS-35236: Revert: Add support for Custom Certificate Authorities for custom signature stores"

Author: openshift-merge-bot[bot]

pkg/customsignaturestore/customsignaturestore.go

@@ -5,46 +5,47 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"net/http"
 	"net/url"
 	"strings"
 	"sync"
 
-	configv1 "github.com/openshift/api/config/v1"
 	configv1listers "github.com/openshift/client-go/config/listers/config/v1"
 	"github.com/openshift/library-go/pkg/verify/store"
 	"github.com/openshift/library-go/pkg/verify/store/parallel"
 	"github.com/openshift/library-go/pkg/verify/store/sigstore"
-	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/klog/v2"
 )
 
 type Store struct {
 	// Name is the name of the ClusterVersion object that configures this store.
 	Name string
 
-	// ClusterVersionLister allows the store to fetch the current ClusterVersion configuration.
-	ClusterVersionLister configv1listers.ClusterVersionLister
+	// Lister allows the store to fetch the current ClusterVersion configuration.
+	Lister configv1listers.ClusterVersionLister
 
-	// HTTPClient construct which respects the customstore CA certs and cluster proxy configuration
-	HTTPClient func(string) (*http.Client, error)
+	// HTTPClient is called once for each Signatures call to ensure
+	// requests are made with the currently-recommended parameters.
+	HTTPClient sigstore.HTTPClient
 
 	// lock allows the store to be locked while mutating or accessing internal state.
 	lock sync.Mutex
 
-	// customStores tracks the most-recently retrieved ClusterVersion configuration.
-	customStores []configv1.SignatureStore
+	// customURIs tracks the most-recently retrieved ClusterVersion configuration.
+	customURIs []*url.URL
 }
 
 // Signatures fetches signatures for the provided digest.
 func (s *Store) Signatures(ctx context.Context, name string, digest string, fn store.Callback) error {
-	customStores, err := s.refreshConfiguration()
+	uris, err := s.refreshConfiguration(ctx)
 	if err != nil {
 		return err
-	} else if customStores == nil {
+	}
+
+	if uris == nil {
 		return nil
-	} else if len(customStores) == 0 {
-		return errors.New("ClusterVersion spec.signatureStores is an empty array. Unset signatureStores entirely if you want to enable the default signature stores")
+	}
+
+	if len(uris) == 0 {
+		return errors.New("ClusterVersion spec.signatureStores is an empty array.  Unset signatureStores entirely if you want to to enable the default signature stores.")
 	}
 
 	allDone := false
@@ -57,66 +58,44 @@ func (s *Store) Signatures(ctx context.Context, name string, digest string, fn s
 		return done, err
 	}
 
-	var errs []error
-	stores := make([]store.Store, 0, len(customStores))
-	for _, customStore := range customStores {
-		uri, err := url.Parse(customStore.URL)
-		if err != nil {
-			errs = append(errs, fmt.Errorf("failed to parse the ClusterVersion spec.signatureStores %w", err))
-			continue
-		}
-		newHttpClient, err := s.HTTPClient(customStore.CA.Name)
-		if err != nil {
-			errs = append(errs, fmt.Errorf("failed to process the ClusterVersion spec.signatureStores %w", err))
-			continue
-		}
-
+	stores := make([]store.Store, 0, len(uris))
+	for i := range uris {
+		uri := *uris[i]
 		stores = append(stores, &sigstore.Store{
-			URI:        uri,
-			HTTPClient: func() (*http.Client, error) { return newHttpClient, nil }})
-	}
-
-	if len(stores) == 0 {
-		return utilerrors.NewAggregate(errs)
+			URI:        &uri,
+			HTTPClient: s.HTTPClient,
+		})
 	}
 	store := &parallel.Store{Stores: stores}
-	if err := store.Signatures(ctx, name, digest, wrapper); allDone {
-		if len(errs) > 0 {
-			klog.V(2).Infof("%s", utilerrors.NewAggregate(errs))
-		}
-		return nil
-	} else if err != nil {
-		errs = append(errs, err)
-		return utilerrors.NewAggregate(errs)
+	if err := store.Signatures(ctx, name, digest, wrapper); err != nil || allDone {
+		return err
 	}
-
-	errs = append(errs, errors.New("ClusterVersion spec.signatureStores exhausted without finding a valid signature"))
-	return utilerrors.NewAggregate(errs)
+	return errors.New("ClusterVersion spec.signatureStores exhausted without finding a valid signature.")
 }
 
-// refreshConfiguration retrieves the latest configuration from the ClusterVersionLister
-// and updates the customStores with the URL and CA information from the retrieved configuration.
-// It returns the updated customStores slice and any error encountered during the retrieval process.
-func (s *Store) refreshConfiguration() ([]configv1.SignatureStore, error) {
-
-	config, err := s.ClusterVersionLister.Get(s.Name)
+func (s *Store) refreshConfiguration(ctx context.Context) ([]*url.URL, error) {
+	config, err := s.Lister.Get(s.Name)
 	if err != nil {
 		return nil, err
 	}
 
-	var customStores = make([]configv1.SignatureStore, 0, len(config.Spec.SignatureStores))
+	var uris []*url.URL
 	if config.Spec.SignatureStores != nil {
+		uris = make([]*url.URL, 0, len(config.Spec.SignatureStores))
 		for _, store := range config.Spec.SignatureStores {
-			url := store.URL
-			caCert := store.CA
-			customStores = append(customStores, configv1.SignatureStore{URL: url, CA: caCert})
+			uri, err := url.Parse(store.URL)
+			if err != nil {
+				return uris, err
+			}
+
+			uris = append(uris, uri)
 		}
 	}
 
 	s.lock.Lock()
 	defer s.lock.Unlock()
-	s.customStores = customStores
-	return customStores, nil
+	s.customURIs = uris
+	return uris, nil
 }
 
 // String returns a description of where this store finds
@@ -125,14 +104,14 @@ func (s *Store) String() string {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
-	if s.customStores == nil {
-		return "ClusterVersion signatureStores not set, falling back to default stores"
-	} else if len(s.customStores) == 0 {
+	if s.customURIs == nil {
+		return "ClusterVersion signatureStores unset, falling back to default stores"
+	} else if len(s.customURIs) == 0 {
 		return "0 ClusterVersion signatureStores"
 	}
-	customStores := make([]string, 0, len(s.customStores))
-	for _, customStore := range s.customStores {
-		customStores = append(customStores, customStore.URL)
+	uris := make([]string, 0, len(s.customURIs))
+	for _, uri := range s.customURIs {
+		uris = append(uris, uri.String())
 	}
-	return fmt.Sprintf("ClusterVersion signatureStores: %s", strings.Join(customStores, ", "))
+	return fmt.Sprintf("ClusterVersion signatureStores: %s", strings.Join(uris, ", "))
 }

pkg/cvo/availableupdates.go

@@ -93,7 +93,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
 	}
 
 	if needFreshFetch {
-		transport, err := optr.getTransport("")
+		transport, err := optr.getTransport()
 		if err != nil {
 			return err
 		}

pkg/cvo/cvo.go

@@ -294,16 +294,16 @@ func (optr *Operator) LoadInitialPayload(ctx context.Context, startingRequiredFe
 	if err != nil {
 		return nil, fmt.Errorf("the local release contents are invalid - no current version can be determined from disk: %v", err)
 	}
-	httpClientConstructor := sigstore.NewCachedHTTPClientConstructor(func() (*http.Client, error) { return optr.HTTPClient("") }, nil)
+	httpClientConstructor := sigstore.NewCachedHTTPClientConstructor(optr.HTTPClient, nil)
 	configClient, err := coreclientsetv1.NewForConfig(restConfig)
 	if err != nil {
 		return nil, fmt.Errorf("unable to create a configuration client: %v", err)
 	}
 
 	customSignatureStore := &customsignaturestore.Store{
-		Name:                 optr.name,
-		ClusterVersionLister: optr.cvLister,
-		HTTPClient:           optr.HTTPClient,
+		Lister:     optr.cvLister,
+		Name:       optr.name,
+		HTTPClient: httpClientConstructor.HTTPClient,
 	}
 
 	// attempt to load a verifier as defined in the payload
@@ -980,8 +980,8 @@ 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(caConfigMap string) (*http.Client, error) {
-	transportOption, err := optr.getTransport(caConfigMap)
+func (optr *Operator) HTTPClient() (*http.Client, error) {
+	transportOption, err := optr.getTransport()
 	if err != nil {
 		return nil, err
 	}

pkg/cvo/egress.go

@@ -11,9 +11,7 @@ import (
 	"golang.org/x/net/http/httpproxy"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
-	"github.com/openshift/cluster-version-operator/pkg/internal"
 	"github.com/openshift/cluster-version-operator/pkg/version"
-	corev1 "k8s.io/api/core/v1"
 )
 
 // Returns a User-Agent to be used for outgoing HTTP requests.
@@ -27,7 +25,7 @@ func (optr *Operator) getUserAgent() string {
 
 // getTransport constructs an HTTP transport configuration, including
 // any custom proxy configuration.
-func (optr *Operator) getTransport(caConfigMap string) (*http.Transport, error) {
+func (optr *Operator) getTransport() (*http.Transport, error) {
 	transport := &http.Transport{}
 
 	proxyConfig, err := optr.getProxyConfig()
@@ -43,7 +41,7 @@ func (optr *Operator) getTransport(caConfigMap string) (*http.Transport, error)
 		}
 	}
 
-	tlsConfig, err := optr.getTLSConfig(caConfigMap)
+	tlsConfig, err := optr.getTLSConfig()
 	if err != nil {
 		return transport, err
 	} else if tlsConfig != nil {
@@ -72,20 +70,8 @@ func (optr *Operator) getProxyConfig() (*httpproxy.Config, error) {
 	}, nil
 }
 
-func (optr *Operator) getTLSConfig(caConfigMap string) (*tls.Config, error) {
-	var namespace, key string
-	var cm *corev1.ConfigMap
-	var err error
-	if caConfigMap == "" {
-		namespace = internal.ConfigManagedNamespace
-		caConfigMap = "trusted-ca-bundle"
-		key = "ca-bundle.crt"
-		cm, err = optr.cmConfigManagedLister.Get(caConfigMap)
-	} else {
-		namespace = internal.ConfigNamespace
-		key = "ca.crt"
-		cm, err = optr.cmConfigLister.Get(caConfigMap)
-	}
+func (optr *Operator) getTLSConfig() (*tls.Config, error) {
+	cm, err := optr.cmConfigManagedLister.Get("trusted-ca-bundle")
 	if apierrors.IsNotFound(err) {
 		return nil, nil
 	}
@@ -95,9 +81,9 @@ func (optr *Operator) getTLSConfig(caConfigMap string) (*tls.Config, error) {
 
 	certPool := x509.NewCertPool()
 
-	if cm.Data[key] != "" {
-		if ok := certPool.AppendCertsFromPEM([]byte(cm.Data[key])); !ok {
-			return nil, fmt.Errorf("unable to add %s certificates from the %s ConfigMap in the %s namespace", key, caConfigMap, namespace)
+	if cm.Data["ca-bundle.crt"] != "" {
+		if ok := certPool.AppendCertsFromPEM([]byte(cm.Data["ca-bundle.crt"])); !ok {
+			return nil, fmt.Errorf("unable to add ca-bundle.crt certificates")
 		}
 	} else {
 		return nil, nil