Skip to content
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
54 changes: 22 additions & 32 deletions hack/import-restrictions.json
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,14 +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"
"vendor/k8s.io/kubernetes/pkg/api/v1"
]
},

Expand Down Expand Up @@ -208,14 +201,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 +229,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 +245,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
58 changes: 49 additions & 9 deletions pkg/api/apihelpers/apitesting/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,60 @@ 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) {
for label := range expectedLabels {
_, _, err := scheme.ConvertFieldLabel(version, kind, label, "")
if err != nil {
t.Errorf("No conversion registered for %s for %s %s", label, version, kind)
}
// 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 _, label := range customLabels {
_, _, err := scheme.ConvertFieldLabel(version, kind, label, "")

for _, externalFieldKey := range f.AllowedExternalFieldKeys {
internalFieldKey, _, err := scheme.ConvertFieldLabel(f.Kind.GroupVersion().String(), f.Kind.Kind, externalFieldKey, "")
if err != nil {
t.Errorf("No conversion registered for %s for %s %s", label, version, kind)
t.Errorf("illegal field conversion %q for %v", externalFieldKey, f.Kind)
continue
}
// we get this by default
if internalFieldKey == "metadata.name" {
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
18 changes: 0 additions & 18 deletions pkg/api/apihelpers/fields.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package apihelpers

import (
"fmt"

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

Expand All @@ -16,19 +14,3 @@ func LegacyMetaV1FieldSelectorConversionWithName(label, value string) (string, s
return runtime.DefaultMetaV1FieldSelectorConversion(label, value)
}
}

// GetFieldLabelConversionFunc returns a field label conversion func, which does the following:
// * returns overrideLabels[label], value, nil if the specified label exists in the overrideLabels map
// * returns label, value, nil if the specified label exists as a key in the supportedLabels map (values in this map are unused, it is intended to be a prototypical label/value map)
// * otherwise, returns an error
func GetFieldLabelConversionFunc(supportedLabels map[string]string, overrideLabels map[string]string) func(label, value string) (string, string, error) {
return func(label, value string) (string, string, error) {
if label, overridden := overrideLabels[label]; overridden {
return label, value, nil
}
if _, supported := supportedLabels[label]; supported {
return label, value, nil
}
return "", "", fmt.Errorf("field label not supported: %s", label)
}
}
20 changes: 12 additions & 8 deletions pkg/authorization/apis/authorization/fields.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package authorization

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

// PolicyBindingToSelectableFields returns a label set that represents the object
// changes to the returned keys require registering conversions for existing versions using Scheme.AddFieldLabelConversionFunc
func PolicyBindingToSelectableFields(policyBinding *PolicyBinding) fields.Set {
return fields.Set{
"metadata.name": policyBinding.Name,
"metadata.namespace": policyBinding.Namespace,
"policyRef.namespace": policyBinding.PolicyRef.Namespace,
"k8s.io/apimachinery/pkg/fields"
runtime "k8s.io/apimachinery/pkg/runtime"
)

func PolicyBindingFieldSelector(obj runtime.Object, fieldSet fields.Set) error {
policyBinding, ok := obj.(*PolicyBinding)
if !ok {
return fmt.Errorf("%T not a PolicyBinding", obj)
}
fieldSet["policyRef.namespace"] = policyBinding.PolicyRef.Namespace
return nil
}
23 changes: 20 additions & 3 deletions pkg/authorization/apis/authorization/v1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,15 +394,32 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
return err
}

if err := scheme.AddFieldLabelConversionFunc("v1", "PolicyBinding",
apihelpers.GetFieldLabelConversionFunc(newer.PolicyBindingToSelectableFields(&newer.PolicyBinding{}), nil),
); err != nil {
return nil
}

func addLegacyFieldSelectorKeyConversions(scheme *runtime.Scheme) error {
if err := scheme.AddFieldLabelConversionFunc(LegacySchemeGroupVersion.String(), "PolicyBinding", legacyPolicyBindingFieldSelectorKeyConversionFunc); err != nil {
return err
}
return nil
}

func addFieldSelectorKeyConversions(scheme *runtime.Scheme) error {
return nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why this is empty?

We don't have any custom field selectors. Yet. I'm placing the skeleton here so people can more easily figure out what to do. This isn't really obvious.

}

// 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 legacyPolicyBindingFieldSelectorKeyConversionFunc(label, value string) (internalLabel, internalValue string, err error) {
switch label {
case "policyRef.namespace":
return label, value, nil
default:
return runtime.DefaultMetaV1FieldSelectorConversion(label, value)
}
}

var _ runtime.NestedObjectDecoder = &PolicyRule{}
var _ runtime.NestedObjectEncoder = &PolicyRule{}

Expand Down
15 changes: 7 additions & 8 deletions pkg/authorization/apis/authorization/v1/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ import (
)

func TestFieldSelectorConversions(t *testing.T) {
converter := runtime.NewScheme()
LegacySchemeBuilder.AddToScheme(converter)

apitesting.TestFieldLabelConversions(t, converter, "v1", "PolicyBinding",
// Ensure all currently returned labels are supported
authorizationapi.PolicyBindingToSelectableFields(&authorizationapi.PolicyBinding{}),
)

apitesting.FieldKeyCheck{
SchemeBuilder: []func(*runtime.Scheme) error{LegacySchemeBuilder.AddToScheme, authorizationapi.LegacySchemeBuilder.AddToScheme},
Kind: LegacySchemeGroupVersion.WithKind("PolicyBinding"),
// Ensure previously supported labels have conversions. DO NOT REMOVE THINGS FROM THIS LIST
AllowedExternalFieldKeys: []string{"policyRef.namespace"},
FieldKeyEvaluatorFn: authorizationapi.PolicyBindingFieldSelector,
}.Check(t)
}
4 changes: 2 additions & 2 deletions pkg/authorization/apis/authorization/v1/register.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ 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, addLegacyFieldSelectorKeyConversions, RegisterDefaults)
AddToSchemeInCoreGroup = LegacySchemeBuilder.AddToScheme

SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes, addConversionFuncs, RegisterDefaults)
SchemeBuilder = runtime.NewSchemeBuilder(addKnownTypes, addConversionFuncs, addFieldSelectorKeyConversions, RegisterDefaults)
AddToScheme = SchemeBuilder.AddToScheme
)

Expand Down
25 changes: 15 additions & 10 deletions pkg/build/apis/build/fields.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
package build

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

// BuildToSelectableFields returns a label set that represents the object
// changes to the returned keys require registering conversions for existing versions using Scheme.AddFieldLabelConversionFunc
func BuildToSelectableFields(build *Build) fields.Set {
return fields.Set{
"metadata.name": build.Name,
"metadata.namespace": build.Namespace,
"status": string(build.Status.Phase),
"podName": GetBuildPodName(build),
import (
"fmt"

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

func BuildFieldSelector(obj runtime.Object, fieldSet fields.Set) error {
build, ok := obj.(*Build)
if !ok {
return fmt.Errorf("%T not a Build", obj)
}
fieldSet["status"] = string(build.Status.Phase)
fieldSet["podName"] = GetBuildPodName(build)

return nil
}
Loading