Update '.drone.yml'

.drone.yml

@@ -0,0 +1,19 @@
+kind: pipeline
+type: kubernetes
+name: Container
+
+steps:
+- name: build & publish
+  image: spritsail/docker-build
+  context: .
+  settings:
+    repo: bv11-cr01.bessems.eu/library/pinniped-server
+    registry: bv11-cr01.bessems.eu
+    tags: latest
+    build_args:
+      - BUILDPLATFORM=linux/amd64
+    mtu: 1450
+    username:
+      from_secret: harbor_username
+    password:
+      from_secret: harbor_password

Dockerfile

@@ -4,7 +4,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 # Prepare to cross-compile by always running the build stage in the build platform, not the target platform.
-FROM --platform=$BUILDPLATFORM golang:1.21.3 as build-env
+FROM --platform=linux/amd64 golang:1.21.3 as build-env
 
 WORKDIR /work
 

deploy/concierge/deployment.yaml

@@ -45,9 +45,8 @@ metadata:
   annotations:
     #! we need to create this service account before we create the secret
     kapp.k14s.io/change-group: "impersonation-proxy.concierge.pinniped.dev/serviceaccount"
-    kubernetes.io/enforce-mountable-secrets: "true"
-secrets: [] #! make sure the token controller does not create any secrets
-automountServiceAccountToken: false
+secrets: #! make sure the token controller does not create any other secrets
+- name: #@ defaultResourceNameWithSuffix("impersonation-proxy")
 ---
 apiVersion: v1
 kind: ConfigMap
@@ -78,8 +77,6 @@ data:
       impersonationCACertificateSecret: (@= defaultResourceNameWithSuffix("impersonation-proxy-ca-certificate") @)
       impersonationSignerSecret: (@= defaultResourceNameWithSuffix("impersonation-proxy-signer-ca-certificate") @)
       agentServiceAccount: (@= defaultResourceNameWithSuffix("kube-cert-agent") @)
-      impersonationProxyServiceAccount: (@= defaultResourceNameWithSuffix("impersonation-proxy") @)
-      impersonationProxyLegacySecret: (@= defaultResourceNameWithSuffix("impersonation-proxy") @)
     labels: (@= json.encode(labels()).rstrip() @)
     kubeCertAgent:
       namePrefix: (@= defaultResourceNameWithSuffix("kube-cert-agent-") @)
@@ -185,6 +182,9 @@ spec:
             - name: podinfo
               mountPath: /etc/podinfo
               readOnly: true
+            - name: impersonation-proxy
+              mountPath: /var/run/secrets/impersonation-proxy.concierge.pinniped.dev/serviceaccount
+              readOnly: true
           env:
             #@ if data.values.https_proxy:
             - name: HTTPS_PROXY
@@ -220,6 +220,12 @@ spec:
         - name: config-volume
           configMap:
             name: #@ defaultResourceNameWithSuffix("config")
+        - name: impersonation-proxy
+          secret:
+            secretName: #@ defaultResourceNameWithSuffix("impersonation-proxy")
+            items: #! make sure our pod does not start until the token controller has a chance to populate the secret
+              - key: token
+                path: token
         - name: podinfo
           downwardAPI:
             items:
@@ -342,3 +348,16 @@ spec:
       loadBalancerIP: #@ data.values.impersonation_proxy_spec.service.load_balancer_ip
       #@ end
       annotations: #@ data.values.impersonation_proxy_spec.service.annotations
+---
+apiVersion: v1
+kind: Secret
+metadata:
+  name: #@ defaultResourceNameWithSuffix("impersonation-proxy")
+  namespace: #@ namespace()
+  labels: #@ labels()
+  annotations:
+    #! wait until the SA exists to create this secret so that the token controller does not delete it
+    #! we have this secret at the end so that kubectl will create the service account first
+    kapp.k14s.io/change-rule: "upsert after upserting impersonation-proxy.concierge.pinniped.dev/serviceaccount"
+    kubernetes.io/service-account.name: #@ defaultResourceNameWithSuffix("impersonation-proxy")
+type: kubernetes.io/service-account-token

deploy/concierge/rbac.yaml

@@ -1,4 +1,4 @@
-#! Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+#! Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
 #! SPDX-License-Identifier: Apache-2.0
 
 #@ load("@ytt:data", "data")
@@ -156,13 +156,6 @@ rules:
   - apiGroups: [ coordination.k8s.io ]
     resources: [ leases ]
     verbs: [ create, get, update ]
-  #! We need to be able to get service accounts and create serviceaccounts/tokens so that we can create short-lived tokens for the impersonation proxy
-  - apiGroups: [""]
-    resources: [ serviceaccounts ]
-    verbs: [ get ]
-  - apiGroups: [""]
-    resources: [ serviceaccounts/token ]
-    verbs: [ create ]
 ---
 kind: RoleBinding
 apiVersion: rbac.authorization.k8s.io/v1

internal/backoff/infinitebackoff.go

@@ -1,44 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package backoff
-
-import (
-	"math"
-	"time"
-)
-
-type InfiniteBackoff struct {
-	// The initial duration.
-	Duration time.Duration
-
-	// Duration is multiplied by Factor each iteration, if the limit imposed by MaxDuration has not been
-	// reached.
-	//
-	Factor float64
-
-	// A limit on step size. Once reached, this value will be used as the interval.
-	MaxDuration time.Duration
-
-	hasStepped bool
-}
-
-// Step returns the next duration in the backoff sequence.
-// It modifies the receiver and is not thread-safe.
-func (b *InfiniteBackoff) Step() time.Duration {
-	if !b.hasStepped {
-		b.hasStepped = true
-		return b.Duration
-	}
-
-	var next time.Duration
-	b.Factor = math.Max(1, b.Factor)
-	// Grow by the factor (which could be 1).
-	next = time.Duration(float64(b.Duration) * b.Factor)
-	// Stop growing the intervals once we exceed the max duration.
-	if b.MaxDuration > 0 && next > b.MaxDuration {
-		next = b.MaxDuration
-	}
-	b.Duration = next
-	return next
-}

internal/backoff/infinitebackoff_test.go

@@ -1,94 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package backoff
-
-import (
-	"math"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/require"
-)
-
-func TestInfiniteBackoff(t *testing.T) {
-	tests := []struct {
-		name             string
-		stepper          Stepper
-		expectedSequence []time.Duration
-	}{
-		{
-			name:    "zero initialization results in 0ns steps",
-			stepper: &InfiniteBackoff{},
-			expectedSequence: func() []time.Duration {
-				results := make([]time.Duration, 1000)
-				for i := 0; i < 1000; i++ {
-					results[i] = time.Duration(0)
-				}
-				return results
-			}(),
-		},
-		{
-			name: "double 5 ns forever",
-			stepper: &InfiniteBackoff{
-				Duration: 5 * time.Nanosecond,
-				Factor:   2,
-			},
-			expectedSequence: func() []time.Duration {
-				// limit to 60 to prevent int64 overflow
-				results := make([]time.Duration, 60)
-				results[0] = 5 * time.Nanosecond
-				for i := 1; i < 60; i++ {
-					results[i] = 2 * results[i-1]
-				}
-				return results
-			}(),
-		},
-		{
-			name: "grows slowly until limit",
-			stepper: &InfiniteBackoff{
-				Duration:    20 * time.Nanosecond,
-				Factor:      1.1,
-				MaxDuration: 40 * time.Nanosecond,
-			},
-			expectedSequence: func() []time.Duration {
-				results := make([]time.Duration, 1000)
-				results[0] = 20 * time.Nanosecond
-				for i := 1; i < 1000; i++ {
-					nanoseconds := 1.1 * float64(results[i-1])
-					results[i] = time.Duration(math.Min(nanoseconds, 40))
-				}
-				return results
-			}(),
-		},
-		{
-			name: "factor less than 1.0 is replaced with 1.0",
-			stepper: &InfiniteBackoff{
-				Duration: 20 * time.Nanosecond,
-				Factor:   0.9,
-			},
-			expectedSequence: func() []time.Duration {
-				results := make([]time.Duration, 1000)
-				for i := 0; i < 1000; i++ {
-					results[i] = 20 * time.Nanosecond
-				}
-				return results
-			}(),
-		},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			require.NotEmpty(t, tt.expectedSequence)
-			for i, expected := range tt.expectedSequence {
-				actual := tt.stepper.Step()
-				require.Equalf(t, expected, actual, "incorrect result on step #%d, previous steps are %v", i, tt.expectedSequence[:i])
-			}
-
-			backoff, ok := tt.stepper.(*InfiniteBackoff)
-			require.True(t, ok)
-			require.NotNil(t, backoff)
-			require.GreaterOrEqual(t, backoff.Factor, 1.0)
-		})
-	}
-}

internal/backoff/stepping.go

@@ -1,56 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package backoff
-
-import (
-	"context"
-	"time"
-
-	"k8s.io/apimachinery/pkg/util/wait"
-)
-
-type Stepper interface {
-	Step() time.Duration
-}
-
-func wrapConditionWithNoPanics(ctx context.Context, condition wait.ConditionWithContextFunc) (done bool, err error) {
-	defer func() {
-		if r := recover(); r != nil {
-			if err2, ok := r.(error); ok {
-				err = err2
-				return
-			}
-		}
-	}()
-
-	return condition(ctx)
-}
-
-func WithContext(ctx context.Context, backoff Stepper, condition wait.ConditionWithContextFunc) error {
-	// Loop forever, unless we reach one of the return statements below.
-	for {
-		// Stop if the context is done.
-		select {
-		case <-ctx.Done():
-			return ctx.Err()
-		default:
-		}
-
-		// Stop trying unless the condition function returns false.
-		// Allow cancellation during the attempt if the condition function respects the ctx.
-		if ok, err := wrapConditionWithNoPanics(ctx, condition); err != nil || ok {
-			return err
-		}
-
-		// Calculate how long to wait before the next step.
-		waitBeforeRetry := backoff.Step()
-
-		// Wait before running again, allowing cancellation during the wait.
-		select {
-		case <-ctx.Done():
-			return ctx.Err()
-		case <-time.After(waitBeforeRetry):
-		}
-	}
-}

internal/backoff/stepping_test.go

