Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add legacy field selector for names #15963

Merged
merged 1 commit into from
Aug 26, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions hack/import-restrictions.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
"vendor/k8s.io/kubernetes/pkg/api",
"vendor/k8s.io/kubernetes/pkg/api/v1",
"github.com/openshift/origin/pkg/api",
"github.com/openshift/origin/pkg/api",
"github.com/openshift/origin/pkg/api/legacy",
"github.com/openshift/origin/pkg/api/install",
"github.com/openshift/origin/pkg/util/namer",
"github.com/openshift/origin/pkg/build/util",
Expand All @@ -67,10 +67,8 @@
"vendor/k8s.io/kubernetes/pkg/apis/extensions/v1beta1",
"vendor/k8s.io/kubernetes/pkg/api/helper",
"vendor/k8s.io/kubernetes/pkg/api/install",
"github.com/openshift/origin/pkg/api",
"github.com/openshift/origin/pkg/deploy/apis/apps/install",
"github.com/openshift/origin/pkg/image/apis/image",
"github.com/openshift/origin/test/util/api"
"github.com/openshift/origin/pkg/image/apis/image"
]
},

Expand Down Expand Up @@ -182,11 +180,7 @@
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
],
"allowedImportPackages": [
"github.com/openshift/origin/pkg/api",
"github.com/openshift/origin/pkg/api/install",
"github.com/openshift/origin/test/util/api"
]
"allowedImportPackages": []
},

{
Expand Down
16 changes: 16 additions & 0 deletions pkg/api/legacy/fields.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package legacy

import (
"k8s.io/apimachinery/pkg/runtime"
)

// LegacyMetaV1FieldSelectorConversionWithName auto-accepts metav1 values for name and namespace AND "name"
// which many of our older resources used.
func LegacyMetaV1FieldSelectorConversionWithName(label, value string) (string, string, error) {
switch label {
case "name":
return "metadata.name", value, nil
default:
return runtime.DefaultMetaV1FieldSelectorConversion(label, value)
}
}
9 changes: 0 additions & 9 deletions pkg/build/apis/build/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,3 @@ func BuildToSelectableFields(build *Build) fields.Set {
"podName": GetBuildPodName(build),
}
}

// BuildConfigToSelectableFields returns a label set that represents the object
// changes to the returned keys require registering conversions for existing versions using Scheme.AddFieldLabelConversionFunc
func BuildConfigToSelectableFields(buildConfig *BuildConfig) fields.Set {
return fields.Set{
"metadata.name": buildConfig.Name,
"metadata.namespace": buildConfig.Namespace,
}
}
15 changes: 12 additions & 3 deletions pkg/build/apis/build/v1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"

oapi "github.com/openshift/origin/pkg/api"
"github.com/openshift/origin/pkg/api/legacy"
newer "github.com/openshift/origin/pkg/build/apis/build"
buildutil "github.com/openshift/origin/pkg/build/util"
imageapi "github.com/openshift/origin/pkg/image/apis/image"
Expand Down Expand Up @@ -174,15 +175,23 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
return err
}

if err := scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.String(), "Build",
oapi.GetFieldLabelConversionFunc(newer.BuildToSelectableFields(&newer.Build{}), nil),
); err != nil {
return err
}

return nil
}

