new approach

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

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,22 @@
+package v1alpha1
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+var GroupVersion = schema.GroupVersion{Group: "authorization.openshift.io", Version: "v1alpha1"}
+
+var (
+	localSchemeBuilder = runtime.NewSchemeBuilder(
+		addKnownTypes,
+	)
+	Install = localSchemeBuilder.AddToScheme
+)
+
+func addKnownTypes(scheme *runtime.Scheme) error {
+	scheme.AddKnownTypes(GroupVersion,
+        &RestrictSubjectBindingsAdmissionConfig{},
+	)
+	return nil
+}

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

@@ -0,0 +1,35 @@
+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.
+	//
+	// 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/restrictusers/restrictusers.go

@@ -22,16 +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"
+	"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/apis/restrictusers/v1alpha1"
 	"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/usercache"
 )
 
 func Register(plugins *admission.Plugins) {
 	plugins.Register("authorization.openshift.io/RestrictSubjectBindings",
 		func(config io.Reader) (admission.Interface, error) {
-			return NewRestrictUsersAdmission()
+			cfg, err := readConfig(config)
+			if err != nil {
+				return nil, err
+			}
+
+            return NewRestrictUsersAdmission(cfg)
 		})
 }
 
+func defaultConfig() *v1alpha1.RestrictSubjectBindingsAdmissionConfig {
+	return &v1alpha1.RestrictSubjectBindingsAdmissionConfig{
+		OpenShiftOAuthDesiredState: v1alpha1.OpenShiftOAuthStateDesired,
+	}
+}
+
+func readConfig(reader io.Reader) (*v1alpha1.RestrictSubjectBindingsAdmissionConfig, error) {
+	obj, err := helpers.ReadYAMLToInternal(reader, v1alpha1.Install)
+	if err != nil {
+		return nil, err
+	}
+	if obj == nil {
+		return nil, nil
+	}
+	config, ok := obj.(*v1alpha1.RestrictSubjectBindingsAdmissionConfig)
+	if !ok {
+		return nil, fmt.Errorf("unexpected config object: %#v", obj)
+	}
+
+	// validate config
+	switch config.OpenShiftOAuthDesiredState {
+	case v1alpha1.OpenShiftOAuthStateDesired, v1alpha1.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 {
 	GroupsFor(string) ([]*userv1.Group, error)
 	HasSynced() bool
@@ -46,8 +83,8 @@ type restrictUsersAdmission struct {
 	roleBindingRestrictionsGetter authorizationtypedclient.RoleBindingRestrictionsGetter
 	userClient                    userclient.Interface
 	kubeClient                    kubernetes.Interface
-	groupCacheFunc                func() (GroupCache, error)
 	groupCache                    GroupCache
+	oauthState                    v1alpha1.OpenShiftOAuthState
 }
 
 var (
@@ -59,9 +96,15 @@ var (
 
 // NewRestrictUsersAdmission configures an admission plugin that enforces
 // restrictions on adding role bindings in a project.
-func NewRestrictUsersAdmission() (admission.Interface, error) {
+func NewRestrictUsersAdmission(cfg *v1alpha1.RestrictSubjectBindingsAdmissionConfig) (admission.Interface, error) {
 	return &restrictUsersAdmission{
 		Handler: admission.NewHandler(admission.Create, admission.Update),
+        oauthState: func() v1alpha1.OpenShiftOAuthState{
+            if cfg != nil {
+                return cfg.OpenShiftOAuthDesiredState
+            }
+            return v1alpha1.OpenShiftOAuthStateDesired
+        }(),
 	}, nil
 }
 
@@ -91,18 +134,16 @@ func (q *restrictUsersAdmission) SetRESTClientConfig(restClientConfig rest.Confi
 }
 
 func (q *restrictUsersAdmission) SetUserInformer(userInformers userinformer.SharedInformerFactory) {
-	// defer the allocation of the group cache until later in the process so we can
-	// ensure we aren't creating informers for the Group resources until this admission
-	// plugin actually runs. If authentication type is OIDC, this plugin should be disabled
-	// resulting in the Group informer never being configured and started.
-	q.groupCacheFunc = func() (GroupCache, error) {
-		if err := userInformers.User().V1().Groups().Informer().AddIndexers(cache.Indexers{
-			usercache.ByUserIndexName: usercache.ByUserIndexKeys,
-		}); err != nil {
-			return nil, err
-		}
-		return usercache.NewGroupCache(userInformers.User().V1().Groups()), nil
+	if q.oauthState == v1alpha1.OpenShiftOAuthStateNotDesired {
+		return
+	}
+
+	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())
 }
 
 // subjectsDelta returns the relative complement of elementsToIgnore in
@@ -201,13 +242,6 @@ func (q *restrictUsersAdmission) Validate(ctx context.Context, a admission.Attri
 		checkers = append(checkers, checker)
 	}
 
-	if q.groupCache == nil && q.groupCacheFunc != nil {
-		q.groupCache, err = q.groupCacheFunc()
-		if err != nil {
-			return admission.NewForbidden(a, fmt.Errorf("could not create group cache: %v", err))
-		}
-	}
-
 	roleBindingRestrictionContext, err := newRoleBindingRestrictionContext(ns,
 		q.kubeClient, q.userClient.UserV1(), q.groupCache)
 	if err != nil {
@@ -247,7 +281,7 @@ func (q *restrictUsersAdmission) ValidateInitialization() error {
 	if q.userClient == nil {
 		return errors.New("RestrictUsersAdmission plugin requires an OpenShift user client")
 	}
-	if q.groupCache == nil && q.groupCacheFunc == nil {
+	if q.groupCache == nil && q.oauthState == v1alpha1.OpenShiftOAuthStateDesired {
 		return errors.New("RestrictUsersAdmission plugin requires a group cache")
 	}
 

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

@@ -359,7 +359,7 @@ func TestAdmission(t *testing.T) {
 		fakeUserClient := fakeuserclient.NewSimpleClientset(tc.userObjects...)
 		fakeAuthorizationClient := fakeauthorizationclient.NewSimpleClientset(tc.authorizationObjects...)
 
-		plugin, err := NewRestrictUsersAdmission()
+		plugin, err := NewRestrictUsersAdmission(nil)
 		if err != nil {
 			t.Errorf("unexpected error initializing admission plugin: %v", err)
 		}

openshift-kube-apiserver/openshiftkubeapiserver/patch.go

@@ -4,7 +4,6 @@ import (
 	"os"
 	"time"
 
-	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/apiserver-library-go/pkg/admission/imagepolicy"
 	"github.com/openshift/apiserver-library-go/pkg/admission/imagepolicy/imagereferencemutators"
 	"github.com/openshift/apiserver-library-go/pkg/admission/quota/clusterresourcequota"
@@ -22,15 +21,13 @@ import (
 	"github.com/openshift/library-go/pkg/apiserver/admission/admissionrestconfig"
 	"github.com/openshift/library-go/pkg/apiserver/apiserverconfig"
 	"github.com/openshift/library-go/pkg/quota/clusterquotamapping"
-	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/quota/v1/generic"
 	genericapiserver "k8s.io/apiserver/pkg/server"
 	clientgoinformers "k8s.io/client-go/informers"
 	corev1informers "k8s.io/client-go/informers/core/v1"
 	"k8s.io/client-go/rest"
 	"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers"
-	"k8s.io/kubernetes/openshift-kube-apiserver/admission/authorization/restrictusers/authncache"
 	"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"
@@ -110,25 +107,7 @@ func OpenShiftKubeAPIServerConfigPatch(genericConfig *genericapiserver.Config, k
 	// END HANDLER CHAIN
 
 	openshiftAPIServiceReachabilityCheck := newOpenshiftAPIServiceReachabilityCheck(genericConfig.PublicAddress)
-
-	oauthAPIServiceTerminationCondition := func() (bool, string) {
-		authnCache := authncache.NewAuthnCache(openshiftInformers.OpenshiftConfigInformers.Config().V1().Authentications())
-		err := wait.PollImmediate(1*time.Second, 30*time.Second, func() (bool, error) {
-			return authnCache.HasSynced(), nil
-		})
-		if err == nil {
-			auth, err := authnCache.Authn()
-			if err == nil && auth != nil {
-				if auth.Spec.Type == configv1.AuthenticationTypeOIDC {
-					// skip the oauthAPIServiceReachabilityCheck if OIDC
-					// has been configured since the oauth apiserver will be down.
-					return true, "authentication type is OIDC, meaning no oauth-apiserver is deployed. Skipping oauth-apiserver availability check"
-				}
-			}
-		}
-		return false, ""
-	}
-	oauthAPIServiceReachabilityCheck := newOAuthAPIServiceReachabilityCheck(genericConfig.PublicAddress, oauthAPIServiceTerminationCondition)
+	oauthAPIServiceReachabilityCheck := newOAuthAPIServiceReachabilityCheck(genericConfig.PublicAddress)
 
 	genericConfig.ReadyzChecks = append(genericConfig.ReadyzChecks, openshiftAPIServiceReachabilityCheck, oauthAPIServiceReachabilityCheck)
 

openshift-kube-apiserver/openshiftkubeapiserver/sdn_readyz_wait.go

@@ -19,28 +19,25 @@ import (
 )
 
 func newOpenshiftAPIServiceReachabilityCheck(ipForKubernetesDefaultService net.IP) *aggregatedAPIServiceAvailabilityCheck {
-	return newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService, "openshift-apiserver", "api", nil)
+	return newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService, "openshift-apiserver", "api")
 }
 
-func newOAuthAPIServiceReachabilityCheck(ipForKubernetesDefaultService net.IP, terminationCondition terminationConditionFunc) *aggregatedAPIServiceAvailabilityCheck {
-	return newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService, "openshift-oauth-apiserver", "api", terminationCondition)
+func newOAuthAPIServiceReachabilityCheck(ipForKubernetesDefaultService net.IP) *aggregatedAPIServiceAvailabilityCheck {
+	return newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService, "openshift-oauth-apiserver", "api")
 }
 
 // if the API service is not found or the termination condition is met, then this check returns quickly.
 // if the endpoint is not accessible within 60 seconds, we report ready no matter what
 // otherwise, wait for up to 60 seconds to be able to reach the apiserver
-func newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService net.IP, namespace, service string, terminationCondition terminationConditionFunc) *aggregatedAPIServiceAvailabilityCheck {
+func newAggregatedAPIServiceReachabilityCheck(ipForKubernetesDefaultService net.IP, namespace, service string) *aggregatedAPIServiceAvailabilityCheck {
 	return &aggregatedAPIServiceAvailabilityCheck{
 		done:                          make(chan struct{}),
 		ipForKubernetesDefaultService: ipForKubernetesDefaultService,
 		namespace:                     namespace,
 		serviceName:                   service,
-		terminationCondition:          terminationCondition,
 	}
 }
 
-type terminationConditionFunc func() (bool, string)
-
 type aggregatedAPIServiceAvailabilityCheck struct {
 	// done indicates that this check is complete (success or failure) and the check should return true
 	done chan struct{}
@@ -53,11 +50,6 @@ type aggregatedAPIServiceAvailabilityCheck struct {
 	namespace string
 	// serviceName is used to get a list of endpoints to directly dial
 	serviceName string
-
-	// terminationCondition is used to determine if conditions are met
-	// to terminate the availability check early. If the conditions are met,
-	// it is expected that true and a message is returned to be logged.
-	terminationCondition terminationConditionFunc
 }
 
 func (c *aggregatedAPIServiceAvailabilityCheck) Name() string {
@@ -84,13 +76,6 @@ func (c *aggregatedAPIServiceAvailabilityCheck) checkForConnection(context gener
 		close(c.done)      // once this method is done, the ready check should return true
 	}()
 
-	if c.terminationCondition != nil {
-		if ok, msg := c.terminationCondition(); ok {
-			klog.V(2).Infof("%s early termination condition met: %s", c.Name(), msg)
-			return
-		}
-	}
-
 	start := time.Now()
 
 	kubeClient, err := kubernetes.NewForConfig(context.LoopbackClientConfig)