avoid the ValidatingAdmissionPolicy admission plugin when it can't work

deploy/concierge/rbac.yaml

@@ -19,7 +19,7 @@ rules:
     resources: [ apiservices ]
     verbs: [ get, list, patch, update, watch ]
   - apiGroups: [ admissionregistration.k8s.io ]
-    resources: [ validatingwebhookconfigurations, mutatingwebhookconfigurations, validatingadmissionpolicybindings ]
+    resources: [ validatingwebhookconfigurations, mutatingwebhookconfigurations, validatingadmissionpolicies, validatingadmissionpolicybindings ]
     verbs: [ get, list, watch ]
   - apiGroups: [ flowcontrol.apiserver.k8s.io ]
     resources: [ flowschemas, prioritylevelconfigurations ]

deploy/supervisor/rbac.yaml

@@ -131,7 +131,7 @@ rules:
     resources: [ apiservices ]
     verbs: [ get, list, patch, update, watch ]
   - apiGroups: [ admissionregistration.k8s.io ]
-    resources: [ validatingwebhookconfigurations, mutatingwebhookconfigurations, validatingadmissionpolicybindings ]
+    resources: [ validatingwebhookconfigurations, mutatingwebhookconfigurations, validatingadmissionpolicies, validatingadmissionpolicybindings ]
     verbs: [ get, list, watch ]
   - apiGroups: [ flowcontrol.apiserver.k8s.io ]
     resources: [ flowschemas, prioritylevelconfigurations ]

internal/admissionpluginconfig/admissionpluginconfg.go

