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/configuration/mutating_webhook_manager.go b/staging/src/k8s.io/apiserver/pkg/admission/configuration/mutating_webhook_manager.go index bf4d0eabf98..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 @@ -18,84 +18,70 @@ 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 + configuration *atomic.Value + 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{ + configuration: &atomic.Value{}, + 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 { + return m.configuration.Load().(*v1beta1.MutatingWebhookConfiguration) +} + +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..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 @@ -17,24 +17,107 @@ 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 +} + +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 { + panic("unsupported") +} +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{}, + } + + // no configurations + informer.lister.list = nil + manager := NewMutatingWebhookConfigurationManager(informer) + if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 { + t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks) + } + + // list err + webhookConfiguration := &v1beta1.MutatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "webhook1"}, + Webhooks: []v1beta1.Webhook{{Name: "webhook1.1"}}, + } + 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) + } + + // 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(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 8f9fd34daae..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 @@ -18,67 +18,69 @@ 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 + configuration *atomic.Value + 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{ + configuration: &atomic.Value{}, + 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 - } - 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 +func (v *ValidatingWebhookConfigurationManager) Webhooks() *v1beta1.ValidatingWebhookConfiguration { + return v.configuration.Load().(*v1beta1.ValidatingWebhookConfiguration) } -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..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 @@ -17,24 +17,107 @@ 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 +} + +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 { + panic("unsupported") +} +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{}, + } + + // no configurations + informer.lister.list = nil + manager := NewValidatingWebhookConfigurationManager(informer) + if configurations := manager.Webhooks(); len(configurations.Webhooks) != 0 { + t.Errorf("expected empty webhooks, but got %v", configurations.Webhooks) + } + + // list error + webhookConfiguration := &v1beta1.ValidatingWebhookConfiguration{ + ObjectMeta: metav1.ObjectMeta{Name: "webhook1"}, + Webhooks: []v1beta1.Webhook{{Name: "webhook1.1"}}, + } + 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) + } + + // 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(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..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 @@ -14,12 +14,10 @@ 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", "//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/mutating/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/mutating/admission.go index 6d62a36f629..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,12 +30,10 @@ 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" 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,8 +67,7 @@ func Register(plugins *admission.Plugins) { // WebhookSource can list dynamic webhook plugins. type WebhookSource interface { - Run(stopCh <-chan struct{}) - Webhooks() (*v1beta1.MutatingWebhookConfiguration, error) + Webhooks() *v1beta1.MutatingWebhookConfiguration } // NewMutatingWebhook returns a generic admission webhook plugin. @@ -146,14 +143,17 @@ 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. func (a *MutatingWebhook) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { namespaceInformer := f.Core().V1().Namespaces() a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister() - a.SetReadyFunc(namespaceInformer.Informer().HasSynced) + 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. @@ -176,35 +176,21 @@ 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) - 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..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 @@ -13,11 +13,9 @@ 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", - "//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/admission.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/validating/admission.go index f68e46fa585..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,11 +30,9 @@ 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" - "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,8 +66,7 @@ func Register(plugins *admission.Plugins) { // WebhookSource can list dynamic webhook plugins. type WebhookSource interface { - Run(stopCh <-chan struct{}) - Webhooks() (*v1beta1.ValidatingWebhookConfiguration, error) + Webhooks() *v1beta1.ValidatingWebhookConfiguration } // NewValidatingAdmissionWebhook returns a generic admission webhook plugin. @@ -141,20 +138,23 @@ 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. func (a *ValidatingAdmissionWebhook) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) { namespaceInformer := f.Core().V1().Namespaces() a.namespaceMatcher.NamespaceLister = namespaceInformer.Lister() - a.SetReadyFunc(namespaceInformer.Informer().HasSynced) + 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. 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,35 +165,19 @@ 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) - 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{}) {}