UPSTREAM: <carry>: Add configuration to RestrictSubjectBindings admission plugin

to allow external components to specify that the oauth stack
will/will not be present on the cluster. When the oauth stack is
not present, this admission plugin is not effective and will be disabled
to prevent configuration and startup of informers for APIs served by the oauth-apiserver

Signed-off-by: Bryce Palmer <[email protected]>

Author: everettraven

openshift-kube-apiserver/admission/authorization/apis/restrictusers/v1alpha1/doc.go

@@ -0,0 +1,4 @@
+// +k8s:deepcopy-gen=package,register
+
+// Package v1alpha is the v1alpha1 version of the API.
+package v1alpha1

openshift-kube-apiserver/admission/authorization/apis/restrictusers/v1alpha1/register.go

@@ -0,0 +1,29 @@
+package v1alpha1
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+var (
+	GroupVersion         = schema.GroupVersion{Group: "authorization.openshift.io", Version: "v1alpha1"}
+	internalGroupVersion = schema.GroupVersion{Group: "authorization.openshift.io", Version: runtime.APIVersionInternal}
+)
+
+var (
+	localSchemeBuilder = runtime.NewSchemeBuilder(
+		addKnownTypes,
+	)
+	Install = localSchemeBuilder.AddToScheme
+)
+
+func addKnownTypes(scheme *runtime.Scheme) error {
+	scheme.AddKnownTypes(GroupVersion,
+		&RestrictSubjectBindingsAdmissionConfig{},
+	)
+
+	scheme.AddKnownTypes(internalGroupVersion,
+		&RestrictSubjectBindingsAdmissionConfig{},
+	)
+	return nil
+}

openshift-kube-apiserver/admission/authorization/apis/restrictusers/v1alpha1/types.go

@@ -0,0 +1,36 @@
+package v1alpha1
+
+import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
+
+// RestrictSubjectBindingsAdmissionConfig is the type
+// used for configuring the authorization.openshift.io/RestrictSubjectBindings
+// admission plugin.
+type RestrictSubjectBindingsAdmissionConfig struct {
+	metav1.TypeMeta `json:",inline"`
+
+	// openshiftOAuthDesiredState specifies the desired state
+	// of the OpenShift oauth-apiserver based on observed configuration.
+	//
+	// Allowed values are Desired and NotDesired.
+    // Defaults to Desired.
+	//
+	// When set to Desired, the authorization.openshift.io/RestrictSubjectBindings
+	// admission plugin will be configured with the expectation that the OpenShift
+	// oauth-apiserver will eventually be running and serving it's APIs.
+	//
+	// When set to NotDesired, the authorization.openshift.io/RestrictSubjectBindings
+	// admission plugin will be configured with the expectation that the OpenShift
+	// oauth-apiserver will not be running.
+	OpenShiftOAuthDesiredState OpenShiftOAuthState `json:"openshiftOAuthDesiredState"`
+}
+
+type OpenShiftOAuthState string
+
+const (
+	OpenShiftOAuthStateDesired    = "Desired"
+	OpenShiftOAuthStateNotDesired = "NotDesired"
+)

openshift-kube-apiserver/admission/authorization/apis/restrictusers/v1alpha1/zz_generated.deepcopy.go

