From 64cd09f7208e7a45d87ab6436c833c984fa6e594 Mon Sep 17 00:00:00 2001 From: Alexander Zielenski Date: Tue, 20 Feb 2024 09:22:18 -0800 Subject: [PATCH] refactor: use match from generic pkg in vap It is same exact code, but uses accessors now --- .../policy/validating/admission_test.go | 28 +++---- .../plugin/policy/validating/dispatcher.go | 8 +- .../plugin/policy/validating/interface.go | 18 ---- .../plugin/policy/validating/matcher.go | 83 ------------------- .../plugin/policy/validating/plugin.go | 2 +- 5 files changed, 19 insertions(+), 120 deletions(-) delete mode 100644 staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/matcher.go diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/admission_test.go index cc8ee5ccbe4..b2ed6519199 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/admission_test.go @@ -261,7 +261,7 @@ func (f validateFunc) Validate( ) } -var _ validating.Matcher = &fakeMatcher{} +var _ generic.PolicyMatcher = &fakeMatcher{} func (f *fakeMatcher) ValidateInitialization() error { return nil @@ -273,11 +273,11 @@ func (f *fakeMatcher) GetNamespace(name string) (*v1.Namespace, error) { type fakeMatcher struct { DefaultMatch bool - DefinitionMatchFuncs map[types.NamespacedName]func(*v1beta1.ValidatingAdmissionPolicy, admission.Attributes) bool - BindingMatchFuncs map[types.NamespacedName]func(*v1beta1.ValidatingAdmissionPolicyBinding, admission.Attributes) bool + DefinitionMatchFuncs map[types.NamespacedName]func(generic.PolicyAccessor, admission.Attributes) bool + BindingMatchFuncs map[types.NamespacedName]func(generic.BindingAccessor, admission.Attributes) bool } -func (f *fakeMatcher) RegisterDefinition(definition *v1beta1.ValidatingAdmissionPolicy, matchFunc func(*v1beta1.ValidatingAdmissionPolicy, admission.Attributes) bool) { +func (f *fakeMatcher) RegisterDefinition(definition *v1beta1.ValidatingAdmissionPolicy, matchFunc func(generic.PolicyAccessor, admission.Attributes) bool) { namespace, name := definition.Namespace, definition.Name key := types.NamespacedName{ Name: name, @@ -286,13 +286,13 @@ func (f *fakeMatcher) RegisterDefinition(definition *v1beta1.ValidatingAdmission if matchFunc != nil { if f.DefinitionMatchFuncs == nil { - f.DefinitionMatchFuncs = make(map[types.NamespacedName]func(*v1beta1.ValidatingAdmissionPolicy, admission.Attributes) bool) + f.DefinitionMatchFuncs = make(map[types.NamespacedName]func(generic.PolicyAccessor, admission.Attributes) bool) } f.DefinitionMatchFuncs[key] = matchFunc } } -func (f *fakeMatcher) RegisterBinding(binding *v1beta1.ValidatingAdmissionPolicyBinding, matchFunc func(*v1beta1.ValidatingAdmissionPolicyBinding, admission.Attributes) bool) { +func (f *fakeMatcher) RegisterBinding(binding *v1beta1.ValidatingAdmissionPolicyBinding, matchFunc func(generic.BindingAccessor, admission.Attributes) bool) { namespace, name := binding.Namespace, binding.Name key := types.NamespacedName{ Name: name, @@ -301,7 +301,7 @@ func (f *fakeMatcher) RegisterBinding(binding *v1beta1.ValidatingAdmissionPolicy if matchFunc != nil { if f.BindingMatchFuncs == nil { - f.BindingMatchFuncs = make(map[types.NamespacedName]func(*v1beta1.ValidatingAdmissionPolicyBinding, admission.Attributes) bool) + f.BindingMatchFuncs = make(map[types.NamespacedName]func(generic.BindingAccessor, admission.Attributes) bool) } f.BindingMatchFuncs[key] = matchFunc } @@ -309,8 +309,8 @@ func (f *fakeMatcher) RegisterBinding(binding *v1beta1.ValidatingAdmissionPolicy // Matches says whether this policy definition matches the provided admission // resource request -func (f *fakeMatcher) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) { - namespace, name := definition.Namespace, definition.Name +func (f *fakeMatcher) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition generic.PolicyAccessor) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) { + namespace, name := definition.GetNamespace(), definition.GetName() key := types.NamespacedName{ Name: name, Namespace: namespace, @@ -325,8 +325,8 @@ func (f *fakeMatcher) DefinitionMatches(a admission.Attributes, o admission.Obje // Matches says whether this policy definition matches the provided admission // resource request -func (f *fakeMatcher) BindingMatches(a admission.Attributes, o admission.ObjectInterfaces, binding *v1beta1.ValidatingAdmissionPolicyBinding) (bool, error) { - namespace, name := binding.Namespace, binding.Name +func (f *fakeMatcher) BindingMatches(a admission.Attributes, o admission.ObjectInterfaces, binding generic.BindingAccessor) (bool, error) { + namespace, name := binding.GetNamespace(), binding.GetName() key := types.NamespacedName{ Name: name, Namespace: namespace, @@ -355,7 +355,7 @@ func setupFakeTest(t *testing.T, comp *fakeCompiler, match *fakeMatcher) *generi func setupTestCommon( t *testing.T, compiler *fakeCompiler, - matcher validating.Matcher, + matcher generic.PolicyMatcher, shouldStartInformers bool, ) *generic.PolicyTestContext[*validating.Policy, *validating.PolicyBinding, validating.Validator] { testContext, testContextCancel, err := generic.NewPolicyTestContext( @@ -367,7 +367,7 @@ func setupTestCommon( func(a authorizer.Authorizer, m *matching.Matcher) generic.Dispatcher[validating.PolicyHook] { coolMatcher := matcher if coolMatcher == nil { - coolMatcher = validating.NewMatcher(m) + coolMatcher = generic.NewPolicyMatcher(m) } return validating.NewDispatcher(a, coolMatcher) }, @@ -549,7 +549,7 @@ func TestDefinitionDoesntMatch(t *testing.T) { } }) - matcher.RegisterDefinition(denyPolicy, func(vap *v1beta1.ValidatingAdmissionPolicy, a admission.Attributes) bool { + matcher.RegisterDefinition(denyPolicy, func(vap generic.PolicyAccessor, a admission.Attributes) bool { // Match names with even-numbered length obj := a.GetObject() diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go index 5af6a3f3092..ef0b13f90e9 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/dispatcher.go @@ -44,7 +44,7 @@ import ( ) type dispatcher struct { - matcher Matcher + matcher generic.PolicyMatcher authz authorizer.Authorizer } @@ -52,7 +52,7 @@ var _ generic.Dispatcher[PolicyHook] = &dispatcher{} func NewDispatcher( authorizer authorizer.Authorizer, - matcher Matcher, + matcher generic.PolicyMatcher, ) generic.Dispatcher[PolicyHook] { return &dispatcher{ matcher: matcher, @@ -124,7 +124,7 @@ func (c *dispatcher) Dispatch(ctx context.Context, a admission.Attributes, o adm var versionedAttr *admission.VersionedAttributes definition := hook.Policy - matches, matchResource, matchKind, err := c.matcher.DefinitionMatches(a, o, definition) + matches, matchResource, matchKind, err := c.matcher.DefinitionMatches(a, o, NewValidatingAdmissionPolicyAccessor(definition)) if err != nil { // Configuration error. addConfigError(err, definition, nil) @@ -143,7 +143,7 @@ func (c *dispatcher) Dispatch(ctx context.Context, a admission.Attributes, o adm for _, binding := range hook.Bindings { // If the key is inside dependentBindings, there is guaranteed to // be a bindingInfo for it - matches, err := c.matcher.BindingMatches(a, o, binding) + matches, err := c.matcher.BindingMatches(a, o, NewValidatingAdmissionPolicyBindingAccessor(binding)) if err != nil { // Configuration error. addConfigError(err, definition, binding) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/interface.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/interface.go index 33cbcce6588..97eeb95504a 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/interface.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/interface.go @@ -21,7 +21,6 @@ import ( celgo "github.com/google/cel-go/cel" - "k8s.io/api/admissionregistration/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -80,23 +79,6 @@ func (v *Variable) GetName() string { return v.Name } -// Matcher is used for matching ValidatingAdmissionPolicy and ValidatingAdmissionPolicyBinding to attributes -type Matcher interface { - admission.InitializationValidator - - // DefinitionMatches says whether this policy definition matches the provided admission - // resource request - DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) - - // BindingMatches says whether this policy definition matches the provided admission - // resource request - BindingMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicyBinding) (bool, error) - - // GetNamespace retrieves the Namespace resource by the given name. The name may be empty, in which case - // GetNamespace must return nil, nil - GetNamespace(name string) (*corev1.Namespace, error) -} - // ValidateResult defines the result of a Validator.Validate operation. type ValidateResult struct { // Decisions specifies the outcome of the validation as well as the details about the decision. diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/matcher.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/matcher.go deleted file mode 100644 index 9d667e46edf..00000000000 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/matcher.go +++ /dev/null @@ -1,83 +0,0 @@ -/* -Copyright 2022 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 validating - -import ( - "k8s.io/api/admissionregistration/v1beta1" - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apiserver/pkg/admission" - "k8s.io/apiserver/pkg/admission/plugin/policy/matching" -) - -var _ matching.MatchCriteria = &matchCriteria{} - -type matchCriteria struct { - constraints *v1beta1.MatchResources -} - -// GetParsedNamespaceSelector returns the converted LabelSelector which implements labels.Selector -func (m *matchCriteria) GetParsedNamespaceSelector() (labels.Selector, error) { - return metav1.LabelSelectorAsSelector(m.constraints.NamespaceSelector) -} - -// GetParsedObjectSelector returns the converted LabelSelector which implements labels.Selector -func (m *matchCriteria) GetParsedObjectSelector() (labels.Selector, error) { - return metav1.LabelSelectorAsSelector(m.constraints.ObjectSelector) -} - -// GetMatchResources returns the matchConstraints -func (m *matchCriteria) GetMatchResources() v1beta1.MatchResources { - return *m.constraints -} - -type matcher struct { - Matcher *matching.Matcher -} - -func NewMatcher(m *matching.Matcher) Matcher { - return &matcher{ - Matcher: m, - } -} - -// ValidateInitialization checks if Matcher is initialized. -func (c *matcher) ValidateInitialization() error { - return c.Matcher.ValidateInitialization() -} - -// DefinitionMatches returns whether this ValidatingAdmissionPolicy matches the provided admission resource request -func (c *matcher) DefinitionMatches(a admission.Attributes, o admission.ObjectInterfaces, definition *v1beta1.ValidatingAdmissionPolicy) (bool, schema.GroupVersionResource, schema.GroupVersionKind, error) { - criteria := matchCriteria{constraints: definition.Spec.MatchConstraints} - return c.Matcher.Matches(a, o, &criteria) -} - -// BindingMatches returns whether this ValidatingAdmissionPolicyBinding matches the provided admission resource request -func (c *matcher) BindingMatches(a admission.Attributes, o admission.ObjectInterfaces, binding *v1beta1.ValidatingAdmissionPolicyBinding) (bool, error) { - if binding.Spec.MatchResources == nil { - return true, nil - } - criteria := matchCriteria{constraints: binding.Spec.MatchResources} - isMatch, _, _, err := c.Matcher.Matches(a, o, &criteria) - return isMatch, err -} - -func (c *matcher) GetNamespace(name string) (*corev1.Namespace, error) { - return c.Matcher.GetNamespace(name) -} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go index db371804495..56b22fc04f2 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go @@ -94,7 +94,7 @@ func NewPlugin(_ io.Reader) *Plugin { ) }, func(a authorizer.Authorizer, m *matching.Matcher) generic.Dispatcher[PolicyHook] { - return NewDispatcher(a, NewMatcher(m)) + return NewDispatcher(a, generic.NewPolicyMatcher(m)) }, ), }