Skip to content

Commit

Permalink
update route field selectors
Browse files Browse the repository at this point in the history
  • Loading branch information
@deads2k
deads2k committed Sep 12, 2017
1 parent 0c5698a commit 02e0217
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 98 deletions.
51 changes: 21 additions & 30 deletions hack/import-restrictions.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,15 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackages": [
"vendor/github.com/google/gofuzz",
"vendor/k8s.io/kubernetes/pkg/api",
"vendor/k8s.io/kubernetes/pkg/api/v1",
"vendor/k8s.io/kubernetes/pkg/api/validation",
"vendor/k8s.io/kubernetes/pkg/apis/rbac",
"github.com/openshift/origin/pkg/api/apihelpers/apitesting",
"github.com/openshift/origin/pkg/api/apihelpers",
"github.com/openshift/origin/pkg/api/install",
"vendor/k8s.io/kubernetes/pkg/api/helper",
"github.com/openshift/origin/pkg/user/apis/user/validation",
Expand All @@ -86,13 +85,12 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackages": [
"vendor/k8s.io/kubernetes/pkg/api",
"vendor/k8s.io/kubernetes/pkg/api/v1",
"github.com/openshift/origin/pkg/api/apihelpers/apitesting",
"github.com/openshift/origin/pkg/api/apihelpers",
"github.com/openshift/origin/pkg/api/install",
"github.com/openshift/origin/pkg/util/namer",
"github.com/openshift/origin/pkg/build/util",
Expand Down Expand Up @@ -131,7 +129,8 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackages": [
"vendor/k8s.io/kubernetes/pkg/api",
Expand All @@ -144,8 +143,6 @@
"vendor/github.com/docker/distribution/manifest/schema1",
"vendor/github.com/docker/distribution/manifest/schema2",
"vendor/github.com/blang/semver",
"github.com/openshift/origin/pkg/api/apihelpers",
"github.com/openshift/origin/pkg/api/apihelpers/apitesting",
"github.com/openshift/origin/pkg/image/apis/image/install",
"github.com/openshift/origin/pkg/image/reference"
]
Expand All @@ -158,12 +155,10 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
],
"allowedImportPackages": [
"github.com/openshift/origin/pkg/api/apihelpers/apitesting",
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
]
],
"allowedImportPackages": []
},

{
Expand All @@ -173,13 +168,12 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackages": [
"vendor/k8s.io/kubernetes/pkg/api",
"vendor/k8s.io/kubernetes/pkg/api/v1",
"github.com/openshift/origin/pkg/api/apihelpers/apitesting",
"github.com/openshift/origin/pkg/api/apihelpers",
"vendor/k8s.io/kubernetes/pkg/registry/core/namespace"
]
},
Expand Down Expand Up @@ -208,14 +202,12 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackages": [
"vendor/k8s.io/kubernetes/pkg/api",
"vendor/k8s.io/kubernetes/pkg/api/v1",
"github.com/openshift/origin/pkg/api/apihelpers",
"github.com/openshift/origin/pkg/api/apihelpers/apitesting",
"github.com/openshift/origin/pkg/api/install"
"vendor/k8s.io/kubernetes/pkg/api/v1"
]
},

Expand All @@ -238,12 +230,12 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackages": [
"vendor/k8s.io/kubernetes/pkg/api",
"vendor/k8s.io/kubernetes/pkg/api/v1",
"github.com/openshift/origin/pkg/api/apihelpers"
"vendor/k8s.io/kubernetes/pkg/api/v1"
]
},

Expand All @@ -254,19 +246,18 @@
],
"allowedImportPackageRoots": [
"vendor/k8s.io/apimachinery",
"vendor/github.com/gogo/protobuf"
"vendor/github.com/gogo/protobuf",
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackages": [
"vendor/k8s.io/kubernetes/pkg/api",
"vendor/k8s.io/kubernetes/pkg/api/v1",
"github.com/openshift/origin/pkg/api/apihelpers/apitesting",
"github.com/openshift/origin/pkg/api/apihelpers"
"vendor/k8s.io/kubernetes/pkg/api/v1"
]
},