@@ -0,0 +1,127 @@
+// Copyright 2024 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package admissionpluginconfig
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/pkg/errors"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/admission/plugin/namespace/lifecycle"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/mutating"
+	"k8s.io/apiserver/pkg/admission/plugin/webhook/validating"
+	"k8s.io/apiserver/pkg/server/options"
+	"k8s.io/client-go/discovery"
+
+	"go.pinniped.dev/internal/kubeclient"
+	"go.pinniped.dev/internal/plog"
+)
+
+// ConfigureAdmissionPlugins may choose to reconfigure the admission plugins present on the given
+// RecommendedOptions by mutating it.
+//
+// The ValidatingAdmissionPolicy feature gate became enabled by default in Kube 1.30.
+// When Pinniped is compiled using the Kube 1.30+ libraries, and when installed onto a Kube cluster older than 1.30,
+// then the new admission ValidatingAdmissionPolicy plugin prevents all our aggregated APIs from working, seemingly
+// because it fails to sync informers created for watching the related resources. As a workaround, ask the k8s API
+// server if it has the ValidatingAdmissionPolicy resource, and configure our admission plugins accordingly.
+func ConfigureAdmissionPlugins(recommendedOptions *options.RecommendedOptions) error {
+	k8sClient, err := kubeclient.New()
+	if err != nil {
+		return fmt.Errorf("failed to create kube client: %w", err)
+	}
+	return configureAdmissionPlugins(k8sClient.Kubernetes.Discovery(), recommendedOptions)
+}
+
+// configureAdmissionPlugins is the same as ConfigureAdmissionPlugins but allows client injection for unit testing.
+func configureAdmissionPlugins(discoveryClient discovery.ServerResourcesInterface, recommendedOptions *options.RecommendedOptions) error {
+	// Check if the API server has such a resource.
+	hasValidatingAdmissionPolicyResource, err := k8sAPIServerHasValidatingAdmissionPolicyResource(discoveryClient)
+	if err != nil {
+		return fmt.Errorf("failed looking up availability of ValidatingAdmissionPolicy resource: %w", err)
+	}
+
+	if hasValidatingAdmissionPolicyResource {
+		// Accept the default admission plugin configuration without any further modification.
+		return nil
+	}
+
+	// Customize the admission plugins to avoid using the new ValidatingAdmissionPolicy plugin.
+	plog.Warning("could not find ValidatingAdmissionPolicy resource on this Kubernetes cluster " +
+		"(which is normal for clusters older than Kubernetes 1.30); " +
+		"disabling ValidatingAdmissionPolicy admission plugins for all Pinniped aggregated API resource types")
+
+	mutateOptionsToUseOldStyleAdmissionPlugins(recommendedOptions)
+
+	return nil
+}
+
+func k8sAPIServerHasValidatingAdmissionPolicyResource(discoveryClient discovery.ServerResourcesInterface) (bool, error) {
+	// Perform discovery. We are looking for ValidatingAdmissionPolicy in group
+	// admissionregistration.k8s.io at any version.
+	resources, err := discoveryClient.ServerPreferredResources()
+	partialErr := &discovery.ErrGroupDiscoveryFailed{}
+	if resources != nil && errors.As(err, &partialErr) {
+		// This is a partial discovery error, most likely caused by Pinniped's own aggregated APIs
+		// not being ready yet since this Pinniped pod is typically in the process of starting up
+		// when this code is reached. Check if the group that we care about is in the error's list
+		// of failed API groups.
+		for groupVersion := range partialErr.Groups {
+			if groupVersion.Group == admissionregistrationv1.GroupName {
+				// There was an error for the specific group that we are trying to find, so
+				// return an error. If we don't arrive here, then it must have been error(s) for
+				// some other group(s) that we are not looking for, so we can ignore those error(s).
+				return false, err
+			}
+		}
+	} else if err != nil {
+		// We got some other type of error aside from a partial failure.
+		return false, fmt.Errorf("failed to perform k8s API discovery: %w", err)
+	}
+
+	// Now look at all discovered groups until we find admissionregistration.k8s.io.
+	wantedGroupWithSlash := fmt.Sprintf("%s/", admissionregistrationv1.GroupName)
+	for _, resourcesPerGV := range resources {
+		if strings.HasPrefix(resourcesPerGV.GroupVersion, wantedGroupWithSlash) {
+			// Found the group, so now look to see if it includes ValidatingAdmissionPolicy as a resource,
+			// which went GA in Kubernetes 1.30, and could be enabled by a feature flag in previous versions.
+			for _, resource := range resourcesPerGV.APIResources {
+				if resource.Kind == "ValidatingAdmissionPolicy" {
+					// Found it!
+					plog.Info("found ValidatingAdmissionPolicy resource on this Kubernetes cluster",
+						"group", resource.Group, "version", resource.Version, "kind", resource.Kind)
+					return true, nil
+				}
+			}
+		}
+	}
+
+	// Didn't findValidatingAdmissionPolicy on this cluster.
+	return false, nil
+}
+
+func mutateOptionsToUseOldStyleAdmissionPlugins(recommendedOptions *options.RecommendedOptions) {
+	plugins := admission.NewPlugins()
+
+	// These lines are copied from server.RegisterAllAdmissionPlugins in k8s.io/apiserver@v0.30.0/pkg/server/plugins.go.
+	lifecycle.Register(plugins)
+	validating.Register(plugins)
+	mutating.Register(plugins)
+	// Note that we are not adding this one:
+	// validatingadmissionpolicy.Register(newAdmissionPlugins)
+
+	// This list is copied from the implementation of NewAdmissionOptions() in k8s.io/apiserver@v0.30.0/pkg/server/options/admission.go
+	recommendedOptions.Admission.RecommendedPluginOrder = []string{
+		lifecycle.PluginName,
+		mutating.PluginName,
+		// Again, note that we are not adding this one:
+		// validatingadmissionpolicy.PluginName,
+		validating.PluginName,
+	}
+
+	// Overwrite the registered plugins with our new, smaller list.
+	recommendedOptions.Admission.Plugins = plugins
+}

internal/admissionpluginconfig/admissionpluginconfg_test.go

