From 68ad63b5e253a03e931a28018fd766c3941cf039 Mon Sep 17 00:00:00 2001 From: Cesar Wong Date: Fri, 15 May 2015 10:48:33 -0400 Subject: [PATCH 1/3] Add operation checking to admission control handlers Adds a new method to the handler interface that returns true only if the admission control handler handles that operation. --- pkg/admission/attributes.go | 6 +- pkg/admission/chain.go | 18 +++ pkg/admission/chain_test.go | 152 ++++++++++++++++++ pkg/admission/handler.go | 44 +++++ pkg/admission/interfaces.go | 17 +- pkg/apiserver/resthandler.go | 48 +++--- plugin/pkg/admission/admit/admission.go | 5 + plugin/pkg/admission/deny/admission.go | 5 + plugin/pkg/admission/limitranger/admission.go | 15 +- .../namespace/autoprovision/admission.go | 15 +- .../namespace/autoprovision/admission_test.go | 13 +- .../admission/namespace/exists/admission.go | 7 +- .../namespace/lifecycle/admission.go | 10 +- .../namespace/lifecycle/admission_test.go | 16 +- .../pkg/admission/resourcequota/admission.go | 16 +- .../admission/resourcequota/admission_test.go | 44 ++--- .../securitycontext/scdeny/admission.go | 9 +- .../securitycontext/scdeny/admission_test.go | 16 ++ .../pkg/admission/serviceaccount/admission.go | 7 +- .../serviceaccount/admission_test.go | 27 ++-- 20 files changed, 384 insertions(+), 106 deletions(-) create mode 100644 pkg/admission/chain_test.go create mode 100644 pkg/admission/handler.go diff --git a/pkg/admission/attributes.go b/pkg/admission/attributes.go index 3726320bbfb..36aa4acdbef 100644 --- a/pkg/admission/attributes.go +++ b/pkg/admission/attributes.go @@ -25,12 +25,12 @@ type attributesRecord struct { kind string namespace string resource string - operation string + operation Operation object runtime.Object userInfo user.Info } -func NewAttributesRecord(object runtime.Object, kind, namespace, resource, operation string, userInfo user.Info) Attributes { +func NewAttributesRecord(object runtime.Object, kind, namespace, resource string, operation Operation, userInfo user.Info) Attributes { return &attributesRecord{ kind: kind, namespace: namespace, @@ -53,7 +53,7 @@ func (record *attributesRecord) GetResource() string { return record.resource } -func (record *attributesRecord) GetOperation() string { +func (record *attributesRecord) GetOperation() Operation { return record.operation } diff --git a/pkg/admission/chain.go b/pkg/admission/chain.go index 42fea1055a9..09d64b9545d 100644 --- a/pkg/admission/chain.go +++ b/pkg/admission/chain.go @@ -36,9 +36,17 @@ func NewFromPlugins(client client.Interface, pluginNames []string, configFilePat return chainAdmissionHandler(plugins) } +// NewChainHandler creates a new chain handler from an array of handlers. Used for testing. +func NewChainHandler(handlers ...Interface) Interface { + return chainAdmissionHandler(handlers) +} + // Admit performs an admission control check using a chain of handlers, and returns immediately on first error func (admissionHandler chainAdmissionHandler) Admit(a Attributes) error { for _, handler := range admissionHandler { + if !handler.Handles(a.GetOperation()) { + continue + } err := handler.Admit(a) if err != nil { return err @@ -46,3 +54,13 @@ func (admissionHandler chainAdmissionHandler) Admit(a Attributes) error { } return nil } + +// Handles will return true if any of the handlers handles the given operation +func (admissionHandler chainAdmissionHandler) Handles(operation Operation) bool { + for _, handler := range admissionHandler { + if handler.Handles(operation) { + return true + } + } + return false +} diff --git a/pkg/admission/chain_test.go b/pkg/admission/chain_test.go new file mode 100644 index 00000000000..bfb3c1d5b1e --- /dev/null +++ b/pkg/admission/chain_test.go @@ -0,0 +1,152 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 admission + +import ( + "fmt" + "testing" +) + +type FakeHandler struct { + *Handler + name string + admit bool + admitCalled bool +} + +func (h *FakeHandler) Admit(a Attributes) (err error) { + h.admitCalled = true + if h.admit { + return nil + } + return fmt.Errorf("Don't admit") +} + +func makeHandler(name string, admit bool, ops ...Operation) Interface { + return &FakeHandler{ + name: name, + admit: admit, + Handler: NewHandler(ops...), + } +} + +func TestAdmit(t *testing.T) { + tests := []struct { + name string + operation Operation + chain chainAdmissionHandler + accept bool + calls map[string]bool + }{ + { + name: "all accept", + operation: Create, + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", true, Delete, Create), + makeHandler("c", true, Create), + }, + calls: map[string]bool{"a": true, "b": true, "c": true}, + accept: true, + }, + { + name: "ignore handler", + operation: Create, + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", false, Delete), + makeHandler("c", true, Create), + }, + calls: map[string]bool{"a": true, "c": true}, + accept: true, + }, + { + name: "ignore all", + operation: Connect, + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", false, Delete), + makeHandler("c", true, Create), + }, + calls: map[string]bool{}, + accept: true, + }, + { + name: "reject one", + operation: Delete, + chain: []Interface{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", false, Delete), + makeHandler("c", true, Create), + }, + calls: map[string]bool{"a": true, "b": true}, + accept: false, + }, + } + for _, test := range tests { + err := test.chain.Admit(NewAttributesRecord(nil, "", "", "", test.operation, nil)) + accepted := (err == nil) + if accepted != test.accept { + t.Errorf("%s: unexpected result of admit call: %v\n", test.name, accepted) + } + for _, h := range test.chain { + fake := h.(*FakeHandler) + _, shouldBeCalled := test.calls[fake.name] + if shouldBeCalled != fake.admitCalled { + t.Errorf("%s: handler %s not called as expected: %v", test.name, fake.name, fake.admitCalled) + continue + } + } + } +} + +func TestHandles(t *testing.T) { + chain := chainAdmissionHandler{ + makeHandler("a", true, Update, Delete, Create), + makeHandler("b", true, Delete, Create), + makeHandler("c", true, Create), + } + + tests := []struct { + name string + operation Operation + chain chainAdmissionHandler + expected bool + }{ + { + name: "all handle", + operation: Create, + expected: true, + }, + { + name: "none handle", + operation: Connect, + expected: false, + }, + { + name: "some handle", + operation: Delete, + expected: true, + }, + } + for _, test := range tests { + handles := chain.Handles(test.operation) + if handles != test.expected { + t.Errorf("Unexpected handles result. Expected: %v. Actual: %v", test.expected, handles) + } + } +} diff --git a/pkg/admission/handler.go b/pkg/admission/handler.go new file mode 100644 index 00000000000..22998eac037 --- /dev/null +++ b/pkg/admission/handler.go @@ -0,0 +1,44 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 admission + +import ( + "github.com/GoogleCloudPlatform/kubernetes/pkg/util" +) + +// Handler is a base for admission control handlers that +// support a predefined set of operations +type Handler struct { + operations util.StringSet +} + +// Handles returns true for methods that this handler supports +func (h *Handler) Handles(operation Operation) bool { + return h.operations.Has(string(operation)) +} + +// NewHandler creates a new base handler that handles the passed +// in operations +func NewHandler(ops ...Operation) *Handler { + operations := util.NewStringSet() + for _, op := range ops { + operations.Insert(string(op)) + } + return &Handler{ + operations: operations, + } +} diff --git a/pkg/admission/interfaces.go b/pkg/admission/interfaces.go index 24b1c0230bd..c282dee9d29 100644 --- a/pkg/admission/interfaces.go +++ b/pkg/admission/interfaces.go @@ -26,7 +26,7 @@ import ( type Attributes interface { GetNamespace() string GetResource() string - GetOperation() string + GetOperation() Operation GetObject() runtime.Object GetKind() string GetUserInfo() user.Info @@ -36,4 +36,19 @@ type Attributes interface { type Interface interface { // Admit makes an admission decision based on the request attributes Admit(a Attributes) (err error) + + // Handles returns true if this admission controller can handle the given operation + // where operation can be one of CREATE, UPDATE, DELETE, or CONNECT + Handles(operation Operation) bool } + +// Operation is the type of resource operation being checked for admission control +type Operation string + +// Operation constants +const ( + Create Operation = "CREATE" + Update Operation = "UPDATE" + Delete Operation = "DELETE" + Connect Operation = "CONNECT" +) diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index b5cb3bbfce0..45d95e8989e 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -293,11 +293,13 @@ func createHandler(r rest.NamedCreater, scope RequestScope, typer runtime.Object return } - userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, scope.Resource, "CREATE", userInfo)) - if err != nil { - errorJSON(err, scope.Codec, w) - return + if admit.Handles(admission.Create) { + userInfo, _ := api.UserFrom(ctx) + err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, scope.Resource, admission.Create, userInfo)) + if err != nil { + errorJSON(err, scope.Codec, w) + return + } } result, err := finishRequest(timeout, func() (runtime.Object, error) { @@ -361,11 +363,13 @@ func PatchResource(r rest.Patcher, scope RequestScope, typer runtime.ObjectTyper ctx = api.WithNamespace(ctx, namespace) // PATCH requires same permission as UPDATE - userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, scope.Resource, "UPDATE", userInfo)) - if err != nil { - errorJSON(err, scope.Codec, w) - return + if admit.Handles(admission.Update) { + userInfo, _ := api.UserFrom(ctx) + err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, scope.Resource, admission.Update, userInfo)) + if err != nil { + errorJSON(err, scope.Codec, w) + return + } } versionedObj, err := converter.ConvertToVersion(obj, scope.APIVersion) @@ -459,11 +463,13 @@ func UpdateResource(r rest.Updater, scope RequestScope, typer runtime.ObjectType return } - userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, scope.Resource, "UPDATE", userInfo)) - if err != nil { - errorJSON(err, scope.Codec, w) - return + if admit.Handles(admission.Update) { + userInfo, _ := api.UserFrom(ctx) + err = admit.Admit(admission.NewAttributesRecord(obj, scope.Kind, namespace, scope.Resource, admission.Update, userInfo)) + if err != nil { + errorJSON(err, scope.Codec, w) + return + } } wasCreated := false @@ -521,11 +527,13 @@ func DeleteResource(r rest.GracefulDeleter, checkBody bool, scope RequestScope, } } - userInfo, _ := api.UserFrom(ctx) - err = admit.Admit(admission.NewAttributesRecord(nil, scope.Kind, namespace, scope.Resource, "DELETE", userInfo)) - if err != nil { - errorJSON(err, scope.Codec, w) - return + if admit.Handles(admission.Delete) { + userInfo, _ := api.UserFrom(ctx) + err = admit.Admit(admission.NewAttributesRecord(nil, scope.Kind, namespace, scope.Resource, admission.Delete, userInfo)) + if err != nil { + errorJSON(err, scope.Codec, w) + return + } } result, err := finishRequest(timeout, func() (runtime.Object, error) { diff --git a/plugin/pkg/admission/admit/admission.go b/plugin/pkg/admission/admit/admission.go index be4f92168e8..29f0e8fe436 100644 --- a/plugin/pkg/admission/admit/admission.go +++ b/plugin/pkg/admission/admit/admission.go @@ -37,6 +37,11 @@ func (alwaysAdmit) Admit(a admission.Attributes) (err error) { return nil } +func (alwaysAdmit) Handles(operation admission.Operation) bool { + return true +} + +// NewAlwaysAdmit creates a new always admit admission handler func NewAlwaysAdmit() admission.Interface { return new(alwaysAdmit) } diff --git a/plugin/pkg/admission/deny/admission.go b/plugin/pkg/admission/deny/admission.go index 2fee60f7bdf..af3c8b03858 100644 --- a/plugin/pkg/admission/deny/admission.go +++ b/plugin/pkg/admission/deny/admission.go @@ -38,6 +38,11 @@ func (alwaysDeny) Admit(a admission.Attributes) (err error) { return admission.NewForbidden(a, errors.New("Admission control is denying all modifications")) } +func (alwaysDeny) Handles(operation admission.Operation) bool { + return true +} + +// NewAlwaysDeny creates an always deny admission handler func NewAlwaysDeny() admission.Interface { return new(alwaysDeny) } diff --git a/plugin/pkg/admission/limitranger/admission.go b/plugin/pkg/admission/limitranger/admission.go index e233cdc90c1..ec6e83806b0 100644 --- a/plugin/pkg/admission/limitranger/admission.go +++ b/plugin/pkg/admission/limitranger/admission.go @@ -40,6 +40,7 @@ func init() { // limitRanger enforces usage limits on a per resource basis in the namespace type limitRanger struct { + *admission.Handler client client.Interface limitFunc LimitFunc indexer cache.Indexer @@ -47,11 +48,6 @@ type limitRanger struct { // Admit admits resources into cluster that do not violate any defined LimitRange in the namespace func (l *limitRanger) Admit(a admission.Attributes) (err error) { - // ignore deletes - if a.GetOperation() == "DELETE" { - return nil - } - obj := a.GetObject() resource := a.GetResource() name := "Unknown" @@ -99,9 +95,15 @@ func NewLimitRanger(client client.Interface, limitFunc LimitFunc) admission.Inte } indexer, reflector := cache.NewNamespaceKeyedIndexerAndReflector(lw, &api.LimitRange{}, 0) reflector.Run() - return &limitRanger{client: client, limitFunc: limitFunc, indexer: indexer} + return &limitRanger{ + Handler: admission.NewHandler(admission.Create, admission.Update), + client: client, + limitFunc: limitFunc, + indexer: indexer, + } } +// Min returns the lesser of its 2 arguments func Min(a int64, b int64) int64 { if a < b { return a @@ -109,6 +111,7 @@ func Min(a int64, b int64) int64 { return b } +// Max returns the greater of its 2 arguments func Max(a int64, b int64) int64 { if a > b { return a diff --git a/plugin/pkg/admission/namespace/autoprovision/admission.go b/plugin/pkg/admission/namespace/autoprovision/admission.go index dd4bc3cd930..334ffa21008 100644 --- a/plugin/pkg/admission/namespace/autoprovision/admission.go +++ b/plugin/pkg/admission/namespace/autoprovision/admission.go @@ -42,15 +42,12 @@ func init() { // It looks at all incoming requests in a namespace context, and if the namespace does not exist, it creates one. // It is useful in deployments that do not want to restrict creation of a namespace prior to its usage. type provision struct { + *admission.Handler client client.Interface store cache.Store } func (p *provision) Admit(a admission.Attributes) (err error) { - // only handle create requests - if a.GetOperation() != "CREATE" { - return nil - } defaultVersion, kind, err := latest.RESTMapper.VersionAndKindForResource(a.GetResource()) if err != nil { return admission.NewForbidden(a, err) @@ -83,6 +80,7 @@ func (p *provision) Admit(a admission.Attributes) (err error) { return nil } +// NewProvision creates a new namespace provision admission control handler func NewProvision(c client.Interface) admission.Interface { store := cache.NewStore(cache.MetaNamespaceKeyFunc) reflector := cache.NewReflector( @@ -99,8 +97,13 @@ func NewProvision(c client.Interface) admission.Interface { 0, ) reflector.Run() + return createProvision(c, store) +} + +func createProvision(c client.Interface, store cache.Store) admission.Interface { return &provision{ - client: c, - store: store, + Handler: admission.NewHandler(admission.Create), + client: c, + store: store, } } diff --git a/plugin/pkg/admission/namespace/autoprovision/admission_test.go b/plugin/pkg/admission/namespace/autoprovision/admission_test.go index 470f193a076..edaa4a1ad9b 100644 --- a/plugin/pkg/admission/namespace/autoprovision/admission_test.go +++ b/plugin/pkg/admission/namespace/autoprovision/admission_test.go @@ -41,7 +41,7 @@ func TestAdmission(t *testing.T) { Containers: []api.Container{{Name: "ctr", Image: "image"}}, }, } - err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "CREATE", nil)) + err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", admission.Create, nil)) if err != nil { t.Errorf("Unexpected error returned from admission handler") } @@ -72,7 +72,7 @@ func TestAdmissionNamespaceExists(t *testing.T) { Containers: []api.Container{{Name: "ctr", Image: "image"}}, }, } - err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "CREATE", nil)) + err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", admission.Create, nil)) if err != nil { t.Errorf("Unexpected error returned from admission handler") } @@ -85,10 +85,7 @@ func TestAdmissionNamespaceExists(t *testing.T) { func TestIgnoreAdmission(t *testing.T) { namespace := "test" mockClient := &testclient.Fake{} - handler := &provision{ - client: mockClient, - store: cache.NewStore(cache.MetaNamespaceKeyFunc), - } + handler := admission.NewChainHandler(createProvision(mockClient, nil)) pod := api.Pod{ ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespace}, Spec: api.PodSpec{ @@ -96,7 +93,7 @@ func TestIgnoreAdmission(t *testing.T) { Containers: []api.Container{{Name: "ctr", Image: "image"}}, }, } - err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "UPDATE", nil)) + err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", admission.Update, nil)) if err != nil { t.Errorf("Unexpected error returned from admission handler") } @@ -123,7 +120,7 @@ func TestAdmissionNamespaceExistsUnknownToHandler(t *testing.T) { Containers: []api.Container{{Name: "ctr", Image: "image"}}, }, } - err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", "CREATE", nil)) + err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespace, "pods", admission.Create, nil)) if err != nil { t.Errorf("Unexpected error returned from admission handler") } diff --git a/plugin/pkg/admission/namespace/exists/admission.go b/plugin/pkg/admission/namespace/exists/admission.go index 944ab332a92..04e97e374ce 100644 --- a/plugin/pkg/admission/namespace/exists/admission.go +++ b/plugin/pkg/admission/namespace/exists/admission.go @@ -42,6 +42,7 @@ func init() { // It rejects all incoming requests in a namespace context if the namespace does not exist. // It is useful in deployments that want to enforce pre-declaration of a Namespace resource. type exists struct { + *admission.Handler client client.Interface store cache.Store } @@ -75,6 +76,7 @@ func (e *exists) Admit(a admission.Attributes) (err error) { return admission.NewForbidden(a, fmt.Errorf("Namespace %s does not exist", a.GetNamespace())) } +// NewExists creates a new namespace exists admission control handler func NewExists(c client.Interface) admission.Interface { store := cache.NewStore(cache.MetaNamespaceKeyFunc) reflector := cache.NewReflector( @@ -92,7 +94,8 @@ func NewExists(c client.Interface) admission.Interface { ) reflector.Run() return &exists{ - client: c, - store: store, + client: c, + store: store, + Handler: admission.NewHandler(admission.Create, admission.Update, admission.Delete), } } diff --git a/plugin/pkg/admission/namespace/lifecycle/admission.go b/plugin/pkg/admission/namespace/lifecycle/admission.go index 0b36388ae0f..d6530a8297c 100644 --- a/plugin/pkg/admission/namespace/lifecycle/admission.go +++ b/plugin/pkg/admission/namespace/lifecycle/admission.go @@ -41,14 +41,12 @@ func init() { // lifecycle is an implementation of admission.Interface. // It enforces life-cycle constraints around a Namespace depending on its Phase type lifecycle struct { + *admission.Handler client client.Interface store cache.Store } func (l *lifecycle) Admit(a admission.Attributes) (err error) { - if a.GetOperation() != "CREATE" { - return nil - } defaultVersion, kind, err := latest.RESTMapper.VersionAndKindForResource(a.GetResource()) if err != nil { return admission.NewForbidden(a, err) @@ -80,6 +78,7 @@ func (l *lifecycle) Admit(a admission.Attributes) (err error) { return admission.NewForbidden(a, fmt.Errorf("Namespace %s is terminating", a.GetNamespace())) } +// NewLifecycle creates a new namespace lifecycle admission control handler func NewLifecycle(c client.Interface) admission.Interface { store := cache.NewStore(cache.MetaNamespaceKeyFunc) reflector := cache.NewReflector( @@ -97,7 +96,8 @@ func NewLifecycle(c client.Interface) admission.Interface { ) reflector.Run() return &lifecycle{ - client: c, - store: store, + Handler: admission.NewHandler(admission.Create), + client: c, + store: store, } } diff --git a/plugin/pkg/admission/namespace/lifecycle/admission_test.go b/plugin/pkg/admission/namespace/lifecycle/admission_test.go index 43ec26336a2..5c3ce76e271 100644 --- a/plugin/pkg/admission/namespace/lifecycle/admission_test.go +++ b/plugin/pkg/admission/namespace/lifecycle/admission_test.go @@ -39,10 +39,9 @@ func TestAdmission(t *testing.T) { store := cache.NewStore(cache.MetaNamespaceIndexFunc) store.Add(namespaceObj) mockClient := &testclient.Fake{} - handler := &lifecycle{ - client: mockClient, - store: store, - } + lfhandler := NewLifecycle(mockClient).(*lifecycle) + lfhandler.store = store + handler := admission.NewChainHandler(lfhandler) pod := api.Pod{ ObjectMeta: api.ObjectMeta{Name: "123", Namespace: namespaceObj.Namespace}, Spec: api.PodSpec{ @@ -50,7 +49,7 @@ func TestAdmission(t *testing.T) { Containers: []api.Container{{Name: "ctr", Image: "image"}}, }, } - err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", "CREATE", nil)) + err := handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", admission.Create, nil)) if err != nil { t.Errorf("Unexpected error returned from admission handler: %v", err) } @@ -60,21 +59,20 @@ func TestAdmission(t *testing.T) { store.Add(namespaceObj) // verify create operations in the namespace cause an error - err = handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", "CREATE", nil)) + err = handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", admission.Create, nil)) if err == nil { t.Errorf("Expected error rejecting creates in a namespace when it is terminating") } // verify update operations in the namespace can proceed - err = handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", "UPDATE", nil)) + err = handler.Admit(admission.NewAttributesRecord(&pod, "Pod", namespaceObj.Namespace, "pods", admission.Update, nil)) if err != nil { t.Errorf("Unexpected error returned from admission handler: %v", err) } // verify delete operations in the namespace can proceed - err = handler.Admit(admission.NewAttributesRecord(nil, "Pod", namespaceObj.Namespace, "pods", "DELETE", nil)) + err = handler.Admit(admission.NewAttributesRecord(nil, "Pod", namespaceObj.Namespace, "pods", admission.Delete, nil)) if err != nil { t.Errorf("Unexpected error returned from admission handler: %v", err) } - } diff --git a/plugin/pkg/admission/resourcequota/admission.go b/plugin/pkg/admission/resourcequota/admission.go index 9d17a96b2ed..af43288fb8d 100644 --- a/plugin/pkg/admission/resourcequota/admission.go +++ b/plugin/pkg/admission/resourcequota/admission.go @@ -42,10 +42,12 @@ func init() { } type quota struct { + *admission.Handler client client.Interface indexer cache.Indexer } +// NewResourceQuota creates a new resource quota admission control handler func NewResourceQuota(client client.Interface) admission.Interface { lw := &cache.ListWatch{ ListFunc: func() (runtime.Object, error) { @@ -57,7 +59,15 @@ func NewResourceQuota(client client.Interface) admission.Interface { } indexer, reflector := cache.NewNamespaceKeyedIndexerAndReflector(lw, &api.ResourceQuota{}, 0) reflector.Run() - return "a{client: client, indexer: indexer} + return createResourceQuota(client, indexer) +} + +func createResourceQuota(client client.Interface, indexer cache.Indexer) admission.Interface { + return "a{ + Handler: admission.NewHandler(admission.Create, admission.Update), + client: client, + indexer: indexer, + } } var resourceToResourceName = map[string]api.ResourceName{ @@ -161,7 +171,7 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli } obj := a.GetObject() // handle max counts for each kind of resource (pods, services, replicationControllers, etc.) - if a.GetOperation() == "CREATE" { + if a.GetOperation() == admission.Create { // TODO v1beta1 had camel case, v1beta3 went to all lower, we can remove this line when we deprecate v1beta1 resourceNormalized := strings.ToLower(a.GetResource()) resourceName := resourceToResourceName[resourceNormalized] @@ -185,7 +195,7 @@ func IncrementUsage(a admission.Attributes, status *api.ResourceQuotaStatus, cli deltaCPU := resourcequota.PodCPU(pod) deltaMemory := resourcequota.PodMemory(pod) // if this is an update, we need to find the delta cpu/memory usage from previous state - if a.GetOperation() == "UPDATE" { + if a.GetOperation() == admission.Update { oldPod, err := client.Pods(a.GetNamespace()).Get(pod.Name) if err != nil { return false, err diff --git a/plugin/pkg/admission/resourcequota/admission_test.go b/plugin/pkg/admission/resourcequota/admission_test.go index 18a44552a96..97992ce8b0e 100644 --- a/plugin/pkg/admission/resourcequota/admission_test.go +++ b/plugin/pkg/admission/resourcequota/admission_test.go @@ -40,10 +40,10 @@ func getResourceRequirements(cpu, memory string) api.ResourceRequirements { func TestAdmissionIgnoresDelete(t *testing.T) { namespace := "default" - handler := NewResourceQuota(&testclient.Fake{}) - err := handler.Admit(admission.NewAttributesRecord(nil, "Pod", namespace, "pods", "DELETE", nil)) + handler := createResourceQuota(&testclient.Fake{}, nil) + err := handler.Admit(admission.NewAttributesRecord(nil, "Pod", namespace, "pods", admission.Delete, nil)) if err != nil { - t.Errorf("ResourceQuota should admit all deletes", err) + t.Errorf("ResourceQuota should admit all deletes: %v", err) } } @@ -67,9 +67,9 @@ func TestIncrementUsagePods(t *testing.T) { r := api.ResourcePods status.Hard[r] = resource.MustParse("2") status.Used[r] = resource.MustParse("1") - dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "pods", "CREATE", nil), status, client) + dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "pods", admission.Create, nil), status, client) if err != nil { - t.Errorf("Unexpected error", err) + t.Errorf("Unexpected error: %v", err) } if !dirty { t.Errorf("Expected the status to get incremented, therefore should have been dirty") @@ -107,9 +107,9 @@ func TestIncrementUsageMemory(t *testing.T) { Volumes: []api.Volume{{Name: "vol"}}, Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, }} - dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE", nil), status, client) + dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", admission.Create, nil), status, client) if err != nil { - t.Errorf("Unexpected error", err) + t.Errorf("Unexpected error: %v", err) } if !dirty { t.Errorf("Expected the status to get incremented, therefore should have been dirty") @@ -148,7 +148,7 @@ func TestExceedUsageMemory(t *testing.T) { Volumes: []api.Volume{{Name: "vol"}}, Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "3Gi")}}, }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected memory usage exceeded error") } @@ -181,9 +181,9 @@ func TestIncrementUsageCPU(t *testing.T) { Volumes: []api.Volume{{Name: "vol"}}, Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("100m", "1Gi")}}, }} - dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE", nil), status, client) + dirty, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", admission.Create, nil), status, client) if err != nil { - t.Errorf("Unexpected error", err) + t.Errorf("Unexpected error: %v", err) } if !dirty { t.Errorf("Expected the status to get incremented, therefore should have been dirty") @@ -222,7 +222,7 @@ func TestUnboundedCPU(t *testing.T) { Volumes: []api.Volume{{Name: "vol"}}, Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("0m", "1Gi")}}, }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected CPU unbounded usage error") } @@ -255,7 +255,7 @@ func TestUnboundedMemory(t *testing.T) { Volumes: []api.Volume{{Name: "vol"}}, Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("250m", "0")}}, }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected memory unbounded usage error") } @@ -288,7 +288,7 @@ func TestExceedUsageCPU(t *testing.T) { Volumes: []api.Volume{{Name: "vol"}}, Containers: []api.Container{{Name: "ctr", Image: "image", Resources: getResourceRequirements("500m", "1Gi")}}, }} - _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(newPod, "Pod", namespace, "pods", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected CPU usage exceeded error") } @@ -314,7 +314,7 @@ func TestExceedUsagePods(t *testing.T) { r := api.ResourcePods status.Hard[r] = resource.MustParse("1") status.Used[r] = resource.MustParse("1") - _, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "pods", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(&api.Pod{}, "Pod", namespace, "pods", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected error because this would exceed your quota") } @@ -336,9 +336,9 @@ func TestIncrementUsageServices(t *testing.T) { r := api.ResourceServices status.Hard[r] = resource.MustParse("2") status.Used[r] = resource.MustParse("1") - dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, "Service", namespace, "services", "CREATE", nil), status, client) + dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, "Service", namespace, "services", admission.Create, nil), status, client) if err != nil { - t.Errorf("Unexpected error", err) + t.Errorf("Unexpected error: %v", err) } if !dirty { t.Errorf("Expected the status to get incremented, therefore should have been dirty") @@ -365,7 +365,7 @@ func TestExceedUsageServices(t *testing.T) { r := api.ResourceServices status.Hard[r] = resource.MustParse("1") status.Used[r] = resource.MustParse("1") - _, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, "Service", namespace, "services", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(&api.Service{}, "Service", namespace, "services", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected error because this would exceed usage") } @@ -387,9 +387,9 @@ func TestIncrementUsageReplicationControllers(t *testing.T) { r := api.ResourceReplicationControllers status.Hard[r] = resource.MustParse("2") status.Used[r] = resource.MustParse("1") - dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, "ReplicationController", namespace, "replicationControllers", "CREATE", nil), status, client) + dirty, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, "ReplicationController", namespace, "replicationControllers", admission.Create, nil), status, client) if err != nil { - t.Errorf("Unexpected error", err) + t.Errorf("Unexpected error: %v", err) } if !dirty { t.Errorf("Expected the status to get incremented, therefore should have been dirty") @@ -416,7 +416,7 @@ func TestExceedUsageReplicationControllers(t *testing.T) { r := api.ResourceReplicationControllers status.Hard[r] = resource.MustParse("1") status.Used[r] = resource.MustParse("1") - _, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, "ReplicationController", namespace, "replicationControllers", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(&api.ReplicationController{}, "ReplicationController", namespace, "replicationControllers", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected error for exceeding hard limits") } @@ -438,7 +438,7 @@ func TestExceedUsageSecrets(t *testing.T) { r := api.ResourceSecrets status.Hard[r] = resource.MustParse("1") status.Used[r] = resource.MustParse("1") - _, err := IncrementUsage(admission.NewAttributesRecord(&api.Secret{}, "Secret", namespace, "secrets", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(&api.Secret{}, "Secret", namespace, "secrets", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected error for exceeding hard limits") } @@ -460,7 +460,7 @@ func TestExceedUsagePersistentVolumeClaims(t *testing.T) { r := api.ResourcePersistentVolumeClaims status.Hard[r] = resource.MustParse("1") status.Used[r] = resource.MustParse("1") - _, err := IncrementUsage(admission.NewAttributesRecord(&api.PersistentVolumeClaim{}, "PersistentVolumeClaim", namespace, "persistentVolumeClaims", "CREATE", nil), status, client) + _, err := IncrementUsage(admission.NewAttributesRecord(&api.PersistentVolumeClaim{}, "PersistentVolumeClaim", namespace, "persistentVolumeClaims", admission.Create, nil), status, client) if err == nil { t.Errorf("Expected error for exceeding hard limits") } diff --git a/plugin/pkg/admission/securitycontext/scdeny/admission.go b/plugin/pkg/admission/securitycontext/scdeny/admission.go index 2362ae3fe5d..373c070f191 100644 --- a/plugin/pkg/admission/securitycontext/scdeny/admission.go +++ b/plugin/pkg/admission/securitycontext/scdeny/admission.go @@ -34,20 +34,21 @@ func init() { // plugin contains the client used by the SecurityContextDeny admission controller type plugin struct { + *admission.Handler client client.Interface } // NewSecurityContextDeny creates a new instance of the SecurityContextDeny admission controller func NewSecurityContextDeny(client client.Interface) admission.Interface { - return &plugin{client} + return &plugin{ + Handler: admission.NewHandler(admission.Create, admission.Update), + client: client, + } } // Admit will deny any SecurityContext that defines options that were not previously available in the api.Container // struct (Capabilities and Privileged) func (p *plugin) Admit(a admission.Attributes) (err error) { - if a.GetOperation() == "DELETE" { - return nil - } if a.GetResource() != string(api.ResourcePods) { return nil } diff --git a/plugin/pkg/admission/securitycontext/scdeny/admission_test.go b/plugin/pkg/admission/securitycontext/scdeny/admission_test.go index f7daf17b715..6a0cd111bf0 100644 --- a/plugin/pkg/admission/securitycontext/scdeny/admission_test.go +++ b/plugin/pkg/admission/securitycontext/scdeny/admission_test.go @@ -63,3 +63,19 @@ func TestAdmission(t *testing.T) { } } } + +func TestHandles(t *testing.T) { + handler := NewSecurityContextDeny(nil) + tests := map[admission.Operation]bool{ + admission.Update: true, + admission.Create: true, + admission.Delete: false, + admission.Connect: false, + } + for op, expected := range tests { + result := handler.Handles(op) + if result != expected { + t.Errorf("Unexpected result for operation %s: %v\n", op, result) + } + } +} diff --git a/plugin/pkg/admission/serviceaccount/admission.go b/plugin/pkg/admission/serviceaccount/admission.go index 8d94ba5a6c1..6b8d0754753 100644 --- a/plugin/pkg/admission/serviceaccount/admission.go +++ b/plugin/pkg/admission/serviceaccount/admission.go @@ -53,6 +53,8 @@ func init() { var _ = admission.Interface(&serviceAccount{}) type serviceAccount struct { + *admission.Handler + // LimitSecretReferences rejects pods that reference secrets their service accounts do not reference LimitSecretReferences bool // MountServiceAccountToken creates Volume and VolumeMounts for the first referenced ServiceAccountToken for the pod's service account @@ -102,6 +104,7 @@ func NewServiceAccount(cl client.Interface) *serviceAccount { ) return &serviceAccount{ + Handler: admission.NewHandler(admission.Create), // TODO: enable this once we've swept secret usage to account for adding secret references to service accounts LimitSecretReferences: false, // Auto mount service account API token secrets @@ -130,10 +133,6 @@ func (s *serviceAccount) Stop() { } func (s *serviceAccount) Admit(a admission.Attributes) (err error) { - // We only care about Pod CREATE operations - if a.GetOperation() != "CREATE" { - return nil - } if a.GetResource() != string(api.ResourcePods) { return nil } diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index 284953bf31f..d7f6b6d0f2e 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -29,9 +29,10 @@ import ( func TestIgnoresNonCreate(t *testing.T) { pod := &api.Pod{} - for _, op := range []string{"UPDATE", "DELETE", "CUSTOM"} { + for _, op := range []admission.Operation{admission.Update, admission.Delete, admission.Connect} { attrs := admission.NewAttributesRecord(pod, "Pod", "myns", string(api.ResourcePods), op, nil) - err := NewServiceAccount(nil).Admit(attrs) + handler := admission.NewChainHandler(NewServiceAccount(nil)) + err := handler.Admit(attrs) if err != nil { t.Errorf("Expected %s operation allowed, got err: %v", op, err) } @@ -40,7 +41,7 @@ func TestIgnoresNonCreate(t *testing.T) { func TestIgnoresNonPodResource(t *testing.T) { pod := &api.Pod{} - attrs := admission.NewAttributesRecord(pod, "Pod", "myns", "CustomResource", "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", "myns", "CustomResource", admission.Create, nil) err := NewServiceAccount(nil).Admit(attrs) if err != nil { t.Errorf("Expected non-pod resource allowed, got err: %v", err) @@ -48,7 +49,7 @@ func TestIgnoresNonPodResource(t *testing.T) { } func TestIgnoresNilObject(t *testing.T) { - attrs := admission.NewAttributesRecord(nil, "Pod", "myns", string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(nil, "Pod", "myns", string(api.ResourcePods), admission.Create, nil) err := NewServiceAccount(nil).Admit(attrs) if err != nil { t.Errorf("Expected nil object allowed allowed, got err: %v", err) @@ -57,7 +58,7 @@ func TestIgnoresNilObject(t *testing.T) { func TestIgnoresNonPodObject(t *testing.T) { obj := &api.Namespace{} - attrs := admission.NewAttributesRecord(obj, "Pod", "myns", string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(obj, "Pod", "myns", string(api.ResourcePods), admission.Create, nil) err := NewServiceAccount(nil).Admit(attrs) if err != nil { t.Errorf("Expected non pod object allowed, got err: %v", err) @@ -77,7 +78,7 @@ func TestIgnoresMirrorPod(t *testing.T) { }, }, } - attrs := admission.NewAttributesRecord(pod, "Pod", "myns", string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", "myns", string(api.ResourcePods), admission.Create, nil) err := NewServiceAccount(nil).Admit(attrs) if err != nil { t.Errorf("Expected mirror pod without service account or secrets allowed, got err: %v", err) @@ -95,7 +96,7 @@ func TestRejectsMirrorPodWithServiceAccount(t *testing.T) { ServiceAccount: "default", }, } - attrs := admission.NewAttributesRecord(pod, "Pod", "myns", string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", "myns", string(api.ResourcePods), admission.Create, nil) err := NewServiceAccount(nil).Admit(attrs) if err == nil { t.Errorf("Expected a mirror pod to be prevented from referencing a service account") @@ -115,7 +116,7 @@ func TestRejectsMirrorPodWithSecretVolumes(t *testing.T) { }, }, } - attrs := admission.NewAttributesRecord(pod, "Pod", "myns", string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", "myns", string(api.ResourcePods), admission.Create, nil) err := NewServiceAccount(nil).Admit(attrs) if err == nil { t.Errorf("Expected a mirror pod to be prevented from referencing a secret volume") @@ -137,7 +138,7 @@ func TestAssignsDefaultServiceAccountAndToleratesMissingAPIToken(t *testing.T) { }) pod := &api.Pod{} - attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), admission.Create, nil) err := admit.Admit(attrs) if err != nil { t.Errorf("Unexpected error: %v", err) @@ -161,7 +162,7 @@ func TestFetchesUncachedServiceAccount(t *testing.T) { admit := NewServiceAccount(client) pod := &api.Pod{} - attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), admission.Create, nil) err := admit.Admit(attrs) if err != nil { t.Errorf("Unexpected error: %v", err) @@ -180,7 +181,7 @@ func TestDeniesInvalidServiceAccount(t *testing.T) { admit := NewServiceAccount(client) pod := &api.Pod{} - attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), admission.Create, nil) err := admit.Admit(attrs) if err == nil { t.Errorf("Expected error for missing service account, got none") @@ -242,7 +243,7 @@ func TestAutomountsAPIToken(t *testing.T) { }, }, } - attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), admission.Create, nil) err := admit.Admit(attrs) if err != nil { t.Errorf("Unexpected error: %v", err) @@ -391,7 +392,7 @@ func TestRejectsUnreferencedSecretVolumes(t *testing.T) { }, }, } - attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), "CREATE", nil) + attrs := admission.NewAttributesRecord(pod, "Pod", ns, string(api.ResourcePods), admission.Create, nil) err := admit.Admit(attrs) if err == nil { t.Errorf("Expected rejection for using a secret the service account does not reference") From 328b1d08173e9240ee2af2d43077b2b9fab23913 Mon Sep 17 00:00:00 2001 From: Cesar Wong Date: Fri, 8 May 2015 15:02:12 -0400 Subject: [PATCH 2/3] Add admission control to the Connect method in the API Server The resource passed to admission control is a ConnectRequest object which includes additional information about the current request. --- pkg/api/rest/rest.go | 15 +++++++++++++++ pkg/apiserver/api_installer.go | 2 +- pkg/apiserver/resthandler.go | 15 ++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/pkg/api/rest/rest.go b/pkg/api/rest/rest.go index fbb80b47f5a..fbb8a131576 100644 --- a/pkg/api/rest/rest.go +++ b/pkg/api/rest/rest.go @@ -246,3 +246,18 @@ type StorageMetadata interface { // PATCH) can respond with. ProducesMIMETypes(verb string) []string } + +// ConnectRequest is an object passed to admission control for Connect operations +type ConnectRequest struct { + // Name is the name of the object on which the connect request was made + Name string + + // Options is the options object passed to the connect request. See the NewConnectOptions method on Connecter + Options runtime.Object + + // ResourcePath is the path for the resource in the REST server (ie. "pods/proxy") + ResourcePath string +} + +// IsAnAPIObject makes ConnectRequest a runtime.Object +func (*ConnectRequest) IsAnAPIObject() {} diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index 7aa68b871c0..2099d5458d6 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -539,7 +539,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag case "CONNECT": for _, method := range connecter.ConnectMethods() { route := ws.Method(method).Path(action.Path). - To(ConnectResource(connecter, reqScope, connectOptionsKind, connectSubpath, connectSubpathKey)). + To(ConnectResource(connecter, reqScope, admit, connectOptionsKind, path, connectSubpath, connectSubpathKey)). Filter(m). Doc("connect " + method + " requests to " + kind). Operation("connect" + method + kind). diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index 45d95e8989e..66733822730 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -142,7 +142,7 @@ func getRequestOptions(req *restful.Request, scope RequestScope, kind string, su } // ConnectResource returns a function that handles a connect request on a rest.Storage object. -func ConnectResource(connecter rest.Connecter, scope RequestScope, connectOptionsKind string, subpath bool, subpathKey string) restful.RouteFunction { +func ConnectResource(connecter rest.Connecter, scope RequestScope, admit admission.Interface, connectOptionsKind, restPath string, subpath bool, subpathKey string) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter namespace, name, err := scope.Namer.Name(req) @@ -157,6 +157,19 @@ func ConnectResource(connecter rest.Connecter, scope RequestScope, connectOption errorJSON(err, scope.Codec, w) return } + if admit.Handles(admission.Connect) { + connectRequest := &rest.ConnectRequest{ + Name: name, + Options: opts, + ResourcePath: restPath, + } + userInfo, _ := api.UserFrom(ctx) + err = admit.Admit(admission.NewAttributesRecord(connectRequest, scope.Kind, namespace, scope.Resource, admission.Connect, userInfo)) + if err != nil { + errorJSON(err, scope.Codec, w) + return + } + } handler, err := connecter.Connect(ctx, name, opts) if err != nil { errorJSON(err, scope.Codec, w) From e95d9c416df0101c8ecd03f27254b34ea3fb0d46 Mon Sep 17 00:00:00 2001 From: Cesar Wong Date: Tue, 12 May 2015 11:15:46 -0400 Subject: [PATCH 3/3] Admission control to prevent exec on privileged pods --- cmd/kube-apiserver/app/plugins.go | 1 + .../exec/denyprivileged/admission.go | 81 +++++++++++++++++ .../exec/denyprivileged/admission_test.go | 91 +++++++++++++++++++ 3 files changed, 173 insertions(+) create mode 100644 plugin/pkg/admission/exec/denyprivileged/admission.go create mode 100644 plugin/pkg/admission/exec/denyprivileged/admission_test.go diff --git a/cmd/kube-apiserver/app/plugins.go b/cmd/kube-apiserver/app/plugins.go index 0e49415ed11..40381f8d043 100644 --- a/cmd/kube-apiserver/app/plugins.go +++ b/cmd/kube-apiserver/app/plugins.go @@ -32,6 +32,7 @@ import ( // Admission policies _ "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/admit" _ "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/deny" + _ "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/exec/denyprivileged" _ "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/limitranger" _ "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/namespace/autoprovision" _ "github.com/GoogleCloudPlatform/kubernetes/plugin/pkg/admission/namespace/exists" diff --git a/plugin/pkg/admission/exec/denyprivileged/admission.go b/plugin/pkg/admission/exec/denyprivileged/admission.go new file mode 100644 index 00000000000..a019198dfbb --- /dev/null +++ b/plugin/pkg/admission/exec/denyprivileged/admission.go @@ -0,0 +1,81 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 denyprivileged + +import ( + "fmt" + "io" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/admission" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/client" +) + +func init() { + admission.RegisterPlugin("DenyExecOnPrivileged", func(client client.Interface, config io.Reader) (admission.Interface, error) { + return NewDenyExecOnPrivileged(client), nil + }) +} + +// denyExecOnPrivileged is an implementation of admission.Interface which says no to a pod/exec on +// a privileged pod +type denyExecOnPrivileged struct { + *admission.Handler + client client.Interface +} + +func (d *denyExecOnPrivileged) Admit(a admission.Attributes) (err error) { + connectRequest, ok := a.GetObject().(*rest.ConnectRequest) + if !ok { + return errors.NewBadRequest("a connect request was received, but could not convert the request object.") + } + // Only handle exec requests on pods + if connectRequest.ResourcePath != "pods/exec" { + return nil + } + pod, err := d.client.Pods(a.GetNamespace()).Get(connectRequest.Name) + if err != nil { + return admission.NewForbidden(a, err) + } + if isPrivileged(pod) { + return admission.NewForbidden(a, fmt.Errorf("Cannot exec into a privileged container")) + } + return nil +} + +// isPrivileged will return true a pod has any privileged containers +func isPrivileged(pod *api.Pod) bool { + for _, c := range pod.Spec.Containers { + if c.SecurityContext == nil { + continue + } + if *c.SecurityContext.Privileged { + return true + } + } + return false +} + +// NewDenyExecOnPrivileged creates a new admission controller that denies an exec operation on a privileged pod +func NewDenyExecOnPrivileged(client client.Interface) admission.Interface { + return &denyExecOnPrivileged{ + Handler: admission.NewHandler(admission.Connect), + client: client, + } +} diff --git a/plugin/pkg/admission/exec/denyprivileged/admission_test.go b/plugin/pkg/admission/exec/denyprivileged/admission_test.go new file mode 100644 index 00000000000..e3e90aa71cd --- /dev/null +++ b/plugin/pkg/admission/exec/denyprivileged/admission_test.go @@ -0,0 +1,91 @@ +/* +Copyright 2015 The Kubernetes Authors All rights reserved. + +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 denyprivileged + +import ( + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/admission" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/client/testclient" + "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" +) + +// TestAdmission verifies a namespace is created on create requests for namespace managed resources +func TestAdmissionAccept(t *testing.T) { + testAdmission(t, acceptPod("podname"), true) +} + +func TestAdmissionDeny(t *testing.T) { + testAdmission(t, denyPod("podname"), false) +} + +func testAdmission(t *testing.T, pod *api.Pod, shouldAccept bool) { + mockClient := &testclient.Fake{ + ReactFn: func(action testclient.FakeAction) (runtime.Object, error) { + if action.Action == "get-pod" && action.Value.(string) == pod.Name { + return pod, nil + } + t.Errorf("Unexpected API call: %#v", action) + return nil, nil + }, + } + handler := &denyExecOnPrivileged{ + client: mockClient, + } + req := &rest.ConnectRequest{Name: pod.Name, ResourcePath: "pods/exec"} + err := handler.Admit(admission.NewAttributesRecord(req, "Pod", "test", "pods", admission.Connect, nil)) + if shouldAccept && err != nil { + t.Errorf("Unexpected error returned from admission handler: %v", err) + } + if !shouldAccept && err == nil { + t.Errorf("An error was expected from the admission handler. Received nil") + } + +} + +func acceptPod(name string) *api.Pod { + return &api.Pod{ + ObjectMeta: api.ObjectMeta{Name: name, Namespace: "test"}, + Spec: api.PodSpec{ + Containers: []api.Container{ + {Name: "ctr1", Image: "image"}, + {Name: "ctr2", Image: "image2"}, + }, + }, + } +} + +func denyPod(name string) *api.Pod { + privileged := true + return &api.Pod{ + ObjectMeta: api.ObjectMeta{Name: name, Namespace: "test"}, + Spec: api.PodSpec{ + Containers: []api.Container{ + {Name: "ctr1", Image: "image"}, + { + Name: "ctr2", + Image: "image2", + SecurityContext: &api.SecurityContext{ + Privileged: &privileged, + }, + }, + }, + }, + } +}