{
"checkedPackages": [
"checkedPackageRoots": [
"github.com/openshift/origin/pkg/api/apihelpers"
],
"allowedImportPackageRoots": [
Expand Down
51 changes: 51 additions & 0 deletions pkg/api/apihelpers/apitesting/fields.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package apitesting
import (
"testing"

"k8s.io/apimachinery/pkg/fields"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
)

func TestFieldLabelConversions(t *testing.T, scheme *runtime.Scheme, version, kind string, expectedLabels map[string]string, customLabels ...string) {
Expand All @@ -20,3 +22,52 @@ func TestFieldLabelConversions(t *testing.T, scheme *runtime.Scheme, version, ki
}
}
}

// FieldKeyCheck gathers information to check if the field key conversions are working correctly. It takes many parameters
// in an attempt to reflect reality
type FieldKeyCheck struct {
SchemeBuilder runtime.SchemeBuilder
Kind schema.GroupVersionKind
AllowedExternalFieldKeys []string
FieldKeyEvaluatorFn FieldKeyEvaluator
}

func (f FieldKeyCheck) Check(t *testing.T) {
scheme := runtime.NewScheme()
f.SchemeBuilder.AddToScheme(scheme)
internalObj, err := scheme.New(f.Kind.GroupKind().WithVersion(runtime.APIVersionInternal))
if err != nil {
t.Errorf("unable to new up %v", f.Kind)
}

for _, externalFieldKey := range f.AllowedExternalFieldKeys {
internalFieldKey, _, err := scheme.ConvertFieldLabel(f.Kind.GroupVersion().String(), f.Kind.Kind, externalFieldKey, "")
if err != nil {
t.Errorf("illegal field conversion %q for %v", externalFieldKey, f.Kind)
continue
}

fieldSet := fields.Set{}
if err := f.FieldKeyEvaluatorFn(internalObj, fieldSet); err != nil {
t.Errorf("unable to valuate field keys for %v: %v", f.Kind, err)
continue
}

found := false
for actualInternalFieldKey := range fieldSet {
if internalFieldKey == actualInternalFieldKey {
found = true
break
}
}
if !found {
t.Errorf("%q converted to %q which has no internal field key match for %v", externalFieldKey, internalFieldKey, f.Kind)
continue
}

}

}

// FieldKeyEvaluator overlaps with the storage mutation func. We use this to confirm that the non-meta fields are actually being handled
type FieldKeyEvaluator func(obj runtime.Object, fieldSet fields.Set) error
23 changes: 14 additions & 9 deletions pkg/route/apis/route/fields.go
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
package route

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

// RouteToSelectableFields returns a label set that represents the object
func RouteToSelectableFields(route *Route) fields.Set {
return fields.Set{
"metadata.name": route.Name,
"metadata.namespace": route.Namespace,
"spec.path": route.Spec.Path,
"spec.host": route.Spec.Host,
"spec.to.name": route.Spec.To.Name,
"k8s.io/apimachinery/pkg/fields"
runtime "k8s.io/apimachinery/pkg/runtime"
)

func RouteFieldSelector(obj runtime.Object, fieldSet fields.Set) error {
route, ok := obj.(*Route)
if !ok {
return fmt.Errorf("%T not a Route", obj)
}
fieldSet["spec.path"] = route.Spec.Path
fieldSet["spec.host"] = route.Spec.Host
fieldSet["spec.to.name"] = route.Spec.To.Name
return nil
}
44 changes: 37 additions & 7 deletions pkg/route/apis/route/v1/conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,43 @@ package v1

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

"github.com/openshift/origin/pkg/api/apihelpers"
routeapi "github.com/openshift/origin/pkg/route/apis/route"
)

func addConversionFuncs(scheme *runtime.Scheme) error {
return scheme.AddFieldLabelConversionFunc("v1", "Route",
apihelpers.GetFieldLabelConversionFunc(routeapi.RouteToSelectableFields(&routeapi.Route{}), nil),
)
func addLegacyFieldConversionFuncs(scheme *runtime.Scheme) error {
if err := scheme.AddFieldLabelConversionFunc(LegacySchemeGroupVersion.String(), "Route", legacyRouteFieldSelectorConversionFunc); err != nil {
return err
}
return nil
}

func addFieldConversionFuncs(scheme *runtime.Scheme) error {
if err := scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.String(), "Route", routeFieldSelectorConversionFunc); err != nil {
return err
}
return nil
}

// because field selectors can vary in support by version they are exposed under, we have one function for each
// groupVersion we're registering for

func legacyRouteFieldSelectorConversionFunc(label, value string) (internalLabel, internalValue string, err error) {
switch label {
case "spec.path",
"spec.host",
"spec.to.name":
return label, value, nil
default:
return runtime.DefaultMetaV1FieldSelectorConversion(label, value)
}
}

func routeFieldSelectorConversionFunc(label, value string) (internalLabel, internalValue string, err error) {
switch label {
case "spec.path",
"spec.host",
"spec.to.name":
return label, value, nil
default:
return runtime.DefaultMetaV1FieldSelectorConversion(label, value)
}
}
22 changes: 14 additions & 8 deletions pkg/route/apis/route/v1/conversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,25 @@ import (
"k8s.io/apimachinery/pkg/runtime"

"github.com/openshift/origin/pkg/api/apihelpers/apitesting"
routeapi "github.com/openshift/origin/pkg/route/apis/route"
"github.com/openshift/origin/pkg/route/apis/route"
)

func TestFieldSelectorConversions(t *testing.T) {
converter := runtime.NewScheme()
LegacySchemeBuilder.AddToScheme(converter)
apitesting.FieldKeyCheck{
SchemeBuilder: []func(*runtime.Scheme) error{LegacySchemeBuilder.AddToScheme, route.LegacySchemeBuilder.AddToScheme},
Kind: LegacySchemeGroupVersion.WithKind("Route"),
// Ensure previously supported labels have conversions. DO NOT REMOVE THINGS FROM THIS LIST
AllowedExternalFieldKeys: []string{"spec.host", "spec.path", "spec.to.name"},
FieldKeyEvaluatorFn: route.RouteFieldSelector,
}.Check(t)

apitesting.TestFieldLabelConversions(t, converter, "v1", "Route",
// Ensure all currently returned labels are supported
routeapi.RouteToSelectableFields(&routeapi.Route{}),
apitesting.FieldKeyCheck{
SchemeBuilder: []func(*runtime.Scheme) error{SchemeBuilder.AddToScheme, route.SchemeBuilder.AddToScheme},
Kind: SchemeGroupVersion.WithKind("Route"),
// Ensure previously supported labels have conversions. DO NOT REMOVE THINGS FROM THIS LIST
"spec.host", "spec.path", "spec.to.name",
)
AllowedExternalFieldKeys: []string{"spec.host", "spec.path", "spec.to.name"},
FieldKeyEvaluatorFn: route.RouteFieldSelector,
}.Check(t)
}

func TestSupportingCamelConstants(t *testing.T) {
Expand Down

0 comments on commit 02e0217

Please sign in to comment.