From 0797d812220be9b76716d366f13215b94b70bf5d Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Sun, 24 Feb 2019 15:18:05 -0500 Subject: [PATCH] Add scope restrictions to webhook admission rules --- .../admissionregistration/fuzzer/fuzzer.go | 7 ++ pkg/apis/admissionregistration/types.go | 24 ++++ .../admissionregistration/v1beta1/defaults.go | 7 ++ .../validation/validation.go | 9 ++ .../admissionregistration/v1beta1/types.go | 24 ++++ .../admission/plugin/webhook/rules/rules.go | 24 +++- .../plugin/webhook/rules/rules_test.go | 116 ++++++++++++++++++ 7 files changed, 210 insertions(+), 1 deletion(-) diff --git a/pkg/apis/admissionregistration/fuzzer/fuzzer.go b/pkg/apis/admissionregistration/fuzzer/fuzzer.go index c68785abaa..5b5b84f2e8 100644 --- a/pkg/apis/admissionregistration/fuzzer/fuzzer.go +++ b/pkg/apis/admissionregistration/fuzzer/fuzzer.go @@ -26,6 +26,13 @@ import ( // Funcs returns the fuzzer functions for the admissionregistration api group. var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} { return []interface{}{ + func(obj *admissionregistration.Rule, c fuzz.Continue) { + c.FuzzNoCustom(obj) // fuzz self without calling this function again + if obj.Scope == nil { + s := admissionregistration.AllScopes + obj.Scope = &s + } + }, func(obj *admissionregistration.Webhook, c fuzz.Continue) { c.FuzzNoCustom(obj) // fuzz self without calling this function again p := admissionregistration.FailurePolicyType("Fail") diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index b95a14b854..a296590e5a 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/pkg/apis/admissionregistration/types.go @@ -49,8 +49,32 @@ type Rule struct { // Depending on the enclosing object, subresources might not be allowed. // Required. Resources []string + + // scope specifies the scope of this rule. + // Valid values are "Cluster", "Namespaced", and "*" + // "Cluster" means that only cluster-scoped resources will match this rule. + // Namespace API objects are cluster-scoped. + // "Namespaced" means that only namespaced resources will match this rule. + // "*" means that there are no scope restrictions. + // Subresources match the scope of their parent resource. + // Default is "*". + // + // +optional + Scope *ScopeType } +type ScopeType string + +const ( + // ClusterScope means that scope is limited to cluster-scoped objects. + // Namespace objects are cluster-scoped. + ClusterScope ScopeType = "Cluster" + // NamespacedScope means that scope is limited to namespaced objects. + NamespacedScope ScopeType = "Namespaced" + // AllScopes means that all scopes are included. + AllScopes ScopeType = "*" +) + type FailurePolicyType string const ( diff --git a/pkg/apis/admissionregistration/v1beta1/defaults.go b/pkg/apis/admissionregistration/v1beta1/defaults.go index 81decaae25..20a318956c 100644 --- a/pkg/apis/admissionregistration/v1beta1/defaults.go +++ b/pkg/apis/admissionregistration/v1beta1/defaults.go @@ -45,3 +45,10 @@ func SetDefaults_Webhook(obj *admissionregistrationv1beta1.Webhook) { *obj.TimeoutSeconds = 30 } } + +func SetDefaults_Rule(obj *admissionregistrationv1beta1.Rule) { + if obj.Scope == nil { + s := admissionregistrationv1beta1.AllScopes + obj.Scope = &s + } +} diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 9cd0da0d9e..955d5d7ad0 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -113,6 +113,12 @@ func validateResourcesNoSubResources(resources []string, fldPath *field.Path) fi return allErrors } +var validScopes = sets.NewString( + string(admissionregistration.ClusterScope), + string(admissionregistration.NamespacedScope), + string(admissionregistration.AllScopes), +) + func validateRule(rule *admissionregistration.Rule, fldPath *field.Path, allowSubResource bool) field.ErrorList { var allErrors field.ErrorList if len(rule.APIGroups) == 0 { @@ -138,6 +144,9 @@ func validateRule(rule *admissionregistration.Rule, fldPath *field.Path, allowSu } else { allErrors = append(allErrors, validateResourcesNoSubResources(rule.Resources, fldPath.Child("resources"))...) } + if rule.Scope != nil && !validScopes.Has(string(*rule.Scope)) { + allErrors = append(allErrors, field.NotSupported(fldPath.Child("scope"), *rule.Scope, validScopes.List())) + } return allErrors } diff --git a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go index 7968372b39..48c3826121 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go +++ b/staging/src/k8s.io/api/admissionregistration/v1beta1/types.go @@ -49,8 +49,32 @@ type Rule struct { // Depending on the enclosing object, subresources might not be allowed. // Required. Resources []string `json:"resources,omitempty" protobuf:"bytes,3,rep,name=resources"` + + // scope specifies the scope of this rule. + // Valid values are "Cluster", "Namespaced", and "*" + // "Cluster" means that only cluster-scoped resources will match this rule. + // Namespace API objects are cluster-scoped. + // "Namespaced" means that only namespaced resources will match this rule. + // "*" means that there are no scope restrictions. + // Subresources match the scope of their parent resource. + // Default is "*". + // + // +optional + Scope *ScopeType `json:"scope,omitempty" protobuf:"bytes,4,rep,name=scope"` } +type ScopeType string + +const ( + // ClusterScope means that scope is limited to cluster-scoped objects. + // Namespace objects are cluster-scoped. + ClusterScope ScopeType = "Cluster" + // NamespacedScope means that scope is limited to namespaced objects. + NamespacedScope ScopeType = "Namespaced" + // AllScopes means that all scopes are included. + AllScopes ScopeType = "*" +) + type FailurePolicyType string const ( diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules.go index 096ab5021d..050885323d 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules.go @@ -20,6 +20,8 @@ import ( "strings" "k8s.io/api/admissionregistration/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/admission" ) @@ -31,7 +33,8 @@ type Matcher struct { // Matches returns if the Attr matches the Rule. func (r *Matcher) Matches() bool { - return r.operation() && + return r.scope() && + r.operation() && r.group() && r.version() && r.resource() @@ -50,6 +53,25 @@ func exactOrWildcard(items []string, requested string) bool { return false } +var namespaceResource = schema.GroupVersionResource{Group: "", Version: "v1", Resource: "namespaces"} + +func (r *Matcher) scope() bool { + if r.Rule.Scope == nil || *r.Rule.Scope == v1beta1.AllScopes { + return true + } + // attr.GetNamespace() is set to the name of the namespace for requests of the namespace object itself. + switch *r.Rule.Scope { + case v1beta1.NamespacedScope: + // first make sure that we are not requesting a namespace object (namespace objects are cluster-scoped) + return r.Attr.GetResource() != namespaceResource && r.Attr.GetNamespace() != metav1.NamespaceNone + case v1beta1.ClusterScope: + // also return true if the request is for a namespace object (namespace objects are cluster-scoped) + return r.Attr.GetResource() == namespaceResource || r.Attr.GetNamespace() == metav1.NamespaceNone + default: + return false + } +} + func (r *Matcher) group() bool { return exactOrWildcard(r.Rule.APIGroups, r.Attr.GetResource().Group) } diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules_test.go index 2827558af2..85fba433eb 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/rules/rules_test.go @@ -17,10 +17,12 @@ limitations under the License. package rules import ( + "fmt" "testing" adreg "k8s.io/api/admissionregistration/v1beta1" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/admission" ) @@ -43,6 +45,30 @@ func a(group, version, resource, subresource, name string, operation admission.O ) } +func namespacedAttributes(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, + false, + nil, + ) +} + +func clusterScopedAttributes(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}, + "", name, + schema.GroupVersionResource{Group: group, Version: version, Resource: resource}, subresource, + operation, + false, + nil, + ) +} + func attrList(a ...admission.Attributes) []admission.Attributes { return a } @@ -299,3 +325,93 @@ func TestResource(t *testing.T) { } } } + +func TestScope(t *testing.T) { + cluster := adreg.ClusterScope + namespace := adreg.NamespacedScope + allscopes := adreg.AllScopes + table := tests{ + "cluster scope": { + rule: adreg.RuleWithOperations{ + Rule: adreg.Rule{ + Resources: []string{"*"}, + Scope: &cluster, + }, + }, + match: attrList( + clusterScopedAttributes("g", "v", "r", "", "name", admission.Create), + clusterScopedAttributes("g", "v", "r", "exec", "name", admission.Create), + clusterScopedAttributes("", "v1", "namespaces", "", "ns", admission.Create), + clusterScopedAttributes("", "v1", "namespaces", "finalize", "ns", admission.Create), + namespacedAttributes("", "v1", "namespaces", "", "ns", admission.Create), + namespacedAttributes("", "v1", "namespaces", "finalize", "ns", admission.Create), + ), + noMatch: attrList( + namespacedAttributes("g", "v", "r", "", "name", admission.Create), + namespacedAttributes("g", "v", "r", "exec", "name", admission.Create), + ), + }, + "namespace scope": { + rule: adreg.RuleWithOperations{ + Rule: adreg.Rule{ + Resources: []string{"*"}, + Scope: &namespace, + }, + }, + match: attrList( + namespacedAttributes("g", "v", "r", "", "name", admission.Create), + namespacedAttributes("g", "v", "r", "exec", "name", admission.Create), + ), + noMatch: attrList( + clusterScopedAttributes("", "v1", "namespaces", "", "ns", admission.Create), + clusterScopedAttributes("", "v1", "namespaces", "finalize", "ns", admission.Create), + namespacedAttributes("", "v1", "namespaces", "", "ns", admission.Create), + namespacedAttributes("", "v1", "namespaces", "finalize", "ns", admission.Create), + clusterScopedAttributes("g", "v", "r", "", "name", admission.Create), + clusterScopedAttributes("g", "v", "r", "exec", "name", admission.Create), + ), + }, + "all scopes": { + rule: adreg.RuleWithOperations{ + Rule: adreg.Rule{ + Resources: []string{"*"}, + Scope: &allscopes, + }, + }, + match: attrList( + namespacedAttributes("g", "v", "r", "", "name", admission.Create), + namespacedAttributes("g", "v", "r", "exec", "name", admission.Create), + clusterScopedAttributes("g", "v", "r", "", "name", admission.Create), + clusterScopedAttributes("g", "v", "r", "exec", "name", admission.Create), + clusterScopedAttributes("", "v1", "namespaces", "", "ns", admission.Create), + clusterScopedAttributes("", "v1", "namespaces", "finalize", "ns", admission.Create), + namespacedAttributes("", "v1", "namespaces", "", "ns", admission.Create), + namespacedAttributes("", "v1", "namespaces", "finalize", "ns", admission.Create), + ), + noMatch: attrList(), + }, + } + keys := sets.NewString() + for name := range table { + keys.Insert(name) + } + for _, name := range keys.List() { + tt := table[name] + for i, m := range tt.match { + t.Run(fmt.Sprintf("%s_match_%d", name, i), func(t *testing.T) { + r := Matcher{tt.rule, m} + if !r.scope() { + t.Errorf("%v: expected match %#v", name, m) + } + }) + } + for i, m := range tt.noMatch { + t.Run(fmt.Sprintf("%s_nomatch_%d", name, i), func(t *testing.T) { + r := Matcher{tt.rule, m} + if r.scope() { + t.Errorf("%v: expected no match %#v", name, m) + } + }) + } + } +}