diff --git a/cmd/kube-apiserver/app/options/options.go b/cmd/kube-apiserver/app/options/options.go index ee69ed645b0..52d4da26dc6 100644 --- a/cmd/kube-apiserver/app/options/options.go +++ b/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, - "client certificate used to prove the identity of the aggragator or kube-apiserver when it proxies requests to a user api-server") - fs.StringVar(&s.ProxyClientKeyFile, "proxy-client-key-file", s.ProxyClientKeyFile, - "client certificate key used to prove the identity of the aggragator or kube-apiserver when it proxies requests to a user api-server") + fs.StringVar(&s.ProxyClientCertFile, "proxy-client-cert-file", s.ProxyClientCertFile, ""+ + "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. 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.") diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 2e92c5fe14a..e370e242b6b 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/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 } diff --git a/hack/.linted_packages b/hack/.linted_packages index 06151acf41f..63486946914 100644 --- a/hack/.linted_packages +++ b/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 diff --git a/pkg/apis/admission/v1alpha1/types.go b/pkg/apis/admission/v1alpha1/types.go index 453b7843632..64e156c22fa 100644 --- a/pkg/apis/admission/v1alpha1/types.go +++ b/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"` diff --git a/pkg/kubeapiserver/admission/init_test.go b/pkg/kubeapiserver/admission/init_test.go index 27440f6c4a3..7176f14a72b 100644 --- a/pkg/kubeapiserver/admission/init_test.go +++ b/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) + } +} diff --git a/pkg/kubeapiserver/admission/initializer.go b/pkg/kubeapiserver/admission/initializer.go index 540029045e9..be56e6efdd6 100644 --- a/pkg/kubeapiserver/admission/initializer.go +++ b/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 { - internalClient internalclientset.Interface - informers informers.SharedInformerFactory - authorizer authorizer.Authorizer - cloudConfig []byte - restMapper meta.RESTMapper - quotaRegistry quota.Registry +// WantsServiceResolver defines a fuction that accepts a ServiceResolver for +// admission plugins that need to make calls to services. +type WantsServiceResolver interface { + SetServiceResolver(ServiceResolver) } -var _ admission.PluginInitializer = pluginInitializer{} +// 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{} // 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 { - return pluginInitializer{ + quotaRegistry quota.Registry) *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) + } } diff --git a/pkg/kubectl/cmd/apply_test.go b/pkg/kubectl/cmd/apply_test.go index 23f17179c46..a559d730452 100644 --- a/pkg/kubectl/cmd/apply_test.go +++ b/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) { diff --git a/pkg/kubectl/cmd/create_deployment_test.go b/pkg/kubectl/cmd/create_deployment_test.go index 067f7e650f1..9dd6720b785 100644 --- a/pkg/kubectl/cmd/create_deployment_test.go +++ b/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 }), } diff --git a/pkg/kubectl/cmd/run_test.go b/pkg/kubectl/cmd/run_test.go index 7f1eed8eee6..e922e70dbef 100644 --- a/pkg/kubectl/cmd/run_test.go +++ b/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 } }), diff --git a/pkg/master/import_known_versions_test.go b/pkg/master/import_known_versions_test.go index 49764685f6a..5047efaf42c 100644 --- a/pkg/master/import_known_versions_test.go +++ b/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) { diff --git a/plugin/pkg/admission/webhook/admission.go b/plugin/pkg/admission/webhook/admission.go index 11d165509e2..7575d7aed80 100644 --- a/plugin/pkg/admission/webhook/admission.go +++ b/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 { - WebhookConfig GenericAdmissionWebhookConfig `json:"webhook"` - } - - d := yaml.NewYAMLOrJSONDecoder(configFile, 4096) - err := d.Decode(&gwhConfig) - - if err != nil { - return nil, err - } - - plugin, err := NewGenericAdmissionWebhook(&gwhConfig.WebhookConfig) - + plugin, err := NewGenericAdmissionWebhook() 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) { - 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 - } - +func NewGenericAdmissionWebhook() (*GenericAdmissionWebhook, error) { return &GenericAdmissionWebhook{ - Handler: admission.NewHandler(admission.Connect, admission.Create, admission.Delete, admission.Update), - webhook: gw, - rules: config.Rules, + Handler: admission.NewHandler( + admission.Connect, + 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 - rules []Rule + hookSource admissioninit.WebhookSource + 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) { - var matched *Rule +func (a *GenericAdmissionWebhook) Admit(attr admission.Attributes) error { + 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 - for i, rule := range a.rules { - if Matches(rule, attr) { - glog.V(2).Infof("rule at index %d matched request", i) - matched = &a.rules[i] + errCh := make(chan error, len(hooks)) + wg := sync.WaitGroup{} + wg.Add(len(hooks)) + for i := range hooks { + 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 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") + if !matches { return nil } // Make the webhook request request := admissionv1alpha1.NewAdmissionReview(attr) - response := a.webhook.RestClient.Post().Body(&request).Do() - - // 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 { + client, err := a.hookClient(h) if err != nil { - glog.V(2).Infof("error contacting webhook backend: %s", 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") + return &ErrCallingWebhook{WebhookName: h.Name, Reason: err} + } + if err := client.Post().Context(ctx).Body(&request).Do().Into(&request); err != nil { + return &ErrCallingWebhook{WebhookName: h.Name, Reason: err} } - return nil + if request.Status.Allowed { + return nil + } + + 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, + } } -// TODO: Allow configuring the serialization strategy +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) +} diff --git a/plugin/pkg/admission/webhook/admission_test.go b/plugin/pkg/admission/webhook/admission_test.go index 1ff95ba18e0..15f331f6510 100644 --- a/plugin/pkg/admission/webhook/admission_test.go +++ b/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" - "path/filepath" - "regexp" + "net/url" + "strings" "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/client-go/tools/clientcmd/api/v1" + "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/admission/v1alpha1" + "k8s.io/kubernetes/pkg/apis/admissionregistration" _ "k8s.io/kubernetes/pkg/apis/admission/install" ) -const ( - errAdmitPrefix = `pods "my-pod" is forbidden: ` -) - -var ( - requestCount int - testRoot string - testServer *httptest.Server -) - -type genericAdmissionWebhookTest struct { - test string - config *GenericAdmissionWebhookConfig - value interface{} +type fakeHookSource struct { + hooks []admissionregistration.ExternalAdmissionHook + err error } -func TestMain(m *testing.M) { - tmpRoot, err := ioutil.TempDir("", "") - - if err != nil { - killTests(err) +func (f *fakeHookSource) List() ([]admissionregistration.ExternalAdmissionHook, error) { + if f.err != nil { + return nil, f.err } - - testRoot = tmpRoot - - // Cleanup - defer os.RemoveAll(testRoot) - - // Create the test webhook server - cert, err := tls.X509KeyPair(clientCert, clientKey) - - if err != nil { - killTests(err) - } - - rootCAs := x509.NewCertPool() - - rootCAs.AppendCertsFromPEM(caCert) - - 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() + return f.hooks, nil } -// 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, - }, - } +type fakeServiceResolver struct { + base url.URL +} - for _, tt := range tests { - _, err := NewGenericAdmissionWebhook(tt.config) - - checkForError(t, tt.test, tt.value, err) +func (f fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) { + if namespace == "failResolve" { + return nil, fmt.Errorf("couldn't resolve service location") } + 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{ - genericAdmissionWebhookTest{ - test: "No matching rule", - config: &GenericAdmissionWebhookConfig{ - KubeConfigFile: filepath.Join(testRoot, "kube-config"), - Rules: []Rule{ - Rule{ - Operations: []admission.Operation{ - admission.Create, - }, - Type: Send, - }, - }, + table := map[string]test{ + "no match": { + hookSource: fakeHookSource{ + hooks: []admissionregistration.ExternalAdmissionHook{{ + Name: "nomatch", + ClientConfig: ccfg("disallow"), + Rules: []admissionregistration.RuleWithOperations{{ + Operations: []admissionregistration.OperationType{admissionregistration.Create}, + }}, + }}, }, - value: nil, + expectAllow: true, }, - genericAdmissionWebhookTest{ - test: "Matching rule skips", - config: &GenericAdmissionWebhookConfig{ - KubeConfigFile: filepath.Join(testRoot, "kube-config"), - Rules: []Rule{ - Rule{ - Operations: []admission.Operation{ - admission.Update, - }, - Type: Skip, - }, - }, + "match & allow": { + hookSource: fakeHookSource{ + hooks: []admissionregistration.ExternalAdmissionHook{{ + Name: "allow", + ClientConfig: ccfg("allow"), + 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), + "match & disallow": { + hookSource: fakeHookSource{ + hooks: []admissionregistration.ExternalAdmissionHook{{ + Name: "disallow", + ClientConfig: ccfg("disallow"), + Rules: matchEverythingRules, + }}, + }, + errorContains: "without explanation", }, - genericAdmissionWebhookTest{ - test: "Matching rule sends (webhook unsuccessful response)", - config: makeGoodConfig(), - value: fmt.Errorf("%serror contacting webhook: 101", errAdmitPrefix), + "match & disallow ii": { + hookSource: fakeHookSource{ + hooks: []admissionregistration.ExternalAdmissionHook{{ + Name: "disallowReason", + ClientConfig: ccfg("disallowReason"), + Rules: matchEverythingRules, + }}, + }, + errorContains: "you shall not pass", }, - 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, + "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, + }}, + }, + expectAllow: true, }, } - for _, tt := range tests { - wh, err := NewGenericAdmissionWebhook(tt.config) + for name, tt := range table { + 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)) - - checkForError(t, tt.test, tt.value, err) + err = wh.Admit(admission.NewAttributesRecord(&object, &oldObject, kind, namespace, name, resource, subResource, operation, &userInfo)) + if tt.expectAllow != (err == nil) { + t.Errorf("%q: expected allowed=%v, but got err=%v", name, tt.expectAllow, 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()) + // 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) + } } - } 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++ - - switch requestCount { - case 1, 4: + fmt.Printf("got req: %v\n", r.URL.Path) + switch r.URL.Path { + case "/internalErr": 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) } } diff --git a/plugin/pkg/admission/webhook/config.go b/plugin/pkg/admission/webhook/config.go deleted file mode 100644 index 8a70ac5de31..00000000000 --- a/plugin/pkg/admission/webhook/config.go +++ /dev/null @@ -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 -} diff --git a/plugin/pkg/admission/webhook/config_test.go b/plugin/pkg/admission/webhook/config_test.go deleted file mode 100644 index 16e342c5913..00000000000 --- a/plugin/pkg/admission/webhook/config_test.go +++ /dev/null @@ -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) - } - } - } -} diff --git a/plugin/pkg/admission/webhook/rules.go b/plugin/pkg/admission/webhook/rules.go index 99521e87c65..2ae2afee5a1 100644 --- a/plugin/pkg/admission/webhook/rules.go +++ b/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 { - for i, item := range items { - if item == requested { - return i + "k8s.io/apiserver/pkg/admission" + "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 } - } - - 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 { + if item == requested { 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 NamespaceMatches(rule Rule, attr admission.Attributes) bool { - if len(rule.Namespaces) == 0 { - return true - } - - return indexOf(rule.Namespaces, attr.GetNamespace()) > -1 +func (r *RuleMatcher) group() bool { + return exactOrWildcard(r.Rule.APIGroups, r.Attr.GetResource().Group) } -// 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 - - if len(attr.GetSubresource()) > 0 { - resource = attr.GetResource().Resource + "/" + attr.GetSubresource() - } - - return indexOf(rule.Resources, resource) > -1 +func (r *RuleMatcher) version() bool { + return exactOrWildcard(r.Rule.APIVersions, r.Attr.GetResource().Version) } -// ResourceNamesMatches returns if the admission.Attributes matches the rule's resource names -func ResourceNamesMatches(rule Rule, attr admission.Attributes) bool { - if len(rule.ResourceNames) == 0 { - return true +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 + // is tested). + if op == admissionregistration.OperationType(attrOp) { + return true + } } - - return indexOf(rule.ResourceNames, attr.GetName()) > -1 + return false +} + +func splitResource(resSub string) (res, sub string) { + parts := strings.SplitN(resSub, "/", 2) + if len(parts) == 2 { + return parts[0], parts[1] + } + return parts[0], "" +} + +func (r *RuleMatcher) resource() bool { + opRes, opSub := r.Attr.GetResource().Resource, r.Attr.GetSubresource() + for _, res := range r.Rule.Resources { + res, sub := splitResource(res) + resMatch := res == "*" || res == opRes + subMatch := sub == "*" || sub == opSub + if resMatch && subMatch { + return true + } + } + return false } diff --git a/plugin/pkg/admission/webhook/rules_test.go b/plugin/pkg/admission/webhook/rules_test.go index 036dd8059ff..dc6a79fbc3a 100644 --- a/plugin/pkg/admission/webhook/rules_test.go +++ b/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 Rule - attrAPIGroup string - attrNamespace string - attrOperation admission.Operation - attrResource string - attrResourceName string - attrSubResource string - shouldMatch bool + rule adreg.RuleWithOperations + match []admission.Attributes + noMatch []admission.Attributes +} +type tests map[string]ruleTest + +func a(group, version, resource, subresource, name string, operation admission.Operation) admission.Attributes { + return admission.NewAttributesRecord( + 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) { - tests := []ruleTest{ - ruleTest{ - test: "apiGroups empty match", - rule: Rule{}, - shouldMatch: true, - }, - ruleTest{ - test: "apiGroup match", - rule: Rule{ - APIGroups: []string{"my-group"}, +func attrList(a ...admission.Attributes) []admission.Attributes { + return a +} + +func TestGroup(t *testing.T) { + table := tests{ + "wildcard": { + rule: adreg.RuleWithOperations{ + Rule: adreg.Rule{ + APIGroups: []string{"*"}, + }, }, - attrAPIGroup: "my-group", - shouldMatch: true, + match: attrList( + a("g", "v", "r", "", "name", admission.Create), + ), }, - ruleTest{ - test: "apiGroup mismatch", - rule: Rule{ - APIGroups: []string{"my-group"}, + "exact": { + rule: adreg.RuleWithOperations{ + Rule: adreg.Rule{ + 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) -} - -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", - }, - } - - runTests(t, "", tests) -} - -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)) + 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) } - 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, ", "))) + for _, m := range tt.noMatch { + r := RuleMatcher{tt.rule, m} + if r.group() { + t.Errorf("%v: expected no match %#v", name, m) } } } } -func testError(t *testing.T, name, msg string) { - t.Errorf("test failed (%s): %s", name, msg) +func TestVersion(t *testing.T) { + 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) + } + } + } } diff --git a/plugin/pkg/admission/webhook/types.go b/plugin/pkg/admission/webhook/types.go deleted file mode 100644 index 157fdb1edc3..00000000000 --- a/plugin/pkg/admission/webhook/types.go +++ /dev/null @@ -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" -) diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/group_version.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/group_version.go index bd0e9612a64..bd4c6d9b586 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/group_version.go +++ b/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"` - Resource string `protobuf:"bytes,2,opt,name=resource"` + Group string `json:"group" protobuf:"bytes,1,opt,name=group"` + 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"` - Version string `protobuf:"bytes,2,opt,name=version"` - Resource string `protobuf:"bytes,3,opt,name=resource"` + Group string `json:"group" protobuf:"bytes,1,opt,name=group"` + Version string `json:"version" protobuf:"bytes,2,opt,name=version"` + 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"` - Kind string `protobuf:"bytes,2,opt,name=kind"` + Group string `json:"group" protobuf:"bytes,1,opt,name=group"` + 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"` - Version string `protobuf:"bytes,2,opt,name=version"` + Group string `json:"group" protobuf:"bytes,1,opt,name=group"` + Version string `json:"version" protobuf:"bytes,2,opt,name=version"` } // Empty returns true if group and version are empty diff --git a/staging/src/k8s.io/client-go/rest/request.go b/staging/src/k8s.io/client-go/rest/request.go index b87ddaff51b..cfb4511bab8 100644 --- a/staging/src/k8s.io/client-go/rest/request.go +++ b/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 { diff --git a/staging/src/k8s.io/client-go/rest/request_test.go b/staging/src/k8s.io/client-go/rest/request_test.go index 15bf851d5a9..cedac794aed 100755 --- a/staging/src/k8s.io/client-go/rest/request_test.go +++ b/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) diff --git a/test/integration/etcd/etcd_storage_path_test.go b/test/integration/etcd/etcd_storage_path_test.go index e2712ab98bb..d487127b338 100644 --- a/test/integration/etcd/etcd_storage_path_test.go +++ b/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)