From f2875274423dac61293069f79eddf1c397e7376a Mon Sep 17 00:00:00 2001 From: hzxuzhonghu Date: Wed, 29 Nov 2017 23:12:19 +0800 Subject: [PATCH 1/3] admission registration use shared informer instead of poll --- .../configuration/mutating_webhook_manager.go | 100 ++++++++------- .../mutating_webhook_manager_test.go | 118 ++++++++++++++++-- .../validating_webhook_manager.go | 90 +++++++------ .../validating_webhook_manager_test.go | 118 ++++++++++++++++-- .../plugin/webhook/mutating/admission.go | 9 +- .../plugin/webhook/validating/admission.go | 11 +- 6 files changed, 316 insertions(+), 130 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go index bf4d0eabf98..fbde83b7d82 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go @@ -18,84 +18,82 @@ package configuration import ( "fmt" - "reflect" "sort" - - "github.com/golang/glog" + "sync/atomic" "k8s.io/api/admissionregistration/v1beta1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/labels" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + admissionregistrationinformers "k8s.io/client-go/informers/admissionregistration/v1beta1" + admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1" + "k8s.io/client-go/tools/cache" ) -type MutatingWebhookConfigurationLister interface { - List(opts metav1.ListOptions) (*v1beta1.MutatingWebhookConfigurationList, error) -} - // MutatingWebhookConfigurationManager collects the mutating webhook objects so that they can be called. type MutatingWebhookConfigurationManager struct { - *poller + ready int32 + configuration *atomic.Value + hasSynced func() bool + lister admissionregistrationlisters.MutatingWebhookConfigurationLister } -func NewMutatingWebhookConfigurationManager(c MutatingWebhookConfigurationLister) *MutatingWebhookConfigurationManager { - getFn := func() (runtime.Object, error) { - list, err := c.List(metav1.ListOptions{}) - if err != nil { - if errors.IsNotFound(err) || errors.IsForbidden(err) { - glog.V(5).Infof("MutatingWebhookConfiguration are disabled due to an error: %v", err) - return nil, ErrDisabled - } - return nil, err - } - return mergeMutatingWebhookConfigurations(list), nil +func NewMutatingWebhookConfigurationManager(informer admissionregistrationinformers.MutatingWebhookConfigurationInformer) *MutatingWebhookConfigurationManager { + manager := &MutatingWebhookConfigurationManager{ + ready: 0, + configuration: &atomic.Value{}, + hasSynced: informer.Informer().HasSynced, + lister: informer.Lister(), } - return &MutatingWebhookConfigurationManager{ - newPoller(getFn), - } + // Start with an empty list + manager.configuration.Store(&v1beta1.MutatingWebhookConfiguration{}) + + // On any change, rebuild the config + informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(_ interface{}) { manager.updateConfiguration() }, + UpdateFunc: func(_, _ interface{}) { manager.updateConfiguration() }, + DeleteFunc: func(_ interface{}) { manager.updateConfiguration() }, + }) + + return manager } // Webhooks returns the merged MutatingWebhookConfiguration. -func (im *MutatingWebhookConfigurationManager) Webhooks() (*v1beta1.MutatingWebhookConfiguration, error) { - configuration, err := im.poller.configuration() +func (m *MutatingWebhookConfigurationManager) Webhooks() (*v1beta1.MutatingWebhookConfiguration, error) { + if atomic.LoadInt32(&m.ready) == 0 { + if !m.hasSynced() { + // Return an error until we've synced + return nil, fmt.Errorf("mutating webhook configuration is not ready") + } + // Remember we're ready + atomic.StoreInt32(&m.ready, 1) + } + return m.configuration.Load().(*v1beta1.MutatingWebhookConfiguration), nil +} + +func (m *MutatingWebhookConfigurationManager) updateConfiguration() { + configurations, err := m.lister.List(labels.Everything()) if err != nil { - return nil, err + utilruntime.HandleError(fmt.Errorf("error updating configuration: %v", err)) + return } - mutatingWebhookConfiguration, ok := configuration.(*v1beta1.MutatingWebhookConfiguration) - if !ok { - return nil, fmt.Errorf("expected type %v, got type %v", reflect.TypeOf(mutatingWebhookConfiguration), reflect.TypeOf(configuration)) - } - return mutatingWebhookConfiguration, nil + m.configuration.Store(mergeMutatingWebhookConfigurations(configurations)) } -func (im *MutatingWebhookConfigurationManager) Run(stopCh <-chan struct{}) { - im.poller.Run(stopCh) -} - -func mergeMutatingWebhookConfigurations( - list *v1beta1.MutatingWebhookConfigurationList, -) *v1beta1.MutatingWebhookConfiguration { - configurations := append([]v1beta1.MutatingWebhookConfiguration{}, list.Items...) +func mergeMutatingWebhookConfigurations(configurations []*v1beta1.MutatingWebhookConfiguration) *v1beta1.MutatingWebhookConfiguration { var ret v1beta1.MutatingWebhookConfiguration // The internal order of webhooks for each configuration is provided by the user // but configurations themselves can be in any order. As we are going to run these // webhooks in serial, they are sorted here to have a deterministic order. - sort.Sort(byName(configurations)) + sort.SliceStable(configurations, MutatingWebhookConfigurationSorter(configurations).ByName) for _, c := range configurations { ret.Webhooks = append(ret.Webhooks, c.Webhooks...) } return &ret } -// byName sorts MutatingWebhookConfiguration by name. These objects are all in -// cluster namespace (aka no namespace) thus they all have unique names. -type byName []v1beta1.MutatingWebhookConfiguration +type MutatingWebhookConfigurationSorter []*v1beta1.MutatingWebhookConfiguration -func (x byName) Len() int { return len(x) } - -func (x byName) Swap(i, j int) { x[i], x[j] = x[j], x[i] } - -func (x byName) Less(i, j int) bool { - return x[i].ObjectMeta.Name < x[j].ObjectMeta.Name +func (a MutatingWebhookConfigurationSorter) ByName(i, j int) bool { + return a[i].Name < a[j].Name } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go index 97333880b09..e6b50aa2320 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go @@ -17,24 +17,118 @@ limitations under the License. package configuration import ( + "fmt" + "reflect" "testing" + "time" "k8s.io/api/admissionregistration/v1beta1" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/labels" + admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1" + "k8s.io/client-go/tools/cache" ) -type disabledMutatingWebhookConfigLister struct{} - -func (l *disabledMutatingWebhookConfigLister) List(options metav1.ListOptions) (*v1beta1.MutatingWebhookConfigurationList, error) { - return nil, errors.NewNotFound(schema.GroupResource{Group: "admissionregistration", Resource: "MutatingWebhookConfigurations"}, "") +type fakeMutatingWebhookConfigSharedInformer struct { + informer *fakeMutatingWebhookConfigInformer + lister *fakeMutatingWebhookConfigLister } -func TestMutatingWebhookConfigDisabled(t *testing.T) { - manager := NewMutatingWebhookConfigurationManager(&disabledMutatingWebhookConfigLister{}) - manager.sync() - _, err := manager.Webhooks() - if err.Error() != ErrDisabled.Error() { - t.Errorf("expected %v, got %v", ErrDisabled, err) + +func (f *fakeMutatingWebhookConfigSharedInformer) Informer() cache.SharedIndexInformer { + return f.informer +} +func (f *fakeMutatingWebhookConfigSharedInformer) Lister() admissionregistrationlisters.MutatingWebhookConfigurationLister { + return f.lister +} + +type fakeMutatingWebhookConfigInformer struct { + eventHandler cache.ResourceEventHandler + hasSynced bool +} + +func (f *fakeMutatingWebhookConfigInformer) AddEventHandler(handler cache.ResourceEventHandler) { + fmt.Println("added handler") + f.eventHandler = handler +} +func (f *fakeMutatingWebhookConfigInformer) AddEventHandlerWithResyncPeriod(handler cache.ResourceEventHandler, resyncPeriod time.Duration) { + panic("unsupported") +} +func (f *fakeMutatingWebhookConfigInformer) GetStore() cache.Store { + panic("unsupported") +} +func (f *fakeMutatingWebhookConfigInformer) GetController() cache.Controller { + panic("unsupported") +} +func (f *fakeMutatingWebhookConfigInformer) Run(stopCh <-chan struct{}) { + panic("unsupported") +} +func (f *fakeMutatingWebhookConfigInformer) HasSynced() bool { + return f.hasSynced +} +func (f *fakeMutatingWebhookConfigInformer) LastSyncResourceVersion() string { + panic("unsupported") +} +func (f *fakeMutatingWebhookConfigInformer) AddIndexers(indexers cache.Indexers) error { + panic("unsupported") +} +func (f *fakeMutatingWebhookConfigInformer) GetIndexer() cache.Indexer { panic("unsupported") } + +type fakeMutatingWebhookConfigLister struct { + list []*v1beta1.MutatingWebhookConfiguration + err error +} + +func (f *fakeMutatingWebhookConfigLister) List(selector labels.Selector) (ret []*v1beta1.MutatingWebhookConfiguration, err error) { + return f.list, f.err +} + +func (f *fakeMutatingWebhookConfigLister) Get(name string) (*v1beta1.MutatingWebhookConfiguration, error) { + panic("unsupported") +} + +func TestGetMutatingWebhookConfig(t *testing.T) { + informer := &fakeMutatingWebhookConfigSharedInformer{ + informer: &fakeMutatingWebhookConfigInformer{}, + lister: &fakeMutatingWebhookConfigLister{}, + } + + // unsynced, error retrieving list + informer.informer.hasSynced = false + informer.lister.list = nil + informer.lister.err = fmt.Errorf("mutating webhook configuration is not ready") + manager := NewMutatingWebhookConfigurationManager(informer) + if _, err := manager.Webhooks(); err == nil { + t.Errorf("expected err, but got none") + } + + // list found, still unsynced + informer.informer.hasSynced = false + informer.lister.list = []*v1beta1.MutatingWebhookConfiguration{} + informer.lister.err = nil + if _, err := manager.Webhooks(); err == nil { + t.Errorf("expected err, but got none") + } + + // items populated, still unsynced + webhookContainer := &v1beta1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "webhook1"}, + Webhooks: []v1beta1.Webhook{{Name: "webhook1.1"}}, + } + informer.informer.hasSynced = false + informer.lister.list = []*v1beta1.MutatingWebhookConfiguration{webhookContainer.DeepCopy()} + informer.lister.err = nil + informer.informer.eventHandler.OnAdd(webhookContainer.DeepCopy()) + if _, err := manager.Webhooks(); err == nil { + t.Errorf("expected err, but got none") + } + + // sync completed + informer.informer.hasSynced = true + hooks, err := manager.Webhooks() + if err != nil { + t.Errorf("unexpected err: %v", err) + } + if !reflect.DeepEqual(hooks.Webhooks, webhookContainer.Webhooks) { + t.Errorf("Expected\n%#v\ngot\n%#v", webhookContainer.Webhooks, hooks.Webhooks) } } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go index 8f9fd34daae..f93068b8037 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go @@ -18,67 +18,81 @@ package configuration import ( "fmt" - "reflect" - - "github.com/golang/glog" + "sort" + "sync/atomic" "k8s.io/api/admissionregistration/v1beta1" - "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/labels" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + admissionregistrationinformers "k8s.io/client-go/informers/admissionregistration/v1beta1" + admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1" + "k8s.io/client-go/tools/cache" ) -type ValidatingWebhookConfigurationLister interface { - List(opts metav1.ListOptions) (*v1beta1.ValidatingWebhookConfigurationList, error) -} - // ValidatingWebhookConfigurationManager collects the validating webhook objects so that they can be called. type ValidatingWebhookConfigurationManager struct { - *poller + ready int32 + configuration *atomic.Value + hasSynced func() bool + lister admissionregistrationlisters.ValidatingWebhookConfigurationLister } -func NewValidatingWebhookConfigurationManager(c ValidatingWebhookConfigurationLister) *ValidatingWebhookConfigurationManager { - getFn := func() (runtime.Object, error) { - list, err := c.List(metav1.ListOptions{}) - if err != nil { - if errors.IsNotFound(err) || errors.IsForbidden(err) { - glog.V(5).Infof("ValidatingWebhookConfiguration are disabled due to an error: %v", err) - return nil, ErrDisabled - } - return nil, err - } - return mergeValidatingWebhookConfigurations(list), nil +func NewValidatingWebhookConfigurationManager(informer admissionregistrationinformers.ValidatingWebhookConfigurationInformer) *ValidatingWebhookConfigurationManager { + manager := &ValidatingWebhookConfigurationManager{ + ready: 0, + configuration: &atomic.Value{}, + hasSynced: informer.Informer().HasSynced, + lister: informer.Lister(), } - return &ValidatingWebhookConfigurationManager{ - newPoller(getFn), - } + // Start with an empty list + manager.configuration.Store(&v1beta1.ValidatingWebhookConfiguration{}) + + // On any change, rebuild the config + informer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(_ interface{}) { manager.updateConfiguration() }, + UpdateFunc: func(_, _ interface{}) { manager.updateConfiguration() }, + DeleteFunc: func(_ interface{}) { manager.updateConfiguration() }, + }) + + return manager } // Webhooks returns the merged ValidatingWebhookConfiguration. -func (im *ValidatingWebhookConfigurationManager) Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error) { - configuration, err := im.poller.configuration() - if err != nil { - return nil, err +func (v *ValidatingWebhookConfigurationManager) Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error) { + if atomic.LoadInt32(&v.ready) == 0 { + if !v.hasSynced() { + // Return an error until we've synced + return nil, fmt.Errorf("validating webhook configuration is not ready") + } + // Remember we're ready + atomic.StoreInt32(&v.ready, 1) } - validatingWebhookConfiguration, ok := configuration.(*v1beta1.ValidatingWebhookConfiguration) - if !ok { - return nil, fmt.Errorf("expected type %v, got type %v", reflect.TypeOf(validatingWebhookConfiguration), reflect.TypeOf(configuration)) - } - return validatingWebhookConfiguration, nil + return v.configuration.Load().(*v1beta1.ValidatingWebhookConfiguration), nil } -func (im *ValidatingWebhookConfigurationManager) Run(stopCh <-chan struct{}) { - im.poller.Run(stopCh) +func (v *ValidatingWebhookConfigurationManager) updateConfiguration() { + configurations, err := v.lister.List(labels.Everything()) + if err != nil { + utilruntime.HandleError(fmt.Errorf("error updating configuration: %v", err)) + return + } + v.configuration.Store(mergeValidatingWebhookConfigurations(configurations)) } func mergeValidatingWebhookConfigurations( - list *v1beta1.ValidatingWebhookConfigurationList, + configurations []*v1beta1.ValidatingWebhookConfiguration, ) *v1beta1.ValidatingWebhookConfiguration { - configurations := list.Items + sort.SliceStable(configurations, ValidatingWebhookConfigurationSorter(configurations).ByName) var ret v1beta1.ValidatingWebhookConfiguration for _, c := range configurations { ret.Webhooks = append(ret.Webhooks, c.Webhooks...) } return &ret } + +type ValidatingWebhookConfigurationSorter []*v1beta1.ValidatingWebhookConfiguration + +func (a ValidatingWebhookConfigurationSorter) ByName(i, j int) bool { + return a[i].Name < a[j].Name +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go index 60ba5367325..929d7b2cfcf 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go @@ -17,24 +17,118 @@ limitations under the License. package configuration import ( + "fmt" + "reflect" "testing" + "time" "k8s.io/api/admissionregistration/v1beta1" - "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/labels" + admissionregistrationlisters "k8s.io/client-go/listers/admissionregistration/v1beta1" + "k8s.io/client-go/tools/cache" ) -type disabledValidatingWebhookConfigLister struct{} - -func (l *disabledValidatingWebhookConfigLister) List(options metav1.ListOptions) (*v1beta1.ValidatingWebhookConfigurationList, error) { - return nil, errors.NewNotFound(schema.GroupResource{Group: "admissionregistration", Resource: "ValidatingWebhookConfigurations"}, "") +type fakeValidatingWebhookConfigSharedInformer struct { + informer *fakeValidatingWebhookConfigInformer + lister *fakeValidatingWebhookConfigLister } -func TestWebhookConfigDisabled(t *testing.T) { - manager := NewValidatingWebhookConfigurationManager(&disabledValidatingWebhookConfigLister{}) - manager.sync() - _, err := manager.Webhooks() - if err.Error() != ErrDisabled.Error() { - t.Errorf("expected %v, got %v", ErrDisabled, err) + +func (f *fakeValidatingWebhookConfigSharedInformer) Informer() cache.SharedIndexInformer { + return f.informer +} +func (f *fakeValidatingWebhookConfigSharedInformer) Lister() admissionregistrationlisters.ValidatingWebhookConfigurationLister { + return f.lister +} + +type fakeValidatingWebhookConfigInformer struct { + eventHandler cache.ResourceEventHandler + hasSynced bool +} + +func (f *fakeValidatingWebhookConfigInformer) AddEventHandler(handler cache.ResourceEventHandler) { + fmt.Println("added handler") + f.eventHandler = handler +} +func (f *fakeValidatingWebhookConfigInformer) AddEventHandlerWithResyncPeriod(handler cache.ResourceEventHandler, resyncPeriod time.Duration) { + panic("unsupported") +} +func (f *fakeValidatingWebhookConfigInformer) GetStore() cache.Store { + panic("unsupported") +} +func (f *fakeValidatingWebhookConfigInformer) GetController() cache.Controller { + panic("unsupported") +} +func (f *fakeValidatingWebhookConfigInformer) Run(stopCh <-chan struct{}) { + panic("unsupported") +} +func (f *fakeValidatingWebhookConfigInformer) HasSynced() bool { + return f.hasSynced +} +func (f *fakeValidatingWebhookConfigInformer) LastSyncResourceVersion() string { + panic("unsupported") +} +func (f *fakeValidatingWebhookConfigInformer) AddIndexers(indexers cache.Indexers) error { + panic("unsupported") +} +func (f *fakeValidatingWebhookConfigInformer) GetIndexer() cache.Indexer { panic("unsupported") } + +type fakeValidatingWebhookConfigLister struct { + list []*v1beta1.ValidatingWebhookConfiguration + err error +} + +func (f *fakeValidatingWebhookConfigLister) List(selector labels.Selector) (ret []*v1beta1.ValidatingWebhookConfiguration, err error) { + return f.list, f.err +} + +func (f *fakeValidatingWebhookConfigLister) Get(name string) (*v1beta1.ValidatingWebhookConfiguration, error) { + panic("unsupported") +} + +func TestGettValidatingWebhookConfig(t *testing.T) { + informer := &fakeValidatingWebhookConfigSharedInformer{ + informer: &fakeValidatingWebhookConfigInformer{}, + lister: &fakeValidatingWebhookConfigLister{}, + } + + // unsynced, error retrieving list + informer.informer.hasSynced = false + informer.lister.list = nil + informer.lister.err = fmt.Errorf("validating webhook configuration is not ready") + manager := NewValidatingWebhookConfigurationManager(informer) + if _, err := manager.Webhooks(); err == nil { + t.Errorf("expected err, but got none") + } + + // list found, still unsynced + informer.informer.hasSynced = false + informer.lister.list = []*v1beta1.ValidatingWebhookConfiguration{} + informer.lister.err = nil + if _, err := manager.Webhooks(); err == nil { + t.Errorf("expected err, but got none") + } + + // items populated, still unsynced + webhookContainer := &v1beta1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "webhook1"}, + Webhooks: []v1beta1.Webhook{{Name: "webhook1.1"}}, + } + informer.informer.hasSynced = false + informer.lister.list = []*v1beta1.ValidatingWebhookConfiguration{webhookContainer.DeepCopy()} + informer.lister.err = nil + informer.informer.eventHandler.OnAdd(webhookContainer.DeepCopy()) + if _, err := manager.Webhooks(); err == nil { + t.Errorf("expected err, but got none") + } + + // sync completed + informer.informer.hasSynced = true + hooks, err := manager.Webhooks() + if err != nil { + t.Errorf("unexpected err: %v", err) + } + if !reflect.DeepEqual(hooks.Webhooks, webhookContainer.Webhooks) { + t.Errorf("Expected\n%#v\ngot\n%#v", webhookContainer.Webhooks, hooks.Webhooks) } } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go index 6d62a36f629..0f50edf6a3e 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go @@ -35,7 +35,6 @@ import ( "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/runtime/serializer/json" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/configuration" genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer" @@ -69,7 +68,6 @@ func Register(plugins *admission.Plugins) { // WebhookSource can list dynamic webhook plugins. type WebhookSource interface { - Run(stopCh <-chan struct{}) Webhooks() (*v1beta1.MutatingWebhookConfiguration, error) } @@ -146,7 +144,6 @@ func (a *MutatingWebhook) SetScheme(scheme *runtime.Scheme) { // WantsExternalKubeClientSet defines a function which sets external ClientSet for admission plugins that need it func (a *MutatingWebhook) SetExternalKubeClientSet(client clientset.Interface) { a.namespaceMatcher.Client = client - a.hookSource = configuration.NewMutatingWebhookConfigurationManager(client.AdmissionregistrationV1beta1().MutatingWebhookConfigurations()) } // SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface. @@ -154,6 +151,7 @@ func (a *MutatingWebhook) SetExternalKubeInformerFactory(f informers.SharedInfor namespaceInformer := f.Core().V1().Namespaces() a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister() a.SetReadyFunc(namespaceInformer.Informer().HasSynced) + a.hookSource = configuration.NewMutatingWebhookConfigurationManager(f.Admissionregistration().V1beta1().MutatingWebhookConfigurations()) } // ValidateInitialization implements the InitializationValidator interface. @@ -176,16 +174,11 @@ func (a *MutatingWebhook) ValidateInitialization() error { if a.defaulter == nil { return fmt.Errorf("MutatingWebhook.defaulter is not properly setup") } - go a.hookSource.Run(wait.NeverStop) return nil } func (a *MutatingWebhook) loadConfiguration(attr admission.Attributes) (*v1beta1.MutatingWebhookConfiguration, error) { hookConfig, err := a.hookSource.Webhooks() - // if Webhook configuration is disabled, fail open - if err == configuration.ErrDisabled { - return &v1beta1.MutatingWebhookConfiguration{}, nil - } if err != nil { e := apierrors.NewServerTimeout(attr.GetResource().GroupResource(), string(attr.GetOperation()), 1) e.ErrStatus.Message = fmt.Sprintf("Unable to refresh the Webhook configuration: %v", err) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go index f68e46fa585..f14d65083de 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" utilruntime "k8s.io/apimachinery/pkg/util/runtime" - "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/admission" "k8s.io/apiserver/pkg/admission/configuration" genericadmissioninit "k8s.io/apiserver/pkg/admission/initializer" @@ -68,7 +67,6 @@ func Register(plugins *admission.Plugins) { // WebhookSource can list dynamic webhook plugins. type WebhookSource interface { - Run(stopCh <-chan struct{}) Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error) } @@ -141,7 +139,6 @@ func (a *ValidatingAdmissionWebhook) SetScheme(scheme *runtime.Scheme) { // WantsExternalKubeClientSet defines a function which sets external ClientSet for admission plugins that need it func (a *ValidatingAdmissionWebhook) SetExternalKubeClientSet(client clientset.Interface) { a.namespaceMatcher.Client = client - a.hookSource = configuration.NewValidatingWebhookConfigurationManager(client.AdmissionregistrationV1beta1().ValidatingWebhookConfigurations()) } // SetExternalKubeInformerFactory implements the WantsExternalKubeInformerFactory interface. @@ -149,12 +146,13 @@ func (a *ValidatingAdmissionWebhook) SetExternalKubeInformerFactory(f informers. namespaceInformer := f.Core().V1().Namespaces() a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister() a.SetReadyFunc(namespaceInformer.Informer().HasSynced) + a.hookSource = configuration.NewValidatingWebhookConfigurationManager(f.Admissionregistration().V1beta1().ValidatingWebhookConfigurations()) } // ValidateInitialization implements the InitializationValidator interface. func (a *ValidatingAdmissionWebhook) ValidateInitialization() error { if a.hookSource == nil { - return fmt.Errorf("ValidatingAdmissionWebhook admission plugin requires a Kubernetes client to be provided") + return fmt.Errorf("ValidatingAdmissionWebhook admission plugin requires a Kubernetes informer to be provided") } if err := a.namespaceMatcher.Validate(); err != nil { return fmt.Errorf("ValidatingAdmissionWebhook.namespaceMatcher is not properly setup: %v", err) @@ -165,16 +163,11 @@ func (a *ValidatingAdmissionWebhook) ValidateInitialization() error { if err := a.convertor.Validate(); err != nil { return fmt.Errorf("ValidatingAdmissionWebhook.convertor is not properly setup: %v", err) } - go a.hookSource.Run(wait.NeverStop) return nil } func (a *ValidatingAdmissionWebhook) loadConfiguration(attr admission.Attributes) (*v1beta1.ValidatingWebhookConfiguration, error) { hookConfig, err := a.hookSource.Webhooks() - // if Webhook configuration is disabled, fail open - if err == configuration.ErrDisabled { - return &v1beta1.ValidatingWebhookConfiguration{}, nil - } if err != nil { e := apierrors.NewServerTimeout(attr.GetResource().GroupResource(), string(attr.GetOperation()), 1) e.ErrStatus.Message = fmt.Sprintf("Unable to refresh the Webhook configuration: %v", err) From ec3925978511cc6b844c5b479c9b30ae21a0136a Mon Sep 17 00:00:00 2001 From: hzxuzhonghu Date: Wed, 6 Dec 2017 11:06:04 +0800 Subject: [PATCH 2/3] add wait ready for mutating/validating webhook configuration --- .../configuration/mutating_webhook_manager.go | 16 +----- .../mutating_webhook_manager_test.go | 49 +++++++------------ .../validating_webhook_manager.go | 16 +----- .../validating_webhook_manager_test.go | 49 +++++++------------ .../admission/plugin/webhook/mutating/BUILD | 1 - .../plugin/webhook/mutating/admission.go | 33 +++++-------- .../plugin/webhook/mutating/admission_test.go | 6 +-- .../admission/plugin/webhook/validating/BUILD | 1 - .../plugin/webhook/validating/admission.go | 31 +++++------- .../webhook/validating/admission_test.go | 6 +-- 10 files changed, 72 insertions(+), 136 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go index fbde83b7d82..3c0990699a4 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go @@ -31,17 +31,13 @@ import ( // MutatingWebhookConfigurationManager collects the mutating webhook objects so that they can be called. type MutatingWebhookConfigurationManager struct { - ready int32 configuration *atomic.Value - hasSynced func() bool lister admissionregistrationlisters.MutatingWebhookConfigurationLister } func NewMutatingWebhookConfigurationManager(informer admissionregistrationinformers.MutatingWebhookConfigurationInformer) *MutatingWebhookConfigurationManager { manager := &MutatingWebhookConfigurationManager{ - ready: 0, configuration: &atomic.Value{}, - hasSynced: informer.Informer().HasSynced, lister: informer.Lister(), } @@ -59,16 +55,8 @@ func NewMutatingWebhookConfigurationManager(informer admissionregistrationinform } // Webhooks returns the merged MutatingWebhookConfiguration. -func (m *MutatingWebhookConfigurationManager) Webhooks() (*v1beta1.MutatingWebhookConfiguration, error) { - if atomic.LoadInt32(&m.ready) == 0 { - if !m.hasSynced() { - // Return an error until we've synced - return nil, fmt.Errorf("mutating webhook configuration is not ready") - } - // Remember we're ready - atomic.StoreInt32(&m.ready, 1) - } - return m.configuration.Load().(*v1beta1.MutatingWebhookConfiguration), nil +func (m *MutatingWebhookConfigurationManager) Webhooks() *v1beta1.MutatingWebhookConfiguration { + return m.configuration.Load().(*v1beta1.MutatingWebhookConfiguration) } func (m *MutatingWebhookConfigurationManager) updateConfiguration() { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go index e6b50aa2320..d6f4f1a4512 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager_test.go @@ -43,7 +43,6 @@ func (f *fakeMutatingWebhookConfigSharedInformer) Lister() admissionregistration type fakeMutatingWebhookConfigInformer struct { eventHandler cache.ResourceEventHandler - hasSynced bool } func (f *fakeMutatingWebhookConfigInformer) AddEventHandler(handler cache.ResourceEventHandler) { @@ -63,7 +62,7 @@ func (f *fakeMutatingWebhookConfigInformer) Run(stopCh <-chan struct{}) { panic("unsupported") } func (f *fakeMutatingWebhookConfigInformer) HasSynced() bool { - return f.hasSynced + panic("unsupported") } func (f *fakeMutatingWebhookConfigInformer) LastSyncResourceVersion() string { panic("unsupported") @@ -92,43 +91,33 @@ func TestGetMutatingWebhookConfig(t *testing.T) { lister: &fakeMutatingWebhookConfigLister{}, } - // unsynced, error retrieving list - informer.informer.hasSynced = false + // no configurations informer.lister.list = nil - informer.lister.err = fmt.Errorf("mutating webhook configuration is not ready") manager := NewMutatingWebhookConfigurationManager(informer) - if _, err := manager.Webhooks(); err == nil { - t.Errorf("expected err, but got none") + if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 { + t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks) } - // list found, still unsynced - informer.informer.hasSynced = false - informer.lister.list = []*v1beta1.MutatingWebhookConfiguration{} - informer.lister.err = nil - if _, err := manager.Webhooks(); err == nil { - t.Errorf("expected err, but got none") - } - - // items populated, still unsynced - webhookContainer := &v1beta1.MutatingWebhookConfiguration{ + // list err + webhookConfiguration := &v1beta1.MutatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{Name: "webhook1"}, Webhooks: []v1beta1.Webhook{{Name: "webhook1.1"}}, } - informer.informer.hasSynced = false - informer.lister.list = []*v1beta1.MutatingWebhookConfiguration{webhookContainer.DeepCopy()} - informer.lister.err = nil - informer.informer.eventHandler.OnAdd(webhookContainer.DeepCopy()) - if _, err := manager.Webhooks(); err == nil { - t.Errorf("expected err, but got none") + informer.lister.list = []*v1beta1.MutatingWebhookConfiguration{webhookConfiguration.DeepCopy()} + informer.lister.err = fmt.Errorf("mutating webhook configuration list error") + informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy()) + if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 { + t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks) } - // sync completed - informer.informer.hasSynced = true - hooks, err := manager.Webhooks() - if err != nil { - t.Errorf("unexpected err: %v", err) + // configuration populated + informer.lister.err = nil + informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy()) + configurations := manager.Webhooks() + if len(configurations.Webhooks) == 0 { + t.Errorf("expected non empty webhooks") } - if !reflect.DeepEqual(hooks.Webhooks, webhookContainer.Webhooks) { - t.Errorf("Expected\n%#v\ngot\n%#v", webhookContainer.Webhooks, hooks.Webhooks) + if !reflect.DeepEqual(configurations.Webhooks, webhookConfiguration.Webhooks) { + t.Errorf("Expected\n%#v\ngot\n%#v", webhookConfiguration.Webhooks, configurations.Webhooks) } } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go index f93068b8037..33644f57fed 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager.go @@ -31,17 +31,13 @@ import ( // ValidatingWebhookConfigurationManager collects the validating webhook objects so that they can be called. type ValidatingWebhookConfigurationManager struct { - ready int32 configuration *atomic.Value - hasSynced func() bool lister admissionregistrationlisters.ValidatingWebhookConfigurationLister } func NewValidatingWebhookConfigurationManager(informer admissionregistrationinformers.ValidatingWebhookConfigurationInformer) *ValidatingWebhookConfigurationManager { manager := &ValidatingWebhookConfigurationManager{ - ready: 0, configuration: &atomic.Value{}, - hasSynced: informer.Informer().HasSynced, lister: informer.Lister(), } @@ -59,16 +55,8 @@ func NewValidatingWebhookConfigurationManager(informer admissionregistrationinfo } // Webhooks returns the merged ValidatingWebhookConfiguration. -func (v *ValidatingWebhookConfigurationManager) Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error) { - if atomic.LoadInt32(&v.ready) == 0 { - if !v.hasSynced() { - // Return an error until we've synced - return nil, fmt.Errorf("validating webhook configuration is not ready") - } - // Remember we're ready - atomic.StoreInt32(&v.ready, 1) - } - return v.configuration.Load().(*v1beta1.ValidatingWebhookConfiguration), nil +func (v *ValidatingWebhookConfigurationManager) Webhooks() *v1beta1.ValidatingWebhookConfiguration { + return v.configuration.Load().(*v1beta1.ValidatingWebhookConfiguration) } func (v *ValidatingWebhookConfigurationManager) updateConfiguration() { diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go index 929d7b2cfcf..6505b2b9b4b 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/validating_webhook_manager_test.go @@ -43,7 +43,6 @@ func (f *fakeValidatingWebhookConfigSharedInformer) Lister() admissionregistrati type fakeValidatingWebhookConfigInformer struct { eventHandler cache.ResourceEventHandler - hasSynced bool } func (f *fakeValidatingWebhookConfigInformer) AddEventHandler(handler cache.ResourceEventHandler) { @@ -63,7 +62,7 @@ func (f *fakeValidatingWebhookConfigInformer) Run(stopCh <-chan struct{}) { panic("unsupported") } func (f *fakeValidatingWebhookConfigInformer) HasSynced() bool { - return f.hasSynced + panic("unsupported") } func (f *fakeValidatingWebhookConfigInformer) LastSyncResourceVersion() string { panic("unsupported") @@ -92,43 +91,33 @@ func TestGettValidatingWebhookConfig(t *testing.T) { lister: &fakeValidatingWebhookConfigLister{}, } - // unsynced, error retrieving list - informer.informer.hasSynced = false + // no configurations informer.lister.list = nil - informer.lister.err = fmt.Errorf("validating webhook configuration is not ready") manager := NewValidatingWebhookConfigurationManager(informer) - if _, err := manager.Webhooks(); err == nil { - t.Errorf("expected err, but got none") + if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 { + t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks) } - // list found, still unsynced - informer.informer.hasSynced = false - informer.lister.list = []*v1beta1.ValidatingWebhookConfiguration{} - informer.lister.err = nil - if _, err := manager.Webhooks(); err == nil { - t.Errorf("expected err, but got none") - } - - // items populated, still unsynced - webhookContainer := &v1beta1.ValidatingWebhookConfiguration{ + // list error + webhookConfiguration := &v1beta1.ValidatingWebhookConfiguration{ ObjectMeta: metav1.ObjectMeta{Name: "webhook1"}, Webhooks: []v1beta1.Webhook{{Name: "webhook1.1"}}, } - informer.informer.hasSynced = false - informer.lister.list = []*v1beta1.ValidatingWebhookConfiguration{webhookContainer.DeepCopy()} - informer.lister.err = nil - informer.informer.eventHandler.OnAdd(webhookContainer.DeepCopy()) - if _, err := manager.Webhooks(); err == nil { - t.Errorf("expected err, but got none") + informer.lister.list = []*v1beta1.ValidatingWebhookConfiguration{webhookConfiguration.DeepCopy()} + informer.lister.err = fmt.Errorf("validating webhook configuration list error") + informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy()) + if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 { + t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks) } - // sync completed - informer.informer.hasSynced = true - hooks, err := manager.Webhooks() - if err != nil { - t.Errorf("unexpected err: %v", err) + // configuration populated + informer.lister.err = nil + informer.informer.eventHandler.OnAdd(webhookConfiguration.DeepCopy()) + configurations := manager.Webhooks() + if len(configurations.Webhooks) == 0 { + t.Errorf("expected non empty webhooks") } - if !reflect.DeepEqual(hooks.Webhooks, webhookContainer.Webhooks) { - t.Errorf("Expected\n%#v\ngot\n%#v", webhookContainer.Webhooks, hooks.Webhooks) + if !reflect.DeepEqual(configurations.Webhooks, webhookConfiguration.Webhooks) { + t.Errorf("Expected\n%#v\ngot\n%#v", webhookConfiguration.Webhooks, configurations.Webhooks) } } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD index 0d46b5d7627..6ed322b2500 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD @@ -14,7 +14,6 @@ go_library( "//vendor/k8s.io/api/admission/v1beta1:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go index 0f50edf6a3e..73e213a6f10 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go @@ -30,7 +30,6 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/runtime/serializer/json" @@ -68,7 +67,7 @@ func Register(plugins *admission.Plugins) { // WebhookSource can list dynamic webhook plugins. type WebhookSource interface { - Webhooks() (*v1beta1.MutatingWebhookConfiguration, error) + Webhooks() *v1beta1.MutatingWebhookConfiguration } // NewMutatingWebhook returns a generic admission webhook plugin. @@ -150,8 +149,11 @@ func (a *MutatingWebhook) SetExternalKubeClientSet(client clientset.Interface) { func (a *MutatingWebhook) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { namespaceInformer := f.Core().V1().Namespaces() a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister() - a.SetReadyFunc(namespaceInformer.Informer().HasSynced) - a.hookSource = configuration.NewMutatingWebhookConfigurationManager(f.Admissionregistration().V1beta1().MutatingWebhookConfigurations()) + mutatingWebhookConfigurationsInformer := f.Admissionregistration().V1beta1().MutatingWebhookConfigurations() + a.hookSource = configuration.NewMutatingWebhookConfigurationManager(mutatingWebhookConfigurationsInformer) + a.SetReadyFunc(func() bool { + return namespaceInformer.Informer().HasSynced() && mutatingWebhookConfigurationsInformer.Informer().HasSynced() + }) } // ValidateInitialization implements the InitializationValidator interface. @@ -177,27 +179,18 @@ func (a *MutatingWebhook) ValidateInitialization() error { return nil } -func (a *MutatingWebhook) loadConfiguration(attr admission.Attributes) (*v1beta1.MutatingWebhookConfiguration, error) { - hookConfig, err := a.hookSource.Webhooks() - if err != nil { - e := apierrors.NewServerTimeout(attr.GetResource().GroupResource(), string(attr.GetOperation()), 1) - e.ErrStatus.Message = fmt.Sprintf("Unable to refresh the Webhook configuration: %v", err) - e.ErrStatus.Reason = "LoadingConfiguration" - e.ErrStatus.Details.Causes = append(e.ErrStatus.Details.Causes, metav1.StatusCause{ - Type: "MutatingWebhookConfigurationFailure", - Message: "An error has occurred while refreshing the MutatingWebhook configuration, no resources can be created/updated/deleted/connected until a refresh succeeds.", - }) - return nil, e - } - return hookConfig, nil +func (a *MutatingWebhook) loadConfiguration(attr admission.Attributes) *v1beta1.MutatingWebhookConfiguration { + hookConfig := a.hookSource.Webhooks() + return hookConfig } // Admit makes an admission decision based on the request attributes. func (a *MutatingWebhook) Admit(attr admission.Attributes) error { - hookConfig, err := a.loadConfiguration(attr) - if err != nil { - return err + if !a.WaitForReady() { + return admission.NewForbidden(attr, fmt.Errorf("not yet ready to handle request")) } + + hookConfig := a.loadConfiguration(attr) hooks := hookConfig.Webhooks ctx := context.TODO() diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission_test.go index 9f92fad1126..523e03762d0 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission_test.go @@ -47,16 +47,16 @@ type fakeHookSource struct { err error } -func (f *fakeHookSource) Webhooks() (*registrationv1beta1.MutatingWebhookConfiguration, error) { +func (f *fakeHookSource) Webhooks() *registrationv1beta1.MutatingWebhookConfiguration { if f.err != nil { - return nil, f.err + return nil } for i, h := range f.hooks { if h.NamespaceSelector == nil { f.hooks[i].NamespaceSelector = &metav1.LabelSelector{} } } - return ®istrationv1beta1.MutatingWebhookConfiguration{Webhooks: f.hooks}, nil + return ®istrationv1beta1.MutatingWebhookConfiguration{Webhooks: f.hooks} } func (f *fakeHookSource) Run(stopCh <-chan struct{}) {} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD index 4226a13912c..a3b6a155937 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD @@ -13,7 +13,6 @@ go_library( "//vendor/k8s.io/api/admission/v1beta1:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go index f14d65083de..8e8c7448fcb 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go @@ -30,7 +30,6 @@ import ( admissionv1beta1 "k8s.io/api/admission/v1beta1" "k8s.io/api/admissionregistration/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -67,7 +66,7 @@ func Register(plugins *admission.Plugins) { // WebhookSource can list dynamic webhook plugins. type WebhookSource interface { - Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error) + Webhooks() *v1beta1.ValidatingWebhookConfiguration } // NewValidatingAdmissionWebhook returns a generic admission webhook plugin. @@ -145,8 +144,11 @@ func (a *ValidatingAdmissionWebhook) SetExternalKubeClientSet(client clientset.I func (a *ValidatingAdmissionWebhook) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { namespaceInformer := f.Core().V1().Namespaces() a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister() - a.SetReadyFunc(namespaceInformer.Informer().HasSynced) - a.hookSource = configuration.NewValidatingWebhookConfigurationManager(f.Admissionregistration().V1beta1().ValidatingWebhookConfigurations()) + validatingWebhookConfigurationsInformer := f.Admissionregistration().V1beta1().ValidatingWebhookConfigurations() + a.hookSource = configuration.NewValidatingWebhookConfigurationManager(validatingWebhookConfigurationsInformer) + a.SetReadyFunc(func() bool { + return namespaceInformer.Informer().HasSynced() && validatingWebhookConfigurationsInformer.Informer().HasSynced() + }) } // ValidateInitialization implements the InitializationValidator interface. @@ -166,27 +168,16 @@ func (a *ValidatingAdmissionWebhook) ValidateInitialization() error { return nil } -func (a *ValidatingAdmissionWebhook) loadConfiguration(attr admission.Attributes) (*v1beta1.ValidatingWebhookConfiguration, error) { - hookConfig, err := a.hookSource.Webhooks() - if err != nil { - e := apierrors.NewServerTimeout(attr.GetResource().GroupResource(), string(attr.GetOperation()), 1) - e.ErrStatus.Message = fmt.Sprintf("Unable to refresh the Webhook configuration: %v", err) - e.ErrStatus.Reason = "LoadingConfiguration" - e.ErrStatus.Details.Causes = append(e.ErrStatus.Details.Causes, metav1.StatusCause{ - Type: "ValidatingWebhookConfigurationFailure", - Message: "An error has occurred while refreshing the ValidatingWebhook configuration, no resources can be created/updated/deleted/connected until a refresh succeeds.", - }) - return nil, e - } - return hookConfig, nil +func (a *ValidatingAdmissionWebhook) loadConfiguration(attr admission.Attributes) *v1beta1.ValidatingWebhookConfiguration { + return a.hookSource.Webhooks() } // Validate makes an admission decision based on the request attributes. func (a *ValidatingAdmissionWebhook) Validate(attr admission.Attributes) error { - hookConfig, err := a.loadConfiguration(attr) - if err != nil { - return err + if !a.WaitForReady() { + return admission.NewForbidden(attr, fmt.Errorf("not yet ready to handle request")) } + hookConfig := a.loadConfiguration(attr) hooks := hookConfig.Webhooks ctx := context.TODO() diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission_test.go index 9a190f41239..77871f01942 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission_test.go @@ -47,16 +47,16 @@ type fakeHookSource struct { err error } -func (f *fakeHookSource) Webhooks() (*registrationv1beta1.ValidatingWebhookConfiguration, error) { +func (f *fakeHookSource) Webhooks() *registrationv1beta1.ValidatingWebhookConfiguration { if f.err != nil { - return nil, f.err + return nil } for i, h := range f.hooks { if h.NamespaceSelector == nil { f.hooks[i].NamespaceSelector = &metav1.LabelSelector{} } } - return ®istrationv1beta1.ValidatingWebhookConfiguration{Webhooks: f.hooks}, nil + return ®istrationv1beta1.ValidatingWebhookConfiguration{Webhooks: f.hooks} } func (f *fakeHookSource) Run(stopCh <-chan struct{}) {} From ea7a71301009fb3e0426ea93f070c27538e59f86 Mon Sep 17 00:00:00 2001 From: hzxuzhonghu Date: Wed, 29 Nov 2017 23:28:53 +0800 Subject: [PATCH 3/3] run update bazel staging-dep --- staging/src/k8s.io/apiserver/Godeps/Godeps.json | 8 ++++++++ .../k8s.io/apiserver/pkg/admission/configuration/BUILD | 8 ++++++++ .../apiserver/pkg/admission/plugin/webhook/mutating/BUILD | 1 - .../pkg/admission/plugin/webhook/validating/BUILD | 1 - 4 files changed, 16 insertions(+), 2 deletions(-) diff --git a/staging/src/k8s.io/apiserver/Godeps/Godeps.json b/staging/src/k8s.io/apiserver/Godeps/Godeps.json index 18797d5b362..df8ae10e9f3 100644 --- a/staging/src/k8s.io/apiserver/Godeps/Godeps.json +++ b/staging/src/k8s.io/apiserver/Godeps/Godeps.json @@ -1790,6 +1790,10 @@ "ImportPath": "k8s.io/client-go/informers", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, + { + "ImportPath": "k8s.io/client-go/informers/admissionregistration/v1beta1", + "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }, { "ImportPath": "k8s.io/client-go/kubernetes", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" @@ -1814,6 +1818,10 @@ "ImportPath": "k8s.io/client-go/kubernetes/typed/core/v1", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" }, + { + "ImportPath": "k8s.io/client-go/listers/admissionregistration/v1beta1", + "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + }, { "ImportPath": "k8s.io/client-go/listers/core/v1", "Rev": "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" diff --git a/staging/src/k8s.io/apiserver/pkg/admission/configuration/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/configuration/BUILD index c892344c3e7..13a7afbd781 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/configuration/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/configuration/BUILD @@ -21,9 +21,12 @@ go_test( "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//vendor/k8s.io/client-go/listers/admissionregistration/v1beta1:go_default_library", + "//vendor/k8s.io/client-go/tools/cache:go_default_library", ], ) @@ -42,8 +45,13 @@ go_library( "//vendor/k8s.io/api/admissionregistration/v1beta1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", + "//vendor/k8s.io/client-go/informers/admissionregistration/v1beta1:go_default_library", + "//vendor/k8s.io/client-go/listers/admissionregistration/v1beta1:go_default_library", + "//vendor/k8s.io/client-go/tools/cache:go_default_library", ], ) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD index 6ed322b2500..85bc608d792 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/BUILD @@ -18,7 +18,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library", diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD index a3b6a155937..e4bf9ced2d6 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/BUILD @@ -16,7 +16,6 @@ go_library( "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/runtime:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/configuration:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission/initializer:go_default_library",