pkg/cvo: Fetch proxy CA certs from openshift-config-managed/trusted-c…

…a-bundle

The API docs [1] recommend avoiding trustedCA (unless you happen to be
the "proxy validator") and instead pulling the trust bundle from this
managed namespace.  The docs also explain that the trusted-ca-bundle
ConfigMap has already been merged with the system certificates, so we
don't need to inject those locally.  If trusted-ca-bundle doesn't
exist, we'll fall back to our local system store.

Our logic was touched most recently in 5968cdf (pkg: switch to
openshift-config for proxy CA, 2019-08-07, openshift#231), which resolved the
"looking in the wrong place" issue by looking at the trustedCA source
(which feeds the proxy validator).  With this commit we switch that
around and look at the proxy validator's output.

[1]: https://github.com/openshift/api/blob/f2a771e1a90ceb4e65f1ca2c8b11fc1ac6a66da8/config/v1/types_proxy.go#L44-L52

pkg/cvo/cvo.go

@@ -115,11 +115,12 @@ type Operator struct {
 	// syncBackoff allows the tests to use a quicker backoff
 	syncBackoff wait.Backoff
 
-	cvLister       configlistersv1.ClusterVersionLister
-	coLister       configlistersv1.ClusterOperatorLister
-	cmConfigLister listerscorev1.ConfigMapNamespaceLister
-	proxyLister    configlistersv1.ProxyLister
-	cacheSynced    []cache.InformerSynced
+	cvLister              configlistersv1.ClusterVersionLister
+	coLister              configlistersv1.ClusterOperatorLister
+	cmConfigLister        listerscorev1.ConfigMapNamespaceLister
+	cmConfigManagedLister listerscorev1.ConfigMapNamespaceLister
+	proxyLister           configlistersv1.ProxyLister
+	cacheSynced           []cache.InformerSynced
 
 	// queue tracks applying updates to a cluster.
 	queue workqueue.RateLimitingInterface
@@ -212,6 +213,7 @@ func New(
 
 	optr.proxyLister = proxyInformer.Lister()
 	optr.cmConfigLister = cmConfigInformer.Lister().ConfigMaps(internal.ConfigNamespace)
+	optr.cmConfigManagedLister = cmConfigInformer.Lister().ConfigMaps(internal.ConfigManagedNamespace)
 
 	// make sure this is initialized after all the listers are initialized
 	optr.upgradeableChecks = optr.defaultUpgradeableChecks()
@@ -743,17 +745,15 @@ func (optr *Operator) HTTPClient() (*http.Client, error) {
 // 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()
+	proxyURL, 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
-		}
+	tlsConfig, err = optr.getTLSConfig()
+	if err != nil {
+		return nil, nil, err
 	}
 	return proxyURL, tlsConfig, nil
 }

pkg/cvo/cvo_test.go

@@ -2628,6 +2628,7 @@ func TestOperator_availableUpdatesSync(t *testing.T) {
 			optr.proxyLister = &clientProxyLister{client: optr.client}
 			optr.coLister = &clientCOLister{client: optr.client}
 			optr.cvLister = &clientCVLister{client: optr.client}
+			optr.cmConfigManagedLister = &cmConfigLister{}
 			optr.eventRecorder = record.NewFakeRecorder(100)
 
 			if tt.handler != nil {

pkg/cvo/egress.go

@@ -11,39 +11,38 @@ import (
 
 // 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, string, error) {
+func (optr *Operator) getHTTPSProxyURL() (*url.URL, error) {
 	proxy, err := optr.proxyLister.Get("cluster")
 
 	if errors.IsNotFound(err) {
-		return nil, "", nil
+		return nil, nil
 	}
 	if err != nil {
-		return nil, "", err
+		return nil, err
 	}
 
 	if &proxy.Spec != nil {
 		if proxy.Spec.HTTPSProxy != "" {
 			proxyURL, err := url.Parse(proxy.Spec.HTTPSProxy)
 			if err != nil {
-				return nil, "", err
+				return nil, err
 			}
-			return proxyURL, proxy.Spec.TrustedCA.Name, nil
+			return proxyURL, nil
 		}
 	}
-	return nil, "", nil
+	return nil, nil
 }
 
-func (optr *Operator) getTLSConfig(cmNameRef string) (*tls.Config, error) {
-	cm, err := optr.cmConfigLister.Get(cmNameRef)
-
+func (optr *Operator) getTLSConfig() (*tls.Config, error) {
+	cm, err := optr.cmConfigManagedLister.Get("trusted-ca-bundle")
+	if errors.IsNotFound(err) {
+		return nil, nil
+	}
 	if err != nil {
 		return nil, err
 	}
 
-	certPool, _ := x509.SystemCertPool()
-	if certPool == nil {
-		certPool = x509.NewCertPool()
-	}
+	certPool := x509.NewCertPool()
 
 	if cm.Data["ca-bundle.crt"] != "" {
 		if ok := certPool.AppendCertsFromPEM([]byte(cm.Data["ca-bundle.crt"])); !ok {

pkg/internal/constants.go

@@ -1,7 +1,8 @@
 package internal
 
 const (
-	ConfigNamespace    = "openshift-config"
-	InstallerConfigMap = "openshift-install"
-	ManifestsConfigMap = "openshift-install-manifests"
+	ConfigNamespace        = "openshift-config"
+	ConfigManagedNamespace = "openshift-config-managed"
+	InstallerConfigMap     = "openshift-install"
+	ManifestsConfigMap     = "openshift-install-manifests"
 )