@@ -1,124 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package backoff
-
-import (
-	"context"
-	"errors"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/require"
-	"k8s.io/apimachinery/pkg/util/wait"
-)
-
-type MockStepper struct {
-	steps       []time.Duration
-	currentStep int
-}
-
-func (m *MockStepper) Step() time.Duration {
-	result := m.steps[m.currentStep]
-	m.currentStep++
-	return result
-}
-
-func TestWithContext(t *testing.T) {
-	tests := []struct {
-		name           string
-		steps          []time.Duration
-		finalCondition wait.ConditionWithContextFunc
-		expectedErr    error
-	}{
-		{
-			name: "cancelling results in cancellation error",
-			steps: []time.Duration{
-				time.Duration(0),
-				time.Duration(0),
-				time.Duration(0),
-			},
-			finalCondition: func(ctx context.Context) (done bool, err error) {
-				return false, nil
-			},
-			expectedErr: context.Canceled,
-		},
-		{
-			name: "when condition is done, exit early",
-			steps: []time.Duration{
-				time.Duration(0),
-				time.Duration(0),
-				time.Duration(0),
-			},
-			finalCondition: func(ctx context.Context) (done bool, err error) {
-				return true, nil
-			},
-			expectedErr: nil,
-		},
-		{
-			name: "when condition returns error, exit early",
-			steps: []time.Duration{
-				time.Duration(0),
-				time.Duration(0),
-				time.Duration(0),
-				time.Duration(0),
-				time.Duration(0),
-			},
-			finalCondition: func(ctx context.Context) (done bool, err error) {
-				return false, errors.New("error from condition")
-			},
-			expectedErr: errors.New("error from condition"),
-		},
-		{
-			name: "when condition panics, cover and exit",
-			steps: []time.Duration{
-				time.Duration(0),
-			},
-			finalCondition: func(ctx context.Context) (done bool, err error) {
-				panic(errors.New("panic error"))
-			},
-			expectedErr: errors.New("panic error"),
-		},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			testContext, cancel := context.WithCancel(context.Background())
-			backoff := &MockStepper{
-				steps: tt.steps,
-			}
-			actualConditionCalls := 0
-
-			err := WithContext(testContext, backoff, func(ctx context.Context) (done bool, err error) {
-				actualConditionCalls++
-
-				if backoff.currentStep >= (len(backoff.steps) - 1) {
-					cancel()
-					return tt.finalCondition(ctx)
-				}
-
-				return false, nil
-			})
-			require.Equal(t, tt.expectedErr, err)
-			require.Equal(t, len(backoff.steps), actualConditionCalls)
-		})
-	}
-
-	t.Run("does not invoke any functions when run with a cancelled context", func(t *testing.T) {
-		testContext, cancel := context.WithCancel(context.Background())
-		cancel()
-
-		stepper := &MockStepper{}
-
-		conditionCalls := 0
-		condition := func(context.Context) (done bool, err error) {
-			conditionCalls++
-			return false, nil
-		}
-
-		err := WithContext(testContext, stepper, condition)
-		require.Equal(t, context.Canceled, err)
-		require.Equal(t, 0, conditionCalls)
-		require.Equal(t, 0, stepper.currentStep)
-	})
-}

internal/concierge/apiserver/apiserver.go

@@ -21,7 +21,6 @@ import (
 	"go.pinniped.dev/internal/pversion"
 	"go.pinniped.dev/internal/registry/credentialrequest"
 	"go.pinniped.dev/internal/registry/whoamirequest"
-	"go.pinniped.dev/internal/tokenclient"
 )
 
 type Config struct {
@@ -37,7 +36,6 @@ type ExtraConfig struct {
 	NegotiatedSerializer          runtime.NegotiatedSerializer
 	LoginConciergeGroupVersion    schema.GroupVersion
 	IdentityConciergeGroupVersion schema.GroupVersion
-	TokenClient                   *tokenclient.TokenClient
 }
 
 type PinnipedServer struct {
@@ -136,24 +134,6 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 		},
 	)
 
