Implement dynamic admission webhooks

Also fix a bug in rest client

cmd/kube-apiserver/app/options/options.go

@@ -214,10 +214,18 @@ func (s *ServerRunOptions) AddFlags(fs *pflag.FlagSet) {
 		"e.g., setting empty UID in update request to its existing value. This flag can be turned off "+
 		"after we fix all the clients that send malformed updates.")
 
-	fs.StringVar(&s.ProxyClientCertFile, "proxy-client-cert-file", s.ProxyClientCertFile,
+	fs.StringVar(&s.ProxyClientCertFile, "proxy-client-cert-file", s.ProxyClientCertFile, ""+
-		"client certificate used to prove the identity of the aggragator or kube-apiserver when it proxies requests to a user api-server")
+		"Client certificate used to prove the identity of the aggregator or kube-apiserver "+
-	fs.StringVar(&s.ProxyClientKeyFile, "proxy-client-key-file", s.ProxyClientKeyFile,
+		"when it must call out during a request. This includes proxying requests to a user "+
-		"client certificate key used to prove the identity of the aggragator or kube-apiserver when it proxies requests to a user api-server")
+		"api-server and calling out to webhook admission plugins. It is expected that this "+
+		"cert includes a signature from the CA in the --requestheader-client-ca-file flag. "+
+		"That CA is published in the 'extension-apiserver-authentication' configmap in "+
+		"the kube-system namespace. Components recieving calls from kube-aggregator should "+
+		"use that CA to perform their half of the mutual TLS verification.")
+	fs.StringVar(&s.ProxyClientKeyFile, "proxy-client-key-file", s.ProxyClientKeyFile, ""+
+		"Private key for the client certificate used to prove the identity of the aggregator or kube-apiserver "+
+		"when it must call out during a request. This includes proxying requests to a user "+
+		"api-server and calling out to webhook admission plugins.")
 
 	fs.BoolVar(&s.EnableAggregatorRouting, "enable-aggregator-routing", s.EnableAggregatorRouting,
 		"Turns on aggregator routing requests to endoints IP rather than cluster IP.")

cmd/kube-apiserver/app/server.go

@@ -424,6 +424,20 @@ func BuildAdmissionPluginInitializer(s *options.ServerRunOptions, client interna
 	quotaRegistry := quotainstall.NewRegistry(nil, nil)
 
 	pluginInitializer := kubeapiserveradmission.NewPluginInitializer(client, sharedInformers, apiAuthorizer, cloudConfig, restMapper, quotaRegistry)
+
+	// Read client cert/key for plugins that need to make calls out
+	if len(s.ProxyClientCertFile) > 0 && len(s.ProxyClientKeyFile) > 0 {
+		certBytes, err := ioutil.ReadFile(s.ProxyClientCertFile)
+		if err != nil {
+			return nil, err
+		}
+		keyBytes, err := ioutil.ReadFile(s.ProxyClientKeyFile)
+		if err != nil {
+			return nil, err
+		}
+		pluginInitializer = pluginInitializer.SetClientCert(certBytes, keyBytes)
+	}
+
 	return pluginInitializer, nil
 }
 

hack/.linted_packages

@@ -68,6 +68,7 @@ pkg/api/v1/node
 pkg/api/v1/service
 pkg/apis/abac/v0
 pkg/apis/abac/v1beta1
+pkg/apis/admission/install
 pkg/apis/admissionregistration/install
 pkg/apis/apps/install
 pkg/apis/apps/v1beta1

pkg/apis/admission/v1alpha1/types.go