func addLegacyFieldLabelConversions(scheme *runtime.Scheme) error {
if err := scheme.AddFieldLabelConversionFunc("v1", "Build",
oapi.GetFieldLabelConversionFunc(newer.BuildToSelectableFields(&newer.Build{}), map[string]string{"name": "metadata.name"}),
); err != nil {
return err
}

if err := scheme.AddFieldLabelConversionFunc("v1", "BuildConfig",
oapi.GetFieldLabelConversionFunc(newer.BuildConfigToSelectableFields(&newer.BuildConfig{}), map[string]string{"name": "metadata.name"}),
); err != nil {
if err := scheme.AddFieldLabelConversionFunc("v1", "BuildConfig", legacy.LegacyMetaV1FieldSelectorConversionWithName); err != nil {
return err
}
return nil
Expand Down
7 changes: 0 additions & 7 deletions pkg/build/apis/build/v1/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ func TestFieldSelectorConversions(t *testing.T) {
// Ensure previously supported labels have conversions. DO NOT REMOVE THINGS FROM THIS LIST
"name", "status", "podName",
)

testutil.CheckFieldLabelConversions(t, "v1", "BuildConfig",
// Ensure all currently returned labels are supported
newer.BuildConfigToSelectableFields(&newer.BuildConfig{}),
// Ensure previously supported labels have conversions. DO NOT REMOVE THINGS FROM THIS LIST
"name",
)
}

func TestBinaryBuildRequestOptions(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/build/apis/build/v1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var (
SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1"}
LegacySchemeGroupVersion = schema.GroupVersion{Group: LegacyGroupName, Version: "v1"}

LegacySchemeBuilder = runtime.NewSchemeBuilder(addLegacyKnownTypes, addConversionFuncs, RegisterDefaults)
LegacySchemeBuilder = runtime.NewSchemeBuilder(addLegacyKnownTypes, addConversionFuncs, RegisterDefaults, addLegacyFieldLabelConversions)
AddToSchemeInCoreGroup = LegacySchemeBuilder.AddToScheme

SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes, addConversionFuncs, RegisterDefaults)
Expand Down
3 changes: 1 addition & 2 deletions pkg/build/registry/buildconfig/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ func NewREST(optsGetter restoptions.Getter) (*REST, error) {
NewFunc: func() runtime.Object { return &buildapi.BuildConfig{} },
NewListFunc: func() runtime.Object { return &buildapi.BuildConfigList{} },
DefaultQualifiedResource: buildapi.Resource("buildconfigs"),
PredicateFunc: buildconfig.Matcher,

CreateStrategy: buildconfig.GroupStrategy,
UpdateStrategy: buildconfig.GroupStrategy,
DeleteStrategy: buildconfig.GroupStrategy,
}

options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: buildconfig.GetAttrs}
options := &generic.StoreOptions{RESTOptions: optsGetter}
if err := store.CompleteWithOptions(options); err != nil {
return nil, err
}
Expand Down
23 changes: 0 additions & 23 deletions pkg/build/registry/buildconfig/strategy.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
package buildconfig

import (
"fmt"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
apirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
kstorage "k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
kapi "k8s.io/kubernetes/pkg/api"

Expand Down Expand Up @@ -125,24 +120,6 @@ func (s legacyStrategy) DefaultGarbageCollectionPolicy() rest.GarbageCollectionP
return rest.OrphanDependents
}

// GetAttrs returns labels and fields of a given object for filtering purposes
func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
buildConfig, ok := obj.(*buildapi.BuildConfig)
if !ok {
return nil, nil, false, fmt.Errorf("not a BuildConfig")
}
return labels.Set(buildConfig.ObjectMeta.Labels), buildapi.BuildConfigToSelectableFields(buildConfig), buildConfig.Initializers != nil, nil
}

// Matcher returns a generic matcher for a given label and field selector.
func Matcher(label labels.Selector, field fields.Selector) kstorage.SelectionPredicate {
return kstorage.SelectionPredicate{
Label: label,
Field: field,
GetAttrs: GetAttrs,
}
}

// CheckGracefulDelete allows a build config to be gracefully deleted.
func (strategy) CheckGracefulDelete(obj runtime.Object, options *metav1.DeleteOptions) bool {
return false
Expand Down
11 changes: 0 additions & 11 deletions pkg/deploy/apis/apps/fields.go

This file was deleted.

6 changes: 0 additions & 6 deletions pkg/deploy/apis/apps/v1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/intstr"

oapi "github.com/openshift/origin/pkg/api"
newer "github.com/openshift/origin/pkg/deploy/apis/apps"
imageapi "github.com/openshift/origin/pkg/image/apis/image"
)
Expand Down Expand Up @@ -119,10 +118,5 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
return err
}

if err := scheme.AddFieldLabelConversionFunc("v1", "DeploymentConfig",
oapi.GetFieldLabelConversionFunc(newer.DeploymentConfigToSelectableFields(&newer.DeploymentConfig{}), nil),
); err != nil {
return err
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this required for buildconfigs but not here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't have a special case to map "name" to "metadata.name" for field selection.

return nil
}
8 changes: 0 additions & 8 deletions pkg/deploy/apis/apps/v1/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
kapiv1 "k8s.io/kubernetes/pkg/api/v1"