@@ -13,6 +13,7 @@ import (
 	"k8s.io/apiserver/pkg/admission/initializer"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/cache"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/apis/rbac"
 
@@ -21,14 +22,53 @@ import (
 	userclient "github.com/openshift/client-go/user/clientset/versioned"
 	userinformer "github.com/openshift/client-go/user/informers/externalversions"
 	"github.com/openshift/library-go/pkg/apiserver/admission/admissionrestconfig"
+	"github.com/openshift/library-go/pkg/config/helpers"
+	restrictusersv1alpha1 "k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/apis/restrictusers/v1alpha1"
 	"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/usercache"
 )
 
+const RestrictSubjectBindingsPluginName = "authorization.openshift.io/RestrictSubjectBindings"
+
 func Register(plugins *admission.Plugins) {
-	plugins.Register("authorization.openshift.io/RestrictSubjectBindings",
-		func(config io.Reader) (admission.Interface, error) {
-			return NewRestrictUsersAdmission()
-		})
+	plugins.Register(RestrictSubjectBindingsPluginName, pluginForConfig)
+}
+
+func pluginForConfig(config io.Reader) (admission.Interface, error) {
+	cfg, err := readConfig(config)
+	if err != nil {
+		return nil, err
+	}
+
+	if cfg != nil && cfg.OpenShiftOAuthDesiredState == restrictusersv1alpha1.OpenShiftOAuthStateNotDesired {
+		klog.Infof("Admission plugin %q configured to expect the OpenShift oauth-apiserver as not being available. This is effectively the same as disabling the plugin, so it will be disabled.", RestrictSubjectBindingsPluginName)
+		return nil, nil
+	}
+
+	return NewRestrictUsersAdmission()
+}
+
+func readConfig(reader io.Reader) (*restrictusersv1alpha1.RestrictSubjectBindingsAdmissionConfig, error) {
+	obj, err := helpers.ReadYAMLToInternal(reader, restrictusersv1alpha1.Install)
+	if err != nil {
+		return nil, err
+	}
+	if obj == nil {
+		return nil, nil
+	}
+	config, ok := obj.(*restrictusersv1alpha1.RestrictSubjectBindingsAdmissionConfig)
+	if !ok {
+		return nil, fmt.Errorf("unexpected config object: %#v", obj)
+	}
+
+	// validate config
+	switch config.OpenShiftOAuthDesiredState {
+	case restrictusersv1alpha1.OpenShiftOAuthStateDesired, restrictusersv1alpha1.OpenShiftOAuthStateNotDesired:
+		// valid, do nothing
+	default:
+		return nil, fmt.Errorf("config is invalid, openshiftOAuthDesiredState must be one of Desired,NotDesired but was %s", config.OpenShiftOAuthDesiredState)
+	}
+
+	return config, nil
 }
 
 type GroupCache interface {
@@ -48,19 +88,16 @@ type restrictUsersAdmission struct {
 	groupCache                    GroupCache
 }
 
-var _ = admissionrestconfig.WantsRESTClientConfig(&restrictUsersAdmission{})
-var _ = WantsUserInformer(&restrictUsersAdmission{})
-var _ = initializer.WantsExternalKubeClientSet(&restrictUsersAdmission{})
-var _ = admission.ValidationInterface(&restrictUsersAdmission{})
+var (
+	_ = admissionrestconfig.WantsRESTClientConfig(&restrictUsersAdmission{})
+	_ = WantsUserInformer(&restrictUsersAdmission{})
+	_ = initializer.WantsExternalKubeClientSet(&restrictUsersAdmission{})
+	_ = admission.ValidationInterface(&restrictUsersAdmission{})
+)
 
 // NewRestrictUsersAdmission configures an admission plugin that enforces
 // restrictions on adding role bindings in a project.
-func NewRestrictUsersAdmission() (admission.Interface, error) {
-	return &restrictUsersAdmission{
-		Handler: admission.NewHandler(admission.Create, admission.Update),
-	}, nil
-}
-
+func NewRestrictUsersAdmission() (admission.Interface, error)
 func (q *restrictUsersAdmission) SetExternalKubeClientSet(c kubernetes.Interface) {
 	q.kubeClient = c
 }
@@ -87,6 +124,11 @@ func (q *restrictUsersAdmission) SetRESTClientConfig(restClientConfig rest.Confi
 }
 
 func (q *restrictUsersAdmission) SetUserInformer(userInformers userinformer.SharedInformerFactory) {
+	if err := userInformers.User().V1().Groups().Informer().AddIndexers(cache.Indexers{
+		usercache.ByUserIndexName: usercache.ByUserIndexKeys,
+	}); err != nil {
+		return
+	}
 	q.groupCache = usercache.NewGroupCache(userInformers.User().V1().Groups())
 }
 
@@ -116,7 +158,6 @@ func subjectsDelta(elementsToIgnore, elements []rbac.Subject) []rbac.Subject {
 // each subject in the binding must be matched by some rolebinding restriction
 // in the namespace.
 func (q *restrictUsersAdmission) Validate(ctx context.Context, a admission.Attributes, _ admission.ObjectInterfaces) (err error) {
-
 	// We only care about rolebindings
 	if a.GetResource().GroupResource() != rbac.Resource("rolebindings") {
 		return nil

openshift-kube-apiserver/admission/authorization/restrictusers/restrictusers.go

@@ -3,10 +3,12 @@ package restrictusers
 import (
 	"context"
 	"fmt"
+	"io"
 	"strings"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -402,3 +404,74 @@ func TestAdmission(t *testing.T) {
 		}
 	}
 }
+
+func TestPluginForConfig(t *testing.T) {
+	testcases := []struct {
+		name           string
+		config         string
+		expectedErr    string
+		expectedPlugin admission.Interface
+	}{
+		{
+			name: "no config, no err, expect plugin",
+			expectedPlugin: func() admission.Interface {
+				plugin, _ := NewRestrictUsersAdmission()
+				return plugin
+			}(),
+		},
+		{
+			name: "config sets openshiftOAuthDesiredState to NotDesired, no err, nil plugin",
+			config: `apiVersion: authorization.openshift.io/v1alpha1
+kind: RestrictSubjectBindingsAdmissionConfig
+openshiftOAuthDesiredState: NotDesired
+            `,
+			expectedPlugin: nil,
+		},
+		{
+			name: "config sets openshiftOAuthDesiredState to Desired, no err, expect plugin",
+			config: `apiVersion: authorization.openshift.io/v1alpha1
+kind: RestrictSubjectBindingsAdmissionConfig
+openshiftOAuthDesiredState: Desired
+            `,
+			expectedPlugin: func() admission.Interface {
+				plugin, _ := NewRestrictUsersAdmission()
+				return plugin
+			}(),
+		},
+		{
+			name: "config sets openshiftOAuthDesiredState to invalid value, err, nil plugin",
+			config: `apiVersion: authorization.openshift.io/v1alpha1
+kind: RestrictSubjectBindingsAdmissionConfig
+openshiftOAuthDesiredState: FooBar
+            `,
+			expectedPlugin: nil,
+			expectedErr:    "config is invalid, openshiftOAuthDesiredState must be one of Desired,NotDesired",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			var reader io.Reader
+			if len(tc.config) > 0 {
+				reader = strings.NewReader(tc.config)
+			}
+
+			plugin, err := pluginForConfig(reader)
+			switch {
+			case len(tc.expectedErr) == 0 && err == nil:
+			case len(tc.expectedErr) == 0 && err != nil:
+				t.Errorf("%s: unexpected error: %v", tc.name, err)
+			case len(tc.expectedErr) != 0 && err == nil:
+				t.Errorf("%s: missing error: %v", tc.name, tc.expectedErr)
+			case len(tc.expectedErr) != 0 && err != nil &&
+				!strings.Contains(err.Error(), tc.expectedErr):
+				t.Errorf("%s: missing error: expected %v, got %v",
+					tc.name, tc.expectedErr, err)
+			}
+
+			if !equality.Semantic.DeepEqual(tc.expectedPlugin, plugin) {
+				t.Errorf("plugin does not match. expected %v, got %v", tc.expectedPlugin, plugin)
+			}
+		})
+	}
+}

openshift-kube-apiserver/admission/authorization/restrictusers/restrictusers_test.go

@@ -27,9 +27,7 @@ import (
 	clientgoinformers "k8s.io/client-go/informers"
 	corev1informers "k8s.io/client-go/informers/core/v1"
 	"k8s.io/client-go/rest"
-	"k8s.io/client-go/tools/cache"
 	"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers"
-	"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/usercache"
 	"k8s.io/kubernetes/openshift-kube-apiserver/admission/autoscaling/managednode"
 	"k8s.io/kubernetes/openshift-kube-apiserver/admission/autoscaling/managementcpusoverride"
 	"k8s.io/kubernetes/openshift-kube-apiserver/admission/scheduler/nodeenv"
@@ -109,7 +107,8 @@ func OpenShiftKubeAPIServerConfigPatch(genericConfig *genericapiserver.Config, k
 	// END HANDLER CHAIN
 
 	openshiftAPIServiceReachabilityCheck := newOpenshiftAPIServiceReachabilityCheck(genericConfig.PublicAddress)
-	oauthAPIServiceReachabilityCheck := newOAuthPIServiceReachabilityCheck(genericConfig.PublicAddress)
+	oauthAPIServiceReachabilityCheck := newOAuthAPIServiceReachabilityCheck(genericConfig.PublicAddress)
+
 	genericConfig.ReadyzChecks = append(genericConfig.ReadyzChecks, openshiftAPIServiceReachabilityCheck, oauthAPIServiceReachabilityCheck)
 
 	genericConfig.AddPostStartHookOrDie("openshift.io-startkubeinformers", func(context genericapiserver.PostStartHookContext) error {
@@ -176,11 +175,6 @@ func newInformers(loopbackClientConfig *rest.Config) (*kubeAPIServerInformers, e
 		OpenshiftUserInformers:     userinformer.NewSharedInformerFactory(userClient, defaultInformerResyncPeriod),
 		OpenshiftConfigInformers:   configv1informer.NewSharedInformerFactory(configClient, defaultInformerResyncPeriod),
 	}
-	if err := ret.OpenshiftUserInformers.User().V1().Groups().Informer().AddIndexers(cache.Indexers{
-		usercache.ByUserIndexName: usercache.ByUserIndexKeys,
-	}); err != nil {
-		return nil, err
-	}
 
 	return ret, nil
 }
@@ -195,12 +189,15 @@ type kubeAPIServerInformers struct {
 func (i *kubeAPIServerInformers) getOpenshiftQuotaInformers() quotainformer.SharedInformerFactory {
 	return i.OpenshiftQuotaInformers
 }
+
 func (i *kubeAPIServerInformers) getOpenshiftSecurityInformers() securityv1informer.SharedInformerFactory {
 	return i.OpenshiftSecurityInformers
 }
+
 func (i *kubeAPIServerInformers) getOpenshiftUserInformers() userinformer.SharedInformerFactory {
 	return i.OpenshiftUserInformers
 }
+
 func (i *kubeAPIServerInformers) getOpenshiftInfraInformers() configv1informer.SharedInformerFactory {
 	return i.OpenshiftConfigInformers
 }

openshift-kube-apiserver/openshiftkubeapiserver/patch.go

@@ -22,7 +22,7 @@ func newOpenshiftAPIServiceReachabilityCheck(ipForKubernetesDefaultService net.I
 	return newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService, "openshift-apiserver", "api")
 }
 
-func newOAuthPIServiceReachabilityCheck(ipForKubernetesDefaultService net.IP) *aggregatedAPIServiceAvailabilityCheck {
+func newOAuthAPIServiceReachabilityCheck(ipForKubernetesDefaultService net.IP) *aggregatedAPIServiceAvailabilityCheck {
 	return newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService, "openshift-oauth-apiserver", "api")
 }