Merge pull request #43813 from liggitt/conditional-post-start-hook

Automatic merge from submit-queue

Make RBAC post-start hook conditional on RBAC authorizer being used

Makes the RBAC post-start hook (and reconciliation) conditional on the RBAC authorizer being used

Ensures we don't set up unnecessary objects.

```release-note
RBAC role and rolebinding auto-reconciliation is now performed only when the RBAC authorization mode is enabled.
```

cmd/kube-apiserver/app/BUILD

@@ -32,12 +32,14 @@ go_library(
         "//pkg/kubeapiserver:go_default_library",
         "//pkg/kubeapiserver/admission:go_default_library",
         "//pkg/kubeapiserver/authenticator:go_default_library",
+        "//pkg/kubeapiserver/authorizer/modes:go_default_library",
         "//pkg/kubeapiserver/options:go_default_library",
         "//pkg/kubeapiserver/server:go_default_library",
         "//pkg/master:go_default_library",
         "//pkg/master/thirdparty:go_default_library",
         "//pkg/master/tunneler:go_default_library",
         "//pkg/registry/cachesize:go_default_library",
+        "//pkg/registry/rbac/rest:go_default_library",
         "//pkg/version:go_default_library",
         "//plugin/pkg/admission/admit:go_default_library",
         "//plugin/pkg/admission/alwayspullimages:go_default_library",

cmd/kube-apiserver/app/server.go

@@ -66,11 +66,13 @@ import (
 	"k8s.io/kubernetes/pkg/kubeapiserver"
 	kubeadmission "k8s.io/kubernetes/pkg/kubeapiserver/admission"
 	kubeauthenticator "k8s.io/kubernetes/pkg/kubeapiserver/authenticator"
+	"k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes"
 	kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options"
 	kubeserver "k8s.io/kubernetes/pkg/kubeapiserver/server"
 	"k8s.io/kubernetes/pkg/master"
 	"k8s.io/kubernetes/pkg/master/tunneler"
 	"k8s.io/kubernetes/pkg/registry/cachesize"
+	rbacrest "k8s.io/kubernetes/pkg/registry/rbac/rest"
 	"k8s.io/kubernetes/pkg/version"
 	"k8s.io/kubernetes/plugin/pkg/auth/authenticator/token/bootstrap"
 )
@@ -353,6 +355,9 @@ func BuildGenericConfig(s *options.ServerRunOptions) (*genericapiserver.Config,
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("invalid authorization config: %v", err)
 	}
+	if !sets.NewString(s.Authorization.Modes()...).Has(modes.ModeRBAC) {
+		genericConfig.DisabledPostStartHooks.Insert(rbacrest.PostStartHookName)
+	}
 
 	genericConfig.AdmissionControl, err = BuildAdmission(s, client, sharedInformers, genericConfig.Authorizer)
 	if err != nil {

hack/make-rules/test-cmd.sh

@@ -36,10 +36,14 @@ function run_kube_apiserver() {
   # Admission Controllers to invoke prior to persisting objects in cluster
   ADMISSION_CONTROL="NamespaceLifecycle,LimitRanger,ResourceQuota"
 
+  # Include RBAC (to exercise bootstrapping), and AlwaysAllow to allow all actions
+  AUTHORIZATION_MODE="RBAC,AlwaysAllow"
+
   "${KUBE_OUTPUT_HOSTBIN}/kube-apiserver" \
     --address="127.0.0.1" \
     --public-address-override="127.0.0.1" \
     --port="${API_PORT}" \
+    --authorization-mode="${AUTHORIZATION_MODE}" \
     --admission-control="${ADMISSION_CONTROL}" \
     --etcd-servers="http://${ETCD_HOST}:${ETCD_PORT}" \
     --public-address-override="127.0.0.1" \

pkg/kubeapiserver/options/authorization.go

@@ -75,14 +75,17 @@ func (s *BuiltInAuthorizationOptions) AddFlags(fs *pflag.FlagSet) {
 
 }
 
-func (s *BuiltInAuthorizationOptions) ToAuthorizationConfig(informerFactory informers.SharedInformerFactory) authorizer.AuthorizationConfig {
+func (s *BuiltInAuthorizationOptions) Modes() []string {
 	modes := []string{}
 	if len(s.Mode) > 0 {
 		modes = strings.Split(s.Mode, ",")
 	}
+	return modes
+}
 
+func (s *BuiltInAuthorizationOptions) ToAuthorizationConfig(informerFactory informers.SharedInformerFactory) authorizer.AuthorizationConfig {
 	return authorizer.AuthorizationConfig{
-		AuthorizationModes:          modes,
+		AuthorizationModes:          s.Modes(),
 		PolicyFile:                  s.PolicyFile,
 		WebhookConfigFile:           s.WebhookConfigFile,
 		WebhookCacheAuthorizedTTL:   s.WebhookCacheAuthorizedTTL,

pkg/registry/rbac/rest/storage_rbac.go

@@ -55,6 +55,8 @@ import (
 	"k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac/bootstrappolicy"
 )
 
+const PostStartHookName = "rbac/bootstrap-roles"
+
 type RESTStorageProvider struct {
 	Authorizer authorizer.Authorizer
 }
@@ -123,7 +125,7 @@ func (p RESTStorageProvider) storage(version schema.GroupVersion, apiResourceCon
 }
 
 func (p RESTStorageProvider) PostStartHook() (string, genericapiserver.PostStartHookFunc, error) {
-	return "rbac/bootstrap-roles", PostStartHook, nil
+	return PostStartHookName, PostStartHook, nil
 }
 
 func PostStartHook(hookContext genericapiserver.PostStartHookContext) error {

staging/src/k8s.io/apiserver/pkg/server/config.go

@@ -96,6 +96,8 @@ type Config struct {
 	EnableContentionProfiling bool
 	EnableMetrics             bool
 
+	DisabledPostStartHooks sets.String
+
 	// Version will enable the /version endpoint if non-nil
 	Version *version.Info
 	// AuditWriter is the destination for audit logs.  If nil, they will not be written.
@@ -203,6 +205,7 @@ func NewConfig(codecs serializer.CodecFactory) *Config {
 		RequestContextMapper:        apirequest.NewRequestContextMapper(),
 		BuildHandlerChainFunc:       DefaultBuildHandlerChain,
 		LegacyAPIGroupPrefixes:      sets.NewString(DefaultLegacyAPIPrefix),
+		DisabledPostStartHooks:      sets.NewString(),
 		HealthzChecks:               []healthz.HealthzChecker{healthz.PingHealthz},
 		EnableIndex:                 true,
 		EnableDiscovery:             true,
@@ -416,6 +419,8 @@ func (c completedConfig) constructServer() (*GenericAPIServer, error) {
 		openAPIConfig: c.OpenAPIConfig,
 
 		postStartHooks:         map[string]postStartHookEntry{},
+		disabledPostStartHooks: c.DisabledPostStartHooks,
+
 		healthzChecks: c.HealthzChecks,
 	}
 

staging/src/k8s.io/apiserver/pkg/server/genericapiserver.go

@@ -143,10 +143,11 @@ type GenericAPIServer struct {
 
 	// PostStartHooks are each called after the server has started listening, in a separate go func for each
 	// with no guarantee of ordering between them.  The map key is a name used for error reporting.
-	// It may kill the process with a panic if it wishes to by returning an error
+	// It may kill the process with a panic if it wishes to by returning an error.
 	postStartHookLock      sync.Mutex
 	postStartHooks         map[string]postStartHookEntry
 	postStartHooksCalled   bool
+	disabledPostStartHooks sets.String
 
 	// healthz checks
 	healthzLock    sync.Mutex

staging/src/k8s.io/apiserver/pkg/server/hooks.go

@@ -65,6 +65,9 @@ func (s *GenericAPIServer) AddPostStartHook(name string, hook PostStartHookFunc)
 	if hook == nil {
 		return nil
 	}
+	if s.disabledPostStartHooks.Has(name) {
+		return nil
+	}
 
 	s.postStartHookLock.Lock()
 	defer s.postStartHookLock.Unlock()