-	s.GenericAPIServer.AddPostStartHookOrDie("fetch-impersonation-proxy-tokens",
-		func(postStartContext genericapiserver.PostStartHookContext) error {
-			plog.Debug("fetch-impersonation-proxy-tokens start hook starting")
-			defer plog.Debug("fetch-impersonation-proxy-tokens start hook completed")
-
-			controllersShutdownWaitGroup.Add(1)
-			go func() {
-				defer controllersShutdownWaitGroup.Done()
-
-				// Start the token client
-				c.ExtraConfig.TokenClient.Start(controllersCtx)
-				plog.Debug("fetch-impersonation-proxy-tokens start hook's background goroutine has finished")
-			}()
-
-			return nil
-		},
-	)
-
 	s.GenericAPIServer.AddPreShutdownHookOrDie("stop-controllers",
 		func() error {
 			plog.Debug("stop-controllers pre shutdown hook starting")

internal/concierge/impersonator/impersonator.go

@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"net/http/httputil"
 	"net/url"
+	"os"
 	"reflect"
 	"regexp"
 	"strings"
@@ -55,7 +56,6 @@ import (
 	"go.pinniped.dev/internal/httputil/securityheader"
 	"go.pinniped.dev/internal/kubeclient"
 	"go.pinniped.dev/internal/plog"
-	"go.pinniped.dev/internal/tokenclient"
 	"go.pinniped.dev/internal/valuelesscontext"
 )
 
@@ -68,38 +68,26 @@ type FactoryFunc func(
 	port int,
 	dynamicCertProvider dynamiccert.Private,
 	impersonationProxySignerCA dynamiccert.Public,
-	impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet,
 ) (func(stopCh <-chan struct{}) error, error)
 
 func New(
 	port int,
 	dynamicCertProvider dynamiccert.Private,
 	impersonationProxySignerCA dynamiccert.Public,
-	impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet,
 ) (func(stopCh <-chan struct{}) error, error) {
-	return newInternal(port, dynamicCertProvider, impersonationProxySignerCA, kubeclient.Secure, impersonationProxyTokenCache, nil, nil, nil)
+	return newInternal(port, dynamicCertProvider, impersonationProxySignerCA, kubeclient.Secure, nil, nil, nil)
 }
 
-//nolint:funlen // It is definitely too complicated. New calls newInternal, which makes another function.
-func newInternal(
+func newInternal( //nolint:funlen // yeah, it's kind of long.
 	port int,
 	dynamicCertProvider dynamiccert.Private,
 	impersonationProxySignerCA dynamiccert.Public,
 	restConfigFunc ptls.RestConfigFunc, // for unit testing, should always be kubeclient.Secure in production
-	cache tokenclient.ExpiringSingletonTokenCacheGet,
-	baseConfig *rest.Config, // for unit testing, should always be nil in production
+	clientOpts []kubeclient.Option, // for unit testing, should always be nil in production
 	recOpts func(*genericoptions.RecommendedOptions), // for unit testing, should always be nil in production
 	recConfig func(*genericapiserver.RecommendedConfig), // for unit testing, should always be nil in production
 ) (func(stopCh <-chan struct{}) error, error) {
 	var listener net.Listener
-	var err error
-
-	if baseConfig == nil {
-		baseConfig, err = rest.InClusterConfig()
-		if err != nil {
-			return nil, err
-		}
-	}
 
 	constructServer := func() (func(stopCh <-chan struct{}) error, error) {
 		// Bare minimum server side scheme to allow for status messages to be encoded.
@@ -129,7 +117,7 @@ func newInternal(
 		// along with the Kube API server's CA.
 		// Note: any changes to the Authentication stack need to be kept in sync with any assumptions made
 		// by getTransportForUser, especially if we ever update the TCR API to start returning bearer tokens.
-		kubeClientUnsafeForProxying, err := kubeclient.New(kubeclient.WithConfig(baseConfig))
+		kubeClientUnsafeForProxying, err := kubeclient.New(clientOpts...)
 		if err != nil {
 			return nil, err
 		}
@@ -180,8 +168,7 @@ func newInternal(
 		)
 
 		// use the custom impersonation proxy service account credentials when reverse proxying to the API server
-
-		kubeClientForProxy, err := getReverseProxyClient(baseConfig, cache)
+		kubeClientForProxy, err := getReverseProxyClient(clientOpts)
 		if err != nil {
 			return nil, fmt.Errorf("failed to build reverse proxy client: %w", err)
 		}
@@ -334,6 +321,11 @@ func newInternal(
 			return nil, fmt.Errorf("invalid mutation of impersonation authorizer detected: %#v", preparedRun.Authorizer)
 		}
 
+		// Sanity check. Assert that we have a functioning token file to use and no bearer token.
+		if len(preparedRun.LoopbackClientConfig.BearerToken) != 0 || len(preparedRun.LoopbackClientConfig.BearerTokenFile) == 0 {
+			return nil, constable.Error("invalid impersonator loopback rest config has wrong bearer token semantics")
+		}
+
 		return preparedRun.Run, nil
 	}
 
@@ -349,16 +341,28 @@ func newInternal(
 	return result, nil
 }
 
-func getReverseProxyClient(baseConfig *rest.Config, cache tokenclient.ExpiringSingletonTokenCacheGet) (*kubeclient.Client, error) {
-	impersonationProxyRestConfig := kubeclient.SecureAnonymousClientConfig(baseConfig)
-
-	authRoundTripper := func(base http.RoundTripper) http.RoundTripper {
-		return &authorizationRoundTripper{
-			cache: cache,
-			base:  base,
-		}
+func getReverseProxyClient(clientOpts []kubeclient.Option) (*kubeclient.Client, error) {
+	// just use the overrides given during unit tests
+	if len(clientOpts) != 0 {
+		return kubeclient.New(clientOpts...)
 	}
-	impersonationProxyRestConfig.Wrap(authRoundTripper)
+
+	// this is the magic path where the impersonation proxy SA token is mounted
+	const tokenFile = "/var/run/secrets/impersonation-proxy.concierge.pinniped.dev/serviceaccount/token" //nolint:gosec // this is not a credential
+
+	// make sure the token file we need exists before trying to use it
+	if _, err := os.Stat(tokenFile); err != nil {
+		return nil, err
+	}
+
+	// build an in cluster config that uses the impersonation proxy token file
+	impersonationProxyRestConfig, err := rest.InClusterConfig()
+	if err != nil {
+		return nil, err
+	}
+	impersonationProxyRestConfig = kubeclient.SecureAnonymousClientConfig(impersonationProxyRestConfig)
+	impersonationProxyRestConfig.BearerTokenFile = tokenFile
+
 	return kubeclient.New(kubeclient.WithConfig(impersonationProxyRestConfig))
 }
 

internal/concierge/impersonator/impersonator_test.go

@@ -55,10 +55,8 @@ import (
 	"go.pinniped.dev/internal/httputil/roundtripper"
 	"go.pinniped.dev/internal/kubeclient"
 	"go.pinniped.dev/internal/testutil/tlsserver"
-	"go.pinniped.dev/internal/tokenclient"
 )
 
-// TODO: add a test without a token?
 func TestImpersonator(t *testing.T) {
 	const (
 		priorityLevelConfigurationsVersion = "v1beta3"
@@ -86,22 +84,24 @@ func TestImpersonator(t *testing.T) {
 	defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.APIPriorityAndFairness, false)()
 
 	tests := []struct {
-		name                            string
-		clientCert                      *clientCert
-		clientImpersonateUser           rest.ImpersonationConfig
-		clientMutateHeaders             func(http.Header)
-		clientNextProtos                []string
-		kubeAPIServerStatusCode         int
-		kubeAPIServerHealthz            http.Handler
-		anonymousAuthDisabled           bool
-		wantKubeAPIServerRequestHeaders http.Header
-		wantError                       string
-		wantConstructionError           string
-		wantAuthorizerAttributes        []authorizer.AttributesRecord
+		name                               string
+		clientCert                         *clientCert
+		clientImpersonateUser              rest.ImpersonationConfig
+		clientMutateHeaders                func(http.Header)
+		clientNextProtos                   []string
+		kubeAPIServerClientBearerTokenFile string
+		kubeAPIServerStatusCode            int
+		kubeAPIServerHealthz               http.Handler
+		anonymousAuthDisabled              bool
+		wantKubeAPIServerRequestHeaders    http.Header
+		wantError                          string
+		wantConstructionError              string
+		wantAuthorizerAttributes           []authorizer.AttributesRecord
 	}{
 		{
-			name:       "happy path",
-			clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			name:                               "happy path",
+			clientCert:                         newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			wantKubeAPIServerRequestHeaders: http.Header{
 				"Impersonate-User":  {"test-username"},
 				"Impersonate-Group": {"test-group1", "test-group2", "system:authenticated"},
@@ -119,8 +119,9 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:       "happy path with forbidden healthz",
-			clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			name:                               "happy path with forbidden healthz",
+			clientCert:                         newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			kubeAPIServerHealthz: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusForbidden)
 				_, _ = w.Write([]byte("no healthz for you"))
@@ -142,8 +143,9 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:       "happy path with unauthorized healthz",
-			clientCert: newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			name:                               "happy path with unauthorized healthz",
+			clientCert:                         newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			kubeAPIServerHealthz: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 				w.WriteHeader(http.StatusUnauthorized)
 				_, _ = w.Write([]byte("no healthz for you"))
@@ -166,8 +168,9 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:       "happy path with upgrade",
-			clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
+			name:                               "happy path with upgrade",
+			clientCert:                         newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			clientMutateHeaders: func(header http.Header) {
 				header.Add("Connection", "Upgrade")
 				header.Add("Upgrade", "spdy/3.1")
@@ -196,8 +199,9 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:       "happy path ignores forwarded header",
-			clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
+			name:                               "happy path ignores forwarded header",
+			clientCert:                         newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			clientMutateHeaders: func(header http.Header) {
 				header.Add("X-Forwarded-For", "example.com")
 			},
@@ -218,8 +222,9 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:       "happy path ignores forwarded header canonicalization",
-			clientCert: newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
+			name:                               "happy path ignores forwarded header canonicalization",
+			clientCert:                         newClientCert(t, ca, "test-username2", []string{"test-group3", "test-group4"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			clientMutateHeaders: func(header http.Header) {
 				header["x-FORWARDED-for"] = append(header["x-FORWARDED-for"], "example.com")
 			},
@@ -240,10 +245,11 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:                    "user is authenticated but the kube API request returns an error",
-			kubeAPIServerStatusCode: http.StatusNotFound,
-			clientCert:              newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
-			wantError:               `the server could not find the requested resource (get namespaces)`,
+			name:                               "user is authenticated but the kube API request returns an error",
+			kubeAPIServerStatusCode:            http.StatusNotFound,
+			clientCert:                         newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          `the server could not find the requested resource (get namespaces)`,
 			wantKubeAPIServerRequestHeaders: http.Header{
 				"Impersonate-User":  {"test-username"},
 				"Impersonate-Group": {"test-group1", "test-group2", "system:authenticated"},
@@ -261,8 +267,9 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:       "when there is no client cert on request, it is an anonymous request",
-			clientCert: &clientCert{},
+			name:                               "when there is no client cert on request, it is an anonymous request",
+			clientCert:                         &clientCert{},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			wantKubeAPIServerRequestHeaders: http.Header{
 				"Impersonate-User":  {"system:anonymous"},
 				"Impersonate-Group": {"system:unauthenticated"},
@@ -287,6 +294,7 @@ func TestImpersonator(t *testing.T) {
 				req := &http.Request{Header: header}
 				req.SetBasicAuth("foo", "bar")
 			},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			wantKubeAPIServerRequestHeaders: http.Header{
 				"Impersonate-User":  {"system:anonymous"},
 				"Impersonate-Group": {"system:unauthenticated"},
@@ -305,15 +313,17 @@ func TestImpersonator(t *testing.T) {
 			},
 		},
 		{
-			name:                     "failed client cert authentication",
-			clientCert:               newClientCert(t, unrelatedCA, "test-username", []string{"test-group1"}),
-			wantError:                "Unauthorized",
-			wantAuthorizerAttributes: nil,
+			name:                               "failed client cert authentication",
+			clientCert:                         newClientCert(t, unrelatedCA, "test-username", []string{"test-group1"}),
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          "Unauthorized",
+			wantAuthorizerAttributes:           nil,
 		},
 		{
-			name:                  "nested impersonation by regular users calls delegating authorizer",
-			clientCert:            newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
-			clientImpersonateUser: rest.ImpersonationConfig{UserName: "some-other-username"},
+			name:                               "nested impersonation by regular users calls delegating authorizer",
+			clientCert:                         newClientCert(t, ca, "test-username", []string{"test-group1", "test-group2"}),
+			clientImpersonateUser:              rest.ImpersonationConfig{UserName: "some-other-username"},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			// this fails because the delegating authorizer in this test only allows system:masters and fails everything else
 			wantError: `users "some-other-username" is forbidden: User "test-username" ` +
 				`cannot impersonate resource "users" in API group "" at the cluster scope: ` +
@@ -349,6 +359,7 @@ func TestImpersonator(t *testing.T) {
 					"alpha.kubernetes.io/identity/user/domain/name": {"a-domain-name"},
 				},
 			},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			wantKubeAPIServerRequestHeaders: http.Header{
 				"Impersonate-User":                                                                {"fire"},
 				"Impersonate-Group":                                                               {"elements", "system:authenticated"},
@@ -466,7 +477,8 @@ func TestImpersonator(t *testing.T) {
 			clientMutateHeaders: func(header http.Header) {
 				header["Impersonate-Uid"] = []string{"root"}
 			},
-			wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          "Internal error occurred: unimplemented functionality - unable to act as current user",
 			wantAuthorizerAttributes: []authorizer.AttributesRecord{
 				{
 					User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -489,7 +501,8 @@ func TestImpersonator(t *testing.T) {
 			clientMutateHeaders: func(header http.Header) {
 				header["imPerSoNaTE-uid"] = []string{"magic"}
 			},
-			wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          "Internal error occurred: unimplemented functionality - unable to act as current user",
 			wantAuthorizerAttributes: []authorizer.AttributesRecord{
 				{
 					User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -516,7 +529,8 @@ func TestImpersonator(t *testing.T) {
 					"something.impersonation-proxy.concierge.pinniped.dev": {"bad data"},
 				},
 			},
-			wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          "Internal error occurred: unimplemented functionality - unable to act as current user",
 			wantAuthorizerAttributes: []authorizer.AttributesRecord{
 				{
 					User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -555,7 +569,8 @@ func TestImpersonator(t *testing.T) {
 					"party~~time": {"danger"},
 				},
 			},
-			wantError: "Internal error occurred: unimplemented functionality - unable to act as current user",
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          "Internal error occurred: unimplemented functionality - unable to act as current user",
 			wantAuthorizerAttributes: []authorizer.AttributesRecord{
 				{
 					User: &user.DefaultInfo{Name: "test-admin", UID: "", Groups: []string{"test-group2", "system:masters", "system:authenticated"}, Extra: nil},
@@ -585,6 +600,7 @@ func TestImpersonator(t *testing.T) {
 					"ROAR": {"tiger"}, // by the time our code sees this key, it is lowercased to "roar"
 				},
 			},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			wantKubeAPIServerRequestHeaders: http.Header{
 				"Impersonate-User":       {"panda"},
 				"Impersonate-Group":      {"other-peeps", "system:authenticated"},
@@ -615,6 +631,11 @@ func TestImpersonator(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:                     "no bearer token file in Kube API server client config",
+			wantConstructionError:    "invalid impersonator loopback rest config has wrong bearer token semantics",
+			wantAuthorizerAttributes: nil,
+		},
 		{
 			name: "unexpected healthz response",
 			kubeAPIServerHealthz: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -630,6 +651,7 @@ func TestImpersonator(t *testing.T) {
 			clientMutateHeaders: func(header http.Header) {
 				header["imPerSonaTE-USer"] = []string{"PANDA"}
 			},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
 			wantError: `users "PANDA" is forbidden: User "test-username" ` +
 				`cannot impersonate resource "users" in API group "" at the cluster scope: ` +
 				`decision made by impersonation-proxy.concierge.pinniped.dev`,
@@ -646,8 +668,9 @@ func TestImpersonator(t *testing.T) {
 			clientMutateHeaders: func(header http.Header) {
 				header["imPerSonaTE-uid"] = []string{"007"}
 			},
-			wantError:                `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID  007  authentication.k8s.io/v1  }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
-			wantAuthorizerAttributes: []authorizer.AttributesRecord{},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID  007  authentication.k8s.io/v1  }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
+			wantAuthorizerAttributes:           []authorizer.AttributesRecord{},
 		},
 		{
 			name:       "future UID header", // no longer future as it exists in Kube v1.22
@@ -655,8 +678,9 @@ func TestImpersonator(t *testing.T) {
 			clientMutateHeaders: func(header http.Header) {
 				header["Impersonate-Uid"] = []string{"008"}
 			},
-			wantError:                `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID  008  authentication.k8s.io/v1  }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
-			wantAuthorizerAttributes: []authorizer.AttributesRecord{},
+			kubeAPIServerClientBearerTokenFile: "required-to-be-set",
+			wantError:                          `an error on the server ("Internal Server Error: \"/api/v1/namespaces\": requested [{UID  008  authentication.k8s.io/v1  }] without impersonating a user") has prevented the request from succeeding (get namespaces)`,
+			wantAuthorizerAttributes:           []authorizer.AttributesRecord{},
 		},
 	}
 	for _, tt := range tests {
@@ -806,8 +830,11 @@ func TestImpersonator(t *testing.T) {
 			// Create the client config that the impersonation server should use to talk to the Kube API server.
 			testKubeAPIServerKubeconfig := rest.Config{
 				Host:            testKubeAPIServer.URL,
+				BearerToken:     "some-service-account-token",
 				TLSClientConfig: rest.TLSClientConfig{CAData: tlsserver.TLSTestServerCA(testKubeAPIServer)},
+				BearerTokenFile: tt.kubeAPIServerClientBearerTokenFile,
 			}
+			clientOpts := []kubeclient.Option{kubeclient.WithConfig(&testKubeAPIServerKubeconfig)}
 
 			// Punch out just enough stuff to make New actually run without error.
 			recOpts := func(options *genericoptions.RecommendedOptions) {
@@ -846,9 +873,7 @@ func TestImpersonator(t *testing.T) {
 			}
 
 			// Create an impersonator.  Use an invalid port number to make sure our listener override works.
-			cache := tokenclient.NewExpiringSingletonTokenCache()
-			cache.Set("some-service-account-token", 1*time.Hour)
-			runner, constructionErr := newInternal(-1000, certKeyContent, caContent, restConfigFunc, cache, &testKubeAPIServerKubeconfig, recOpts, recConfig)
+			runner, constructionErr := newInternal(-1000, certKeyContent, caContent, restConfigFunc, clientOpts, recOpts, recConfig)
 			if len(tt.wantConstructionError) > 0 {
 				require.EqualError(t, constructionErr, tt.wantConstructionError)
 				require.Nil(t, runner)
@@ -897,7 +922,7 @@ func TestImpersonator(t *testing.T) {
 			client, err := kubeclient.New(kubeclient.WithConfig(clientKubeconfig))
 			require.NoError(t, err)
 
-			// The fake Kube API server knows how to list namespaces, so make that request using the client
+			// The fake Kube API server knows how to to list namespaces, so make that request using the client
 			// through the impersonator.
 			listResponse, err := client.Kubernetes.CoreV1().Namespaces().List(ctx, metav1.ListOptions{})
 			if len(tt.wantError) > 0 {

internal/concierge/impersonator/roundtripper.go

@@ -1,34 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package impersonator
-
-import (
-	"fmt"
-	"net/http"
-
-	utilnet "k8s.io/apimachinery/pkg/util/net"
-
-	"go.pinniped.dev/internal/tokenclient"
-)
-
-type authorizationRoundTripper struct {
-	cache tokenclient.ExpiringSingletonTokenCacheGet
-	base  http.RoundTripper
-}
-
-var _ utilnet.RoundTripperWrapper = (*authorizationRoundTripper)(nil)
-
-func (rt *authorizationRoundTripper) WrappedRoundTripper() http.RoundTripper {
-	return rt.base
-}
-
-func (rt *authorizationRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
-	req = utilnet.CloneRequest(req)
-	token := rt.cache.Get()
-	if token == "" {
-		return nil, fmt.Errorf("no token available")
-	}
-	req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token))
-	return rt.base.RoundTrip(req)
-}

internal/concierge/impersonator/roundtripper_test.go

@@ -1,107 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package impersonator
-
-import (
-	"errors"
-	"net/http"
-	"testing"
-
-	"github.com/stretchr/testify/require"
-	"golang.org/x/oauth2"
-
-	"go.pinniped.dev/internal/tokenclient"
-)
-
-func TestWrappedRoundTripper(t *testing.T) {
-	var base = new(oauth2.Transport)
-
-	roundTripper := authorizationRoundTripper{
-		base: base,
-	}
-
-	require.Equal(t, base, roundTripper.WrappedRoundTripper())
-}
-
-type fakeRoundTripper struct {
-	request  *http.Request
-	response *http.Response
-	err      error
-}
-
-func (t *fakeRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) {
-	t.request = request
-	return t.response, t.err
-}
-
-var _ http.RoundTripper = (*fakeRoundTripper)(nil)
-
-type fakeCache struct {
-	token string
-}
-
-func (c *fakeCache) Get() string {
-	return c.token
-}
-
-var _ tokenclient.ExpiringSingletonTokenCacheGet = (*fakeCache)(nil)
-
-func TestRoundTrip(t *testing.T) {
-	fakeResponse := new(http.Response)
-	for _, tt := range []struct {
-		name         string
-		token        string
-		baseResponse *http.Response
-		baseError    string
-		wantResponse *http.Response
-		wantError    string
-	}{
-		{
-			name:         "happy path",
-			token:        "token",
-			baseResponse: fakeResponse,
-			baseError:    "error from base",
-			wantResponse: fakeResponse,
-			wantError:    "error from base",
-		},
-		{
-			name:      "no token available",
-			token:     "", // since the cache always returns a non-pointer string, this indicates empty
-			wantError: "no token available",
-		},
-	} {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			base := &fakeRoundTripper{
-				response: new(http.Response),
-				err:      errors.New(tt.baseError),
-			}
-
-			cache := &fakeCache{
-				token: tt.token,
-			}
-
-			roundTripper := &authorizationRoundTripper{
-				cache: cache,
-				base:  base,
-			}
-
-			//nolint:noctx // this is test code
-			request, err := http.NewRequest("GET", "https://example.com", nil)
-			require.NoError(t, err)
-			defer request.Body.Close()
-
-			response, err := roundTripper.RoundTrip(request)
-			require.Equal(t, tt.wantResponse, response)
-			require.ErrorContains(t, err, tt.wantError)
-			defer response.Body.Close()
-
-			if tt.token != "" {
-				require.Equal(t, "Bearer "+tt.token, base.request.Header.Get("Authorization"))
-			} else {
-				require.Empty(t, base.request)
-			}
-		})
-	}
-}

internal/concierge/server/server.go

@@ -12,8 +12,6 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
-	authenticationv1 "k8s.io/api/authentication/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
@@ -40,7 +38,6 @@ import (
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/pversion"
 	"go.pinniped.dev/internal/registry/credentialrequest"
-	"go.pinniped.dev/internal/tokenclient"
 )
 
 // App is an object that represents the pinniped-concierge application.
@@ -139,8 +136,6 @@ func (a *App) runServer(ctx context.Context) error {
 	// injected suffix).
 	scheme, loginGV, identityGV := conciergescheme.New(*cfg.APIGroupSuffix)
 
-	impersonationProxyTokenCache := tokenclient.NewExpiringSingletonTokenCache()
-
 	// Prepare to start the controllers, but defer actually starting them until the
 	// post start hook of the aggregated API server.
 	buildControllers, err := controllermanager.PrepareControllers(
@@ -159,7 +154,6 @@ func (a *App) runServer(ctx context.Context) error {
 			AuthenticatorCache:               authenticators,
 			// This port should be safe to cast because the config reader already validated it.
 			ImpersonationProxyServerPort: int(*cfg.ImpersonationProxyServerPort),
-			ImpersonationProxyTokenCache: impersonationProxyTokenCache,
 		},
 	)
 	if err != nil {
@@ -187,22 +181,6 @@ func (a *App) runServer(ctx context.Context) error {
 		return fmt.Errorf("could not configure aggregated API server: %w", err)
 	}
 
-	oneDayInSeconds := int64(24 * 60 * 60)
-	k8sClient, err := kubeclient.New()
-	if err != nil {
-		return fmt.Errorf("could not create default kubernetes client: %w", err)
-	}
-	aggregatedAPIServerConfig.ExtraConfig.TokenClient = tokenclient.New(
-		podInfo.Namespace,
-		cfg.NamesConfig.ImpersonationProxyServiceAccount,
-		k8sClient.Kubernetes,
-		func(tokenRequestStatus authenticationv1.TokenRequestStatus, ttl metav1.Duration) error {
-			impersonationProxyTokenCache.Set(tokenRequestStatus.Token, ttl.Duration)
-			return nil
-		},
-		plog.New(),
-		tokenclient.WithExpirationSeconds(oneDayInSeconds))
-
 	// Complete the aggregated API server config and make a server instance.
 	server, err := aggregatedAPIServerConfig.Complete().New()
 	if err != nil {

internal/config/concierge/config.go

@@ -161,12 +161,6 @@ func validateNames(names *NamesConfigSpec) error {
 	if names.AgentServiceAccount == "" {
 		missingNames = append(missingNames, "agentServiceAccount")
 	}
-	if names.ImpersonationProxyServiceAccount == "" {
-		missingNames = append(missingNames, "impersonationProxyServiceAccount")
-	}
-	if names.ImpersonationProxyLegacySecret == "" {
-		missingNames = append(missingNames, "impersonationProxyLegacySecret")
-	}
 	if len(missingNames) > 0 {
 		return constable.Error("missing required names: " + strings.Join(missingNames, ", "))
 	}

internal/config/concierge/config_test.go

@@ -47,8 +47,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 				  extraName: extraName-value
 				labels:
 				  myLabelKey1: myLabelValue1
@@ -82,8 +80,6 @@ func TestFromPath(t *testing.T) {
 					ImpersonationCACertificateSecret:  "impersonationCACertificateSecret-value",
 					ImpersonationSignerSecret:         "impersonationSignerSecret-value",
 					AgentServiceAccount:               "agentServiceAccount-value",
-					ImpersonationProxyServiceAccount:  "impersonationProxyServiceAccount-value",
-					ImpersonationProxyLegacySecret:    "impersonationProxyLegacySecret-value",
 				},
 				Labels: map[string]string{
 					"myLabelKey1": "myLabelValue1",
@@ -125,8 +121,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 				  extraName: extraName-value
 				labels:
 				  myLabelKey1: myLabelValue1
@@ -162,8 +156,6 @@ func TestFromPath(t *testing.T) {
 					ImpersonationCACertificateSecret:  "impersonationCACertificateSecret-value",
 					ImpersonationSignerSecret:         "impersonationSignerSecret-value",
 					AgentServiceAccount:               "agentServiceAccount-value",
-					ImpersonationProxyServiceAccount:  "impersonationProxyServiceAccount-value",
-					ImpersonationProxyLegacySecret:    "impersonationProxyLegacySecret-value",
 				},
 				Labels: map[string]string{
 					"myLabelKey1": "myLabelValue1",
@@ -205,8 +197,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 				  extraName: extraName-value
 				labels:
 				  myLabelKey1: myLabelValue1
@@ -243,8 +233,6 @@ func TestFromPath(t *testing.T) {
 					ImpersonationCACertificateSecret:  "impersonationCACertificateSecret-value",
 					ImpersonationSignerSecret:         "impersonationSignerSecret-value",
 					AgentServiceAccount:               "agentServiceAccount-value",
-					ImpersonationProxyServiceAccount:  "impersonationProxyServiceAccount-value",
-					ImpersonationProxyLegacySecret:    "impersonationProxyLegacySecret-value",
 				},
 				Labels: map[string]string{
 					"myLabelKey1": "myLabelValue1",
@@ -276,7 +264,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
 				log:
 				  level: all
 				  format: snorlax
@@ -297,8 +284,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantConfig: &Config{
 				DiscoveryInfo: DiscoveryInfoSpec{
@@ -323,8 +308,6 @@ func TestFromPath(t *testing.T) {
 					ImpersonationCACertificateSecret:  "impersonationCACertificateSecret-value",
 					ImpersonationSignerSecret:         "impersonationSignerSecret-value",
 					AgentServiceAccount:               "agentServiceAccount-value",
-					ImpersonationProxyServiceAccount:  "impersonationProxyServiceAccount-value",
-					ImpersonationProxyLegacySecret:    "impersonationProxyLegacySecret-value",
 				},
 				Labels: map[string]string{},
 				KubeCertAgentConfig: KubeCertAgentSpec{
@@ -339,7 +322,7 @@ func TestFromPath(t *testing.T) {
 			wantError: "validate names: missing required names: servingCertificateSecret, credentialIssuer, " +
 				"apiService, impersonationLoadBalancerService, " +
 				"impersonationClusterIPService, impersonationTLSCertificateSecret, impersonationCACertificateSecret, " +
-				"impersonationSignerSecret, agentServiceAccount, impersonationProxyServiceAccount, impersonationProxyLegacySecret",
+				"impersonationSignerSecret, agentServiceAccount",
 		},
 		{
 			name: "Missing apiService name",
@@ -354,8 +337,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: apiService",
 		},
@@ -372,8 +353,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: credentialIssuer",
 		},
@@ -390,8 +369,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: servingCertificateSecret",
 		},
@@ -408,8 +385,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: impersonationLoadBalancerService",
 		},
@@ -426,8 +401,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: impersonationClusterIPService",
 		},
@@ -444,8 +417,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: impersonationTLSCertificateSecret",
 		},
@@ -462,8 +433,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: impersonationCACertificateSecret",
 		},
@@ -480,47 +449,9 @@ func TestFromPath(t *testing.T) {
 				  impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
 				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: impersonationSignerSecret",
 		},
-		{
-			name: "Missing impersonationProxyServiceAccount name",
-			yaml: here.Doc(`
-				---
-				names:
-				  servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate
-				  credentialIssuer: pinniped-config
-				  apiService: pinniped-api
-				  impersonationLoadBalancerService: impersonationLoadBalancerService-value
-				  impersonationClusterIPService: impersonationClusterIPService-value
-				  impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
-				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
-				  impersonationSignerSecret: impersonationSignerSecret-value
-				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
-			`),
-			wantError: "validate names: missing required names: impersonationProxyServiceAccount",
-		},
-		{
-			name: "Missing impersonationProxyLegacySecret name",
-			yaml: here.Doc(`
-				---
-				names:
-				  servingCertificateSecret: pinniped-concierge-api-tls-serving-certificate
-				  credentialIssuer: pinniped-config
-				  apiService: pinniped-api
-				  impersonationLoadBalancerService: impersonationLoadBalancerService-value
-				  impersonationClusterIPService: impersonationClusterIPService-value
-				  impersonationTLSCertificateSecret: impersonationTLSCertificateSecret-value
-				  impersonationCACertificateSecret: impersonationCACertificateSecret-value
-				  impersonationSignerSecret: impersonationSignerSecret-value
-				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-			`),
-			wantError: "validate names: missing required names: impersonationProxyLegacySecret",
-		},
 		{
 			name: "Missing several required names",
 			yaml: here.Doc(`
@@ -533,8 +464,6 @@ func TestFromPath(t *testing.T) {
 				  impersonationClusterIPService: impersonationClusterIPService-value
 				  impersonationSignerSecret: impersonationSignerSecret-value
 				  agentServiceAccount: agentServiceAccount-value
-				  impersonationProxyServiceAccount: impersonationProxyServiceAccount-value
-				  impersonationProxyLegacySecret: impersonationProxyLegacySecret-value
 			`),
 			wantError: "validate names: missing required names: " +
 				"impersonationTLSCertificateSecret, impersonationCACertificateSecret",

internal/config/concierge/types.go

@@ -45,8 +45,6 @@ type NamesConfigSpec struct {
 	ImpersonationCACertificateSecret  string `json:"impersonationCACertificateSecret"`
 	ImpersonationSignerSecret         string `json:"impersonationSignerSecret"`
 	AgentServiceAccount               string `json:"agentServiceAccount"`
-	ImpersonationProxyServiceAccount  string `json:"impersonationProxyServiceAccount"`
-	ImpersonationProxyLegacySecret    string `json:"impersonationProxyLegacySecret"`
 }
 
 // ServingCertificateConfigSpec contains the configuration knobs for the API's

internal/controller/impersonatorconfig/impersonator_config.go

@@ -47,7 +47,6 @@ import (
 	"go.pinniped.dev/internal/dynamiccert"
 	"go.pinniped.dev/internal/endpointaddr"
 	"go.pinniped.dev/internal/plog"
-	"go.pinniped.dev/internal/tokenclient"
 )
 
 const (
@@ -88,8 +87,6 @@ type impersonatorConfigController struct {
 	tlsServingCertDynamicCertProvider dynamiccert.Private
 	infoLog                           logr.Logger
 	debugLog                          logr.Logger
-
-	impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet
 }
 
 func NewImpersonatorConfigController(
@@ -112,7 +109,6 @@ func NewImpersonatorConfigController(
 	impersonationSignerSecretName string,
 	impersonationSigningCertProvider dynamiccert.Provider,
 	log logr.Logger,
-	impersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet,
 ) controllerlib.Controller {
 	secretNames := sets.NewString(tlsSecretName, caSecretName, impersonationSignerSecretName)
 	log = log.WithName("impersonator-config-controller")
@@ -140,7 +136,6 @@ func NewImpersonatorConfigController(
 				tlsServingCertDynamicCertProvider: dynamiccert.NewServingCert("impersonation-proxy-serving-cert"),
 				infoLog:                           log.V(plog.KlogLevelInfo),
 				debugLog:                          log.V(plog.KlogLevelDebug),
-				impersonationProxyTokenCache:      impersonationProxyTokenCache,
 			},
 		},
 		withInformer(credentialIssuerInformer,
@@ -493,7 +488,6 @@ func (c *impersonatorConfigController) ensureImpersonatorIsStarted(syncCtx contr
 		c.impersonationProxyPort,
 		c.tlsServingCertDynamicCertProvider,
 		c.impersonationSigningCertProvider,
-		c.impersonationProxyTokenCache,
 	)
 	if err != nil {
 		return err

internal/controller/impersonatorconfig/impersonator_config_test.go

@@ -45,7 +45,6 @@ import (
 	"go.pinniped.dev/internal/kubeclient"
 	"go.pinniped.dev/internal/plog"
 	"go.pinniped.dev/internal/testutil"
-	"go.pinniped.dev/internal/tokenclient"
 )
 
 func TestImpersonatorConfigControllerOptions(t *testing.T) {
@@ -94,7 +93,6 @@ func TestImpersonatorConfigControllerOptions(t *testing.T) {
 				caSignerName,
 				nil,
 				plog.Logr(), //nolint:staticcheck // old test with no log assertions
-				nil,
 			)
 			credIssuerInformerFilter = observableWithInformerOption.GetFilterForInformer(credIssuerInformer)
 			servicesInformerFilter = observableWithInformerOption.GetFilterForInformer(servicesInformer)
@@ -310,7 +308,6 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 			port int,
 			dynamicCertProvider dynamiccert.Private,
 			impersonationProxySignerCAProvider dynamiccert.Public,
-			impersationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet,
 		) (func(stopCh <-chan struct{}) error, error) {
 			impersonatorFuncWasCalled++
 			r.Equal(8444, port)
@@ -583,8 +580,7 @@ func TestImpersonatorConfigControllerSync(t *testing.T) {
 				impersonatorFunc,
 				mTLSClientCertCASecretName,
 				mTLSClientCertProvider,
-				plog.Logr(), //nolint:staticcheck  // old test with no log assertions,
-				nil,
+				plog.Logr(), //nolint:staticcheck  // old test with no log assertions
 			)
 			controllerlib.TestWrap(t, subject, func(syncer controllerlib.Syncer) controllerlib.Syncer {
 				tlsServingCertDynamicCertProvider = syncer.(*impersonatorConfigController).tlsServingCertDynamicCertProvider

internal/controller/secrets/secrets.go

@@ -1,92 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package secrets
-
-import (
-	"fmt"
-
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/labels"
-	corev1informers "k8s.io/client-go/informers/core/v1"
-	"k8s.io/client-go/kubernetes"
-
-	pinnipedcontroller "go.pinniped.dev/internal/controller"
-	"go.pinniped.dev/internal/controllerlib"
-	"go.pinniped.dev/internal/plog"
-)
-
-func NewServiceAccountTokenCleanupController(
-	namespace string,
-	legacySecretName string,
-	k8sClient kubernetes.Interface,
-	secretInformer corev1informers.SecretInformer,
-	withInformer pinnipedcontroller.WithInformerOptionFunc,
-	logger plog.Logger,
-) controllerlib.Controller {
-	name := "service-account-token-cleanup-controller"
-	return controllerlib.New(controllerlib.Config{
-		Name: name,
-		Syncer: &serviceAccountTokenCleanupController{
-			name:             name,
-			namespace:        namespace,
-			legacySecretName: legacySecretName,
-			k8sClient:        k8sClient,
-			secretInformer:   secretInformer,
-			logger:           logger.WithName(name),
-		},
-	},
-		withInformer(
-			secretInformer,
-			pinnipedcontroller.SimpleFilterWithSingletonQueue(func(obj metav1.Object) bool {
-				secret, ok := obj.(*corev1.Secret)
-
-				return obj.GetNamespace() == namespace &&
-					obj.GetName() == legacySecretName &&
-					ok &&
-					secret.Type == corev1.SecretTypeServiceAccountToken
-			}),
-			controllerlib.InformerOption{},
-		))
-}
-
-type serviceAccountTokenCleanupController struct {
-	name             string
-	namespace        string
-	legacySecretName string
-	k8sClient        kubernetes.Interface
-	secretInformer   corev1informers.SecretInformer
-	logger           plog.Logger
-}
-
-func (c serviceAccountTokenCleanupController) Sync(syncCtx controllerlib.Context) error {
-	secrets, err := c.secretInformer.Lister().Secrets(c.namespace).List(labels.Everything())
-	if err != nil {
-		return fmt.Errorf("unable to list all secrets in namespace %s: %w", c.namespace, err)
-	}
-	c.logger.Info(fmt.Sprintf("You have now arrived in the %s controller, found %d secrets", c.name, len(secrets)))
-
-	foundSecret := false
-	for _, secret := range secrets {
-		if secret.Name == c.legacySecretName && secret.Type == corev1.SecretTypeServiceAccountToken {
-			foundSecret = true
-		}
-	}
-
-	c.logger.Info(fmt.Sprintf(
-		"The %s controller has found a secret of name %s to delete with type %s",
-		c.name,
-		c.legacySecretName,
-		corev1.SecretTypeServiceAccountToken,
-	))
-
-	if foundSecret {
-		err = c.k8sClient.CoreV1().Secrets(c.namespace).Delete(syncCtx.Context, c.legacySecretName, metav1.DeleteOptions{})
-		if err != nil {
-			return fmt.Errorf("unable to delete secret %s in namespace %s: %w", c.legacySecretName, c.namespace, err)
-		}
-	}
-
-	return nil
-}

internal/controller/secrets/secrets_test.go

@@ -1,296 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package secrets
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"fmt"
-	"testing"
-
-	"github.com/stretchr/testify/require"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	kubeinformers "k8s.io/client-go/informers"
-	kubernetesfake "k8s.io/client-go/kubernetes/fake"
-	kubetesting "k8s.io/client-go/testing"
-
-	"go.pinniped.dev/internal/controllerlib"
-	"go.pinniped.dev/internal/plog"
-	"go.pinniped.dev/internal/testutil"
-)
-
-func TestNewServiceAccountTokenCleanupController(t *testing.T) {
-	namespace := "a-namespace"
-	legacySecretName := "a-secret"
-	observableWithInformerOption := testutil.NewObservableWithInformerOption()
-	secretsInformer := kubeinformers.NewSharedInformerFactory(nil, 0).Core().V1().Secrets()
-
-	var log bytes.Buffer
-	_ = NewServiceAccountTokenCleanupController(
-		namespace,
-		legacySecretName,
-		nil, // not needed for this test
-		secretsInformer,
-		observableWithInformerOption.WithInformer,
-		plog.TestLogger(t, &log),
-	)
-
-	secretsInformerFilter := observableWithInformerOption.GetFilterForInformer(secretsInformer)
-
-	legacySecret := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: legacySecretName, Namespace: namespace}, Type: corev1.SecretTypeServiceAccountToken}
-	wrongName := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrongName", Namespace: namespace}, Type: corev1.SecretTypeServiceAccountToken}
-	wrongType := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrongType", Namespace: namespace}, Type: "other-type"}
-	wrongNamespace := &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "wrongNamespace", Namespace: "wrong-namespace"}, Type: corev1.SecretTypeServiceAccountToken}
-	wrongObject := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "config-map", Namespace: namespace}}
-
-	require.False(t, secretsInformerFilter.Add(wrongName))
-	require.False(t, secretsInformerFilter.Update(wrongName, wrongNamespace))
-	require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongName))
-	require.False(t, secretsInformerFilter.Delete(wrongName))
-
-	require.False(t, secretsInformerFilter.Add(wrongObject))
-	require.False(t, secretsInformerFilter.Update(wrongObject, wrongNamespace))
-	require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongObject))
-	require.False(t, secretsInformerFilter.Delete(wrongObject))
-
-	require.False(t, secretsInformerFilter.Add(wrongNamespace))
-	require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongObject))
-	require.False(t, secretsInformerFilter.Update(wrongObject, wrongNamespace))
-	require.False(t, secretsInformerFilter.Delete(wrongNamespace))
-
-	require.False(t, secretsInformerFilter.Add(wrongType))
-	require.False(t, secretsInformerFilter.Update(wrongType, wrongNamespace))
-	require.False(t, secretsInformerFilter.Update(wrongNamespace, wrongType))
-	require.False(t, secretsInformerFilter.Delete(wrongType))
-
-	require.True(t, secretsInformerFilter.Add(legacySecret))
-	require.True(t, secretsInformerFilter.Update(legacySecret, wrongNamespace))
-	require.True(t, secretsInformerFilter.Update(wrongNamespace, legacySecret))
-	require.True(t, secretsInformerFilter.Delete(legacySecret))
-}
-
-func TestSync(t *testing.T) {
-	kubeAPIClient := kubernetesfake.NewSimpleClientset()
-	kubeInformerClient := kubernetesfake.NewSimpleClientset()
-
-	kubeInformers := kubeinformers.NewSharedInformerFactory(
-		kubeInformerClient,
-		0,
-	)
-
-	namespace := "some-namespace"
-	secretToDelete := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "secret-to-delete",
-			Namespace: namespace,
-		},
-		Type: corev1.SecretTypeServiceAccountToken,
-	}
-	secretWithWrongName := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "wrong-name",
-			Namespace: namespace,
-		},
-		Type: corev1.SecretTypeServiceAccountToken,
-	}
-	secretWithWrongType := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "secret-to-leave-alone",
-			Namespace: namespace,
-		},
-	}
-	secretWithWrongNamespace := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "secret-to-delete",
-			Namespace: "other",
-		},
-		Type: corev1.SecretTypeServiceAccountToken,
-	}
-
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretToDelete))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretToDelete))
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongName))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongName))
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongType))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongType))
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongNamespace))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongNamespace))
-
-	var log bytes.Buffer
-
-	controller := NewServiceAccountTokenCleanupController(
-		namespace,
-		"secret-to-delete",
-		kubeAPIClient,
-		kubeInformers.Core().V1().Secrets(),
-		controllerlib.WithInformer,
-		plog.TestLogger(t, &log),
-	)
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	// Must start informers before calling TestRunSynchronously().
-	kubeInformers.Start(ctx.Done())
-
-	controllerlib.TestRunSynchronously(t, controller)
-
-	err := controllerlib.TestSync(t, controller, controllerlib.Context{
-		Context: ctx,
-	})
-	require.NoError(t, err)
-	t.Log(log.String())
-
-	expectedActions := []kubetesting.Action{kubetesting.NewDeleteAction(
-		schema.GroupVersionResource{
-			Group:    "",
-			Version:  "v1",
-			Resource: "secrets",
-		},
-		namespace,
-		"secret-to-delete",
-	)}
-	actualActions := kubeAPIClient.Actions()
-	require.Equal(t, expectedActions, actualActions)
-}
-
-func TestSync_NoSecretToDelete(t *testing.T) {
-	kubeAPIClient := kubernetesfake.NewSimpleClientset()
-	kubeInformerClient := kubernetesfake.NewSimpleClientset()
-
-	kubeInformers := kubeinformers.NewSharedInformerFactory(
-		kubeInformerClient,
-		0,
-	)
-
-	namespace := "some-namespace"
-	secretWithWrongName := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "wrong-name",
-			Namespace: namespace,
-		},
-		Type: corev1.SecretTypeServiceAccountToken,
-	}
-	secretWithWrongType := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "secret-to-leave-alone",
-			Namespace: namespace,
-		},
-	}
-	secretWithWrongNamespace := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "secret-to-delete",
-			Namespace: "other",
-		},
-		Type: corev1.SecretTypeServiceAccountToken,
-	}
-
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongName))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongName))
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongType))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongType))
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretWithWrongNamespace))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretWithWrongNamespace))
-
-	var log bytes.Buffer
-
-	controller := NewServiceAccountTokenCleanupController(
-		namespace,
-		"secret-to-delete",
-		kubeAPIClient,
-		kubeInformers.Core().V1().Secrets(),
-		controllerlib.WithInformer,
-		plog.TestLogger(t, &log),
-	)
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	// Must start informers before calling TestRunSynchronously().
-	kubeInformers.Start(ctx.Done())
-
-	controllerlib.TestRunSynchronously(t, controller)
-
-	err := controllerlib.TestSync(t, controller, controllerlib.Context{
-		Context: ctx,
-	})
-	require.NoError(t, err)
-	t.Log(log.String())
-
-	actualActions := kubeAPIClient.Actions()
-	require.Empty(t, actualActions)
-}
-
-func TestSync_ReturnsAPIErrors(t *testing.T) {
-	kubeAPIClient := kubernetesfake.NewSimpleClientset()
-	kubeInformerClient := kubernetesfake.NewSimpleClientset()
-
-	kubeInformers := kubeinformers.NewSharedInformerFactory(
-		kubeInformerClient,
-		0,
-	)
-
-	errorMessage := "error from API client"
-	kubeAPIClient.PrependReactor(
-		"delete",
-		"secrets",
-		func(a kubetesting.Action) (bool, runtime.Object, error) {
-			return true, nil, errors.New(errorMessage)
-		},
-	)
-	namespace := "some-namespace"
-
-	var log bytes.Buffer
-
-	legacySecretName := "secret-to-delete"
-	secretToDelete := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      legacySecretName,
-			Namespace: namespace,
-		},
-		Type: corev1.SecretTypeServiceAccountToken,
-	}
-
-	require.NoError(t, kubeAPIClient.Tracker().Add(secretToDelete))
-	require.NoError(t, kubeInformerClient.Tracker().Add(secretToDelete))
-
-	controller := NewServiceAccountTokenCleanupController(
-		namespace,
-		legacySecretName,
-		kubeAPIClient,
-		kubeInformers.Core().V1().Secrets(),
-		controllerlib.WithInformer,
-		plog.TestLogger(t, &log),
-	)
-
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-
-	// Must start informers before calling TestRunSynchronously().
-	kubeInformers.Start(ctx.Done())
-
-	controllerlib.TestRunSynchronously(t, controller)
-
-	err := controllerlib.TestSync(t, controller, controllerlib.Context{
-		Context: ctx,
-	})
-	require.ErrorContains(t, err, fmt.Sprintf("unable to delete secret %s in namespace %s: %s", legacySecretName, namespace, errorMessage))
-	t.Log(log.String())
-
-	expectedActions := []kubetesting.Action{kubetesting.NewDeleteAction(
-		schema.GroupVersionResource{
-			Group:    "",
-			Version:  "v1",
-			Resource: "secrets",
-		},
-		namespace,
-		legacySecretName,
-	)}
-	actualActions := kubeAPIClient.Actions()
-	require.Equal(t, expectedActions, actualActions)
-}

