Improve error messaging for validating admission policy authz

Kubernetes-commit: 1ad6fd7a0fa454cc3302b579dc73eb5c9afec49a
This commit is contained in:
Joe Betz 2024-10-25 13:52:34 -04:00 committed by Kubernetes Publisher
parent 9da95682e0
commit c259217340
4 changed files with 7 additions and 682 deletions

View File

@ -1,152 +0,0 @@
/*
Copyright 2023 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 (
"context"
"encoding/json"
"sort"
"strings"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
)
type authzResult struct {
authorized authorizer.Decision
reason string
err error
}
type cachingAuthorizer struct {
authorizer authorizer.Authorizer
decisions map[string]authzResult
}
func newCachingAuthorizer(in authorizer.Authorizer) authorizer.Authorizer {
return &cachingAuthorizer{
authorizer: in,
decisions: make(map[string]authzResult),
}
}
// The attribute accessors known to cache key construction. If this fails to compile, the cache
// implementation may need to be updated.
var _ authorizer.Attributes = (interface {
GetUser() user.Info
GetVerb() string
IsReadOnly() bool
GetNamespace() string
GetResource() string
GetSubresource() string
GetName() string
GetAPIGroup() string
GetAPIVersion() string
IsResourceRequest() bool
GetPath() string
GetFieldSelector() (fields.Requirements, error)
GetLabelSelector() (labels.Requirements, error)
})(nil)
// The user info accessors known to cache key construction. If this fails to compile, the cache
// implementation may need to be updated.
var _ user.Info = (interface {
GetName() string
GetUID() string
GetGroups() []string
GetExtra() map[string][]string
})(nil)
// Authorize returns an authorization decision by delegating to another Authorizer. If an equivalent
// check has already been performed, a cached result is returned. Not safe for concurrent use.
func (ca *cachingAuthorizer) Authorize(ctx context.Context, a authorizer.Attributes) (authorizer.Decision, string, error) {
type SerializableAttributes struct {
authorizer.AttributesRecord
LabelSelector string
}
serializableAttributes := SerializableAttributes{
AttributesRecord: authorizer.AttributesRecord{
Verb: a.GetVerb(),
Namespace: a.GetNamespace(),
APIGroup: a.GetAPIGroup(),
APIVersion: a.GetAPIVersion(),
Resource: a.GetResource(),
Subresource: a.GetSubresource(),
Name: a.GetName(),
ResourceRequest: a.IsResourceRequest(),
Path: a.GetPath(),
},
}
// in the error case, we won't honor this field selector, so the cache doesn't need it.
if fieldSelector, err := a.GetFieldSelector(); len(fieldSelector) > 0 {
serializableAttributes.FieldSelectorRequirements, serializableAttributes.FieldSelectorParsingErr = fieldSelector, err
}
if labelSelector, _ := a.GetLabelSelector(); len(labelSelector) > 0 {
// the labels requirements have private elements so those don't help us serialize to a unique key
serializableAttributes.LabelSelector = labelSelector.String()
}
if u := a.GetUser(); u != nil {
di := &user.DefaultInfo{
Name: u.GetName(),
UID: u.GetUID(),
}
// Differently-ordered groups or extras could cause otherwise-equivalent checks to
// have distinct cache keys.
if groups := u.GetGroups(); len(groups) > 0 {
di.Groups = make([]string, len(groups))
copy(di.Groups, groups)
sort.Strings(di.Groups)
}
if extra := u.GetExtra(); len(extra) > 0 {
di.Extra = make(map[string][]string, len(extra))
for k, vs := range extra {
vdupe := make([]string, len(vs))
copy(vdupe, vs)
sort.Strings(vdupe)
di.Extra[k] = vdupe
}
}
serializableAttributes.User = di
}
var b strings.Builder
if err := json.NewEncoder(&b).Encode(serializableAttributes); err != nil {
return authorizer.DecisionNoOpinion, "", err
}
key := b.String()
if cached, ok := ca.decisions[key]; ok {
return cached.authorized, cached.reason, cached.err
}
authorized, reason, err := ca.authorizer.Authorize(ctx, a)
ca.decisions[key] = authzResult{
authorized: authorized,
reason: reason,
err: err,
}
return authorized, reason, err
}

View File

@ -1,523 +0,0 @@
/*
Copyright 2023 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 (
"context"
"errors"
"fmt"
"testing"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/authorization/authorizer"
genericfeatures "k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
)
func mustParseLabelSelector(str string) labels.Requirements {
ret, err := labels.Parse(str)
if err != nil {
panic(err)
}
retRequirements, _ /*selectable*/ := ret.Requirements()
return retRequirements
}
func TestCachingAuthorizer(t *testing.T) {
featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.AuthorizeWithSelectors, true)
type result struct {
decision authorizer.Decision
reason string
error error
}
type invocation struct {
attributes authorizer.Attributes
expected result
}
for _, tc := range []struct {
name string
calls []invocation
backend []result
}{
{
name: "hit",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{Name: "test name"},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
attributes: authorizer.AttributesRecord{Name: "test name"},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
{
name: "hit with differently-ordered groups",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
User: &user.DefaultInfo{
Groups: []string{"a", "b", "c"},
},
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
attributes: authorizer.AttributesRecord{
User: &user.DefaultInfo{
Groups: []string{"c", "b", "a"},
},
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
{
name: "hit with differently-ordered extra",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
User: &user.DefaultInfo{
Extra: map[string][]string{
"k": {"a", "b", "c"},
},
},
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
attributes: authorizer.AttributesRecord{
User: &user.DefaultInfo{
Extra: map[string][]string{
"k": {"c", "b", "a"},
},
},
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
{
name: "miss due to different name",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{Name: "alpha"},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason alpha",
error: fmt.Errorf("test error alpha"),
},
},
{
attributes: authorizer.AttributesRecord{Name: "beta"},
expected: result{
decision: authorizer.DecisionDeny,
reason: "test reason beta",
error: fmt.Errorf("test error beta"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason alpha",
error: fmt.Errorf("test error alpha"),
},
{
decision: authorizer.DecisionDeny,
reason: "test reason beta",
error: fmt.Errorf("test error beta"),
},
},
},
{
name: "miss due to different user",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
User: &user.DefaultInfo{Name: "alpha"},
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason alpha",
error: fmt.Errorf("test error alpha"),
},
},
{
attributes: authorizer.AttributesRecord{
User: &user.DefaultInfo{Name: "beta"},
},
expected: result{
decision: authorizer.DecisionDeny,
reason: "test reason beta",
error: fmt.Errorf("test error beta"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason alpha",
error: fmt.Errorf("test error alpha"),
},
{
decision: authorizer.DecisionDeny,
reason: "test reason beta",
error: fmt.Errorf("test error beta"),
},
},
},
{
name: "honor good field selector",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
Name: "test name",
FieldSelectorRequirements: fields.ParseSelectorOrDie("foo=bar").Requirements(),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
attributes: authorizer.AttributesRecord{
Name: "test name",
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason 2",
error: fmt.Errorf("test error 2"),
},
},
{
// now this should be cached
attributes: authorizer.AttributesRecord{
Name: "test name",
FieldSelectorRequirements: fields.ParseSelectorOrDie("foo=bar").Requirements(),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
{
decision: authorizer.DecisionAllow,
reason: "test reason 2",
error: fmt.Errorf("test error 2"),
},
},
},
{
name: "ignore malformed field selector first",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
Name: "test name",
FieldSelectorParsingErr: errors.New("malformed"),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
// notice that this does not have the malformed field selector.
// it should use the cached result
attributes: authorizer.AttributesRecord{
Name: "test name",
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
{
name: "ignore malformed field selector second",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
Name: "test name",
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
// this should use the broader cached value because the selector will be ignored
attributes: authorizer.AttributesRecord{
Name: "test name",
FieldSelectorParsingErr: errors.New("malformed"),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
{
name: "honor good label selector",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
Name: "test name",
LabelSelectorRequirements: mustParseLabelSelector("foo=bar"),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
attributes: authorizer.AttributesRecord{
Name: "test name",
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason 2",
error: fmt.Errorf("test error 2"),
},
},
{
// now this should be cached
attributes: authorizer.AttributesRecord{
Name: "test name",
LabelSelectorRequirements: mustParseLabelSelector("foo=bar"),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
attributes: authorizer.AttributesRecord{
Name: "test name",
LabelSelectorRequirements: mustParseLabelSelector("diff=zero"),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason 3",
error: fmt.Errorf("test error 3"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
{
decision: authorizer.DecisionAllow,
reason: "test reason 2",
error: fmt.Errorf("test error 2"),
},
{
decision: authorizer.DecisionAllow,
reason: "test reason 3",
error: fmt.Errorf("test error 3"),
},
},
},
{
name: "ignore malformed label selector first",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
Name: "test name",
LabelSelectorParsingErr: errors.New("malformed mess"),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
// notice that this does not have the malformed field selector.
// it should use the cached result
attributes: authorizer.AttributesRecord{
Name: "test name",
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
{
name: "ignore malformed label selector second",
calls: []invocation{
{
attributes: authorizer.AttributesRecord{
Name: "test name",
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
{
// this should use the broader cached value because the selector will be ignored
attributes: authorizer.AttributesRecord{
Name: "test name",
LabelSelectorParsingErr: errors.New("malformed mess"),
},
expected: result{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
backend: []result{
{
decision: authorizer.DecisionAllow,
reason: "test reason",
error: fmt.Errorf("test error"),
},
},
},
} {
t.Run(tc.name, func(t *testing.T) {
var misses int
frontend := newCachingAuthorizer(func() authorizer.Authorizer {
return authorizer.AuthorizerFunc(func(_ context.Context, attributes authorizer.Attributes) (authorizer.Decision, string, error) {
if misses >= len(tc.backend) {
t.Fatalf("got more than expected %d backend invocations", len(tc.backend))
}
result := tc.backend[misses]
misses++
return result.decision, result.reason, result.error
})
}())
for i, invocation := range tc.calls {
decision, reason, err := frontend.Authorize(context.TODO(), invocation.attributes)
if decision != invocation.expected.decision {
t.Errorf("(call %d of %d) expected decision %v, got %v", i+1, len(tc.calls), invocation.expected.decision, decision)
}
if reason != invocation.expected.reason {
t.Errorf("(call %d of %d) expected reason %q, got %q", i+1, len(tc.calls), invocation.expected.reason, reason)
}
if err.Error() != invocation.expected.error.Error() {
t.Errorf("(call %d of %d) expected error %q, got %q", i+1, len(tc.calls), invocation.expected.error.Error(), err.Error())
}
}
if len(tc.backend) > misses {
t.Errorf("expected %d backend invocations, got %d", len(tc.backend), misses)
}
})
}
}

View File

@ -41,13 +41,13 @@ import (
// validator implements the Validator interface
type validator struct {
celMatcher matchconditions.Matcher
validationFilter cel.Filter
auditAnnotationFilter cel.Filter
messageFilter cel.Filter
validationFilter cel.ConditionEvaluator
auditAnnotationFilter cel.ConditionEvaluator
messageFilter cel.ConditionEvaluator
failPolicy *v1.FailurePolicyType
}
func NewValidator(validationFilter cel.Filter, celMatcher matchconditions.Matcher, auditAnnotationFilter, messageFilter cel.Filter, failPolicy *v1.FailurePolicyType) Validator {
func NewValidator(validationFilter cel.ConditionEvaluator, celMatcher matchconditions.Matcher, auditAnnotationFilter, messageFilter cel.ConditionEvaluator, failPolicy *v1.FailurePolicyType) Validator {
return &validator{
celMatcher: celMatcher,
validationFilter: validationFilter,

View File

@ -42,7 +42,7 @@ import (
"k8s.io/apiserver/pkg/cel/environment"
)
var _ cel.Filter = &fakeCelFilter{}
var _ cel.ConditionEvaluator = &fakeCelFilter{}
type fakeCelFilter struct {
evaluations []cel.EvaluationResult
@ -1035,8 +1035,8 @@ func TestContextCanceled(t *testing.T) {
fakeAttr := admission.NewAttributesRecord(nil, nil, schema.GroupVersionKind{}, "default", "foo", schema.GroupVersionResource{}, "", admission.Create, nil, false, nil)
fakeVersionedAttr, _ := admission.NewVersionedAttributes(fakeAttr, schema.GroupVersionKind{}, nil)
fc := cel.NewFilterCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
f := fc.Compile([]cel.ExpressionAccessor{&ValidationCondition{Expression: "[1,2,3,4,5,6,7,8,9,10].map(x, [1,2,3,4,5,6,7,8,9,10].map(y, x*y)) == []"}}, cel.OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.StoredExpressions)
fc := cel.NewConditionCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
f := fc.CompileCondition([]cel.ExpressionAccessor{&ValidationCondition{Expression: "[1,2,3,4,5,6,7,8,9,10].map(x, [1,2,3,4,5,6,7,8,9,10].map(y, x*y)) == []"}}, cel.OptionalVariableDeclarations{HasParams: false, HasAuthorizer: false}, environment.StoredExpressions)
v := validator{
failPolicy: &fail,
celMatcher: &fakeCELMatcher{matches: true},