Add controller support for spec.tls field.

Signed-off-by: Matt Moyer <moyerm@vmware.com>

internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher.go

@@ -6,7 +6,11 @@ package upstreamwatcher
 
 import (
 	"context"
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/base64"
 	"fmt"
+	"net/http"
 	"net/url"
 	"sort"
 	"time"
@@ -48,10 +52,12 @@ const (
 	reasonMissingKeys          = "SecretMissingKeys"
 	reasonSuccess              = "Success"
 	reasonUnreachable          = "Unreachable"
+	reasonInvalidTLSConfig     = "InvalidTLSConfig"
 	reasonInvalidResponse      = "InvalidResponse"
 
 	// Errors that are generated by our reconcile process.
-	errFailureStatus = constable.Error("UpstreamOIDCProvider has a failing condition")
+	errFailureStatus  = constable.Error("UpstreamOIDCProvider has a failing condition")
+	errNoCertificates = constable.Error("no certificates found")
 )
 
 // IDPCache is a thread safe cache that holds a list of validated upstream OIDC IDP configurations.
@@ -59,13 +65,39 @@ type IDPCache interface {
 	SetIDPList([]provider.UpstreamOIDCIdentityProvider)
 }
 
+// lruValidatorCache caches the *oidc.Provider associated with a particular issuer/TLS configuration.
+type lruValidatorCache struct{ cache *cache.Expiring }
+
+func (c *lruValidatorCache) getProvider(spec *v1alpha1.UpstreamOIDCProviderSpec) *oidc.Provider {
+	if result, ok := c.cache.Get(c.cacheKey(spec)); ok {
+		return result.(*oidc.Provider)
+	}
+	return nil
+}
+
+func (c *lruValidatorCache) putProvider(spec *v1alpha1.UpstreamOIDCProviderSpec, provider *oidc.Provider) {
+	c.cache.Set(c.cacheKey(spec), provider, validatorCacheTTL)
+}
+
+func (c *lruValidatorCache) cacheKey(spec *v1alpha1.UpstreamOIDCProviderSpec) interface{} {
+	var key struct{ issuer, caBundle string }
+	key.issuer = spec.Issuer
+	if spec.TLS != nil {
+		key.caBundle = spec.TLS.CertificateAuthorityData
+	}
+	return key
+}
+
 type controller struct {
 	cache          IDPCache
 	log            logr.Logger
 	client         pinnipedclientset.Interface
 	providers      idpinformers.UpstreamOIDCProviderInformer
 	secrets        corev1informers.SecretInformer
-	validatorCache *cache.Expiring
+	validatorCache interface {
+		getProvider(spec *v1alpha1.UpstreamOIDCProviderSpec) *oidc.Provider
+		putProvider(spec *v1alpha1.UpstreamOIDCProviderSpec, provider *oidc.Provider)
+	}
 }
 
 // New instantiates a new controllerlib.Controller which will populate the provided IDPCache.
@@ -82,7 +114,7 @@ func New(
 		client:         client,
 		providers:      providers,
 		secrets:        secrets,
-		validatorCache: cache.NewExpiring(),
+		validatorCache: &lruValidatorCache{cache: cache.NewExpiring()},
 	}
 	filter := pinnipedcontroller.MatchAnythingFilter(pinnipedcontroller.SingletonQueue())
 	return controllerlib.New(
@@ -197,15 +229,22 @@ func (c *controller) validateSecret(upstream *v1alpha1.UpstreamOIDCProvider, res
 // validateIssuer validates the .spec.issuer field, performs OIDC discovery, and returns the appropriate OIDCDiscoverySucceeded condition.
 func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, result *provider.UpstreamOIDCIdentityProvider) *v1alpha1.Condition {
 	// Get the provider (from cache if possible).
-	var discoveredProvider *oidc.Provider
+	discoveredProvider := c.validatorCache.getProvider(&upstream.Spec)
-	if cached, ok := c.validatorCache.Get(upstream.Spec.Issuer); ok {
-		discoveredProvider = cached.(*oidc.Provider)
-	}
 
 	// If the provider does not exist in the cache, do a fresh discovery lookup and save to the cache.
 	if discoveredProvider == nil {
-		var err error
+		tlsConfig, err := getTLSConfig(upstream)
-		discoveredProvider, err = oidc.NewProvider(ctx, upstream.Spec.Issuer)
+		if err != nil {
+			return &v1alpha1.Condition{
+				Type:    typeOIDCDiscoverySucceeded,
+				Status:  v1alpha1.ConditionFalse,
+				Reason:  reasonInvalidTLSConfig,
+				Message: err.Error(),
+			}
+		}
+		httpClient := &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}}
+
+		discoveredProvider, err = oidc.NewProvider(oidc.ClientContext(ctx, httpClient), upstream.Spec.Issuer)
 		if err != nil {
 			return &v1alpha1.Condition{
 				Type:    typeOIDCDiscoverySucceeded,
@@ -216,7 +255,7 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.Upst
 		}
 
 		// Update the cache with the newly discovered value.
-		c.validatorCache.Set(upstream.Spec.Issuer, discoveredProvider, validatorCacheTTL)
+		c.validatorCache.putProvider(&upstream.Spec, discoveredProvider)
 	}
 
 	// Parse out and validate the discovered authorize endpoint.
@@ -248,6 +287,28 @@ func (c *controller) validateIssuer(ctx context.Context, upstream *v1alpha1.Upst
 	}
 }
 
+func getTLSConfig(upstream *v1alpha1.UpstreamOIDCProvider) (*tls.Config, error) {
+	result := tls.Config{
+		MinVersion: tls.VersionTLS12,
+	}
+
+	if upstream.Spec.TLS == nil || upstream.Spec.TLS.CertificateAuthorityData == "" {
+		return &result, nil
+	}
+
+	bundle, err := base64.StdEncoding.DecodeString(upstream.Spec.TLS.CertificateAuthorityData)
+	if err != nil {
+		return nil, fmt.Errorf("spec.certificateAuthorityData is invalid: %w", err)
+	}
+
+	result.RootCAs = x509.NewCertPool()
+	if !result.RootCAs.AppendCertsFromPEM(bundle) {
+		return nil, fmt.Errorf("spec.certificateAuthorityData is invalid: %w", errNoCertificates)
+	}
+
+	return &result, nil
+}
+
 func (c *controller) updateStatus(ctx context.Context, upstream *v1alpha1.UpstreamOIDCProvider, conditions []*v1alpha1.Condition) {
 	log := c.log.WithValues("namespace", upstream.Namespace, "name", upstream.Name)
 	updated := upstream.DeepCopy()

internal/controller/supervisorconfig/upstreamwatcher/upstreamwatcher_test.go

@@ -5,9 +5,9 @@ package upstreamwatcher
 
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"net/http"
-	"net/http/httptest"
 	"net/url"
 	"strings"
 	"testing"
@@ -25,6 +25,7 @@ import (
 	pinnipedinformers "go.pinniped.dev/generated/1.19/client/supervisor/informers/externalversions"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/oidc/provider"
+	"go.pinniped.dev/internal/testutil"
 	"go.pinniped.dev/internal/testutil/testlogger"
 )
 
@@ -34,7 +35,8 @@ func TestController(t *testing.T) {
 	earlier := metav1.NewTime(now.Add(-1 * time.Hour).UTC())
 
 	// Start another test server that answers discovery successfully.
-	testIssuer := newTestIssuer(t)
+	testIssuerCA, testIssuerURL := newTestIssuer(t)
+	testIssuerCABase64 := base64.StdEncoding.EncodeToString([]byte(testIssuerCA))
 	testIssuerAuthorizeURL, err := url.Parse("https://example.com/authorize")
 	require.NoError(t, err)
 
@@ -65,7 +67,8 @@ func TestController(t *testing.T) {
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
 				Spec: v1alpha1.UpstreamOIDCProviderSpec{
-					Issuer:              testIssuer.URL,
+					Issuer:              testIssuerURL,
+					TLS:                 &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
 					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
 					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
 				},
@@ -106,7 +109,8 @@ func TestController(t *testing.T) {
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
 				Spec: v1alpha1.UpstreamOIDCProviderSpec{
-					Issuer:              testIssuer.URL,
+					Issuer:              testIssuerURL,
+					TLS:                 &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
 					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
 					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
 				},
@@ -151,7 +155,8 @@ func TestController(t *testing.T) {
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
 				Spec: v1alpha1.UpstreamOIDCProviderSpec{
-					Issuer:              testIssuer.URL,
+					Issuer:              testIssuerURL,
+					TLS:                 &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
 					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
 					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
 				},
@@ -190,6 +195,102 @@ func TestController(t *testing.T) {
 				},
 			}},
 		},
+		{
+			name: "TLS CA bundle is invalid base64",
+			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"},
+				Spec: v1alpha1.UpstreamOIDCProviderSpec{
+					Issuer: testIssuerURL,
+					TLS: &v1alpha1.TLSSpec{
+						CertificateAuthorityData: "invalid-base64",
+					},
+					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
+					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")},
+				},
+			}},
+			inputSecrets: []runtime.Object{&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName},
+				Type:       "secrets.pinniped.dev/oidc-client",
+				Data:       testValidSecretData,
+			}},
+			wantErr: controllerlib.ErrSyntheticRequeue.Error(),
+			wantLogs: []string{
+				`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
+				`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7" "reason"="InvalidTLSConfig" "status"="False" "type"="OIDCDiscoverySucceeded"`,
+				`upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`,
+			},
+			wantResultingCache: []provider.UpstreamOIDCIdentityProvider{},
+			wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
+				Status: v1alpha1.UpstreamOIDCProviderStatus{
+					Phase: "Error",
+					Conditions: []v1alpha1.Condition{
+						{
+							Type:               "ClientCredentialsValid",
+							Status:             "True",
+							LastTransitionTime: now,
+							Reason:             "Success",
+							Message:            "loaded client credentials",
+						},
+						{
+							Type:               "OIDCDiscoverySucceeded",
+							Status:             "False",
+							LastTransitionTime: now,
+							Reason:             "InvalidTLSConfig",
+							Message:            `spec.certificateAuthorityData is invalid: illegal base64 data at input byte 7`,
+						},
+					},
+				},
+			}},
+		},
+		{
+			name: "TLS CA bundle does not have any certificates",
+			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"},
+				Spec: v1alpha1.UpstreamOIDCProviderSpec{
+					Issuer: testIssuerURL,
+					TLS: &v1alpha1.TLSSpec{
+						CertificateAuthorityData: base64.StdEncoding.EncodeToString([]byte("not-a-pem-ca-bundle")),
+					},
+					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
+					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")},
+				},
+			}},
+			inputSecrets: []runtime.Object{&corev1.Secret{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testSecretName},
+				Type:       "secrets.pinniped.dev/oidc-client",
+				Data:       testValidSecretData,
+			}},
+			wantErr: controllerlib.ErrSyntheticRequeue.Error(),
+			wantLogs: []string{
+				`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="loaded client credentials" "reason"="Success" "status"="True" "type"="ClientCredentialsValid"`,
+				`upstream-observer "level"=0 "msg"="updated condition" "name"="test-name" "namespace"="test-namespace" "message"="spec.certificateAuthorityData is invalid: no certificates found" "reason"="InvalidTLSConfig" "status"="False" "type"="OIDCDiscoverySucceeded"`,
+				`upstream-observer "error"="UpstreamOIDCProvider has a failing condition" "msg"="found failing condition" "message"="spec.certificateAuthorityData is invalid: no certificates found" "name"="test-name" "namespace"="test-namespace" "reason"="InvalidTLSConfig" "type"="OIDCDiscoverySucceeded"`,
+			},
+			wantResultingCache: []provider.UpstreamOIDCIdentityProvider{},
+			wantResultingUpstreams: []v1alpha1.UpstreamOIDCProvider{{
+				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
+				Status: v1alpha1.UpstreamOIDCProviderStatus{
+					Phase: "Error",
+					Conditions: []v1alpha1.Condition{
+						{
+							Type:               "ClientCredentialsValid",
+							Status:             "True",
+							LastTransitionTime: now,
+							Reason:             "Success",
+							Message:            "loaded client credentials",
+						},
+						{
+							Type:               "OIDCDiscoverySucceeded",
+							Status:             "False",
+							LastTransitionTime: now,
+							Reason:             "InvalidTLSConfig",
+							Message:            `spec.certificateAuthorityData is invalid: no certificates found`,
+						},
+					},
+				},
+			}},
+		},
 		{
 			name: "issuer is invalid URL",
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
@@ -240,7 +341,8 @@ func TestController(t *testing.T) {
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
 				Spec: v1alpha1.UpstreamOIDCProviderSpec{
-					Issuer:              testIssuer.URL + "/invalid",
+					Issuer:              testIssuerURL + "/invalid",
+					TLS:                 &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
 					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
 					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
 				},
@@ -285,7 +387,8 @@ func TestController(t *testing.T) {
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName},
 				Spec: v1alpha1.UpstreamOIDCProviderSpec{
-					Issuer:              testIssuer.URL + "/insecure",
+					Issuer:              testIssuerURL + "/insecure",
+					TLS:                 &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
 					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
 					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
 				},
@@ -330,7 +433,8 @@ func TestController(t *testing.T) {
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: "test-name"},
 				Spec: v1alpha1.UpstreamOIDCProviderSpec{
-					Issuer:              testIssuer.URL,
+					Issuer:              testIssuerURL,
+					TLS:                 &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
 					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
 					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: append(testAdditionalScopes, "xyz", "openid")},
 				},
@@ -373,7 +477,8 @@ func TestController(t *testing.T) {
 			inputUpstreams: []runtime.Object{&v1alpha1.UpstreamOIDCProvider{
 				ObjectMeta: metav1.ObjectMeta{Namespace: testNamespace, Name: testName, Generation: 1234},
 				Spec: v1alpha1.UpstreamOIDCProviderSpec{
-					Issuer:              testIssuer.URL,
+					Issuer:              testIssuerURL,
+					TLS:                 &v1alpha1.TLSSpec{CertificateAuthorityData: testIssuerCABase64},
 					Client:              v1alpha1.OIDCClient{SecretName: testSecretName},
 					AuthorizationConfig: v1alpha1.OIDCAuthorizationConfig{AdditionalScopes: testAdditionalScopes},
 				},
@@ -486,10 +591,9 @@ func normalizeUpstreams(upstreams []v1alpha1.UpstreamOIDCProvider, now metav1.Ti
 	return result
 }
 
-func newTestIssuer(t *testing.T) *httptest.Server {
+func newTestIssuer(t *testing.T) (string, string) {
 	mux := http.NewServeMux()
-	testServer := httptest.NewServer(mux)
+	caBundlePEM, testURL := testutil.TLSTestServer(t, mux.ServeHTTP)
-	t.Cleanup(testServer.Close)
 
 	type providerJSON struct {
 		Issuer   string `json:"issuer"`
@@ -502,7 +606,7 @@ func newTestIssuer(t *testing.T) *httptest.Server {
 	mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("content-type", "application/json")
 		_ = json.NewEncoder(w).Encode(&providerJSON{
-			Issuer:  testServer.URL,
+			Issuer:  testURL,
 			AuthURL: "https://example.com/authorize",
 		})
 	})
@@ -511,7 +615,7 @@ func newTestIssuer(t *testing.T) *httptest.Server {
 	mux.HandleFunc("/invalid/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("content-type", "application/json")
 		_ = json.NewEncoder(w).Encode(&providerJSON{
-			Issuer:  testServer.URL + "/invalid",
+			Issuer:  testURL + "/invalid",
 			AuthURL: "%",
 		})
 	})
@@ -520,10 +624,10 @@ func newTestIssuer(t *testing.T) *httptest.Server {
 	mux.HandleFunc("/insecure/.well-known/openid-configuration", func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("content-type", "application/json")
 		_ = json.NewEncoder(w).Encode(&providerJSON{
-			Issuer:  testServer.URL + "/insecure",
+			Issuer:  testURL + "/insecure",
 			AuthURL: "http://example.com/authorize",
 		})
 	})
 
-	return testServer
+	return caBundlePEM, testURL
 }