Merge pull request #238 from stlaz/server_cert_fails

Bug 2026860: ocp provider: don't fail client creation if oauth-server cert is not present

Author: openshift-merge-robot

providers/openshift/provider.go

@@ -27,6 +27,7 @@ import (
 
 	authenticationv1 "k8s.io/api/authentication/v1"
 	authorizationv1 "k8s.io/api/authorization/v1"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -157,12 +158,16 @@ func (p *OpenShiftProvider) newOpenShiftClient() (*http.Client, error) {
 		return nil, err
 	}
 
+	cachedKey := metadataHash
+
 	oauthServerCert, err := p.configMapLister.ConfigMaps("openshift-config-managed").Get("oauth-serving-cert")
-	if err != nil {
+	if err != nil && !apierrors.IsNotFound(err) {
 		return nil, err
 	}
+	if oauthServerCert != nil {
+		cachedKey += oauthServerCert.ResourceVersion
+	}
 
-	cachedKey := metadataHash + oauthServerCert.ResourceVersion
 	if httpClient, ok := p.httpClientCache.Load(cachedKey); ok {
 		return httpClient.(*http.Client), nil
 	}
@@ -173,8 +178,10 @@ func (p *OpenShiftProvider) newOpenShiftClient() (*http.Client, error) {
 		return nil, err
 	}
 
-	if ok := pool.AppendCertsFromPEM([]byte(oauthServerCert.Data["ca-bundle.crt"])); !ok {
-		log.Println("failed to add the oauth-server certificate to the OpenShift client CA bundle")
+	if oauthServerCert != nil {
+		if ok := pool.AppendCertsFromPEM([]byte(oauthServerCert.Data["ca-bundle.crt"])); !ok {
+			log.Println("failed to add the oauth-server certificate to the OpenShift client CA bundle")
+		}
 	}
 
 	httpClient := &http.Client{

providers/openshift/provider_test.go

@@ -154,14 +154,6 @@ func TestNewOpenShiftClient(t *testing.T) {
 	}
 
 	indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
-	err = indexer.Add(&corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "oauth-serving-cert",
-			Namespace: "openshift-config-managed",
-		},
-	})
-	require.NoError(t, err)
-
 	p := &OpenShiftProvider{
 		configMapLister: corev1listers.NewConfigMapLister(indexer),
 	}
@@ -171,14 +163,35 @@ func TestNewOpenShiftClient(t *testing.T) {
 	p.authorizer = &mockAuthorizer{}
 	p.SetReviewCAs([]string{tmpfile.Name()})
 
+	// missing oauth serving cert should not cause failures
+	noServerCertClient, err := p.newOpenShiftClient()
+	if err != nil {
+		t.Fatalf("failed to create an OpenShift Client: %v", err)
+	}
+
+	err = indexer.Add(&corev1.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			ResourceVersion: "someVersion",
+			Name:            "oauth-serving-cert",
+			Namespace:       "openshift-config-managed",
+		},
+	})
+	require.NoError(t, err)
+	p.configMapLister = corev1listers.NewConfigMapLister(indexer)
+
 	client, err := p.newOpenShiftClient()
 	if err != nil {
-		t.Fatalf("failed to create an OpenShift Client")
+		t.Fatalf("failed to create an OpenShift Client: %v", err)
+	}
+
+	if noServerCertClient == client {
+		p.httpClientCache.Range(func(key, _ interface{}) bool { t.Logf("%s", key); return true })
+		t.Fatalf("clients should be different when the oauth-server cert is present compared to when it isn't")
 	}
 
 	newClient, err := p.newOpenShiftClient()
 	if err != nil {
-		t.Fatalf("failed to create a new OpenShift Client")
+		t.Fatalf("failed to create a new OpenShift Client: %v", err)
 	}
 
 	// caching should make sure the clients are the same