diff --git a/pkg/registry/batch/cronjob/BUILD b/pkg/registry/batch/cronjob/BUILD index 81a10e3c55..0a894be868 100644 --- a/pkg/registry/batch/cronjob/BUILD +++ b/pkg/registry/batch/cronjob/BUILD @@ -19,14 +19,10 @@ go_library( "//pkg/api:go_default_library", "//pkg/apis/batch:go_default_library", "//pkg/apis/batch/validation:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/fields:go_default_library", - "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", - "//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/rest:go_default_library", - "//vendor/k8s.io/apiserver/pkg/storage:go_default_library", "//vendor/k8s.io/apiserver/pkg/storage/names:go_default_library", ], ) @@ -38,7 +34,6 @@ go_test( tags = ["automanaged"], deps = [ "//pkg/api:go_default_library", - "//pkg/api/testing:go_default_library", "//pkg/apis/batch:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apiserver/pkg/endpoints/request:go_default_library", diff --git a/pkg/registry/batch/cronjob/storage/storage.go b/pkg/registry/batch/cronjob/storage/storage.go index 56ba8aff2f..f4ada18bc6 100644 --- a/pkg/registry/batch/cronjob/storage/storage.go +++ b/pkg/registry/batch/cronjob/storage/storage.go @@ -40,7 +40,6 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { Copier: api.Scheme, NewFunc: func() runtime.Object { return &batch.CronJob{} }, NewListFunc: func() runtime.Object { return &batch.CronJobList{} }, - PredicateFunc: cronjob.MatchCronJob, QualifiedResource: batch.Resource("cronjobs"), WatchCacheSize: cachesize.GetWatchCacheSizeByResource("cronjobs"), @@ -48,7 +47,7 @@ func NewREST(optsGetter generic.RESTOptionsGetter) (*REST, *StatusREST) { UpdateStrategy: cronjob.Strategy, DeleteStrategy: cronjob.Strategy, } - options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: cronjob.GetAttrs} + options := &generic.StoreOptions{RESTOptions: optsGetter} if err := store.CompleteWithOptions(options); err != nil { panic(err) // TODO: Propagate error up } diff --git a/pkg/registry/batch/cronjob/strategy.go b/pkg/registry/batch/cronjob/strategy.go index a78b9176d4..1da7c50724 100644 --- a/pkg/registry/batch/cronjob/strategy.go +++ b/pkg/registry/batch/cronjob/strategy.go @@ -17,16 +17,10 @@ limitations under the License. package cronjob import ( - "fmt" - - "k8s.io/apimachinery/pkg/fields" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" - "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/registry/rest" - "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/batch" @@ -105,28 +99,3 @@ func (cronJobStatusStrategy) PrepareForUpdate(ctx genericapirequest.Context, obj func (cronJobStatusStrategy) ValidateUpdate(ctx genericapirequest.Context, obj, old runtime.Object) field.ErrorList { return field.ErrorList{} } - -// CronJobToSelectableFields returns a field set that represents the object for matching purposes. -func CronJobToSelectableFields(cronJob *batch.CronJob) fields.Set { - return generic.ObjectMetaFieldsSet(&cronJob.ObjectMeta, true) -} - -// GetAttrs returns labels and fields of a given object for filtering purposes. -func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, bool, error) { - cronJob, ok := obj.(*batch.CronJob) - if !ok { - return nil, nil, false, fmt.Errorf("given object is not a scheduled job.") - } - return labels.Set(cronJob.ObjectMeta.Labels), CronJobToSelectableFields(cronJob), cronJob.Initializers != nil, nil -} - -// MatchCronJob is the filter used by the generic etcd backend to route -// watch events from etcd to clients of the apiserver only interested in specific -// labels/fields. -func MatchCronJob(label labels.Selector, field fields.Selector) storage.SelectionPredicate { - return storage.SelectionPredicate{ - Label: label, - Field: field, - GetAttrs: GetAttrs, - } -} diff --git a/pkg/registry/batch/cronjob/strategy_test.go b/pkg/registry/batch/cronjob/strategy_test.go index 42bde0c354..4dd34ee4ad 100644 --- a/pkg/registry/batch/cronjob/strategy_test.go +++ b/pkg/registry/batch/cronjob/strategy_test.go @@ -23,7 +23,6 @@ import ( genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/kubernetes/pkg/api" - apitesting "k8s.io/kubernetes/pkg/api/testing" "k8s.io/kubernetes/pkg/apis/batch" ) @@ -170,13 +169,3 @@ func TestCronJobStatusStrategy(t *testing.T) { t.Errorf("Incoming resource version on update should not be mutated") } } - -// FIXME: this is failing conversion.go -func TestSelectableFieldLabelConversions(t *testing.T) { - apitesting.TestSelectableFieldLabelConversionsOfKind(t, - "batch/v2alpha1", - "CronJob", - CronJobToSelectableFields(&batch.CronJob{}), - nil, - ) -} diff --git a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go index b5705f6fdc..be51729f53 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/generic/registry/store.go @@ -1267,8 +1267,23 @@ func (e *Store) CompleteWithOptions(options *generic.StoreOptions) error { if options.RESTOptions == nil { return fmt.Errorf("options for %s must have RESTOptions set", e.QualifiedResource.String()) } - if options.AttrFunc == nil { - return fmt.Errorf("options for %s must have AttrFunc set", e.QualifiedResource.String()) + + attrFunc := options.AttrFunc + if attrFunc == nil { + if isNamespaced { + attrFunc = storage.DefaultNamespaceScopedAttr + } else { + attrFunc = storage.DefaultClusterScopedAttr + } + } + if e.PredicateFunc == nil { + e.PredicateFunc = func(label labels.Selector, field fields.Selector) storage.SelectionPredicate { + return storage.SelectionPredicate{ + Label: label, + Field: field, + GetAttrs: attrFunc, + } + } } opts, err := options.RESTOptions.GetRESTOptions(e.QualifiedResource) @@ -1349,7 +1364,7 @@ func (e *Store) CompleteWithOptions(options *generic.StoreOptions) error { prefix, keyFunc, e.NewListFunc, - options.AttrFunc, + attrFunc, triggerFunc, ) } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go b/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go index 95636ccd72..8421e2103d 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/selection_predicate.go @@ -17,6 +17,7 @@ limitations under the License. package storage import ( + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -26,6 +27,48 @@ import ( // In any failure to parse given object, it returns error. type AttrFunc func(obj runtime.Object) (labels.Set, fields.Set, bool, error) +// FieldMutationFunc allows the mutation of the field selection fields. It is mutating to +// avoid the extra allocation on this common path +type FieldMutationFunc func(obj runtime.Object, fieldSet fields.Set) error + +func DefaultClusterScopedAttr(obj runtime.Object) (labels.Set, fields.Set, bool, error) { + metadata, err := meta.Accessor(obj) + if err != nil { + return nil, nil, false, err + } + fieldSet := fields.Set{ + "metadata.name": metadata.GetName(), + } + + return labels.Set(metadata.GetLabels()), fieldSet, metadata.GetInitializers() != nil, nil +} + +func DefaultNamespaceScopedAttr(obj runtime.Object) (labels.Set, fields.Set, bool, error) { + metadata, err := meta.Accessor(obj) + if err != nil { + return nil, nil, false, err + } + fieldSet := fields.Set{ + "metadata.name": metadata.GetName(), + "metadata.namespace": metadata.GetNamespace(), + } + + return labels.Set(metadata.GetLabels()), fieldSet, metadata.GetInitializers() != nil, nil +} + +func (f AttrFunc) WithFieldMutation(fieldMutator FieldMutationFunc) AttrFunc { + return func(obj runtime.Object) (labels.Set, fields.Set, bool, error) { + labelSet, fieldSet, initialized, err := f(obj) + if err != nil { + return nil, nil, false, err + } + if err := fieldMutator(obj, fieldSet); err != nil { + return nil, nil, false, err + } + return labelSet, fieldSet, initialized, nil + } +} + // SelectionPredicate is used to represent the way to select objects from api storage. type SelectionPredicate struct { Label labels.Selector