UPSTREAM: <carry>: Move Group informer configuration into RestrictSub…

…jectBindings admission plugin initialization to prevent Group informers being configured when the plugin is disabled. This is necessary for when the OpenShift OAuth stack is not present and the plugin is disabled as part of that. Signed-off-by: Bryce Palmer <[email protected]>
openshift · Jan 29, 2025 · 03e0545 · 03e0545
1 parent 8ac36bf
commit 03e0545
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 14 deletions.
diff --git a/openshift-kube-apiserver/admission/authorization/restrictusers/restrictusers.go b/openshift-kube-apiserver/admission/authorization/restrictusers/restrictusers.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"
 
@@ -48,10 +49,12 @@ 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.
@@ -87,6 +90,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 +124,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

diff --git a/openshift-kube-apiserver/openshiftkubeapiserver/patch.go b/openshift-kube-apiserver/openshiftkubeapiserver/patch.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
 }

diff --git a/openshift-kube-apiserver/openshiftkubeapiserver/sdn_readyz_wait.go b/openshift-kube-apiserver/openshiftkubeapiserver/sdn_readyz_wait.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")
 }