impersonator: remove redundant deleteKnownImpersonationHeaders logic

WithImpersonation already deletes impersonation headers and has done
so since the early days:

https://github.com/kubernetes/kubernetes/pull/36769

ensureNoImpersonationHeaders will still reject any request that has
impersonation headers set so we will always fail closed.

Signed-off-by: Monis Khan <mok@vmware.com>

internal/concierge/impersonator/impersonator.go

@@ -174,10 +174,6 @@ func newInternal( //nolint:funlen // yeah, it's kind of long.
 			}))
 			handler = filterlatency.TrackStarted(handler, "impersonationproxy")
 
-			handler = filterlatency.TrackCompleted(handler)
-			handler = deleteKnownImpersonationHeaders(handler)
-			handler = filterlatency.TrackStarted(handler, "deleteimpersonationheaders")
-
 			// The standard Kube handler chain (authn, authz, impersonation, audit, etc).
 			// See the genericapiserver.DefaultBuildHandlerChain func for details.
 			handler = defaultBuildHandlerChainFunc(handler, c)
@@ -372,34 +368,6 @@ func isTokenCredReq(reqInfo *genericapirequest.RequestInfo) bool {
 	return true
 }
 
-func deleteKnownImpersonationHeaders(delegate http.Handler) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		// remove known impersonation headers while avoiding mutation of input request
-		// unknown future impersonation headers will still get caught by our later checks
-		if ensureNoImpersonationHeaders(r) != nil {
-			r = r.Clone(r.Context())
-
-			impersonationHeaders := []string{
-				transport.ImpersonateUserHeader,
-				transport.ImpersonateGroupHeader,
-			}
-
-			for k := range r.Header {
-				if !strings.HasPrefix(k, transport.ImpersonateUserExtraHeaderPrefix) {
-					continue
-				}
-				impersonationHeaders = append(impersonationHeaders, k)
-			}
-
-			for _, header := range impersonationHeaders {
-				r.Header.Del(header) // delay mutation until the end when we are done iterating over the map
-			}
-		}
-
-		delegate.ServeHTTP(w, r)
-	})
-}
-
 // No-op wrapping around RequestFunc to allow for comparisons.
 type comparableAuthenticator struct {
 	authenticator.RequestFunc

internal/concierge/impersonator/impersonator_test.go

@@ -1628,114 +1628,6 @@ func requireCanBindToPort(t *testing.T, port int) {
 	require.NoError(t, ln.Close())
 }
 
-func Test_deleteKnownImpersonationHeaders(t *testing.T) {
-	tests := []struct {
-		name          string
-		headers, want http.Header
-	}{
-		{
-			name: "no impersonation",
-			headers: map[string][]string{
-				"a":               {"b"},
-				"Accept-Encoding": {"gzip"},
-				"User-Agent":      {"test-user-agent"},
-			},
-			want: map[string][]string{
-				"a":               {"b"},
-				"Accept-Encoding": {"gzip"},
-				"User-Agent":      {"test-user-agent"},
-			},
-		},
-		{
-			name: "impersonate user header is dropped",
-			headers: map[string][]string{
-				"a":                {"b"},
-				"Impersonate-User": {"panda"},
-				"Accept-Encoding":  {"gzip"},
-				"User-Agent":       {"test-user-agent"},
-			},
-			want: map[string][]string{
-				"a":               {"b"},
-				"Accept-Encoding": {"gzip"},
-				"User-Agent":      {"test-user-agent"},
-			},
-		},
-		{
-			name: "all known impersonate headers are dropped",
-			headers: map[string][]string{
-				"Accept-Encoding":           {"gzip"},
-				"Authorization":             {"Bearer some-service-account-token"},
-				"Impersonate-Extra-Extra-1": {"some", "extra", "stuff"},
-				"Impersonate-Extra-Extra-2": {"some", "more", "extra", "stuff"},
-				"Impersonate-Group":         {"test-group-1", "test-group-2"},
-				"Impersonate-User":          {"test-user"},
-				"User-Agent":                {"test-user-agent"},
-			},
-			want: map[string][]string{
-				"Accept-Encoding": {"gzip"},
-				"Authorization":   {"Bearer some-service-account-token"},
-				"User-Agent":      {"test-user-agent"},
-			},
-		},
-		{
-			name: "future UID header is not dropped",
-			headers: map[string][]string{
-				"Accept-Encoding":           {"gzip"},
-				"Authorization":             {"Bearer some-service-account-token"},
-				"Impersonate-Extra-Extra-1": {"some", "extra", "stuff"},
-				"Impersonate-Extra-Extra-2": {"some", "more", "extra", "stuff"},
-				"Impersonate-Group":         {"test-group-1", "test-group-2"},
-				"Impersonate-User":          {"test-user"},
-				"Impersonate-Uid":           {"008"},
-				"User-Agent":                {"test-user-agent"},
-			},
-			want: map[string][]string{
-				"Accept-Encoding": {"gzip"},
-				"Authorization":   {"Bearer some-service-account-token"},
-				"User-Agent":      {"test-user-agent"},
-				"Impersonate-Uid": {"008"},
-			},
-		},
-		{
-			name: "future UID header is not dropped, no other headers",
-			headers: map[string][]string{
-				"Impersonate-Uid": {"009"},
-			},
-			want: map[string][]string{
-				"Impersonate-Uid": {"009"},
-			},
-		},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			inputReq := (&http.Request{Header: tt.headers}).WithContext(context.Background())
-			inputReqCopy := inputReq.Clone(inputReq.Context())
-
-			var called bool
-			delegate := http.HandlerFunc(func(w http.ResponseWriter, outputReq *http.Request) {
-				called = true
-				require.Nil(t, w)
-
-				// assert only headers mutated
-				outputReqCopy := outputReq.Clone(outputReq.Context())
-				outputReqCopy.Header = tt.headers
-				require.Equal(t, inputReqCopy, outputReqCopy)
-
-				require.Equal(t, tt.want, outputReq.Header)
-
-				if ensureNoImpersonationHeaders(inputReq) == nil {
-					require.True(t, inputReq == outputReq, "expect req to passed through when no modification needed")
-				}
-			})
-
-			deleteKnownImpersonationHeaders(delegate).ServeHTTP(nil, inputReq)
-			require.Equal(t, inputReqCopy, inputReq) // assert no mutation occurred
-			require.True(t, called)
-		})
-	}
-}
-
 func Test_withBearerTokenPreservation(t *testing.T) {
 	tests := []struct {
 		name    string