@@ -0,0 +1,233 @@
+// Copyright 2024 the Pinniped contributors. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package admissionpluginconfig
+
+import (
+	"errors"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
+	appsv1 "k8s.io/api/apps/v1"
+	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"
+	"k8s.io/apiserver/pkg/admission"
+	"k8s.io/apiserver/pkg/server/options"
+	"k8s.io/client-go/discovery"
+	kubernetesfake "k8s.io/client-go/kubernetes/fake"
+	k8stesting "k8s.io/client-go/testing"
+	kubetesting "k8s.io/client-go/testing"
+)
+
+func TestConfigureAdmissionPlugins(t *testing.T) {
+	defaultPlugins := admission.NewPlugins()
+	defaultPlugins.Register("fake-plugin1", func(config io.Reader) (admission.Interface, error) { return nil, nil })
+	defaultPlugins.Register("fake-plugin2", func(config io.Reader) (admission.Interface, error) { return nil, nil })
+
+	defaultPluginsRegistered := []string{"fake-plugin1", "fake-plugin2"}
+	defaultRecommendedPluginOrder := []string{"fake-plugin2", "fake-plugin1"}
+
+	customOldStylePluginsRegistered := []string{"MutatingAdmissionWebhook", "NamespaceLifecycle", "ValidatingAdmissionWebhook"}
+	customOldStyleRecommendedPluginOrder := []string{"NamespaceLifecycle", "MutatingAdmissionWebhook", "ValidatingAdmissionWebhook"}
+
+	coreResources := &metav1.APIResourceList{
+		GroupVersion: corev1.SchemeGroupVersion.String(),
+		APIResources: []metav1.APIResource{
+			{Name: "pods", Namespaced: true, Kind: "Pod"},
+		},
+	}
+
+	appsResources := &metav1.APIResourceList{
+		GroupVersion: appsv1.SchemeGroupVersion.String(),
+		APIResources: []metav1.APIResource{
+			{Name: "deployments", Namespaced: true, Kind: "Deployment"},
+			{Name: "deployments/scale", Namespaced: true, Kind: "Scale", Group: "apps", Version: "v1"},
+		},
+	}
+
+	newStyleAdmissionResourcesWithValidatingAdmissionPolicies := &metav1.APIResourceList{
+		GroupVersion: admissionregistrationv1.SchemeGroupVersion.String(),
+		APIResources: []metav1.APIResource{
+			{Name: "validatingwebhookconfigurations", Kind: "ValidatingWebhookConfiguration"},
+			{Name: "validatingadmissionpolicies", Kind: "ValidatingAdmissionPolicy"},
+		},
+	}
+
+	oldStyleAdmissionResourcesWithoutValidatingAdmissionPolicies := &metav1.APIResourceList{
+		GroupVersion: admissionregistrationv1.SchemeGroupVersion.String(),
+		APIResources: []metav1.APIResource{
+			{Name: "validatingwebhookconfigurations", Kind: "ValidatingWebhookConfiguration"},
+		},
+	}
+
+	tests := []struct {
+		name                       string
+		availableAPIResources      []*metav1.APIResourceList
+		discoveryErr               error
+		wantErr                    string
+		wantRegisteredPlugins      []string
+		wantRecommendedPluginOrder []string
+	}{
+		{
+			name: "when there is a ValidatingAdmissionPolicy resource, then we do not change the plugin configuration",
+			availableAPIResources: []*metav1.APIResourceList{
+				coreResources,
+				newStyleAdmissionResourcesWithValidatingAdmissionPolicies,
+				appsResources,
+			},
+			wantRegisteredPlugins:      defaultPluginsRegistered,
+			wantRecommendedPluginOrder: defaultRecommendedPluginOrder,
+		},
+		{
+			name: "when there is no ValidatingAdmissionPolicy resource, as there would not be in an old Kubernetes cluster, then we change the plugin configuration to be more like it was for old versions of Kubernetes",
+			availableAPIResources: []*metav1.APIResourceList{
+				coreResources,
+				oldStyleAdmissionResourcesWithoutValidatingAdmissionPolicies,
+				appsResources,
+			},
+			wantRegisteredPlugins:      customOldStylePluginsRegistered,
+			wantRecommendedPluginOrder: customOldStyleRecommendedPluginOrder,
+		},
+		{
+			name:                       "when there is a total error returned by discovery",
+			discoveryErr:               errors.New("total error from API discovery client"),
+			wantErr:                    "failed looking up availability of ValidatingAdmissionPolicy resource: failed to perform k8s API discovery: total error from API discovery client",
+			wantRegisteredPlugins:      defaultPluginsRegistered,
+			wantRecommendedPluginOrder: defaultRecommendedPluginOrder,
+		},
+		{
+			name: "when there is a partial error returned by discovery which does include the group of interest, then we cannot ignore the error, because we could not discover anything about that group",
+			availableAPIResources: []*metav1.APIResourceList{
+				coreResources,
+				oldStyleAdmissionResourcesWithoutValidatingAdmissionPolicies,
+				appsResources,
+			},
+			discoveryErr: &discovery.ErrGroupDiscoveryFailed{Groups: map[schema.GroupVersion]error{
+				schema.GroupVersion{Group: "someGroup", Version: "v1"}:                    errors.New("fake error for someGroup"),
+				schema.GroupVersion{Group: "admissionregistration.k8s.io", Version: "v1"}: errors.New("fake error for admissionregistration"),
+			}},
+			wantErr:                    "failed looking up availability of ValidatingAdmissionPolicy resource: unable to retrieve the complete list of server APIs: admissionregistration.k8s.io/v1: fake error for admissionregistration, someGroup/v1: fake error for someGroup",
+			wantRegisteredPlugins:      defaultPluginsRegistered,
+			wantRecommendedPluginOrder: defaultRecommendedPluginOrder,
+		},
+		{
+			name: "when there is a partial error returned by discovery on an new-style cluster which does not include the group of interest, then we can ignore the error and use the default plugins",
+			availableAPIResources: []*metav1.APIResourceList{
+				coreResources,
+				newStyleAdmissionResourcesWithValidatingAdmissionPolicies,
+				appsResources,
+			},
+			discoveryErr: &discovery.ErrGroupDiscoveryFailed{Groups: map[schema.GroupVersion]error{
+				schema.GroupVersion{Group: "someGroup", Version: "v1"}:      errors.New("fake error for someGroup"),
+				schema.GroupVersion{Group: "someOtherGroup", Version: "v1"}: errors.New("fake error for someOtherGroup"),
+			}},
+			wantRegisteredPlugins:      defaultPluginsRegistered,
+			wantRecommendedPluginOrder: defaultRecommendedPluginOrder,
+		},
+		{
+			name: "when there is a partial error returned by discovery on an old-style cluster which does not include the group of interest, then we can ignore the error and customize the plugins",
+			availableAPIResources: []*metav1.APIResourceList{
+				coreResources,
+				oldStyleAdmissionResourcesWithoutValidatingAdmissionPolicies,
+				appsResources,
+			},
+			discoveryErr: &discovery.ErrGroupDiscoveryFailed{Groups: map[schema.GroupVersion]error{
+				schema.GroupVersion{Group: "someGroup", Version: "v1"}:      errors.New("fake error for someGroup"),
+				schema.GroupVersion{Group: "someOtherGroup", Version: "v1"}: errors.New("fake error for someOtherGroup"),
+			}},
+			wantRegisteredPlugins:      customOldStylePluginsRegistered,
+			wantRecommendedPluginOrder: customOldStyleRecommendedPluginOrder,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			kubeClient := kubernetesfake.NewSimpleClientset()
+			kubeClient.Fake.Resources = tt.availableAPIResources
+
+			// Unfortunately, kubernetesfake.NewSimpleClientset() does not support using reactors to
+			// cause discovery to return errors. Instead, we will make our own fake implementation of the
+			// discovery client's interface and only mock the parts that we need for this test.
+			discoveryClient := newFakeDiscoveryClient(kubeClient)
+
+			if tt.discoveryErr != nil {
+				kubeClient.PrependReactor(
+					"get",
+					"resource",
+					func(a kubetesting.Action) (bool, runtime.Object, error) {
+						return true, nil, tt.discoveryErr
+					},
+				)
+			}
+
+			opts := &options.RecommendedOptions{
+				Admission: &options.AdmissionOptions{
+					Plugins:                defaultPlugins,
+					RecommendedPluginOrder: defaultRecommendedPluginOrder,
+				},
+			}
+			// Sanity checks on opts before we use it.
+			require.Equal(t, defaultPlugins, opts.Admission.Plugins)
+			require.Equal(t, defaultPluginsRegistered, opts.Admission.Plugins.Registered())
+			require.Equal(t, defaultRecommendedPluginOrder, opts.Admission.RecommendedPluginOrder)
+
+			// Call the function under test.
+			err := configureAdmissionPlugins(discoveryClient, opts)
+
+			if tt.wantErr == "" {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, tt.wantErr)
+			}
+
+			// Check the expected side effects of the function under test, if any.
+			require.Equal(t, tt.wantRegisteredPlugins, opts.Admission.Plugins.Registered())
+			require.Equal(t, tt.wantRecommendedPluginOrder, opts.Admission.RecommendedPluginOrder)
+		})
+	}
+}
+
+type fakeDiscoveryClient struct {
+	fakeClientSet *kubernetesfake.Clientset
+}
+
+var _ discovery.ServerResourcesInterface = &fakeDiscoveryClient{}
+
+func newFakeDiscoveryClient(fakeClientSet *kubernetesfake.Clientset) *fakeDiscoveryClient {
+	return &fakeDiscoveryClient{
+		fakeClientSet: fakeClientSet,
+	}
+}
+
+// This is the only function from the discovery.DiscoveryInterface that we care to fake for this test.
+// The rest of the functions are here only to satisfy the interface.
+func (f *fakeDiscoveryClient) ServerPreferredResources() ([]*metav1.APIResourceList, error) {
+	action := k8stesting.ActionImpl{
+		Verb:     "get",
+		Resource: schema.GroupVersionResource{Resource: "resource"},
+	}
+	// Wire in actions just enough that we can cause errors for the test when we want them.
+	// Ignoring the first return value because we don't need it for this test.
+	_, err := f.fakeClientSet.Invokes(action, nil)
+	// Still return the "partial" results even where there was an error, similar enough to how the real API works.
+	return f.fakeClientSet.Resources, err
+}
+
+func (f *fakeDiscoveryClient) ServerResourcesForGroupVersion(_ string) (*metav1.APIResourceList, error) {
+	return nil, nil
+}
+
+func (f *fakeDiscoveryClient) ServerGroupsAndResources() ([]*metav1.APIGroup, []*metav1.APIResourceList, error) {
+	return nil, nil, nil
+}
+
+func (f *fakeDiscoveryClient) ServerPreferredNamespacedResources() ([]*metav1.APIResourceList, error) {
+	return nil, nil
+}

