Creation and deletion of OIDC Provider discovery endpoints from config

- The OIDCProviderConfigWatcherController synchronizes the
  OIDCProviderConfig settings to dynamically mount and unmount the
  OIDC discovery endpoints for each provider
- Integration test passes but unit tests need to be added still

cmd/pinniped-supervisor/main.go

@@ -23,8 +23,7 @@ import (
 	"go.pinniped.dev/internal/controller/supervisorconfig"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/downward"
-	"go.pinniped.dev/internal/oidc/discovery"
+	"go.pinniped.dev/internal/oidc/provider"
-	"go.pinniped.dev/internal/oidc/issuerprovider"
 )
 
 const (
@@ -32,10 +31,8 @@ const (
 	defaultResyncInterval = 3 * time.Minute
 )
 
-func start(ctx context.Context, l net.Listener, discoveryHandler http.Handler) {
+func start(ctx context.Context, l net.Listener, handler http.Handler) {
-	server := http.Server{
+	server := http.Server{Handler: handler}
-		Handler: discoveryHandler,
-	}
 
 	errCh := make(chan error)
 	go func() {
@@ -63,14 +60,14 @@ func waitForSignal() os.Signal {
 
 func startControllers(
 	ctx context.Context,
-	issuerProvider *issuerprovider.Provider,
+	issuerProvider *provider.Manager,
 	pinnipedInformers pinnipedinformers.SharedInformerFactory,
 ) {
 	// Create controller manager.
 	controllerManager := controllerlib.
 		NewManager().
 		WithController(
-			supervisorconfig.NewDynamicConfigWatcherController(
+			supervisorconfig.NewOIDCProviderConfigWatcherController(
 				issuerProvider,
 				pinnipedInformers.Config().V1alpha1().OIDCProviderConfigs(),
 				controllerlib.WithInformer,
@@ -113,8 +110,8 @@ func run(serverInstallationNamespace string) error {
 		pinnipedinformers.WithNamespace(serverInstallationNamespace),
 	)
 
-	issuerProvider := issuerprovider.New()
+	oidProvidersManager := provider.NewManager(http.NotFoundHandler())
-	startControllers(ctx, issuerProvider, pinnipedInformers)
+	startControllers(ctx, oidProvidersManager, pinnipedInformers)
 
 	//nolint: gosec // Intentionally binding to all network interfaces.
 	l, err := net.Listen("tcp", ":80")
@@ -123,7 +120,7 @@ func run(serverInstallationNamespace string) error {
 	}
 	defer l.Close()
 
-	start(ctx, l, discovery.New(issuerProvider))
+	start(ctx, l, oidProvidersManager)
 	klog.InfoS("supervisor is ready", "address", l.Addr().String())
 
 	gotSignal := waitForSignal()

internal/controller/supervisorconfig/dynamic_config_watcher.go

@@ -1,114 +0,0 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package supervisorconfig
-
-import (
-	"fmt"
-	"net/url"
-
-	k8serrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/klog/v2"
-
-	configinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions/config/v1alpha1"
-	pinnipedcontroller "go.pinniped.dev/internal/controller"
-	"go.pinniped.dev/internal/controllerlib"
-)
-
-// IssuerSetter can be notified of a valid issuer with its SetIssuer function. If there is no
-// longer any valid issuer, then nil can be passed to this interface.
-//
-// If the IssuerSetter doesn't like the provided issuer, it can return an error.
-//
-// Implementations of this type should be thread-safe to support calls from multiple goroutines.
-type IssuerSetter interface {
-	SetIssuer(issuer *url.URL) error
-}
-
-type dynamicConfigWatcherController struct {
-	issuerSetter IssuerSetter
-	opcInformer  configinformers.OIDCProviderConfigInformer
-}
-
-// NewDynamicConfigWatcherController creates a controllerlib.Controller that watches
-// OIDCProviderConfig objects and notifies a callback object of their creation or deletion.
-func NewDynamicConfigWatcherController(
-	issuerObserver IssuerSetter,
-	opcInformer configinformers.OIDCProviderConfigInformer,
-	withInformer pinnipedcontroller.WithInformerOptionFunc,
-) controllerlib.Controller {
-	return controllerlib.New(
-		controllerlib.Config{
-			Name: "DynamicConfigWatcherController",
-			Syncer: &dynamicConfigWatcherController{
-				issuerSetter: issuerObserver,
-				opcInformer:  opcInformer,
-			},
-		},
-		withInformer(
-			opcInformer,
-			pinnipedcontroller.NoOpFilter(),
-			controllerlib.InformerOption{},
-		),
-	)
-}
-
-// Sync implements controllerlib.Syncer.
-func (c *dynamicConfigWatcherController) Sync(ctx controllerlib.Context) error {
-	// TODO Watch the configmap to find the issuer name, ingress url, etc.
-	// TODO Update some kind of in-memory representation of the configuration so the discovery endpoint can use it.
-	// TODO The discovery endpoint would return an error until all missing configuration options are
-	// filled in.
-
-	opc, err := c.opcInformer.
-		Lister().
-		OIDCProviderConfigs(ctx.Key.Namespace).
-		Get(ctx.Key.Name)
-	notFound := k8serrors.IsNotFound(err)
-	if err != nil && !notFound {
-		return fmt.Errorf("failed to get %s/%s oidcproviderconfig: %w", ctx.Key.Namespace, ctx.Key.Name, err)
-	}
-
-	if notFound {
-		klog.InfoS(
-			"dynamicConfigWatcherController Sync found no oidcproviderconfig",
-			"oidcproviderconfig",
-			klog.KRef(ctx.Key.Namespace, ctx.Key.Name),
-		)
-		if err := c.issuerSetter.SetIssuer(nil); err != nil {
-			klog.InfoS(
-				"dynamicConfigWatcherController Sync failed to set issuer",
-				"err",
-				err,
-			)
-		}
-		return nil
-	}
-
-	url, err := url.Parse(opc.Spec.Issuer)
-	if err != nil {
-		klog.InfoS(
-			"dynamicConfigWatcherController Sync failed to parse issuer",
-			"err",
-			err,
-		)
-		return nil
-	}
-
-	klog.InfoS(
-		"dynamicConfigWatcherController Sync issuer",
-		"oidcproviderconfig",
-		klog.KObj(opc),
-		"issuer",
-		url,
-	)
-	if err := c.issuerSetter.SetIssuer(url); err != nil {
-		klog.InfoS(
-			"dynamicConfigWatcherController Sync failed to set issuer",
-			"err",
-			err,
-		)
-	}
-
-	return nil
-}

internal/controller/supervisorconfig/oidcproviderconfig_watcher.go

@@ -0,0 +1,91 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package supervisorconfig
+
+import (
+	"net/url"
+
+	"k8s.io/apimachinery/pkg/labels"
+	"k8s.io/klog/v2"
+
+	configinformers "go.pinniped.dev/generated/1.19/client/informers/externalversions/config/v1alpha1"
+	pinnipedcontroller "go.pinniped.dev/internal/controller"
+	"go.pinniped.dev/internal/controllerlib"
+	"go.pinniped.dev/internal/oidc/provider"
+)
+
+// ProvidersSetter can be notified of all known valid providers with its SetIssuer function.
+// If there are no longer any valid issuers, then it can be called with no arguments.
+// Implementations of this type should be thread-safe to support calls from multiple goroutines.
+type ProvidersSetter interface {
+	SetProviders(oidcProviders ...*provider.OIDCProvider)
+}
+
+type oidcProviderConfigWatcherController struct {
+	providerSetter ProvidersSetter
+	opcInformer    configinformers.OIDCProviderConfigInformer
+}
+
+// NewOIDCProviderConfigWatcherController creates a controllerlib.Controller that watches
+// OIDCProviderConfig objects and notifies a callback object of the collection of provider configs.
+func NewOIDCProviderConfigWatcherController(
+	issuerObserver ProvidersSetter,
+	opcInformer configinformers.OIDCProviderConfigInformer,
+	withInformer pinnipedcontroller.WithInformerOptionFunc,
+) controllerlib.Controller {
+	return controllerlib.New(
+		controllerlib.Config{
+			Name: "OIDCProviderConfigWatcherController",
+			Syncer: &oidcProviderConfigWatcherController{
+				providerSetter: issuerObserver,
+				opcInformer:    opcInformer,
+			},
+		},
+		withInformer(
+			opcInformer,
+			pinnipedcontroller.NoOpFilter(),
+			controllerlib.InformerOption{},
+		),
+	)
+}
+
+// Sync implements controllerlib.Syncer.
+func (c *oidcProviderConfigWatcherController) Sync(ctx controllerlib.Context) error {
+	all, err := c.opcInformer.Lister().List(labels.Everything())
+	if err != nil {
+		return err
+	}
+
+	oidcProviders := make([]*provider.OIDCProvider, 0)
+	for _, opc := range all {
+		issuerURL, err := url.Parse(opc.Spec.Issuer)
+		if err != nil {
+			klog.InfoS(
+				"OIDCProviderConfigWatcherController Sync failed to parse issuer",
+				"err",
+				err,
+			)
+			continue
+		}
+		oidcProvider := &provider.OIDCProvider{Issuer: issuerURL}
+		err = oidcProvider.Validate()
+		if err != nil {
+			klog.InfoS(
+				"OIDCProviderConfigWatcherController Sync could failed to validate OIDCProviderConfig",
+				"err",
+				err,
+			)
+			continue
+		}
+		oidcProviders = append(oidcProviders, oidcProvider)
+		klog.InfoS(
+			"OIDCProviderConfigWatcherController Sync accepted OIDCProviderConfig",
+			"issuer",
+			issuerURL,
+		)
+	}
+
+	c.providerSetter.SetProviders(oidcProviders...)
+	return nil
+}

internal/oidc/discovery/discovery.go

@@ -8,16 +8,13 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"net/url"
-
-	"go.pinniped.dev/internal/oidc"
 )
 
 // Metadata holds all fields (that we care about) from the OpenID Provider Metadata section in the
 // OpenID Connect Discovery specification:
 // https://openid.net/specs/openid-connect-discovery-1_0.html#rfc.section.3.
 type Metadata struct {
-	// vvvRequiredvvv
+	// vvv Required vvv
 
 	Issuer string `json:"issuer"`
 
@@ -29,44 +26,28 @@ type Metadata struct {
 	SubjectTypesSupported            []string `json:"subject_types_supported"`
 	IDTokenSigningAlgValuesSupported []string `json:"id_token_signing_alg_values_supported"`
 
-	// ^^^Required^^^
+	// ^^^ Required ^^^
 
-	// vvvOptionalvvv
+	// vvv Optional vvv
 
 	TokenEndpointAuthMethodsSupported           []string `json:"token_endpoint_auth_methods_supported"`
 	TokenEndpointAuthSigningAlgoValuesSupported []string `json:"token_endpoint_auth_signing_alg_values_supported"`
 	ScopesSupported                             []string `json:"scopes_supported"`
 	ClaimsSupported                             []string `json:"claims_supported"`
 
-	// ^^^Optional^^^
+	// ^^^ Optional ^^^
 }
 
-// IssuerGetter holds onto an issuer which can be retrieved via its GetIssuer function. If there is
+// New returns an http.Handler that serves an OIDC discovery endpoint.
-// no valid issuer, then nil will be returned.
+func New(issuerURL string) http.Handler {
-//
-// Implementations of this type should be thread-safe to support calls from multiple goroutines.
-type IssuerGetter interface {
-	GetIssuer() *url.URL
-}
-
-// New returns an http.Handler that will use information from the provided IssuerGetter to serve an
-// OIDC discovery endpoint.
-func New(ig IssuerGetter) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 
-		issuer := ig.GetIssuer()
-		if issuer == nil || r.URL.Path != issuer.Path+oidc.WellKnownURLPath {
-			http.Error(w, `{"error": "OIDC discovery not available (unknown issuer)"}`, http.StatusNotFound)
-			return
-		}
-
 		if r.Method != http.MethodGet {
 			http.Error(w, `{"error": "Method not allowed (try GET)"}`, http.StatusMethodNotAllowed)
 			return
 		}
 
-		issuerURL := issuer.String()
 		oidcConfig := Metadata{
 			Issuer:                            issuerURL,
 			AuthorizationEndpoint:             fmt.Sprintf("%s/oauth2/v0/auth", issuerURL),

internal/oidc/discovery/discovery_test.go

@@ -7,20 +7,18 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
-	"net/url"
 	"testing"
 
 	"github.com/stretchr/testify/require"
 
 	"go.pinniped.dev/internal/oidc"
-	"go.pinniped.dev/internal/oidc/issuerprovider"
 )
 
 func TestDiscovery(t *testing.T) {
 	tests := []struct {
 		name string
 
-		issuer *url.URL
+		issuer string
 		method string
 		path   string
 
@@ -29,37 +27,8 @@ func TestDiscovery(t *testing.T) {
 		wantBody        interface{}
 	}{
 		{
-			name:       "nil issuer",
+			name:            "happy path",
-			method:     http.MethodGet,
+			issuer:          "https://some-issuer.com/some/path",
-			path:       oidc.WellKnownURLPath,
-			wantStatus: http.StatusNotFound,
-			wantBody: map[string]string{
-				"error": "OIDC discovery not available (unknown issuer)",
-			},
-		},
-		{
-			name:       "root path mismatch",
-			issuer:     must(url.Parse("https://some-issuer.com/some/path")),
-			method:     http.MethodGet,
-			path:       "/some/other/path" + oidc.WellKnownURLPath,
-			wantStatus: http.StatusNotFound,
-			wantBody: map[string]string{
-				"error": "OIDC discovery not available (unknown issuer)",
-			},
-		},
-		{
-			name:       "well-known path mismatch",
-			issuer:     must(url.Parse("https://some-issuer.com/some/path")),
-			method:     http.MethodGet,
-			path:       "/some/path/that/is/not/the/well-known/path",
-			wantStatus: http.StatusNotFound,
-			wantBody: map[string]string{
-				"error": "OIDC discovery not available (unknown issuer)",
-			},
-		},
-		{
-			name:            "issuer path matches",
-			issuer:          must(url.Parse("https://some-issuer.com/some/path")),
 			method:          http.MethodGet,
 			path:            "/some/path" + oidc.WellKnownURLPath,
 			wantStatus:      http.StatusOK,
@@ -80,7 +49,7 @@ func TestDiscovery(t *testing.T) {
 		},
 		{
 			name:       "bad method",
-			issuer:     must(url.Parse("https://some-issuer.com")),
+			issuer:     "https://some-issuer.com",
 			method:     http.MethodPost,
 			path:       oidc.WellKnownURLPath,
 			wantStatus: http.StatusMethodNotAllowed,
@@ -92,11 +61,7 @@ func TestDiscovery(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			p := issuerprovider.New()
+			handler := New(test.issuer)
-			err := p.SetIssuer(test.issuer)
-			require.NoError(t, err)
-
-			handler := New(p)
 			req := httptest.NewRequest(test.method, test.path, nil)
 			rsp := httptest.NewRecorder()
 			handler.ServeHTTP(rsp, req)
@@ -115,10 +80,3 @@ func TestDiscovery(t *testing.T) {
 		})
 	}
 }
-
-func must(u *url.URL, err error) *url.URL {
-	if err != nil {
-		panic(err)
-	}
-	return u
-}

internal/oidc/issuerprovider/issuerprovider.go

@@ -1,80 +0,0 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-// Package issuerprovider provides a thread-safe type that can hold on to an OIDC issuer name.
-package issuerprovider
-
-import (
-	"net/url"
-	"strings"
-	"sync"
-
-	"go.pinniped.dev/internal/constable"
-)
-
-// Provider is a type that can hold onto an issuer value, which may be nil.
-//
-// It is thread-safe.
-type Provider struct {
-	mu     sync.RWMutex
-	issuer *url.URL
-}
-
-// New returns an empty Provider, i.e., one that holds a nil issuer.
-func New() *Provider {
-	return &Provider{}
-}
-
-// SetIssuer validates and sets the provided issuer. If validation fails, SetIssuer will return
-// an error.
-func (p *Provider) SetIssuer(issuer *url.URL) error {
-	if err := p.validateIssuer(issuer); err != nil {
-		return err
-	}
-	p.setIssuer(issuer)
-	return nil
-}
-
-func (p *Provider) validateIssuer(issuer *url.URL) error {
-	if issuer == nil {
-		return nil
-	}
-
-	if issuer.Scheme != "https" && removeMeAfterWeNoLongerNeedHTTPIssuerSupport(issuer.Scheme) {
-		return constable.Error(`issuer must have "https" scheme`)
-	}
-
-	if issuer.User != nil {
-		return constable.Error(`issuer must not have username or password`)
-	}
-
-	if strings.HasSuffix(issuer.Path, "/") {
-		return constable.Error(`issuer must not have trailing slash in path`)
-	}
-
-	if issuer.RawQuery != "" {
-		return constable.Error(`issuer must not have query`)
-	}
-
-	if issuer.Fragment != "" {
-		return constable.Error(`issuer must not have fragment`)
-	}
-
-	return nil
-}
-
-func (p *Provider) setIssuer(issuer *url.URL) {
-	p.mu.Lock()
-	defer p.mu.Unlock()
-	p.issuer = issuer
-}
-
-func (p *Provider) GetIssuer() *url.URL {
-	p.mu.RLock()
-	defer p.mu.RUnlock()
-	return p.issuer
-}
-
-func removeMeAfterWeNoLongerNeedHTTPIssuerSupport(scheme string) bool {
-	return scheme != "http"
-}

internal/oidc/provider/manager.go

@@ -0,0 +1,115 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package provider
+
+import (
+	"net/http"
+	"net/url"
+	"strings"
+	"sync"
+
+	"k8s.io/klog/v2"
+
+	"go.pinniped.dev/internal/oidc"
+	"go.pinniped.dev/internal/oidc/discovery"
+)
+
+// Manager can manage multiple active OIDC providers. It acts as a request router for them.
+//
+// It is thread-safe.
+type Manager struct {
+	mu               sync.RWMutex
+	providerHandlers map[string]*providerHandler // map of issuer name to providerHandler
+	nextHandler      http.Handler                // the next handler in a chain, called when this manager didn't know how to handle a request
+}
+
+// New returns an empty Manager.
+// nextHandler will be invoked for any requests that could not be handled by this manager's providers.
+func NewManager(nextHandler http.Handler) *Manager {
+	return &Manager{providerHandlers: make(map[string]*providerHandler), nextHandler: nextHandler}
+}
+
+type providerHandler struct {
+	provider         *OIDCProvider
+	discoveryHandler http.Handler
+}
+
+func (h *providerHandler) Issuer() *url.URL {
+	return h.provider.Issuer
+}
+
+// SetProviders adds or updates all the given providerHandlers using each provider's issuer string
+// as the name of the provider to decide if it is an add or update operation.
+//
+// It also removes any providerHandlers that were previously added but were not passed in to
+// the current invocation.
+//
+// This method assumes that all of the OIDCProvider arguments have already been validated
+// by someone else before they are passed to this method.
+func (c *Manager) SetProviders(oidcProviders ...*OIDCProvider) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	// Add all of the incoming providers.
+	for _, incomingProvider := range oidcProviders {
+		issuerString := incomingProvider.Issuer.String()
+		c.providerHandlers[issuerString] = &providerHandler{
+			provider:         incomingProvider,
+			discoveryHandler: discovery.New(issuerString),
+		}
+		klog.InfoS("oidc provider manager added or updated issuer", "issuer", issuerString)
+	}
+	// Remove any providers that we previously handled but no longer exist.
+	for issuerKey := range c.providerHandlers {
+		if !findIssuerInListOfProviders(issuerKey, oidcProviders) {
+			delete(c.providerHandlers, issuerKey)
+			klog.InfoS("oidc provider manager removed issuer", "issuer", issuerKey)
+		}
+	}
+}
+
+// ServeHTTP implements the http.Handler interface.
+func (c *Manager) ServeHTTP(resp http.ResponseWriter, req *http.Request) {
+	providerHandler := c.findProviderHandlerByIssuerURL(req.Host, req.URL.Path)
+	if providerHandler != nil {
+		if req.URL.Path == providerHandler.Issuer().Path+oidc.WellKnownURLPath {
+			providerHandler.discoveryHandler.ServeHTTP(resp, req)
+			return // handled!
+		}
+		klog.InfoS(
+			"oidc provider manager found issuer but could not handle request",
+			"method", req.Method,
+			"host", req.Host,
+			"path", req.URL.Path,
+		)
+	} else {
+		klog.InfoS(
+			"oidc provider manager could not find issuer to handle request",
+			"method", req.Method,
+			"host", req.Host,
+			"path", req.URL.Path,
+		)
+	}
+	// Didn't know how to handle this request, so send it along the chain for further processing.
+	c.nextHandler.ServeHTTP(resp, req)
+}
+
+func (c *Manager) findProviderHandlerByIssuerURL(host, path string) *providerHandler {
+	for _, providerHandler := range c.providerHandlers {
+		pi := providerHandler.Issuer()
+		// TODO do we need to compare scheme? not sure how to get it from the http.Request object
+		if host == pi.Host && strings.HasPrefix(path, pi.Path) { // TODO probably need better logic here? also maybe needs some of the logic from inside ServeMux
+			return providerHandler
+		}
+	}
+	return nil
+}
+
+func findIssuerInListOfProviders(issuer string, oidcProviders []*OIDCProvider) bool {
+	for _, provider := range oidcProviders {
+		if provider.Issuer.String() == issuer {
+			return true
+		}
+	}
+	return false
+}

internal/oidc/provider/oidcprovider.go

@@ -0,0 +1,50 @@
+// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package provider
+
+import (
+	"net/url"
+	"strings"
+
+	"go.pinniped.dev/internal/constable"
+)
+
+// OIDCProvider represents all of the settings and state for an OIDC provider.
+type OIDCProvider struct {
+	Issuer *url.URL
+}
+
+// Validate returns an error if there is anything wrong with the provider settings, or
+// returns nil if there is nothing wrong with the settings.
+func (p *OIDCProvider) Validate() error {
+	if p.Issuer == nil {
+		return constable.Error(`provider must have an issuer`)
+	}
+
+	if p.Issuer.Scheme != "https" && p.removeMeAfterWeNoLongerNeedHTTPIssuerSupport(p.Issuer.Scheme) {
+		return constable.Error(`issuer must have "https" scheme`)
+	}
+
+	if p.Issuer.User != nil {
+		return constable.Error(`issuer must not have username or password`)
+	}
+
+	if strings.HasSuffix(p.Issuer.Path, "/") {
+		return constable.Error(`issuer must not have trailing slash in path`)
+	}
+
+	if p.Issuer.RawQuery != "" {
+		return constable.Error(`issuer must not have query`)
+	}
+
+	if p.Issuer.Fragment != "" {
+		return constable.Error(`issuer must not have fragment`)
+	}
+
+	return nil
+}
+
+func (p *OIDCProvider) removeMeAfterWeNoLongerNeedHTTPIssuerSupport(scheme string) bool {
+	return scheme != "http"
+}

internal/oidc/provider/oidcprovider_test.go

@@ -1,7 +1,7 @@
 // Copyright 2020 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
-package issuerprovider
+package provider
 
 import (
 	"net/url"
@@ -10,15 +10,16 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestProvider(t *testing.T) {
+func TestOIDCProviderValidations(t *testing.T) {
 	tests := []struct {
 		name      string
 		issuer    *url.URL
 		wantError string
 	}{
 		{
-			name:   "nil issuer",
+			name:      "provider must have an issuer",
-			issuer: nil,
+			issuer:    nil,
+			wantError: "provider must have an issuer",
 		},
 		{
 			name:      "no scheme",
@@ -67,14 +68,12 @@ func TestProvider(t *testing.T) {
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			p := New()
+			p := OIDCProvider{Issuer: tt.issuer}
-			err := p.SetIssuer(tt.issuer)
+			err := p.Validate()
 			if tt.wantError != "" {
 				require.EqualError(t, err, tt.wantError)
-				require.Nil(t, p.GetIssuer())
 			} else {
 				require.NoError(t, err)
-				require.Equal(t, tt.issuer, p.GetIssuer())
 			}
 		})
 	}

test/integration/supervisor_discovery_test.go

@@ -13,9 +13,11 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"go.pinniped.dev/generated/1.19/apis/config/v1alpha1"
+	pinnipedclientset "go.pinniped.dev/generated/1.19/client/clientset/versioned"
 	"go.pinniped.dev/internal/here"
 	"go.pinniped.dev/test/library"
 )
@@ -24,7 +26,6 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
 	env := library.IntegrationEnv(t)
 	client := library.NewPinnipedClientset(t)
 
-	httpClient := &http.Client{}
 	ns := env.SupervisorNamespace
 	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
 	defer cancel()
@@ -32,7 +33,6 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
 	// Temporarily remove any existing OIDCProviderConfigs from the cluster so we can test from a clean slate.
 	originalConfigList, err := client.ConfigV1alpha1().OIDCProviderConfigs(ns).List(ctx, metav1.ListOptions{})
 	require.NoError(t, err)
-
 	for _, config := range originalConfigList.Items {
 		err := client.ConfigV1alpha1().OIDCProviderConfigs(ns).Delete(ctx, config.Name, metav1.DeleteOptions{})
 		require.NoError(t, err)
@@ -52,51 +52,85 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
 	})
 
 	// Test that there is no default discovery endpoint available when there are no OIDCProviderConfigs.
+	requireDiscoveryEndpointIsNotFound(t, fmt.Sprintf("http://%s", env.SupervisorAddress))
+
+	// Define several unique issuer strings.
+	issuer1 := fmt.Sprintf("http://%s/nested/issuer1", env.SupervisorAddress)
+	issuer2 := fmt.Sprintf("http://%s/nested/issuer2", env.SupervisorAddress)
+	issuer3 := fmt.Sprintf("http://%s/issuer3", env.SupervisorAddress)
+	issuer4 := fmt.Sprintf("http://%s/issuer4", env.SupervisorAddress)
+
+	// When OIDCProviderConfig are created in sequence they each cause a discovery endpoint to appear only for as long as the OIDCProviderConfig exists.
+	createdOIDCProviderConfig1 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer1, "from-integration-test1")
+	requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, createdOIDCProviderConfig1, client, ns, issuer1)
+	createdOIDCProviderConfig2 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer2, "from-integration-test2")
+	requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, createdOIDCProviderConfig2, client, ns, issuer2)
+
+	// When multiple OIDCProviderConfigs exist at the same time they each serve a unique discovery endpoint.
+	createdOIDCProviderConfig3 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer3, "from-integration-test3")
+	createdOIDCProviderConfig4 := requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t, client, ns, issuer4, "from-integration-test4")
+	requireWellKnownEndpointIsWorking(t, issuer3) // discovery for issuer3 is still working after issuer4 started working
+
+	// When they are deleted they stop serving discovery endpoints.
+	requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, createdOIDCProviderConfig3, client, ns, issuer2)
+	requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t, createdOIDCProviderConfig4, client, ns, issuer2)
+}
+
+func requireDiscoveryEndpointIsNotFound(t *testing.T, issuerName string) {
+	t.Helper()
+	httpClient := &http.Client{}
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
 	requestNonExistentPath, err := http.NewRequestWithContext(
 		ctx,
 		http.MethodGet,
-		fmt.Sprintf("http://%s/.well-known/openid-configuration", env.SupervisorAddress),
+		fmt.Sprintf("%s/.well-known/openid-configuration", issuerName),
 		nil,
 	)
+
+	var response *http.Response
+	assert.Eventually(t, func() bool {
+		response, err = httpClient.Do(requestNonExistentPath) //nolint:bodyclose
+		return err == nil && response.StatusCode == http.StatusNotFound
+	}, 10*time.Second, 200*time.Millisecond)
 	require.NoError(t, err)
-	notFoundResponse, err := httpClient.Do(requestNonExistentPath)
+	require.Equal(t, http.StatusNotFound, response.StatusCode)
+	err = response.Body.Close()
 	require.NoError(t, err)
-	require.Equal(t, 404, notFoundResponse.StatusCode)
+}
-	err = notFoundResponse.Body.Close()
+
+func requireCreatingOIDCProviderConfigCausesWellKnownEndpointToAppear(t *testing.T, client pinnipedclientset.Interface, ns string, issuerName string, oidcProviderConfigName string) *v1alpha1.OIDCProviderConfig {
+	t.Helper()
+	newOIDCProviderConfig := createOIDCProviderConfig(t, oidcProviderConfigName, client, ns, issuerName)
+	requireWellKnownEndpointIsWorking(t, issuerName)
+	return newOIDCProviderConfig
+}
+
+func requireDeletingOIDCProviderConfigCausesWellKnownEndpointToDisappear(t *testing.T, existingOIDCProviderConfig *v1alpha1.OIDCProviderConfig, client pinnipedclientset.Interface, ns string, issuerName string) {
+	t.Helper()
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	// Delete the OIDCProviderConfig.
+	err := client.ConfigV1alpha1().OIDCProviderConfigs(ns).Delete(ctx, existingOIDCProviderConfig.Name, metav1.DeleteOptions{})
 	require.NoError(t, err)
 
-	// Create a new OIDCProviderConfig with a known issuer.
+	// Fetch that same discovery endpoint as before, but now it should not exist anymore. Give it some time for the endpoint to go away.
-	issuer := fmt.Sprintf("http://%s/nested/issuer", env.SupervisorAddress)
+	requireDiscoveryEndpointIsNotFound(t, issuerName)
-	newOIDCProviderConfig := v1alpha1.OIDCProviderConfig{
+}
-		TypeMeta: metav1.TypeMeta{
-			Kind:       "OIDCProviderConfig",
-			APIVersion: v1alpha1.SchemeGroupVersion.String(),
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "nested-issuser-config-from-integration-test",
-			Namespace: ns,
-		},
-		Spec: v1alpha1.OIDCProviderConfigSpec{
-			Issuer: issuer,
-		},
-	}
-	_, err = client.ConfigV1alpha1().OIDCProviderConfigs(ns).Create(ctx, &newOIDCProviderConfig, metav1.CreateOptions{})
-	require.NoError(t, err)
 
-	// When this test has finished, clean up the new OIDCProviderConfig.
+func requireWellKnownEndpointIsWorking(t *testing.T, issuerName string) {
-	t.Cleanup(func() {
+	t.Helper()
-		cleanupCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	httpClient := &http.Client{}
-		defer cancel()
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
 
-		err = client.ConfigV1alpha1().OIDCProviderConfigs(ns).Delete(cleanupCtx, newOIDCProviderConfig.Name, metav1.DeleteOptions{})
+	// Define a request to the new discovery endpoint which should have been created by an OIDCProviderConfig.
-		require.NoError(t, err)
-	})
-
-	// Define a request to the new discovery endpoint which should have been created for the above OIDCProviderConfig.
 	requestDiscoveryEndpoint, err := http.NewRequestWithContext(
 		ctx,
 		http.MethodGet,
-		fmt.Sprintf("http://%s/nested/issuer/.well-known/openid-configuration", env.SupervisorAddress),
+		fmt.Sprintf("%s/.well-known/openid-configuration", issuerName),
 		nil,
 	)
 	require.NoError(t, err)
@@ -104,7 +138,7 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
 	// Fetch that discovery endpoint. Give it some time for the endpoint to come into existence.
 	var response *http.Response
 	assert.Eventually(t, func() bool {
-		response, err = httpClient.Do(requestDiscoveryEndpoint) //nolint:bodyclose // the body is closed below after it is read
+		response, err = httpClient.Do(requestDiscoveryEndpoint) //nolint:bodyclose
 		return err == nil && response.StatusCode == http.StatusOK
 	}, 10*time.Second, 200*time.Millisecond)
 	require.NoError(t, err)
@@ -129,8 +163,45 @@ func TestSupervisorOIDCDiscovery(t *testing.T) {
       "subject_types_supported": ["public"],
       "id_token_signing_alg_values_supported": ["RS256"]
     }`)
-	expectedJSON := fmt.Sprintf(expectedResultTemplate, issuer, issuer, issuer, issuer)
+	expectedJSON := fmt.Sprintf(expectedResultTemplate, issuerName, issuerName, issuerName, issuerName)
 
 	require.Equal(t, "application/json", response.Header.Get("content-type"))
 	require.JSONEq(t, expectedJSON, string(responseBody))
 }
+
+func createOIDCProviderConfig(t *testing.T, oidcProviderConfigName string, client pinnipedclientset.Interface, ns string, issuerName string) *v1alpha1.OIDCProviderConfig {
+	t.Helper()
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	newOIDCProviderConfig := v1alpha1.OIDCProviderConfig{
+		TypeMeta: metav1.TypeMeta{
+			Kind:       "OIDCProviderConfig",
+			APIVersion: v1alpha1.SchemeGroupVersion.String(),
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      oidcProviderConfigName,
+			Namespace: ns,
+		},
+		Spec: v1alpha1.OIDCProviderConfigSpec{
+			Issuer: issuerName,
+		},
+	}
+	createdOIDCProviderConfig, err := client.ConfigV1alpha1().OIDCProviderConfigs(ns).Create(ctx, &newOIDCProviderConfig, metav1.CreateOptions{})
+	require.NoError(t, err)
+
+	// When this test has finished, be sure to clean up the new OIDCProviderConfig.
+	t.Cleanup(func() {
+		cleanupCtx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+		defer cancel()
+
+		err = client.ConfigV1alpha1().OIDCProviderConfigs(ns).Delete(cleanupCtx, newOIDCProviderConfig.Name, metav1.DeleteOptions{})
+		notFound := k8serrors.IsNotFound(err)
+		// It's okay if it is not found, because it might have been deleted by another part of this test.
+		if !notFound {
+			require.NoError(t, err)
+		}
+	})
+
+	return createdOIDCProviderConfig
+}