internal/controllermanager/prepare_controllers.go

@@ -25,7 +25,6 @@ import (
 	"go.pinniped.dev/internal/controller/authenticator/webhookcachefiller"
 	"go.pinniped.dev/internal/controller/impersonatorconfig"
 	"go.pinniped.dev/internal/controller/kubecertagent"
-	"go.pinniped.dev/internal/controller/secrets"
 	"go.pinniped.dev/internal/controllerinit"
 	"go.pinniped.dev/internal/controllerlib"
 	"go.pinniped.dev/internal/deploymentref"
@@ -35,7 +34,6 @@ import (
 	"go.pinniped.dev/internal/kubeclient"
 	"go.pinniped.dev/internal/leaderelection"
 	"go.pinniped.dev/internal/plog"
-	"go.pinniped.dev/internal/tokenclient"
 )
 
 const (
@@ -83,9 +81,6 @@ type Config struct {
 	// (Note that the impersonation proxy also accepts client certs signed by the Kube API server's cert.)
 	ImpersonationSigningCertProvider dynamiccert.Provider
 
-	// ImpersonationProxyTokenCache holds short-lived tokens for the impersonation proxy service account.
-	ImpersonationProxyTokenCache tokenclient.ExpiringSingletonTokenCacheGet
-
 	// ServingCertDuration is the validity period, in seconds, of the API serving certificate.
 	ServingCertDuration time.Duration
 
@@ -281,7 +276,6 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol
 				c.NamesConfig.ImpersonationSignerSecret,
 				c.ImpersonationSigningCertProvider,
 				plog.Logr(), //nolint:staticcheck // old controller with lots of log statements
-				c.ImpersonationProxyTokenCache,
 			),
 			singletonWorker,
 		).
@@ -312,17 +306,6 @@ func PrepareControllers(c *Config) (controllerinit.RunnerBuilder, error) { //nol
 				plog.New(),
 			),
 			singletonWorker,
-		).
-		WithController(
-			secrets.NewServiceAccountTokenCleanupController(
-				c.ServerInstallationInfo.Namespace,
-				c.NamesConfig.ImpersonationProxyLegacySecret,
-				client.Kubernetes,
-				informers.installationNamespaceK8s.Core().V1().Secrets(),
-				controllerlib.WithInformer,
-				plog.New(),
-			),
-			singletonWorker,
 		)
 
 	return controllerinit.Prepare(controllerManager.Start, leaderElector,

internal/federationdomain/oidc/oidc.go

@@ -144,7 +144,10 @@ func FositeOauth2Helper(
 		RefreshTokenLifespan:  timeoutsConfiguration.RefreshTokenLifespan,
 
 		ScopeStrategy: fosite.ExactScopeStrategy,
-		EnforcePKCE:   true,
+		// The only public client is pinniped-cli, so this combination of PKCE settings requires PKCE for the
+		// pinniped-cli client and does not require PKCE for any dynamically configured OIDCClients.
+		EnforcePKCE:                 false,
+		EnforcePKCEForPublicClients: true,
 
 		// "offline_access" as per https://openid.net/specs/openid-connect-core-1_0.html#OfflineAccess
 		RefreshTokenScopes: []string{oidcapi.ScopeOfflineAccess},

internal/tokenclient/expiringtokencache.go

@@ -1,50 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package tokenclient
-
-import (
-	"time"
-
-	"k8s.io/apimachinery/pkg/util/cache"
-)
-
-const tokenCacheKey = "token"
-
-type ExpiringSingletonTokenCacheGet interface {
-	Get() string
-}
-
-type ExpiringSingletonTokenCache interface {
-	ExpiringSingletonTokenCacheGet
-	Set(token string, ttl time.Duration)
-}
-
-type expiringCacheImpl struct {
-	cache *cache.Expiring
-}
-
-var _ ExpiringSingletonTokenCacheGet = &expiringCacheImpl{}
-var _ ExpiringSingletonTokenCache = &expiringCacheImpl{}
-
-func NewExpiringSingletonTokenCache() ExpiringSingletonTokenCache {
-	return &expiringCacheImpl{cache: cache.NewExpiring()}
-}
-
-func (e *expiringCacheImpl) Get() string {
-	maybeToken, ok := e.cache.Get(tokenCacheKey)
-	if !ok {
-		return ""
-	}
-
-	token, ok := maybeToken.(string)
-	if !ok {
-		return ""
-	}
-
-	return token
-}
-
-func (e *expiringCacheImpl) Set(token string, ttl time.Duration) {
-	e.cache.Set(tokenCacheKey, token, ttl)
-}

internal/tokenclient/expiringtokencache_test.go

@@ -1,35 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package tokenclient
-
-import (
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/require"
-	k8sCache "k8s.io/apimachinery/pkg/util/cache"
-)
-
-func TestExpiringSingletonTokenCache(t *testing.T) {
-	cache := NewExpiringSingletonTokenCache()
-	require.NotNil(t, cache)
-	require.Empty(t, cache.Get())
-
-	cache.Set("i am a 12 hour token", 12*time.Hour)
-	require.Equal(t, "i am a 12 hour token", cache.Get())
-
-	cache.Set("i am a 0-TTL token", time.Duration(0))
-	time.Sleep(1 * time.Millisecond)
-	require.Empty(t, cache.Get())
-
-	cache.Set("i am a very short token", 1*time.Millisecond)
-	time.Sleep(2 * time.Millisecond)
-	require.Empty(t, cache.Get())
-}
-
-func TestExpiringSingletonTokenCache_WithNonString(t *testing.T) {
-	cache := &expiringCacheImpl{cache: k8sCache.NewExpiring()}
-	cache.cache.Set(tokenCacheKey, true, 1*time.Hour)
-	require.Empty(t, cache.Get())
-}

internal/tokenclient/tokenclient.go

@@ -1,146 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package tokenclient
-
-import (
-	"context"
-	"fmt"
-	"time"
-
-	"github.com/pkg/errors"
-	authenticationv1 "k8s.io/api/authentication/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/utils/clock"
-
-	"go.pinniped.dev/internal/backoff"
-	"go.pinniped.dev/internal/plog"
-)
-
-type WhatToDoWithTokenFunc func(authenticationv1.TokenRequestStatus, metav1.Duration) error
-
-type TokenClient struct {
-	namespace          string
-	serviceAccountName string
-	k8sClient          kubernetes.Interface
-	whatToDoWithToken  WhatToDoWithTokenFunc
-	expirationSeconds  int64
-	clock              clock.Clock
-	logger             plog.Logger
-}
-
-type Opt func(client *TokenClient)
-
-func WithExpirationSeconds(expirationSeconds int64) Opt {
-	return func(client *TokenClient) {
-		client.expirationSeconds = expirationSeconds
-	}
-}
-
-func New(
-	namespace string,
-	serviceAccountName string,
-	k8sClient kubernetes.Interface,
-	whatToDoWithToken WhatToDoWithTokenFunc,
-	logger plog.Logger,
-	opts ...Opt,
-) *TokenClient {
-	client := &TokenClient{
-		namespace:          namespace,
-		serviceAccountName: serviceAccountName,
-		k8sClient:          k8sClient,
-		whatToDoWithToken:  whatToDoWithToken,
-		expirationSeconds:  600,
-		clock:              clock.RealClock{},
-		logger:             logger,
-	}
-	for _, opt := range opts {
-		opt(client)
-	}
-	return client
-}
-
-type howToFetchTokenFromAPIServer func(tokenRequest *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error)
-
-func (tokenClient TokenClient) Start(ctx context.Context) {
-	sleeper := make(chan time.Time, 1)
-	// Make sure that the <-sleeper below gets run once immediately.
-	sleeper <- time.Now()
-	for {
-		select {
-		case <-ctx.Done():
-			tokenClient.logger.Info("TokenClient was cancelled and is stopping")
-			return
-		case <-sleeper:
-			var tokenTTL metav1.Duration
-			err := backoff.WithContext(ctx, &backoff.InfiniteBackoff{
-				Duration:    10 * time.Millisecond,
-				MaxDuration: 5 * time.Second,
-				Factor:      2.0,
-			}, func(ctx context.Context) (bool, error) {
-				var err error
-				var tokenRequestStatus authenticationv1.TokenRequestStatus
-				tokenRequestStatus, tokenTTL, err = tokenClient.fetchToken(
-					func(tokenRequest *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
-						return tokenClient.k8sClient.CoreV1().ServiceAccounts(tokenClient.namespace).CreateToken(
-							ctx,
-							tokenClient.serviceAccountName,
-							tokenRequest,
-							metav1.CreateOptions{})
-					},
-				)
-
-				if err != nil {
-					tokenClient.logger.Warning(fmt.Sprintf("Could not fetch token: %s\n", err))
-					// We got an error. Swallow it and ask for retry.
-					return false, nil
-				}
-
-				err = tokenClient.whatToDoWithToken(tokenRequestStatus, tokenTTL)
-				if err != nil {
-					tokenClient.logger.Warning(fmt.Sprintf("unable to pass token to the caller: %s\n", err))
-					// We got an error. Swallow it and ask for retry.
-					return false, nil
-				}
-				// We got a token. Stop backing off.
-				return true, nil
-			})
-			if err != nil {
-				// We were cancelled during our WithContext. We know it was not due to some other
-				// error because our last argument to WithContext above never returns any errors.
-				return
-			}
-			// Schedule ourselves to wake up in the future.
-			time.AfterFunc(tokenTTL.Duration*4/5, func() {
-				sleeper <- time.Now()
-			})
-		}
-	}
-}
-
-func (tokenClient TokenClient) fetchToken(
-	howToFetchTokenFromAPIServer howToFetchTokenFromAPIServer,
-) (authenticationv1.TokenRequestStatus, metav1.Duration, error) {
-	tokenClient.logger.Debug(fmt.Sprintf("refreshing cache at time=%s\n", tokenClient.clock.Now().Format(time.RFC3339)))
-
-	tokenRequestInput := &authenticationv1.TokenRequest{
-		Spec: authenticationv1.TokenRequestSpec{
-			ExpirationSeconds: &tokenClient.expirationSeconds,
-		},
-	}
-
-	tokenRequest, err := howToFetchTokenFromAPIServer(tokenRequestInput)
-
-	emptyMetav1Duration := metav1.Duration{Duration: 0}
-	if err != nil {
-		return authenticationv1.TokenRequestStatus{}, emptyMetav1Duration, errors.Wrap(err, "error creating token")
-	}
-
-	if tokenRequest == nil {
-		return authenticationv1.TokenRequestStatus{}, emptyMetav1Duration, errors.New("tokenRequest is nil after request")
-	}
-
-	ttl := metav1.Duration{Duration: tokenRequest.Status.ExpirationTimestamp.Sub(tokenClient.clock.Now())}
-	return tokenRequest.Status, ttl, nil
-}

internal/tokenclient/tokenclient_test.go

@@ -1,202 +0,0 @@
-// Copyright 2023 the Pinniped contributors. All Rights Reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package tokenclient
-
-import (
-	"bytes"
-	"errors"
-	"testing"
-	"time"
-
-	"github.com/stretchr/testify/require"
-	authenticationv1 "k8s.io/api/authentication/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/utils/clock"
-	clocktesting "k8s.io/utils/clock/testing"
-
-	"go.pinniped.dev/internal/plog"
-)
-
-func TestNew(t *testing.T) {
-	mockWhatToDoWithTokenFunc := *new(WhatToDoWithTokenFunc)
-	mockClient := new(kubernetes.Clientset)
-	mockTime := time.Now()
-	mockClock := clocktesting.NewFakeClock(mockTime)
-	var log bytes.Buffer
-	testLogger := plog.TestLogger(t, &log)
-
-	type args struct {
-		namespace          string
-		serviceAccountName string
-		k8sClient          *kubernetes.Clientset
-		whatToDoWithToken  WhatToDoWithTokenFunc
-		logger             plog.Logger
-		opts               []Opt
-	}
-
-	tests := []struct {
-		name     string
-		args     args
-		expected *TokenClient
-	}{
-		{
-			name: "defaults",
-			args: args{
-				namespace:          "namespace",
-				serviceAccountName: "serviceAccountName",
-				k8sClient:          mockClient,
-				whatToDoWithToken:  mockWhatToDoWithTokenFunc,
-				logger:             testLogger,
-			},
-			expected: &TokenClient{
-				namespace:          "namespace",
-				serviceAccountName: "serviceAccountName",
-				k8sClient:          mockClient,
-				whatToDoWithToken:  mockWhatToDoWithTokenFunc,
-				expirationSeconds:  600,
-				clock:              clock.RealClock{},
-				logger:             testLogger,
-			},
-		},
-		{
-			name: "with all opts",
-			args: args{
-				namespace:          "custom-namespace",
-				serviceAccountName: "custom-serviceAccountName",
-				k8sClient:          mockClient,
-				whatToDoWithToken:  mockWhatToDoWithTokenFunc,
-				logger:             testLogger,
-				opts: []Opt{
-					WithExpirationSeconds(777),
-					withClock(mockClock),
-				},
-			},
-			expected: &TokenClient{
-				namespace:          "custom-namespace",
-				serviceAccountName: "custom-serviceAccountName",
-				k8sClient:          mockClient,
-				whatToDoWithToken:  mockWhatToDoWithTokenFunc,
-				expirationSeconds:  777,
-				clock:              mockClock,
-				logger:             testLogger,
-			},
-		},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			actual := New(
-				tt.args.namespace,
-				tt.args.serviceAccountName,
-				tt.args.k8sClient,
-				tt.args.whatToDoWithToken,
-				tt.args.logger,
-				tt.args.opts...,
-			)
-
-			require.Equal(t, tt.expected, actual)
-		})
-	}
-}
-
-// withClock should only be used for testing.
-func withClock(clock clock.Clock) Opt {
-	return func(client *TokenClient) {
-		client.clock = clock
-	}
-}
-
-func TestFetchToken(t *testing.T) {
-	mockTime := metav1.Now()
-
-	type expected struct {
-		tokenRequestStatus authenticationv1.TokenRequestStatus
-		ttl                metav1.Duration
-		errMessage         string
-	}
-
-	tests := []struct {
-		name                         string
-		expirationSeconds            int64
-		howToFetchTokenFromAPIServer howToFetchTokenFromAPIServer
-		expected                     expected
-	}{
-		{
-			name:              "happy path",
-			expirationSeconds: 555,
-			howToFetchTokenFromAPIServer: func(_ *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
-				tokenRequest := authenticationv1.TokenRequest{
-					Status: authenticationv1.TokenRequestStatus{
-						Token:               "token value",
-						ExpirationTimestamp: metav1.NewTime(mockTime.Add(25 * time.Minute)),
-					},
-				}
-				return &tokenRequest, nil
-			},
-			expected: expected{
-				tokenRequestStatus: authenticationv1.TokenRequestStatus{
-					Token:               "token value",
-					ExpirationTimestamp: metav1.NewTime(mockTime.Add(25 * time.Minute)),
-				},
-				ttl: metav1.Duration{
-					Duration: 25 * time.Minute,
-				},
-			},
-		},
-		{
-			name:              "returns errors from howToFetchTokenFromAPIServer",
-			expirationSeconds: 444,
-			howToFetchTokenFromAPIServer: func(_ *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
-				return nil, errors.New("has an error")
-			},
-			expected: expected{
-				errMessage: "error creating token: has an error",
-			},
-		},
-		{
-			name:              "errors when howToFetchTokenFromAPIServer returns nil",
-			expirationSeconds: 333,
-			howToFetchTokenFromAPIServer: func(_ *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
-				return nil, nil
-			},
-			expected: expected{
-				errMessage: "tokenRequest is nil after request",
-			},
-		},
-	}
-
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			wrappedFunc := func(tokenRequest *authenticationv1.TokenRequest) (*authenticationv1.TokenRequest, error) {
-				require.NotNil(t, tokenRequest)
-				require.Equal(t, tt.expirationSeconds, *tokenRequest.Spec.ExpirationSeconds)
-				require.Empty(t, tokenRequest.Spec.Audiences)
-				require.Empty(t, tokenRequest.Spec.BoundObjectRef)
-				return tt.howToFetchTokenFromAPIServer(tokenRequest)
-			}
-
-			mockClock := clocktesting.NewFakeClock(mockTime.Time)
-			var log bytes.Buffer
-
-			tokenClient := TokenClient{
-				expirationSeconds: tt.expirationSeconds,
-				clock:             mockClock,
-				logger:            plog.TestLogger(t, &log),
-			}
-
-			tokenRequestStatus, ttl, err := tokenClient.fetchToken(
-				wrappedFunc,
-			)
-
-			if tt.expected.errMessage != "" {
-				require.ErrorContains(t, err, tt.expected.errMessage)
-			} else {
-				require.Equal(t, tt.expected.tokenRequestStatus, tokenRequestStatus)
-				require.Equal(t, tt.expected.ttl, ttl)
-			}
-		})
-	}
-}

test/integration/concierge_impersonation_proxy_test.go

@@ -2051,7 +2051,6 @@ func createServiceAccountToken(ctx context.Context, t *testing.T, adminClient ku
 			Delete(context.Background(), serviceAccount.Name, metav1.DeleteOptions{}))
 	})
 
-	// TODO: What is this used for?
 	secret, err := adminClient.CoreV1().Secrets(namespaceName).Create(ctx, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "int-test-service-account-token-",

test/integration/concierge_whoami_test.go

@@ -89,7 +89,6 @@ func TestWhoAmI_ServiceAccount_Legacy_Parallel(t *testing.T) {
 	}, metav1.CreateOptions{})
 	require.NoError(t, err)
 
-	// TODO: What is this used for?
 	secret, err := kubeClient.Secrets(ns.Name).Create(ctx, &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "test-whoami-",