internal/concierge/server/server.go

@@ -23,6 +23,7 @@ import (
 	"k8s.io/client-go/rest"
 
 	conciergeopenapi "go.pinniped.dev/generated/latest/client/concierge/openapi"
+	"go.pinniped.dev/internal/admissionpluginconfig"
 	"go.pinniped.dev/internal/certauthority/dynamiccertauthority"
 	"go.pinniped.dev/internal/clientcertissuer"
 	"go.pinniped.dev/internal/concierge/apiserver"
@@ -247,6 +248,11 @@ func getAggregatedAPIServerConfig(
 	// This port is configurable. It should be safe to cast because the config reader already validated it.
 	recommendedOptions.SecureServing.BindPort = int(aggregatedAPIServerPort)
 
+	err := admissionpluginconfig.ConfigureAdmissionPlugins(recommendedOptions)
+	if err != nil {
+		return nil, fmt.Errorf("failed to configure admission plugins on recommended options: %w", err)
+	}
+
 	// secure TLS for connections coming from and going to the Kube API server
 	// this is best effort because not all options provide the right hooks to override TLS config
 	// since our only client is the Kube API server, this uses the most secure TLS config

internal/supervisor/server/server.go

@@ -43,6 +43,7 @@ import (
 	"go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/typed/config/v1alpha1"
 	supervisorinformers "go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions"
 	supervisoropenapi "go.pinniped.dev/generated/latest/client/supervisor/openapi"
+	"go.pinniped.dev/internal/admissionpluginconfig"
 	"go.pinniped.dev/internal/apiserviceref"
 	"go.pinniped.dev/internal/config/featuregates"
 	"go.pinniped.dev/internal/config/supervisor"
@@ -622,6 +623,11 @@ func getAggregatedAPIServerConfig(
 	// This port is configurable. It should be safe to cast because the config reader already validated it.
 	recommendedOptions.SecureServing.BindPort = int(aggregatedAPIServerPort)
 
+	err := admissionpluginconfig.ConfigureAdmissionPlugins(recommendedOptions)
+	if err != nil {
+		return nil, fmt.Errorf("failed to configure admission plugins on recommended options: %w", err)
+	}
+
 	// secure TLS for connections coming from and going to the Kube API server
 	// this is best effort because not all options provide the right hooks to override TLS config
 	// since our only client is the Kube API server, this uses the most secure TLS config