@@ -30,7 +30,7 @@ type AdmissionReview struct {
 	// Since this admission controller is non-mutating the webhook should avoid setting this in its response to avoid the
 	// cost of deserializing it.
 	// +optional
-	Spec AdmissionReviewSpec `json:"spec" protobuf:"bytes,1,opt,name=spec"`
+	Spec AdmissionReviewSpec `json:"spec,omitempty" protobuf:"bytes,1,opt,name=spec"`
 	// Status is filled in by the webhook and indicates whether the admission request should be permitted.
 	// +optional
 	Status AdmissionReviewStatus `json:"status,omitempty" protobuf:"bytes,2,opt,name=status"`

pkg/kubeapiserver/admission/init_test.go

@@ -17,10 +17,12 @@ limitations under the License.
 package admission
 
 import (
+	"net/url"
 	"testing"
 
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
+	"k8s.io/kubernetes/pkg/apis/admissionregistration"
 )
 
 // TestAuthorizer is a testing struct for testing that fulfills the authorizer interface.
@@ -32,18 +34,22 @@ func (t *TestAuthorizer) Authorize(a authorizer.Attributes) (authorized bool, re
 
 var _ authorizer.Authorizer = &TestAuthorizer{}
 
+type doNothingAdmission struct{}
+
+func (doNothingAdmission) Admit(a admission.Attributes) error { return nil }
+func (doNothingAdmission) Handles(o admission.Operation) bool { return false }
+func (doNothingAdmission) Validate() error                    { return nil }
+
 // WantAuthorizerAdmission is a testing struct that fulfills the WantsAuthorizer
 // interface.
 type WantAuthorizerAdmission struct {
+	doNothingAdmission
 	auth authorizer.Authorizer
 }
 
 func (self *WantAuthorizerAdmission) SetAuthorizer(a authorizer.Authorizer) {
 	self.auth = a
 }
-func (self *WantAuthorizerAdmission) Admit(a admission.Attributes) error { return nil }
-func (self *WantAuthorizerAdmission) Handles(o admission.Operation) bool { return false }
-func (self *WantAuthorizerAdmission) Validate() error                    { return nil }
 
 var _ admission.Interface = &WantAuthorizerAdmission{}
 var _ WantsAuthorizer = &WantAuthorizerAdmission{}
@@ -60,6 +66,7 @@ func TestWantsAuthorizer(t *testing.T) {
 }
 
 type WantsCloudConfigAdmissionPlugin struct {
+	doNothingAdmission
 	cloudConfig []byte
 }
 
@@ -67,10 +74,6 @@ func (self *WantsCloudConfigAdmissionPlugin) SetCloudConfig(cloudConfig []byte)
 	self.cloudConfig = cloudConfig
 }
 
-func (self *WantsCloudConfigAdmissionPlugin) Admit(a admission.Attributes) error { return nil }
-func (self *WantsCloudConfigAdmissionPlugin) Handles(o admission.Operation) bool { return false }
-func (self *WantsCloudConfigAdmissionPlugin) Validate() error                    { return nil }
-
 func TestCloudConfigAdmissionPlugin(t *testing.T) {
 	cloudConfig := []byte("cloud-configuration")
 	initializer := NewPluginInitializer(nil, nil, &TestAuthorizer{}, cloudConfig, nil, nil)
@@ -81,3 +84,65 @@ func TestCloudConfigAdmissionPlugin(t *testing.T) {
 		t.Errorf("Expected cloud config to be initialized but found nil")
 	}
 }
+
+type fakeServiceResolver struct{}
+
+func (f *fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) {
+	return nil, nil
+}
+
+type serviceWanter struct {
+	doNothingAdmission
+	got ServiceResolver
+}
+
+func (s *serviceWanter) SetServiceResolver(sr ServiceResolver) { s.got = sr }
+
+func TestWantsServiceResolver(t *testing.T) {
+	sw := &serviceWanter{}
+	fsr := &fakeServiceResolver{}
+	i := &PluginInitializer{}
+	i.SetServiceResolver(fsr).Initialize(sw)
+	if got, ok := sw.got.(*fakeServiceResolver); !ok || got != fsr {
+		t.Errorf("plumbing fail - %v %v#", ok, got)
+	}
+}
+
+type clientCertWanter struct {
+	doNothingAdmission
+	gotCert, gotKey []byte
+}
+
+func (s *clientCertWanter) SetClientCert(cert, key []byte) { s.gotCert, s.gotKey = cert, key }
+
+func TestWantsClientCert(t *testing.T) {
+	i := &PluginInitializer{}
+	ccw := &clientCertWanter{}
+	i.SetClientCert([]byte("cert"), []byte("key")).Initialize(ccw)
+	if string(ccw.gotCert) != "cert" || string(ccw.gotKey) != "key" {
+		t.Errorf("plumbing fail - %v %v", ccw.gotCert, ccw.gotKey)
+	}
+}
+
+type fakeHookSource struct{}
+
+func (f *fakeHookSource) List() ([]admissionregistration.ExternalAdmissionHook, error) {
+	return nil, nil
+}
+
+type hookSourceWanter struct {
+	doNothingAdmission
+	got WebhookSource
+}
+
+func (s *hookSourceWanter) SetWebhookSource(w WebhookSource) { s.got = w }
+
+func TestWantsWebhookSource(t *testing.T) {
+	hsw := &hookSourceWanter{}
+	fhs := &fakeHookSource{}
+	i := &PluginInitializer{}
+	i.SetWebhookSource(fhs).Initialize(hsw)
+	if got, ok := hsw.got.(*fakeHookSource); !ok || got != fhs {
+		t.Errorf("plumbing fail - %v %v#", ok, got)
+	}
+}

pkg/kubeapiserver/admission/initializer.go

@@ -17,9 +17,12 @@ limitations under the License.
 package admission
 
 import (
+	"net/url"
+
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authorization/authorizer"
+	"k8s.io/kubernetes/pkg/apis/admissionregistration"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
 	"k8s.io/kubernetes/pkg/quota"
@@ -61,25 +64,62 @@ type WantsQuotaRegistry interface {
 	admission.Validator
 }
 
-type pluginInitializer struct {
+// WantsServiceResolver defines a fuction that accepts a ServiceResolver for
+// admission plugins that need to make calls to services.
+type WantsServiceResolver interface {
+	SetServiceResolver(ServiceResolver)
+}
+
+// WantsClientCert defines a fuction that accepts a cert & key for admission
+// plugins that need to make calls and prove their identity.
+type WantsClientCert interface {
+	SetClientCert(cert, key []byte)
+}
+
+// WantsWebhookSource defines a function that accepts a webhook lister for the
+// dynamic webhook plugin.
+type WantsWebhookSource interface {
+	SetWebhookSource(WebhookSource)
+}
+
+// ServiceResolver knows how to convert a service reference into an actual
+// location.
+type ServiceResolver interface {
+	ResolveEndpoint(namespace, name string) (*url.URL, error)
+}
+
+// WebhookSource can list dynamic webhook plugins.
+type WebhookSource interface {
+	List() ([]admissionregistration.ExternalAdmissionHook, error)
+}
+
+type PluginInitializer struct {
 	internalClient  internalclientset.Interface
 	informers       informers.SharedInformerFactory
 	authorizer      authorizer.Authorizer
 	cloudConfig     []byte
 	restMapper      meta.RESTMapper
 	quotaRegistry   quota.Registry
+	serviceResolver ServiceResolver
+	webhookSource   WebhookSource
+
+	// for proving we are apiserver in call-outs
+	clientCert []byte
+	clientKey  []byte
 }
 
-var _ admission.PluginInitializer = pluginInitializer{}
+var _ admission.PluginInitializer = &PluginInitializer{}
 
 // NewPluginInitializer constructs new instance of PluginInitializer
+// TODO: switch these parameters to use the builder pattern or just make them
+// all public, this construction method is pointless boilerplate.
 func NewPluginInitializer(internalClient internalclientset.Interface,
 	sharedInformers informers.SharedInformerFactory,
 	authz authorizer.Authorizer,
 	cloudConfig []byte,
 	restMapper meta.RESTMapper,
-	quotaRegistry quota.Registry) admission.PluginInitializer {
+	quotaRegistry quota.Registry) *PluginInitializer {
-	return pluginInitializer{
+	return &PluginInitializer{
 		internalClient: internalClient,
 		informers:      sharedInformers,
 		authorizer:     authz,
@@ -89,9 +129,30 @@ func NewPluginInitializer(internalClient internalclientset.Interface,
 	}
 }
 
+// SetServiceResolver sets the service resolver which is needed by some plugins.
+func (i *PluginInitializer) SetServiceResolver(s ServiceResolver) *PluginInitializer {
+	i.serviceResolver = s
+	return i
+}
+
+// SetClientCert sets the client cert & key (identity used for calling out to
+// web hooks) which is needed by some plugins.
+func (i *PluginInitializer) SetClientCert(cert, key []byte) *PluginInitializer {
+	i.clientCert = cert
+	i.clientKey = key
+	return i
+}
+
+// SetWebhookSource sets the webhook source-- admittedly this is probably
+// specific to the external admission hook plugin.
+func (i *PluginInitializer) SetWebhookSource(w WebhookSource) *PluginInitializer {
+	i.webhookSource = w
+	return i
+}
+
 // Initialize checks the initialization interfaces implemented by each plugin
 // and provide the appropriate initialization data
-func (i pluginInitializer) Initialize(plugin admission.Interface) {
+func (i *PluginInitializer) Initialize(plugin admission.Interface) {
 	if wants, ok := plugin.(WantsInternalKubeClientSet); ok {
 		wants.SetInternalKubeClientSet(i.internalClient)
 	}
@@ -115,4 +176,25 @@ func (i pluginInitializer) Initialize(plugin admission.Interface) {
 	if wants, ok := plugin.(WantsQuotaRegistry); ok {
 		wants.SetQuotaRegistry(i.quotaRegistry)
 	}
+
+	if wants, ok := plugin.(WantsServiceResolver); ok {
+		if i.serviceResolver == nil {
+			panic("An admission plugin wants the service resolver, but it was not provided.")
+		}
+		wants.SetServiceResolver(i.serviceResolver)
+	}
+
+	if wants, ok := plugin.(WantsClientCert); ok {
+		if i.clientCert == nil || i.clientKey == nil {
+			panic("An admission plugin wants a client cert/key, but they were not provided.")
+		}
+		wants.SetClientCert(i.clientCert, i.clientKey)
+	}
+
+	if wants, ok := plugin.(WantsWebhookSource); ok {
+		if i.webhookSource == nil {
+			panic("An admission plugin wants a webhook source, but it was not provided.")
+		}
+		wants.SetWebhookSource(i.webhookSource)
+	}
 }

pkg/kubectl/cmd/apply_test.go

@@ -987,9 +987,8 @@ func TestRunApplySetLastApplied(t *testing.T) {
 		if buf.String() != test.expectedOut {
 			t.Fatalf("%s: unexpected output: %s\nexpected: %s", test.name, buf.String(), test.expectedOut)
 		}
-
 	}
-
+	cmdutil.BehaviorOnFatal(func(str string, code int) {})
 }
 
 func checkPatchString(t *testing.T, req *http.Request) {

pkg/kubectl/cmd/create_deployment_test.go

@@ -39,7 +39,7 @@ func TestCreateDeployment(t *testing.T) {
 		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
 			return &http.Response{
 				StatusCode: http.StatusOK,
-				Body:       ioutil.NopCloser(&bytes.Buffer{}),
+				Body:       ioutil.NopCloser(bytes.NewBuffer([]byte("{}"))),
 			}, nil
 		}),
 	}

pkg/kubectl/cmd/run_test.go

@@ -171,7 +171,7 @@ func TestRunArgsFollowDashRules(t *testing.T) {
 				} else {
 					return &http.Response{
 						StatusCode: http.StatusOK,
-						Body:       ioutil.NopCloser(&bytes.Buffer{}),
+						Body:       ioutil.NopCloser(bytes.NewBuffer([]byte("{}"))),
 					}, nil
 				}
 			}),

pkg/master/import_known_versions_test.go

@@ -85,6 +85,7 @@ var typesAllowedTags = map[reflect.Type]bool{
 	reflect.TypeOf(metav1.DeleteOptions{}):        true,
 	reflect.TypeOf(metav1.GroupVersionKind{}):     true,
 	reflect.TypeOf(metav1.GroupVersionResource{}): true,
+	reflect.TypeOf(metav1.Status{}):               true,
 }
 
 func ensureNoTags(t *testing.T, gvk schema.GroupVersionKind, tp reflect.Type, parents []reflect.Type) {

plugin/pkg/admission/webhook/admission.go

@@ -14,25 +14,31 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-// Package webhook checks a webhook for configured operation admission
+// Package webhook delegates admission checks to dynamically configured webhooks.
 package webhook
 
 import (
-	"errors"
+	"context"
 	"fmt"
 	"io"
+	"sync"
+	"time"
 
 	"github.com/golang/glog"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	"k8s.io/apimachinery/pkg/util/yaml"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/apiserver/pkg/util/webhook"
+	"k8s.io/client-go/rest"
-
 	"k8s.io/kubernetes/pkg/api"
 	admissionv1alpha1 "k8s.io/kubernetes/pkg/apis/admission/v1alpha1"
+	"k8s.io/kubernetes/pkg/apis/admissionregistration"
+	admissioninit "k8s.io/kubernetes/pkg/kubeapiserver/admission"
 
-	// install the clientgo admissiony API for use with api registry
+	// install the clientgo admission API for use with api registry
 	_ "k8s.io/kubernetes/pkg/apis/admission/install"
 )
 
@@ -42,22 +48,22 @@ var (
 	}
 )
 
+type ErrCallingWebhook struct {
+	WebhookName string
+	Reason      error
+}
+
+func (e *ErrCallingWebhook) Error() string {
+	if e.Reason != nil {
+		return fmt.Sprintf("failed calling admission webhook %q: %v", e.WebhookName, e.Reason)
+	}
+	return fmt.Sprintf("failed calling admission webhook %q; no further details available", e.WebhookName)
+}
+
 // Register registers a plugin
 func Register(plugins *admission.Plugins) {
 	plugins.Register("GenericAdmissionWebhook", func(configFile io.Reader) (admission.Interface, error) {
-		var gwhConfig struct {
+		plugin, err := NewGenericAdmissionWebhook()
-			WebhookConfig GenericAdmissionWebhookConfig `json:"webhook"`
-		}
-
-		d := yaml.NewYAMLOrJSONDecoder(configFile, 4096)
-		err := d.Decode(&gwhConfig)
-
-		if err != nil {
-			return nil, err
-		}
-
-		plugin, err := NewGenericAdmissionWebhook(&gwhConfig.WebhookConfig)
-
 		if err != nil {
 			return nil, err
 		}
@@ -67,98 +73,150 @@ func Register(plugins *admission.Plugins) {
 }
 
 // NewGenericAdmissionWebhook returns a generic admission webhook plugin.
-func NewGenericAdmissionWebhook(config *GenericAdmissionWebhookConfig) (admission.Interface, error) {
+func NewGenericAdmissionWebhook() (*GenericAdmissionWebhook, error) {
-	err := normalizeConfig(config)
-
-	if err != nil {
-		return nil, err
-	}
-
-	gw, err := webhook.NewGenericWebhook(api.Registry, api.Codecs, config.KubeConfigFile, groupVersions, config.RetryBackoff)
-
-	if err != nil {
-		return nil, err
-	}
-
 	return &GenericAdmissionWebhook{
-		Handler: admission.NewHandler(admission.Connect, admission.Create, admission.Delete, admission.Update),
+		Handler: admission.NewHandler(
-		webhook: gw,
+			admission.Connect,
-		rules:   config.Rules,
+			admission.Create,
+			admission.Delete,
+			admission.Update,
+		),
+		negotiatedSerializer: serializer.NegotiatedSerializerWrapper(runtime.SerializerInfo{
+			Serializer: api.Codecs.LegacyCodec(admissionv1alpha1.SchemeGroupVersion),
+		}),
 	}, nil
 }
 
 // GenericAdmissionWebhook is an implementation of admission.Interface.
 type GenericAdmissionWebhook struct {
 	*admission.Handler
-	webhook *webhook.GenericWebhook
+	hookSource           admissioninit.WebhookSource
-	rules   []Rule
+	serviceResolver      admissioninit.ServiceResolver
+	negotiatedSerializer runtime.NegotiatedSerializer
+	clientCert           []byte
+	clientKey            []byte
+}
+
+var (
+	_ = admissioninit.WantsServiceResolver(&GenericAdmissionWebhook{})
+	_ = admissioninit.WantsClientCert(&GenericAdmissionWebhook{})
+	_ = admissioninit.WantsWebhookSource(&GenericAdmissionWebhook{})
+)
+
+func (a *GenericAdmissionWebhook) SetServiceResolver(sr admissioninit.ServiceResolver) {
+	a.serviceResolver = sr
+}
+
+func (a *GenericAdmissionWebhook) SetClientCert(cert, key []byte) {
+	a.clientCert = cert
+	a.clientKey = key
+}
+
+func (a *GenericAdmissionWebhook) SetWebhookSource(ws admissioninit.WebhookSource) {
+	a.hookSource = ws
 }
 
 // Admit makes an admission decision based on the request attributes.
-func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) (err error) {
+func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) error {
-	var matched *Rule
+	hooks, err := a.hookSource.List()
+	if err != nil {
+		return fmt.Errorf("failed listing hooks: %v", err)
+	}
+	ctx := context.TODO()
 
-	// Process all declared rules to attempt to find a match
+	errCh := make(chan error, len(hooks))
-	for i, rule := range a.rules {
+	wg := sync.WaitGroup{}
-		if Matches(rule, attr) {
+	wg.Add(len(hooks))
-			glog.V(2).Infof("rule at index %d matched request", i)
+	for i := range hooks {
-			matched = &a.rules[i]
+		go func(hook *admissionregistration.ExternalAdmissionHook) {
+			defer wg.Done()
+			if err := a.callHook(ctx, hook, attr); err == nil {
+				return
+			} else if callErr, ok := err.(*ErrCallingWebhook); ok {
+				glog.Warningf("Failed calling webhook %v: %v", hook.Name, callErr)
+				utilruntime.HandleError(callErr)
+				// Since we are failing open to begin with, we do not send an error down the channel
+			} else {
+				glog.Warningf("rejected by webhook %v %t: %v", hook.Name, err, err)
+				errCh <- err
+			}
+		}(&hooks[i])
+	}
+	wg.Wait()
+	close(errCh)
+
+	var errs []error
+	for e := range errCh {
+		errs = append(errs, e)
+	}
+	if len(errs) == 0 {
+		return nil
+	}
+	if len(errs) > 1 {
+		for i := 1; i < len(errs); i++ {
+			// TODO: merge status errors; until then, just return the first one.
+			utilruntime.HandleError(errs[i])
+		}
+	}
+	return errs[0]
+}
+
+func (a *GenericAdmissionWebhook) callHook(ctx context.Context, h *admissionregistration.ExternalAdmissionHook, attr admission.Attributes) error {
+	matches := false
+	for _, r := range h.Rules {
+		m := RuleMatcher{Rule: r, Attr: attr}
+		if m.Matches() {
+			matches = true
 			break
 		}
 	}
-
+	if !matches {
-	if matched == nil {
-		glog.V(2).Infof("rule explicitly allowed the request: no rule matched the admission request")
-		return nil
-	}
-
-	// The matched rule skips processing this request
-	if matched.Type == Skip {
-		glog.V(2).Infof("rule explicitly allowed the request")
 		return nil
 	}
 
 	// Make the webhook request
 	request := admissionv1alpha1.NewAdmissionReview(attr)
-	response := a.webhook.RestClient.Post().Body(&request).Do()
+	client, err := a.hookClient(h)
-
-	// Handle webhook response
-	if err := response.Error(); err != nil {
-		return a.handleError(attr, matched.FailAction, err)
-	}
-
-	var statusCode int
-	if response.StatusCode(&statusCode); statusCode < 200 || statusCode >= 300 {
-		return a.handleError(attr, matched.FailAction, fmt.Errorf("error contacting webhook: %d", statusCode))
-	}
-
-	if err := response.Into(&request); err != nil {
-		return a.handleError(attr, matched.FailAction, err)
-	}
-
-	if !request.Status.Allowed {
-		if request.Status.Result != nil && len(request.Status.Result.Reason) > 0 {
-			return a.handleError(attr, Deny, fmt.Errorf("webhook backend denied the request: %s", request.Status.Result.Reason))
-		}
-		return a.handleError(attr, Deny, errors.New("webhook backend denied the request"))
-	}
-
-	// The webhook admission controller DOES NOT allow mutation of the admission request so nothing else is required
-
-	return nil
-}
-
-func (a *GenericAdmissionWebhook) handleError(attr admission.Attributes, allowIfErr FailAction, err error) error {
 	if err != nil {
-		glog.V(2).Infof("error contacting webhook backend: %s", err)
+		return &ErrCallingWebhook{WebhookName: h.Name, Reason: err}
-		if allowIfErr != Allow {
-			glog.V(2).Infof("resource not allowed due to webhook backend failure: %s", err)
-			return admission.NewForbidden(attr, err)
 	}
-		glog.V(2).Infof("resource allowed in spite of webhook backend failure")
+	if err := client.Post().Context(ctx).Body(&request).Do().Into(&request); err != nil {
+		return &ErrCallingWebhook{WebhookName: h.Name, Reason: err}
 	}
 
+	if request.Status.Allowed {
 		return nil
 	}
 
-// TODO: Allow configuring the serialization strategy
+	if request.Status.Result == nil {
+		return fmt.Errorf("admission webhook %q denied the request without explanation", h.Name)
+	}
+
+	return &apierrors.StatusError{
+		ErrStatus: *request.Status.Result,
+	}
+}
+
+func (a *GenericAdmissionWebhook) hookClient(h *admissionregistration.ExternalAdmissionHook) (*rest.RESTClient, error) {
+	u, err := a.serviceResolver.ResolveEndpoint(h.ClientConfig.Service.Namespace, h.ClientConfig.Service.Name)
+	if err != nil {
+		return nil, err
+	}
+
+	// TODO: cache these instead of constructing one each time
+	cfg := &rest.Config{
+		Host:    u.Host,
+		APIPath: u.Path,
+		TLSClientConfig: rest.TLSClientConfig{
+			CAData:   h.ClientConfig.CABundle,
+			CertData: a.clientCert,
+			KeyData:  a.clientKey,
+		},
+		UserAgent: "kube-apiserver-admission",
+		Timeout:   30 * time.Second,
+		ContentConfig: rest.ContentConfig{
+			NegotiatedSerializer: a.negotiatedSerializer,
+		},
+	}
+	return rest.UnversionedRESTClientFor(cfg)
+}

plugin/pkg/admission/webhook/admission_test.go

@@ -17,171 +17,81 @@ limitations under the License.
 package webhook
 
 import (
-	"bytes"
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
-	"os"
+	"net/url"
-	"path/filepath"
+	"strings"
-	"regexp"
 	"testing"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/authentication/user"
-	"k8s.io/client-go/pkg/api"
+	"k8s.io/kubernetes/pkg/api"
-	"k8s.io/client-go/tools/clientcmd/api/v1"
 	"k8s.io/kubernetes/pkg/apis/admission/v1alpha1"
+	"k8s.io/kubernetes/pkg/apis/admissionregistration"
 
 	_ "k8s.io/kubernetes/pkg/apis/admission/install"
 )
 
-const (
+type fakeHookSource struct {
-	errAdmitPrefix = `pods "my-pod" is forbidden: `
+	hooks []admissionregistration.ExternalAdmissionHook
-)
+	err   error
-
-var (
-	requestCount int
-	testRoot     string
-	testServer   *httptest.Server
-)
-
-type genericAdmissionWebhookTest struct {
-	test   string
-	config *GenericAdmissionWebhookConfig
-	value  interface{}
 }
 
-func TestMain(m *testing.M) {
+func (f *fakeHookSource) List() ([]admissionregistration.ExternalAdmissionHook, error) {
-	tmpRoot, err := ioutil.TempDir("", "")
+	if f.err != nil {
-
+		return nil, f.err
-	if err != nil {
+	}
-		killTests(err)
+	return f.hooks, nil
 }
 
-	testRoot = tmpRoot
+type fakeServiceResolver struct {
-
+	base url.URL
-	// Cleanup
-	defer os.RemoveAll(testRoot)
-
-	// Create the test webhook server
-	cert, err := tls.X509KeyPair(clientCert, clientKey)
-
-	if err != nil {
-		killTests(err)
 }
 
-	rootCAs := x509.NewCertPool()
+func (f fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) {
-
+	if namespace == "failResolve" {
-	rootCAs.AppendCertsFromPEM(caCert)
+		return nil, fmt.Errorf("couldn't resolve service location")
-
-	testServer = httptest.NewUnstartedServer(http.HandlerFunc(webhookHandler))
-
-	testServer.TLS = &tls.Config{
-		Certificates: []tls.Certificate{cert},
-		ClientCAs:    rootCAs,
-		ClientAuth:   tls.RequireAndVerifyClientCert,
-	}
-
-	// Create the test webhook server
-	testServer.StartTLS()
-
-	// Cleanup
-	defer testServer.Close()
-
-	// Create an invalid and valid Kubernetes configuration file
-	var kubeConfig bytes.Buffer
-
-	err = json.NewEncoder(&kubeConfig).Encode(v1.Config{
-		Clusters: []v1.NamedCluster{
-			{
-				Cluster: v1.Cluster{
-					Server: testServer.URL,
-					CertificateAuthorityData: caCert,
-				},
-			},
-		},
-		AuthInfos: []v1.NamedAuthInfo{
-			{
-				AuthInfo: v1.AuthInfo{
-					ClientCertificateData: clientCert,
-					ClientKeyData:         clientKey,
-				},
-			},
-		},
-	})
-
-	if err != nil {
-		killTests(err)
-	}
-
-	// The files needed on disk for the webhook tests
-	files := map[string][]byte{
-		"ca.pem":         caCert,
-		"client.pem":     clientCert,
-		"client-key.pem": clientKey,
-		"kube-config":    kubeConfig.Bytes(),
-	}
-
-	// Write the certificate files to disk or fail
-	for fileName, fileData := range files {
-		if err := ioutil.WriteFile(filepath.Join(testRoot, fileName), fileData, 0400); err != nil {
-			killTests(err)
-		}
-	}
-
-	// Run the tests
-	m.Run()
-}
-
-// TestNewGenericAdmissionWebhook tests that NewGenericAdmissionWebhook works as expected
-func TestNewGenericAdmissionWebhook(t *testing.T) {
-	tests := []genericAdmissionWebhookTest{
-		genericAdmissionWebhookTest{
-			test:   "Empty webhook config",
-			config: &GenericAdmissionWebhookConfig{},
-			value:  fmt.Errorf("kubeConfigFile is required"),
-		},
-		genericAdmissionWebhookTest{
-			test: "Broken webhook config",
-			config: &GenericAdmissionWebhookConfig{
-				KubeConfigFile: filepath.Join(testRoot, "kube-config.missing"),
-				Rules: []Rule{
-					Rule{
-						Type: Skip,
-					},
-				},
-			},
-			value: fmt.Errorf("stat .*kube-config.missing.*"),
-		},
-		genericAdmissionWebhookTest{
-			test: "Valid webhook config",
-			config: &GenericAdmissionWebhookConfig{
-				KubeConfigFile: filepath.Join(testRoot, "kube-config"),
-				Rules: []Rule{
-					Rule{
-						Type: Skip,
-					},
-				},
-			},
-			value: nil,
-		},
-	}
-
-	for _, tt := range tests {
-		_, err := NewGenericAdmissionWebhook(tt.config)
-
-		checkForError(t, tt.test, tt.value, err)
 	}
+	u := f.base
+	u.Path = name
+	return &u, nil
 }
 
 // TestAdmit tests that GenericAdmissionWebhook#Admit works as expected
 func TestAdmit(t *testing.T) {
-	configWithFailAction := makeGoodConfig()
+	// Create the test webhook server
+	sCert, err := tls.X509KeyPair(serverCert, serverKey)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rootCAs := x509.NewCertPool()
+	rootCAs.AppendCertsFromPEM(caCert)
+	testServer := httptest.NewUnstartedServer(http.HandlerFunc(webhookHandler))
+	testServer.TLS = &tls.Config{
+		Certificates: []tls.Certificate{sCert},
+		ClientCAs:    rootCAs,
+		ClientAuth:   tls.RequireAndVerifyClientCert,
+	}
+	testServer.StartTLS()
+	defer testServer.Close()
+	serverURL, err := url.ParseRequestURI(testServer.URL)
+	if err != nil {
+		t.Fatalf("this should never happen? %v", err)
+	}
+	wh, err := NewGenericAdmissionWebhook()
+	if err != nil {
+		t.Fatal(err)
+	}
+	wh.serviceResolver = fakeServiceResolver{*serverURL}
+	wh.clientCert = clientCert
+	wh.clientKey = clientKey
+
+	// Set up a test object for the call
 	kind := api.Kind("Pod").WithVersion("v1")
 	name := "my-pod"
 	namespace := "webhook-test"
@@ -209,161 +119,145 @@ func TestAdmit(t *testing.T) {
 		UID:  "webhook-test",
 	}
 
-	configWithFailAction.Rules[0].FailAction = Allow
+	type test struct {
+		hookSource    fakeHookSource
+		expectAllow   bool
+		errorContains string
+	}
+	ccfg := func(result string) admissionregistration.AdmissionHookClientConfig {
+		return admissionregistration.AdmissionHookClientConfig{
+			Service: admissionregistration.ServiceReference{
+				Name: result,
+			},
+			CABundle: caCert,
+		}
+	}
+	matchEverythingRules := []admissionregistration.RuleWithOperations{{
+		Operations: []admissionregistration.OperationType{admissionregistration.OperationAll},
+		Rule: admissionregistration.Rule{
+			APIGroups:   []string{"*"},
+			APIVersions: []string{"*"},
+			Resources:   []string{"*/*"},
+		},
+	}}
 
-	tests := []genericAdmissionWebhookTest{
+	table := map[string]test{
-		genericAdmissionWebhookTest{
+		"no match": {
-			test: "No matching rule",
+			hookSource: fakeHookSource{
-			config: &GenericAdmissionWebhookConfig{
+				hooks: []admissionregistration.ExternalAdmissionHook{{
-				KubeConfigFile: filepath.Join(testRoot, "kube-config"),
+					Name:         "nomatch",
-				Rules: []Rule{
+					ClientConfig: ccfg("disallow"),
-					Rule{
+					Rules: []admissionregistration.RuleWithOperations{{
-						Operations: []admission.Operation{
+						Operations: []admissionregistration.OperationType{admissionregistration.Create},
-							admission.Create,
+					}},
+				}},
 			},
-						Type: Send,
+			expectAllow: true,
 		},
+		"match & allow": {
+			hookSource: fakeHookSource{
+				hooks: []admissionregistration.ExternalAdmissionHook{{
+					Name:         "allow",
+					ClientConfig: ccfg("allow"),
+					Rules:        matchEverythingRules,
+				}},
 			},
+			expectAllow: true,
 		},
-			value: nil,
+		"match & disallow": {
+			hookSource: fakeHookSource{
+				hooks: []admissionregistration.ExternalAdmissionHook{{
+					Name:         "disallow",
+					ClientConfig: ccfg("disallow"),
+					Rules:        matchEverythingRules,
+				}},
 			},
-		genericAdmissionWebhookTest{
+			errorContains: "without explanation",
-			test: "Matching rule skips",
-			config: &GenericAdmissionWebhookConfig{
-				KubeConfigFile: filepath.Join(testRoot, "kube-config"),
-				Rules: []Rule{
-					Rule{
-						Operations: []admission.Operation{
-							admission.Update,
 		},
-						Type: Skip,
+		"match & disallow ii": {
+			hookSource: fakeHookSource{
+				hooks: []admissionregistration.ExternalAdmissionHook{{
+					Name:         "disallowReason",
+					ClientConfig: ccfg("disallowReason"),
+					Rules:        matchEverythingRules,
+				}},
 			},
+			errorContains: "you shall not pass",
 		},
+		"match & fail (but allow because fail open)": {
+			hookSource: fakeHookSource{
+				hooks: []admissionregistration.ExternalAdmissionHook{{
+					Name:         "internalErr A",
+					ClientConfig: ccfg("internalErr"),
+					Rules:        matchEverythingRules,
+				}, {
+					Name:         "invalidReq B",
+					ClientConfig: ccfg("invalidReq"),
+					Rules:        matchEverythingRules,
+				}, {
+					Name:         "invalidResp C",
+					ClientConfig: ccfg("invalidResp"),
+					Rules:        matchEverythingRules,
+				}},
 			},
-			value: nil,
+			expectAllow: true,
-		},
-		genericAdmissionWebhookTest{
-			test:   "Matching rule sends (webhook internal server error)",
-			config: makeGoodConfig(),
-			value:  fmt.Errorf(`%san error on the server ("webhook internal server error") has prevented the request from succeeding`, errAdmitPrefix),
-		},
-		genericAdmissionWebhookTest{
-			test:   "Matching rule sends (webhook unsuccessful response)",
-			config: makeGoodConfig(),
-			value:  fmt.Errorf("%serror contacting webhook: 101", errAdmitPrefix),
-		},
-		genericAdmissionWebhookTest{
-			test:   "Matching rule sends (webhook unmarshallable response)",
-			config: makeGoodConfig(),
-			value:  fmt.Errorf("%scouldn't get version/kind; json parse error: json: cannot unmarshal string into Go value of type struct.*", errAdmitPrefix),
-		},
-		genericAdmissionWebhookTest{
-			test:   "Matching rule sends (webhook error but allowed via fail action)",
-			config: configWithFailAction,
-			value:  nil,
-		},
-		genericAdmissionWebhookTest{
-			test:   "Matching rule sends (webhook request denied without reason)",
-			config: makeGoodConfig(),
-			value:  fmt.Errorf("%swebhook backend denied the request", errAdmitPrefix),
-		},
-		genericAdmissionWebhookTest{
-			test:   "Matching rule sends (webhook request denied with reason)",
-			config: makeGoodConfig(),
-			value:  fmt.Errorf("%swebhook backend denied the request: you shall not pass", errAdmitPrefix),
-		},
-		genericAdmissionWebhookTest{
-			test:   "Matching rule sends (webhook request allowed)",
-			config: makeGoodConfig(),
-			value:  nil,
 		},
 	}
 
-	for _, tt := range tests {
+	for name, tt := range table {
-		wh, err := NewGenericAdmissionWebhook(tt.config)
+		wh.hookSource = &tt.hookSource
 
-		if err != nil {
-			t.Errorf("%s: unexpected error: %v", tt.test, err)
-		} else {
 		err = wh.Admit(admission.NewAttributesRecord(&object, &oldObject, kind, namespace, name, resource, subResource, operation, &userInfo))
-
+		if tt.expectAllow != (err == nil) {
-			checkForError(t, tt.test, tt.value, err)
+			t.Errorf("%q: expected allowed=%v, but got err=%v", name, tt.expectAllow, err)
+		}
+		// ErrWebhookRejected is not an error for our purposes
+		if tt.errorContains != "" {
+			if err == nil || !strings.Contains(err.Error(), tt.errorContains) {
+				t.Errorf("%q: expected an error saying %q, but got %v", name, tt.errorContains, err)
 			}
 		}
 	}
-
-func checkForError(t *testing.T, test string, expected, actual interface{}) {
-	aErr, _ := actual.(error)
-	eErr, _ := expected.(error)
-
-	if eErr != nil {
-		if aErr == nil {
-			t.Errorf("%s: expected an error", test)
-		} else if eErr.Error() != aErr.Error() && !regexp.MustCompile(eErr.Error()).MatchString(aErr.Error()) {
-			t.Errorf("%s: unexpected error message to match:\n  Expected: %s\n  Actual:   %s", test, eErr.Error(), aErr.Error())
-		}
-	} else {
-		if aErr != nil {
-			t.Errorf("%s: unexpected error: %v", test, aErr)
-		}
-	}
-}
-
-func killTests(err error) {
-	panic(fmt.Sprintf("Unable to bootstrap tests: %v", err))
-}
-
-func makeGoodConfig() *GenericAdmissionWebhookConfig {
-	return &GenericAdmissionWebhookConfig{
-		KubeConfigFile: filepath.Join(testRoot, "kube-config"),
-		Rules: []Rule{
-			Rule{
-				Operations: []admission.Operation{
-					admission.Update,
-				},
-				Type: Send,
-			},
-		},
-	}
 }
 
 func webhookHandler(w http.ResponseWriter, r *http.Request) {
-	requestCount++
+	fmt.Printf("got req: %v\n", r.URL.Path)
-
+	switch r.URL.Path {
-	switch requestCount {
+	case "/internalErr":
-	case 1, 4:
 		http.Error(w, "webhook internal server error", http.StatusInternalServerError)
 		return
-	case 2:
+	case "/invalidReq":
 		w.WriteHeader(http.StatusSwitchingProtocols)
 		w.Write([]byte("webhook invalid request"))
 		return
-	case 3:
+	case "/invalidResp":
 		w.Header().Set("Content-Type", "application/json")
 		w.Write([]byte("webhook invalid response"))
-	case 5:
+	case "/disallow":
 		w.Header().Set("Content-Type", "application/json")
 		json.NewEncoder(w).Encode(&v1alpha1.AdmissionReview{
 			Status: v1alpha1.AdmissionReviewStatus{
 				Allowed: false,
 			},
 		})
-	case 6:
+	case "/disallowReason":
 		w.Header().Set("Content-Type", "application/json")
 		json.NewEncoder(w).Encode(&v1alpha1.AdmissionReview{
 			Status: v1alpha1.AdmissionReviewStatus{
 				Allowed: false,
 				Result: &metav1.Status{
-					Reason: "you shall not pass",
+					Message: "you shall not pass",
 				},
 			},
 		})
-	case 7:
+	case "/allow":
 		w.Header().Set("Content-Type", "application/json")
 		json.NewEncoder(w).Encode(&v1alpha1.AdmissionReview{
 			Status: v1alpha1.AdmissionReviewStatus{
 				Allowed: true,
 			},
 		})
+	default:
+		http.NotFound(w, r)
 	}
 }

plugin/pkg/admission/webhook/config.go

@@ -1,120 +0,0 @@
-/*
-Copyright 2017 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-// Package webhook checks a webhook for configured operation admission
-package webhook
-
-import (
-	"fmt"
-	"strings"
-	"time"
-
-	"k8s.io/apiserver/pkg/admission"
-)
-
-const (
-	defaultFailAction         = Deny
-	defaultRetryBackoff       = time.Duration(500) * time.Millisecond
-	errInvalidFailAction      = "webhook rule (%d) has an invalid fail action: %s should be either %v or %v"
-	errInvalidRuleOperation   = "webhook rule (%d) has an invalid operation (%d): %s"
-	errInvalidRuleType        = "webhook rule (%d) has an invalid type: %s should be either %v or %v"
-	errMissingKubeConfigFile  = "kubeConfigFile is required"
-	errOneRuleRequired        = "webhook requires at least one rule defined"
-	errRetryBackoffOutOfRange = "webhook retry backoff is invalid: %v should be between %v and %v"
-	minRetryBackoff           = time.Duration(1) * time.Millisecond
-	maxRetryBackoff           = time.Duration(5) * time.Minute
-)
-
-func normalizeConfig(config *GenericAdmissionWebhookConfig) error {
-	allowFailAction := string(Allow)
-	denyFailAction := string(Deny)
-	connectOperation := string(admission.Connect)
-	createOperation := string(admission.Create)
-	deleteOperation := string(admission.Delete)
-	updateOperation := string(admission.Update)
-	sendRuleType := string(Send)
-	skipRuleType := string(Skip)
-
-	// Validate the kubeConfigFile property is present
-	if config.KubeConfigFile == "" {
-		return fmt.Errorf(errMissingKubeConfigFile)
-	}
-
-	// Normalize and validate the retry backoff
-	if config.RetryBackoff == 0 {
-		config.RetryBackoff = defaultRetryBackoff
-	} else {
-		// Unmarshalling gives nanoseconds so convert to milliseconds
-		config.RetryBackoff *= time.Millisecond
-	}
-
-	if config.RetryBackoff < minRetryBackoff || config.RetryBackoff > maxRetryBackoff {
-		return fmt.Errorf(errRetryBackoffOutOfRange, config.RetryBackoff, minRetryBackoff, maxRetryBackoff)
-	}
-
-	// Validate that there is at least one rule
-	if len(config.Rules) == 0 {
-		return fmt.Errorf(errOneRuleRequired)
-	}
-
-	for i, rule := range config.Rules {
-		// Normalize and validate the fail action
-		failAction := strings.ToUpper(string(rule.FailAction))
-
-		switch failAction {
-		case "":
-			config.Rules[i].FailAction = defaultFailAction
-		case allowFailAction:
-			config.Rules[i].FailAction = Allow
-		case denyFailAction:
-			config.Rules[i].FailAction = Deny
-		default:
-			return fmt.Errorf(errInvalidFailAction, i, rule.FailAction, Allow, Deny)
-		}
-
-		// Normalize and validate the rule operation(s)
-		for j, operation := range rule.Operations {
-			operation := strings.ToUpper(string(operation))
-
-			switch operation {
-			case connectOperation:
-				config.Rules[i].Operations[j] = admission.Connect
-			case createOperation:
-				config.Rules[i].Operations[j] = admission.Create
-			case deleteOperation:
-				config.Rules[i].Operations[j] = admission.Delete
-			case updateOperation:
-				config.Rules[i].Operations[j] = admission.Update
-			default:
-				return fmt.Errorf(errInvalidRuleOperation, i, j, rule.Operations[j])
-			}
-		}
-
-		// Normalize and validate the rule type
-		ruleType := strings.ToUpper(string(rule.Type))
-
-		switch ruleType {
-		case sendRuleType:
-			config.Rules[i].Type = Send
-		case skipRuleType:
-			config.Rules[i].Type = Skip
-		default:
-			return fmt.Errorf(errInvalidRuleType, i, rule.Type, Send, Skip)
-		}
-	}
-
-	return nil
-}

plugin/pkg/admission/webhook/config_test.go

@@ -1,220 +0,0 @@
-/*
-Copyright 2017 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package webhook
-
-import (
-	"fmt"
-	"reflect"
-	"testing"
-	"time"
-
-	"k8s.io/apiserver/pkg/admission"
-)
-
-func TestConfigNormalization(t *testing.T) {
-	defaultRules := []Rule{
-		Rule{
-			Type: Skip,
-		},
-	}
-	highRetryBackoff := (maxRetryBackoff / time.Millisecond) + (time.Duration(1) * time.Millisecond)
-	kubeConfigFile := "/tmp/kube/config"
-	lowRetryBackoff := time.Duration(-1)
-	normalizedValidRules := []Rule{
-		Rule{
-			APIGroups:  []string{""},
-			FailAction: Allow,
-			Namespaces: []string{"my-ns"},
-			Operations: []admission.Operation{
-				admission.Connect,
-				admission.Create,
-				admission.Delete,
-				admission.Update,
-			},
-			Resources:     []string{"pods"},
-			ResourceNames: []string{"my-name"},
-			Type:          Send,
-		},
-		Rule{
-			FailAction: Deny,
-			Type:       Skip,
-		},
-	}
-	rawValidRules := []Rule{
-		Rule{
-			APIGroups:  []string{""},
-			FailAction: FailAction("AlLoW"),
-			Namespaces: []string{"my-ns"},
-			Operations: []admission.Operation{
-				admission.Operation("connect"),
-				admission.Operation("CREATE"),
-				admission.Operation("DeLeTe"),
-				admission.Operation("UPdaTE"),
-			},
-			Resources:     []string{"pods"},
-			ResourceNames: []string{"my-name"},
-			Type:          RuleType("SenD"),
-		},
-		Rule{
-			FailAction: Deny,
-			Type:       Skip,
-		},
-	}
-	unknownFailAction := FailAction("Unknown")
-	unknownOperation := admission.Operation("Unknown")
-	unknownRuleType := RuleType("Allow")
-	tests := []struct {
-		test             string
-		rawConfig        GenericAdmissionWebhookConfig
-		normalizedConfig GenericAdmissionWebhookConfig
-		err              error
-	}{
-		{
-			test:      "kubeConfigFile was not provided (error)",
-			rawConfig: GenericAdmissionWebhookConfig{},
-			err:       fmt.Errorf(errMissingKubeConfigFile),
-		},
-		{
-			test: "retryBackoff was not provided (use default)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				Rules:          defaultRules,
-			},
-			normalizedConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				RetryBackoff:   defaultRetryBackoff,
-				Rules:          defaultRules,
-			},
-		},
-		{
-			test: "retryBackoff was below minimum value (error)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				RetryBackoff:   lowRetryBackoff,
-				Rules:          defaultRules,
-			},
-			err: fmt.Errorf(errRetryBackoffOutOfRange, lowRetryBackoff*time.Millisecond, minRetryBackoff, maxRetryBackoff),
-		},
-		{
-			test: "retryBackoff was above maximum value (error)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				RetryBackoff:   highRetryBackoff,
-				Rules:          defaultRules,
-			},
-			err: fmt.Errorf(errRetryBackoffOutOfRange, highRetryBackoff*time.Millisecond, minRetryBackoff, maxRetryBackoff),
-		},
-		{
-			test: "rules should have at least one rule (error)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-			},
-			err: fmt.Errorf(errOneRuleRequired),
-		},
-		{
-			test: "fail action was not provided (use default)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				Rules: []Rule{
-					Rule{
-						Type: Skip,
-					},
-				},
-			},
-			normalizedConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				RetryBackoff:   defaultRetryBackoff,
-				Rules: []Rule{
-					Rule{
-						FailAction: defaultFailAction,
-						Type:       Skip,
-					},
-				},
-			},
-		},
-		{
-			test: "rule has invalid fail action (error)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				Rules: []Rule{
-					Rule{
-						FailAction: unknownFailAction,
-					},
-				},
-			},
-			err: fmt.Errorf(errInvalidFailAction, 0, unknownFailAction, Allow, Deny),
-		},
-		{
-			test: "rule has invalid operation (error)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				Rules: []Rule{
-					Rule{
-						Operations: []admission.Operation{unknownOperation},
-					},
-				},
-			},
-			err: fmt.Errorf(errInvalidRuleOperation, 0, 0, unknownOperation),
-		},
-		{
-			test: "rule has invalid type (error)",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				Rules: []Rule{
-					Rule{
-						Type: unknownRuleType,
-					},
-				},
-			},
-			err: fmt.Errorf(errInvalidRuleType, 0, unknownRuleType, Send, Skip),
-		},
-		{
-			test: "valid configuration",
-			rawConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				Rules:          rawValidRules,
-			},
-			normalizedConfig: GenericAdmissionWebhookConfig{
-				KubeConfigFile: kubeConfigFile,
-				RetryBackoff:   defaultRetryBackoff,
-				Rules:          normalizedValidRules,
-			},
-			err: nil,
-		},
-	}
-
-	for _, tt := range tests {
-		err := normalizeConfig(&tt.rawConfig)
-		if err == nil {
-			if tt.err != nil {
-				// Ensure that expected errors are produced
-				t.Errorf("%s: expected error but did not produce one", tt.test)
-			} else if !reflect.DeepEqual(tt.rawConfig, tt.normalizedConfig) {
-				// Ensure that valid configurations are structured properly
-				t.Errorf("%s: normalized config mismtach. got: %v expected: %v", tt.test, tt.rawConfig, tt.normalizedConfig)
-			}
-		} else {
-			if tt.err == nil {
-				// Ensure that unexpected errors are not produced
-				t.Errorf("%s: unexpected error: %v", tt.test, err)
-			} else if err != nil && tt.err != nil && err.Error() != tt.err.Error() {
-				// Ensure that expected errors are formated properly
-				t.Errorf("%s: error message mismatch. got: '%v' expected: '%v'", tt.test, err, tt.err)
-			}
-		}
-	}
-}

plugin/pkg/admission/webhook/rules.go

@@ -17,46 +17,31 @@ limitations under the License.
 // Package webhook checks a webhook for configured operation admission
 package webhook
 
-import "k8s.io/apiserver/pkg/admission"
+import (
+	"strings"
 
-func indexOf(items []string, requested string) int {
+	"k8s.io/apiserver/pkg/admission"
-	for i, item := range items {
+	"k8s.io/kubernetes/pkg/apis/admissionregistration"
+)
+
+type RuleMatcher struct {
+	Rule admissionregistration.RuleWithOperations
+	Attr admission.Attributes
+}
+
+func (r *RuleMatcher) Matches() bool {
+	return r.operation() &&
+		r.group() &&
+		r.version() &&
+		r.resource()
+}
+
+func exactOrWildcard(items []string, requested string) bool {
+	for _, item := range items {
+		if item == "*" {
+			return true
+		}
 		if item == requested {
-			return i
-		}
-	}
-
-	return -1
-}
-
-// APIGroupMatches returns if the admission.Attributes matches the rule's API groups
-func APIGroupMatches(rule Rule, attr admission.Attributes) bool {
-	if len(rule.APIGroups) == 0 {
-		return true
-	}
-
-	return indexOf(rule.APIGroups, attr.GetResource().Group) > -1
-}
-
-// Matches returns if the admission.Attributes matches the rule
-func Matches(rule Rule, attr admission.Attributes) bool {
-	return APIGroupMatches(rule, attr) &&
-		NamespaceMatches(rule, attr) &&
-		OperationMatches(rule, attr) &&
-		ResourceMatches(rule, attr) &&
-		ResourceNamesMatches(rule, attr)
-}
-
-// OperationMatches returns if the admission.Attributes matches the rule's operation
-func OperationMatches(rule Rule, attr admission.Attributes) bool {
-	if len(rule.Operations) == 0 {
-		return true
-	}
-
-	aOp := attr.GetOperation()
-
-	for _, rOp := range rule.Operations {
-		if aOp == rOp {
 			return true
 		}
 	}
@@ -64,35 +49,46 @@ func OperationMatches(rule Rule, attr admission.Attributes) bool {
 	return false
 }
 
-// NamespaceMatches returns if the admission.Attributes matches the rule's namespaces
+func (r *RuleMatcher) group() bool {
-func NamespaceMatches(rule Rule, attr admission.Attributes) bool {
+	return exactOrWildcard(r.Rule.APIGroups, r.Attr.GetResource().Group)
-	if len(rule.Namespaces) == 0 {
+}
+
+func (r *RuleMatcher) version() bool {
+	return exactOrWildcard(r.Rule.APIVersions, r.Attr.GetResource().Version)
+}
+
+func (r *RuleMatcher) operation() bool {
+	attrOp := r.Attr.GetOperation()
+	for _, op := range r.Rule.Operations {
+		if op == admissionregistration.OperationAll {
 			return true
 		}
-
+		// The constants are the same such that this is a valid cast (and this
-	return indexOf(rule.Namespaces, attr.GetNamespace()) > -1
+		// is tested).
-}
+		if op == admissionregistration.OperationType(attrOp) {
-
-// ResourceMatches returns if the admission.Attributes matches the rule's resource (and optional subresource)
-func ResourceMatches(rule Rule, attr admission.Attributes) bool {
-	if len(rule.Resources) == 0 {
 			return true
 		}
-
+	}
-	resource := attr.GetResource().Resource
+	return false
-
-	if len(attr.GetSubresource()) > 0 {
-		resource = attr.GetResource().Resource + "/" + attr.GetSubresource()
 }
 
-	return indexOf(rule.Resources, resource) > -1
+func splitResource(resSub string) (res, sub string) {
+	parts := strings.SplitN(resSub, "/", 2)
+	if len(parts) == 2 {
+		return parts[0], parts[1]
+	}
+	return parts[0], ""
 }
 
-// ResourceNamesMatches returns if the admission.Attributes matches the rule's resource names
+func (r *RuleMatcher) resource() bool {
-func ResourceNamesMatches(rule Rule, attr admission.Attributes) bool {
+	opRes, opSub := r.Attr.GetResource().Resource, r.Attr.GetSubresource()
-	if len(rule.ResourceNames) == 0 {
+	for _, res := range r.Rule.Resources {
+		res, sub := splitResource(res)
+		resMatch := res == "*" || res == opRes
+		subMatch := sub == "*" || sub == opSub
+		if resMatch && subMatch {
 			return true
 		}
-
+	}
-	return indexOf(rule.ResourceNames, attr.GetName()) > -1
+	return false
 }

plugin/pkg/admission/webhook/rules_test.go

@@ -17,293 +17,284 @@ limitations under the License.
 package webhook
 
 import (
-	"fmt"
-	"strings"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apiserver/pkg/admission"
-	"k8s.io/client-go/pkg/api"
+	adreg "k8s.io/kubernetes/pkg/apis/admissionregistration"
 )
 
 type ruleTest struct {
-	test             string
+	rule    adreg.RuleWithOperations
-	rule             Rule
+	match   []admission.Attributes
-	attrAPIGroup     string
+	noMatch []admission.Attributes
-	attrNamespace    string
+}
-	attrOperation    admission.Operation
+type tests map[string]ruleTest
-	attrResource     string
+
-	attrResourceName string
+func a(group, version, resource, subresource, name string, operation admission.Operation) admission.Attributes {
-	attrSubResource  string
+	return admission.NewAttributesRecord(
-	shouldMatch      bool
+		nil, nil,
+		schema.GroupVersionKind{Group: group, Version: version, Kind: "k" + resource},
+		"ns", name,
+		schema.GroupVersionResource{Group: group, Version: version, Resource: resource}, subresource,
+		operation,
+		nil,
+	)
 }
 
-func TestAPIGroupMatches(t *testing.T) {
+func attrList(a ...admission.Attributes) []admission.Attributes {
-	tests := []ruleTest{
+	return a
-		ruleTest{
+}
-			test:        "apiGroups empty match",
+
-			rule:        Rule{},
+func TestGroup(t *testing.T) {
-			shouldMatch: true,
+	table := tests{
+		"wildcard": {
+			rule: adreg.RuleWithOperations{
+				Rule: adreg.Rule{
+					APIGroups: []string{"*"},
 				},
-		ruleTest{
-			test: "apiGroup match",
-			rule: Rule{
-				APIGroups: []string{"my-group"},
 			},
-			attrAPIGroup: "my-group",
+			match: attrList(
-			shouldMatch:  true,
+				a("g", "v", "r", "", "name", admission.Create),
+			),
 		},
-		ruleTest{
+		"exact": {
-			test: "apiGroup mismatch",
+			rule: adreg.RuleWithOperations{
-			rule: Rule{
+				Rule: adreg.Rule{
-				APIGroups: []string{"my-group"},
+					APIGroups: []string{"g1", "g2"},
 				},
-			attrAPIGroup: "your-group",
+			},
-			shouldMatch:  false,
+			match: attrList(
+				a("g1", "v", "r", "", "name", admission.Create),
+				a("g2", "v2", "r3", "", "name", admission.Create),
+			),
+			noMatch: attrList(
+				a("g3", "v", "r", "", "name", admission.Create),
+				a("g4", "v", "r", "", "name", admission.Create),
+			),
 		},
 	}
 
-	runTests(t, "apiGroups", tests)
+	for name, tt := range table {
+		for _, m := range tt.match {
+			r := RuleMatcher{tt.rule, m}
+			if !r.group() {
+				t.Errorf("%v: expected match %#v", name, m)
 			}
-
-func TestMatches(t *testing.T) {
-	tests := []ruleTest{
-		ruleTest{
-			test:        "empty rule matches",
-			rule:        Rule{},
-			shouldMatch: true,
-		},
-		ruleTest{
-			test: "all properties match",
-			rule: Rule{
-				APIGroups:     []string{"my-group"},
-				Namespaces:    []string{"my-ns"},
-				Operations:    []admission.Operation{admission.Create},
-				ResourceNames: []string{"my-name"},
-				Resources:     []string{"pods/status"},
-			},
-			shouldMatch:      true,
-			attrAPIGroup:     "my-group",
-			attrNamespace:    "my-ns",
-			attrOperation:    admission.Create,
-			attrResource:     "pods",
-			attrResourceName: "my-name",
-			attrSubResource:  "status",
-		},
-		ruleTest{
-			test: "no properties match",
-			rule: Rule{
-				APIGroups:     []string{"my-group"},
-				Namespaces:    []string{"my-ns"},
-				Operations:    []admission.Operation{admission.Create},
-				ResourceNames: []string{"my-name"},
-				Resources:     []string{"pods/status"},
-			},
-			shouldMatch:      false,
-			attrAPIGroup:     "your-group",
-			attrNamespace:    "your-ns",
-			attrOperation:    admission.Delete,
-			attrResource:     "secrets",
-			attrResourceName: "your-name",
-		},
 		}
-
+		for _, m := range tt.noMatch {
-	runTests(t, "", tests)
+			r := RuleMatcher{tt.rule, m}
-}
+			if r.group() {
-
+				t.Errorf("%v: expected no match %#v", name, m)
-func TestNamespaceMatches(t *testing.T) {
-	tests := []ruleTest{
-		ruleTest{
-			test:        "namespaces empty match",
-			rule:        Rule{},
-			shouldMatch: true,
-		},
-		ruleTest{
-			test: "namespace match",
-			rule: Rule{
-				Namespaces: []string{"my-ns"},
-			},
-			attrNamespace: "my-ns",
-			shouldMatch:   true,
-		},
-		ruleTest{
-			test: "namespace mismatch",
-			rule: Rule{
-				Namespaces: []string{"my-ns"},
-			},
-			attrNamespace: "your-ns",
-			shouldMatch:   false,
-		},
-	}
-
-	runTests(t, "namespaces", tests)
-}
-
-func TestOperationMatches(t *testing.T) {
-	tests := []ruleTest{
-		ruleTest{
-			test:        "operations empty match",
-			rule:        Rule{},
-			shouldMatch: true,
-		},
-		ruleTest{
-			test: "operation match",
-			rule: Rule{
-				Operations: []admission.Operation{admission.Create},
-			},
-			attrOperation: admission.Create,
-			shouldMatch:   true,
-		},
-		ruleTest{
-			test: "operation mismatch",
-			rule: Rule{
-				Operations: []admission.Operation{admission.Create},
-			},
-			attrOperation: admission.Delete,
-			shouldMatch:   false,
-		},
-	}
-
-	runTests(t, "operations", tests)
-}
-
-func TestResourceMatches(t *testing.T) {
-	tests := []ruleTest{
-		ruleTest{
-			test:        "resources empty match",
-			rule:        Rule{},
-			shouldMatch: true,
-		},
-		ruleTest{
-			test: "resource match",
-			rule: Rule{
-				Resources: []string{"pods"},
-			},
-			attrResource: "pods",
-			shouldMatch:  true,
-		},
-		ruleTest{
-			test: "resource mismatch",
-			rule: Rule{
-				Resources: []string{"pods"},
-			},
-			attrResource: "secrets",
-			shouldMatch:  false,
-		},
-		ruleTest{
-			test: "resource with subresource match",
-			rule: Rule{
-				Resources: []string{"pods/status"},
-			},
-			attrResource:    "pods",
-			attrSubResource: "status",
-			shouldMatch:     true,
-		},
-		ruleTest{
-			test: "resource with subresource mismatch",
-			rule: Rule{
-				Resources: []string{"pods"},
-			},
-			attrResource:    "pods",
-			attrSubResource: "status",
-			shouldMatch:     false,
-		},
-	}
-
-	runTests(t, "resources", tests)
-}
-
-func TestResourceNameMatches(t *testing.T) {
-	tests := []ruleTest{
-		ruleTest{
-			test:        "resourceNames empty match",
-			rule:        Rule{},
-			shouldMatch: true,
-		},
-		ruleTest{
-			test: "resourceName match",
-			rule: Rule{
-				ResourceNames: []string{"my-name"},
-			},
-			attrResourceName: "my-name",
-			shouldMatch:      true,
-		},
-		ruleTest{
-			test: "resourceName mismatch",
-			rule: Rule{
-				ResourceNames: []string{"my-name"},
-			},
-			attrResourceName: "your-name",
-			shouldMatch:      false,
-		},
-	}
-
-	runTests(t, "resourceNames", tests)
-}
-
-func runTests(t *testing.T, prop string, tests []ruleTest) {
-	for _, tt := range tests {
-		if tt.attrResource == "" {
-			tt.attrResource = "pods"
-		}
-
-		res := api.Resource(tt.attrResource).WithVersion("version")
-
-		if tt.attrAPIGroup != "" {
-			res.Group = tt.attrAPIGroup
-		}
-
-		attr := admission.NewAttributesRecord(nil, nil, api.Kind("Pod").WithVersion("version"), tt.attrNamespace, tt.attrResourceName, res, tt.attrSubResource, tt.attrOperation, nil)
-		var attrVal string
-		var ruleVal []string
-		var matches bool
-
-		switch prop {
-		case "":
-			matches = Matches(tt.rule, attr)
-		case "apiGroups":
-			attrVal = tt.attrAPIGroup
-			matches = APIGroupMatches(tt.rule, attr)
-			ruleVal = tt.rule.APIGroups
-		case "namespaces":
-			attrVal = tt.attrNamespace
-			matches = NamespaceMatches(tt.rule, attr)
-			ruleVal = tt.rule.Namespaces
-		case "operations":
-			attrVal = string(tt.attrOperation)
-			matches = OperationMatches(tt.rule, attr)
-			ruleVal = make([]string, len(tt.rule.Operations))
-
-			for _, rOp := range tt.rule.Operations {
-				ruleVal = append(ruleVal, string(rOp))
-			}
-		case "resources":
-			attrVal = tt.attrResource
-			matches = ResourceMatches(tt.rule, attr)
-			ruleVal = tt.rule.Resources
-		case "resourceNames":
-			attrVal = tt.attrResourceName
-			matches = ResourceNamesMatches(tt.rule, attr)
-			ruleVal = tt.rule.ResourceNames
-		default:
-			t.Errorf("Unexpected test property: %s", prop)
-		}
-
-		if matches && !tt.shouldMatch {
-			if prop == "" {
-				testError(t, tt.test, "Expected match")
-			} else {
-				testError(t, tt.test, fmt.Sprintf("Expected %s rule property not to match %s against one of the following: %s", prop, attrVal, strings.Join(ruleVal, ", ")))
-			}
-		} else if !matches && tt.shouldMatch {
-			if prop == "" {
-				testError(t, tt.test, "Unexpected match")
-			} else {
-				testError(t, tt.test, fmt.Sprintf("Expected %s rule property to match %s against one of the following: %s", prop, attrVal, strings.Join(ruleVal, ", ")))
 			}
 		}
 	}
 }
 
-func testError(t *testing.T, name, msg string) {
+func TestVersion(t *testing.T) {
-	t.Errorf("test failed (%s): %s", name, msg)
+	table := tests{
+		"wildcard": {
+			rule: adreg.RuleWithOperations{
+				Rule: adreg.Rule{
+					APIVersions: []string{"*"},
+				},
+			},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+			),
+		},
+		"exact": {
+			rule: adreg.RuleWithOperations{
+				Rule: adreg.Rule{
+					APIVersions: []string{"v1", "v2"},
+				},
+			},
+			match: attrList(
+				a("g1", "v1", "r", "", "name", admission.Create),
+				a("g2", "v2", "r", "", "name", admission.Create),
+			),
+			noMatch: attrList(
+				a("g1", "v3", "r", "", "name", admission.Create),
+				a("g2", "v4", "r", "", "name", admission.Create),
+			),
+		},
+	}
+	for name, tt := range table {
+		for _, m := range tt.match {
+			r := RuleMatcher{tt.rule, m}
+			if !r.version() {
+				t.Errorf("%v: expected match %#v", name, m)
+			}
+		}
+		for _, m := range tt.noMatch {
+			r := RuleMatcher{tt.rule, m}
+			if r.version() {
+				t.Errorf("%v: expected no match %#v", name, m)
+			}
+		}
+	}
+}
+
+func TestOperation(t *testing.T) {
+	table := tests{
+		"wildcard": {
+			rule: adreg.RuleWithOperations{Operations: []adreg.OperationType{adreg.OperationAll}},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("g", "v", "r", "", "name", admission.Update),
+				a("g", "v", "r", "", "name", admission.Delete),
+				a("g", "v", "r", "", "name", admission.Connect),
+			),
+		},
+		"create": {
+			rule: adreg.RuleWithOperations{Operations: []adreg.OperationType{adreg.Create}},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+			),
+			noMatch: attrList(
+				a("g", "v", "r", "", "name", admission.Update),
+				a("g", "v", "r", "", "name", admission.Delete),
+				a("g", "v", "r", "", "name", admission.Connect),
+			),
+		},
+		"update": {
+			rule: adreg.RuleWithOperations{Operations: []adreg.OperationType{adreg.Update}},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Update),
+			),
+			noMatch: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("g", "v", "r", "", "name", admission.Delete),
+				a("g", "v", "r", "", "name", admission.Connect),
+			),
+		},
+		"delete": {
+			rule: adreg.RuleWithOperations{Operations: []adreg.OperationType{adreg.Delete}},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Delete),
+			),
+			noMatch: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("g", "v", "r", "", "name", admission.Update),
+				a("g", "v", "r", "", "name", admission.Connect),
+			),
+		},
+		"connect": {
+			rule: adreg.RuleWithOperations{Operations: []adreg.OperationType{adreg.Connect}},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Connect),
+			),
+			noMatch: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("g", "v", "r", "", "name", admission.Update),
+				a("g", "v", "r", "", "name", admission.Delete),
+			),
+		},
+		"multiple": {
+			rule: adreg.RuleWithOperations{Operations: []adreg.OperationType{adreg.Update, adreg.Delete}},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Update),
+				a("g", "v", "r", "", "name", admission.Delete),
+			),
+			noMatch: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("g", "v", "r", "", "name", admission.Connect),
+			),
+		},
+	}
+	for name, tt := range table {
+		for _, m := range tt.match {
+			r := RuleMatcher{tt.rule, m}
+			if !r.operation() {
+				t.Errorf("%v: expected match %#v", name, m)
+			}
+		}
+		for _, m := range tt.noMatch {
+			r := RuleMatcher{tt.rule, m}
+			if r.operation() {
+				t.Errorf("%v: expected no match %#v", name, m)
+			}
+		}
+	}
+}
+
+func TestResource(t *testing.T) {
+	table := tests{
+		"no subresources": {
+			rule: adreg.RuleWithOperations{
+				Rule: adreg.Rule{
+					Resources: []string{"*"},
+				},
+			},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("2", "v", "r2", "", "name", admission.Create),
+			),
+			noMatch: attrList(
+				a("g", "v", "r", "exec", "name", admission.Create),
+				a("2", "v", "r2", "proxy", "name", admission.Create),
+			),
+		},
+		"r & subresources": {
+			rule: adreg.RuleWithOperations{
+				Rule: adreg.Rule{
+					Resources: []string{"r/*"},
+				},
+			},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("g", "v", "r", "exec", "name", admission.Create),
+			),
+			noMatch: attrList(
+				a("2", "v", "r2", "", "name", admission.Create),
+				a("2", "v", "r2", "proxy", "name", admission.Create),
+			),
+		},
+		"r & subresources or r2": {
+			rule: adreg.RuleWithOperations{
+				Rule: adreg.Rule{
+					Resources: []string{"r/*", "r2"},
+				},
+			},
+			match: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("g", "v", "r", "exec", "name", admission.Create),
+				a("2", "v", "r2", "", "name", admission.Create),
+			),
+			noMatch: attrList(
+				a("2", "v", "r2", "proxy", "name", admission.Create),
+			),
+		},
+		"proxy or exec": {
+			rule: adreg.RuleWithOperations{
+				Rule: adreg.Rule{
+					Resources: []string{"*/proxy", "*/exec"},
+				},
+			},
+			match: attrList(
+				a("g", "v", "r", "exec", "name", admission.Create),
+				a("2", "v", "r2", "proxy", "name", admission.Create),
+				a("2", "v", "r3", "proxy", "name", admission.Create),
+			),
+			noMatch: attrList(
+				a("g", "v", "r", "", "name", admission.Create),
+				a("2", "v", "r2", "", "name", admission.Create),
+				a("2", "v", "r4", "scale", "name", admission.Create),
+			),
+		},
+	}
+	for name, tt := range table {
+		for _, m := range tt.match {
+			r := RuleMatcher{tt.rule, m}
+			if !r.resource() {
+				t.Errorf("%v: expected match %#v", name, m)
+			}
+		}
+		for _, m := range tt.noMatch {
+			r := RuleMatcher{tt.rule, m}
+			if r.resource() {
+				t.Errorf("%v: expected no match %#v", name, m)
+			}
+		}
+	}
 }

plugin/pkg/admission/webhook/types.go

@@ -1,69 +0,0 @@
-/*
-Copyright 2017 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-// Package webhook checks a webhook for configured operation admission
-package webhook
-
-import (
-	"time"
-
-	"k8s.io/apiserver/pkg/admission"
-)
-
-// FailAction is the action to take whenever a webhook fails and there are no more retries available
-type FailAction string
-
-const (
-	// Allow is the fail action taken whenever the webhook call fails and there are no more retries
-	Allow FailAction = "ALLOW"
-	// Deny is the fail action taken whenever the webhook call fails and there are no more retries
-	Deny FailAction = "DENY"
-)
-
-// GenericAdmissionWebhookConfig holds configuration for an admission webhook
-type GenericAdmissionWebhookConfig struct {
-	KubeConfigFile string        `json:"kubeConfigFile"`
-	RetryBackoff   time.Duration `json:"retryBackoff"`
-	Rules          []Rule        `json:"rules"`
-}
-
-// Rule is the type defining an admission rule in the admission controller configuration file
-type Rule struct {
-	// APIGroups is a list of API groups that contain the resource this rule applies to.
-	APIGroups []string `json:"apiGroups"`
-	// FailAction is the action to take whenever the webhook fails and there are no more retries (Default: DENY)
-	FailAction FailAction `json:"failAction"`
-	// Namespaces is a list of namespaces this rule applies to.
-	Namespaces []string `json:"namespaces"`
-	// Operations is a list of admission operations this rule applies to.
-	Operations []admission.Operation `json:"operations"`
-	// Resources is a list of resources this rule applies to.
-	Resources []string `json:"resources"`
-	// ResourceNames is a list of resource names this rule applies to.
-	ResourceNames []string `json:"resourceNames"`
-	// Type is the admission rule type
-	Type RuleType `json:"type"`
-}
-
-// RuleType is the type of admission rule
-type RuleType string
-
-const (
-	// Send is the rule type for when a matching admission.Attributes should be sent to the webhook
-	Send RuleType = "SEND"
-	// Skip is the rule type for when a matching admission.Attributes should not be sent to the webhook
-	Skip RuleType = "SKIP"
-)

staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/group_version.go

@@ -29,8 +29,8 @@ import (
 //
 // +protobuf.options.(gogoproto.goproto_stringer)=false
 type GroupResource struct {
-	Group    string `protobuf:"bytes,1,opt,name=group"`
+	Group    string `json:"group" protobuf:"bytes,1,opt,name=group"`
-	Resource string `protobuf:"bytes,2,opt,name=resource"`
+	Resource string `json:"resource" protobuf:"bytes,2,opt,name=resource"`
 }
 
 func (gr *GroupResource) String() string {
@@ -45,9 +45,9 @@ func (gr *GroupResource) String() string {
 //
 // +protobuf.options.(gogoproto.goproto_stringer)=false
 type GroupVersionResource struct {
-	Group    string `protobuf:"bytes,1,opt,name=group"`
+	Group    string `json:"group" protobuf:"bytes,1,opt,name=group"`
-	Version  string `protobuf:"bytes,2,opt,name=version"`
+	Version  string `json:"version" protobuf:"bytes,2,opt,name=version"`
-	Resource string `protobuf:"bytes,3,opt,name=resource"`
+	Resource string `json:"resource" protobuf:"bytes,3,opt,name=resource"`
 }
 
 func (gvr *GroupVersionResource) String() string {
@@ -59,8 +59,8 @@ func (gvr *GroupVersionResource) String() string {
 //
 // +protobuf.options.(gogoproto.goproto_stringer)=false
 type GroupKind struct {
-	Group string `protobuf:"bytes,1,opt,name=group"`
+	Group string `json:"group" protobuf:"bytes,1,opt,name=group"`
-	Kind  string `protobuf:"bytes,2,opt,name=kind"`
+	Kind  string `json:"kind" protobuf:"bytes,2,opt,name=kind"`
 }
 
 func (gk *GroupKind) String() string {
@@ -88,8 +88,8 @@ func (gvk GroupVersionKind) String() string {
 //
 // +protobuf.options.(gogoproto.goproto_stringer)=false
 type GroupVersion struct {
-	Group   string `protobuf:"bytes,1,opt,name=group"`
+	Group   string `json:"group" protobuf:"bytes,1,opt,name=group"`
-	Version string `protobuf:"bytes,2,opt,name=version"`
+	Version string `json:"version" protobuf:"bytes,2,opt,name=version"`
 }
 
 // Empty returns true if group and version are empty

staging/src/k8s.io/client-go/rest/request.go

@@ -1148,6 +1148,9 @@ func (r Result) Into(obj runtime.Object) error {
 	if r.decoder == nil {
 		return fmt.Errorf("serializer for %s doesn't exist", r.contentType)
 	}
+	if len(r.body) == 0 {
+		return fmt.Errorf("0-length response")
+	}
 
 	out, _, err := r.decoder.Decode(r.body, nil, obj)
 	if err != nil || out == obj {

staging/src/k8s.io/client-go/rest/request_test.go

@@ -329,6 +329,16 @@ func TestResultIntoWithErrReturnsErr(t *testing.T) {
 	}
 }
 
+func TestResultIntoWithNoBodyReturnsErr(t *testing.T) {
+	res := Result{
+		body:    []byte{},
+		decoder: scheme.Codecs.LegacyCodec(v1.SchemeGroupVersion),
+	}
+	if err := res.Into(&v1.Pod{}); err == nil || !strings.Contains(err.Error(), "0-length") {
+		t.Errorf("should have complained about 0 length body")
+	}
+}
+
 func TestURLTemplate(t *testing.T) {
 	uri, _ := url.Parse("http://localhost")
 	r := NewRequest(nil, "POST", uri, "", ContentConfig{GroupVersion: &schema.GroupVersion{Group: "test"}}, Serializers{}, nil, nil)

test/integration/etcd/etcd_storage_path_test.go

@@ -381,6 +381,10 @@ var ephemeralWhiteList = createEphemeralWhiteList(
 	// k8s.io/kubernetes/pkg/apis/policy/v1beta1
 	gvr("policy", "v1beta1", "evictions"), // not stored in etcd, deals with evicting kapiv1.Pod
 	// --
+
+	// k8s.io/kubernetes/pkg/apis/admission/v1alpha1
+	gvr("admission.k8s.io", "v1alpha1", "admissionreviews"), // not stored in etcd, call out to webhooks.
+	// --
 )
 
 // Only add kinds to this list when there is no mapping from GVK to GVR (and thus there is no way to create the object)