refactor: handle paramKind directly

remove hacks that might conceal errors
This commit is contained in:
Alexander Zielenski 2024-02-20 09:22:35 -08:00
parent 6d5133f3ec
commit acf1d850c6
4 changed files with 35 additions and 47 deletions

View File

@ -18,14 +18,13 @@ package generic
import (
"k8s.io/api/admissionregistration/v1beta1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
)
type PolicyAccessor interface {
GetName() string
GetNamespace() string
GetParamKind() *schema.GroupVersionKind
GetParamKind() *v1beta1.ParamKind
GetMatchConstraints() *v1beta1.MatchResources
}

View File

@ -83,8 +83,11 @@ type compiledPolicyEntry[E Evaluator] struct {
}
type PolicyHook[P runtime.Object, B runtime.Object, E Evaluator] struct {
Policy P
Bindings []B
Policy P
Bindings []B
// ParamInformer is the informer for the param CRD for this policy, or nil if
// there is no param or if there was a configuration error
ParamInformer informers.GenericInformer
ParamScope meta.RESTScope
@ -157,7 +160,7 @@ func (s *policySource[P, B, E]) Run(ctx context.Context) error {
}
defer func() {
if err := s.policyInformer.RemoveEventHandler(handle); err != nil {
utilruntime.HandleError(fmt.Errorf("failed to remove policy event handler: %v", err))
utilruntime.HandleError(fmt.Errorf("failed to remove policy event handler: %w", err))
}
}()
@ -167,7 +170,7 @@ func (s *policySource[P, B, E]) Run(ctx context.Context) error {
}
defer func() {
if err := s.bindingInformer.RemoveEventHandler(bindingHandle); err != nil {
utilruntime.HandleError(fmt.Errorf("failed to remove binding event handler: %v", err))
utilruntime.HandleError(fmt.Errorf("failed to remove binding event handler: %w", err))
}
}()
@ -231,7 +234,7 @@ func (s *policySource[P, B, E]) refreshPolicies() {
if err != nil {
// An error was generated while syncing policies. Mark it as dirty again
// so we can retry later
utilruntime.HandleError(fmt.Errorf("encountered error syncing policies: %v. Rescheduling policy sync", err))
utilruntime.HandleError(fmt.Errorf("encountered error syncing policies: %w. Rescheduling policy sync", err))
s.notify()
}
}
@ -299,8 +302,26 @@ func (s *policySource[P, B, E]) calculatePolicyData() ([]PolicyHook[P, B, E], er
continue
}
var parsedParamKind *schema.GroupVersionKind
policyAccessor := s.newPolicyAccessor(policySpec)
paramInformer, paramScope, configurationError := s.ensureParamsForPolicyLocked(policyAccessor.GetParamKind())
if paramKind := policyAccessor.GetParamKind(); paramKind != nil {
groupVersion, err := schema.ParseGroupVersion(paramKind.APIVersion)
if err != nil {
errs = append(errs, fmt.Errorf("failed to parse paramKind APIVersion: %w", err))
continue
}
parsedParamKind = &schema.GroupVersionKind{
Group: groupVersion.Group,
Version: groupVersion.Version,
Kind: paramKind.Kind,
}
// TEMPORARY UNTIL WE HAVE SHARED PARAM INFORMERS
usedParams[*parsedParamKind] = struct{}{}
}
paramInformer, paramScope, configurationError := s.ensureParamsForPolicyLocked(parsedParamKind)
result = append(result, PolicyHook[P, B, E]{
Policy: policySpec,
Bindings: bindingSpecs,
@ -310,11 +331,6 @@ func (s *policySource[P, B, E]) calculatePolicyData() ([]PolicyHook[P, B, E], er
ConfigurationError: configurationError,
})
// TEMPORARY UNTIL WE HAVE SHARED PARAM INFORMERS
if paramKind := policyAccessor.GetParamKind(); paramKind != nil {
usedParams[*paramKind] = struct{}{}
}
// Should queue a re-sync for policy sync error. If our shared param
// informer can notify us when CRD discovery changes we can remove this
// and just rely on the informer to notify us when the CRDs change

View File

@ -23,7 +23,6 @@ import (
"k8s.io/api/admissionregistration/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/admission/plugin/policy/generic"
"k8s.io/apiserver/pkg/admission/plugin/policy/matching"
@ -111,10 +110,9 @@ func TestPolicySourceHasSyncedInitialList(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "policy2",
},
ParamKind: &schema.GroupVersionKind{
Group: "policy.example.com",
Version: "v1",
Kind: "FakeParam",
ParamKind: &v1beta1.ParamKind{
APIVersion: "policy.example.com/v1",
Kind: "FakeParam",
},
},
))
@ -179,7 +177,7 @@ type FakePolicy struct {
metav1.TypeMeta
metav1.ObjectMeta
ParamKind *schema.GroupVersionKind
ParamKind *v1beta1.ParamKind
}
var _ generic.PolicyAccessor = &FakePolicy{}
@ -201,7 +199,7 @@ func (fp *FakePolicy) GetNamespace() string {
return fp.Namespace
}
func (fp *FakePolicy) GetParamKind() *schema.GroupVersionKind {
func (fp *FakePolicy) GetParamKind() *v1beta1.ParamKind {
return fp.ParamKind
}

View File

@ -18,7 +18,6 @@ package validating
import (
"k8s.io/api/admissionregistration/v1beta1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/admission/plugin/policy/generic"
)
@ -47,32 +46,8 @@ func (v *validatingAdmissionPolicyAccessor) GetName() string {
return v.Name
}
func (v *validatingAdmissionPolicyAccessor) GetParamKind() *schema.GroupVersionKind {
paramKind := v.Spec.ParamKind
if paramKind == nil {
return nil
}
groupVersion, err := schema.ParseGroupVersion(paramKind.APIVersion)
if err != nil {
// A validatingadmissionpolicy which passes validation should have
// a parseable APIVersion for its ParamKind, so this should never happen
// if the policy is valid.
//
// Return a bogus but non-nil GVK that will throw an error about the
// invalid APIVersion when the param is looked up.
return &schema.GroupVersionKind{
Group: paramKind.APIVersion,
Version: "",
Kind: paramKind.Kind,
}
}
return &schema.GroupVersionKind{
Group: groupVersion.Group,
Version: groupVersion.Version,
Kind: paramKind.Kind,
}
func (v *validatingAdmissionPolicyAccessor) GetParamKind() *v1beta1.ParamKind {
return v.Spec.ParamKind
}
func (v *validatingAdmissionPolicyAccessor) GetMatchConstraints() *v1beta1.MatchResources {