newer "github.com/openshift/origin/pkg/deploy/apis/apps"
testutil "github.com/openshift/origin/test/util/api"
)

func TestTriggerRoundTrip(t *testing.T) {
Expand Down Expand Up @@ -223,10 +222,3 @@ func newInt32(val int32) *int32 {
func newIntOrString(ios intstr.IntOrString) *intstr.IntOrString {
return &ios
}

func TestFieldSelectors(t *testing.T) {
testutil.CheckFieldLabelConversions(t, "v1", "DeploymentConfig",
// Ensure all currently returned labels are supported
newer.DeploymentConfigToSelectableFields(&newer.DeploymentConfig{}),
)
}
3 changes: 1 addition & 2 deletions pkg/deploy/registry/deployconfig/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,14 @@ func NewREST(optsGetter restoptions.Getter) (*REST, *StatusREST, *ScaleREST, err
Copier: kapi.Scheme,
NewFunc: func() runtime.Object { return &deployapi.DeploymentConfig{} },
NewListFunc: func() runtime.Object { return &deployapi.DeploymentConfigList{} },
PredicateFunc: deployconfig.Matcher,
DefaultQualifiedResource: deployapi.Resource("deploymentconfigs"),

CreateStrategy: deployconfig.GroupStrategy,
UpdateStrategy: deployconfig.GroupStrategy,
DeleteStrategy: deployconfig.GroupStrategy,
}

options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: deployconfig.GetAttrs}
options := &generic.StoreOptions{RESTOptions: optsGetter}
if err := store.CompleteWithOptions(options); err != nil {
return nil, nil, nil, err
}
Expand Down
22 changes: 0 additions & 22 deletions pkg/deploy/registry/deployconfig/strategy.go
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
package deployconfig

import (
"fmt"
"reflect"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
apirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
kstorage "k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
kapi "k8s.io/kubernetes/pkg/api"

Expand Down Expand Up @@ -162,21 +158,3 @@ func (statusStrategy) PrepareForUpdate(ctx apirequest.Context, obj, old runtime.
func (statusStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runtime.Object) field.ErrorList {
return validation.ValidateDeploymentConfigStatusUpdate(obj.(*deployapi.DeploymentConfig), old.(*deployapi.DeploymentConfig))
}

// GetAttrs returns labels and fields of a given object for filtering purposes
func GetAttrs(obj runtime.Object) (labels.Set, fields.Set, bool, error) {
deploymentConfig, ok := obj.(*deployapi.DeploymentConfig)
if !ok {
return nil, nil, false, fmt.Errorf("not a DeploymentConfig")
}
return labels.Set(deploymentConfig.ObjectMeta.Labels), deployapi.DeploymentConfigToSelectableFields(deploymentConfig), deploymentConfig.Initializers != nil, nil
}

// Matcher returns a generic matcher for a given label and field selector.
func Matcher(label labels.Selector, field fields.Selector) kstorage.SelectionPredicate {
return kstorage.SelectionPredicate{
Label: label,
Field: field,
GetAttrs: GetAttrs,
}
}
8 changes: 0 additions & 8 deletions pkg/image/apis/image/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,6 @@ package image

import "k8s.io/apimachinery/pkg/fields"

// ImageToSelectableFields returns a label set that represents the object.
func ImageToSelectableFields(image *Image) fields.Set {
return fields.Set{
"metadata.name": image.Name,
"metadata.namespace": image.Namespace,
}
}

// ImageStreamToSelectableFields returns a label set that represents the object.
func ImageStreamToSelectableFields(ir *ImageStream) fields.Set {
return fields.Set{
Expand Down
6 changes: 0 additions & 6 deletions pkg/image/apis/image/v1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,6 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
return err
}

if err := scheme.AddFieldLabelConversionFunc("v1", "Image",
oapi.GetFieldLabelConversionFunc(newer.ImageToSelectableFields(&newer.Image{}), nil),
); err != nil {
return err
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this required for buildconfigs but not here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't have name -> metadata.name

if err := scheme.AddFieldLabelConversionFunc("v1", "ImageStream",
oapi.GetFieldLabelConversionFunc(newer.ImageStreamToSelectableFields(&newer.ImageStream{}), map[string]string{"name": "metadata.name"}),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing port to the new LegacyMetaV1FieldSelectorConversionWithName ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing port to the new LegacyMetaV1FieldSelectorConversionWithName?

didn't forget this one. This field selector actually lists many fields so the conversion can't be done via a straight default. It will require more thought and code.

); err != nil {
Expand Down
4 changes: 0 additions & 4 deletions pkg/image/apis/image/v1/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ func TestRoundTripVersionedObject(t *testing.T) {
}

func TestFieldSelectors(t *testing.T) {
testutil.CheckFieldLabelConversions(t, "v1", "Image",
// Ensure all currently returned labels are supported
newer.ImageToSelectableFields(&newer.Image{}),
)
testutil.CheckFieldLabelConversions(t, "v1", "ImageStream",
// Ensure all currently returned labels are supported
newer.ImageStreamToSelectableFields(&newer.ImageStream{}),
Expand Down
3 changes: 1 addition & 2 deletions pkg/image/registry/image/etcd/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,14 @@ func NewREST(optsGetter restoptions.Getter) (*REST, error) {
Copier: kapi.Scheme,
NewFunc: func() runtime.Object { return &imageapi.Image{} },
NewListFunc: func() runtime.Object { return &imageapi.ImageList{} },
PredicateFunc: image.Matcher,
DefaultQualifiedResource: imageapi.Resource("images"),

CreateStrategy: image.Strategy,
UpdateStrategy: image.Strategy,
DeleteStrategy: image.Strategy,
}

options := &generic.StoreOptions{RESTOptions: optsGetter, AttrFunc: image.GetAttrs}
options := &generic.StoreOptions{RESTOptions: optsGetter}
if err := store.CompleteWithOptions(options); err != nil {
return nil, err
}
Expand Down
27 changes: 0 additions & 27 deletions pkg/image/registry/image/strategy.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,11 @@ package image
import (
"fmt"

"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
apirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/registry/rest"
kstorage "k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
kapi "k8s.io/kubernetes/pkg/api"

Expand Down Expand Up @@ -125,26 +121,3 @@ func (s imageStrategy) PrepareForUpdate(ctx apirequest.Context, obj, old runtime
func (imageStrategy) ValidateUpdate(ctx apirequest.Context, obj, old runtime.Object) field.ErrorList {
return validation.ValidateImageUpdate(old.(*imageapi.Image), obj.(*imageapi.Image))
}

// GetAttrs returns labels and fields of a given object for filtering purposes
func GetAttrs(o runtime.Object) (labels.Set, fields.Set, bool, error) {
obj, ok := o.(*imageapi.Image)
if !ok {
return nil, nil, false, fmt.Errorf("not an Image")
}
return labels.Set(obj.Labels), SelectableFields(obj), obj.Initializers != nil, nil
}

// Matcher returns a generic matcher for a given label and field selector.
func Matcher(label labels.Selector, field fields.Selector) kstorage.SelectionPredicate {
return kstorage.SelectionPredicate{
Label: label,
Field: field,
GetAttrs: GetAttrs,
}
}

// SelectableFields returns a field set that can be used for filter selection
func SelectableFields(obj *imageapi.Image) fields.Set {
return generic.ObjectMetaFieldsSet(&obj.ObjectMeta, false)
}
7 changes: 0 additions & 7 deletions pkg/oauth/apis/oauth/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ func OAuthAuthorizeTokenToSelectableFields(obj *OAuthAuthorizeToken) fields.Set
}
}

// OAuthClientToSelectableFields returns a label set that represents the object
func OAuthClientToSelectableFields(obj *OAuthClient) fields.Set {
return fields.Set{
"metadata.name": obj.Name,
}
}

// OAuthClientAuthorizationToSelectableFields returns a label set that represents the object
func OAuthClientAuthorizationToSelectableFields(obj *OAuthClientAuthorization) fields.Set {
return fields.Set{
Expand Down
Loading