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

Make RESTMapper scope aware, use it to inform kubectl and swagger behavior #3944

Merged
40 changes: 38 additions & 2 deletions pkg/api/latest/latest.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,43 @@ func init() {
return interfaces, true
},
)
mapper.Add(api.Scheme, true, "v1beta1", "v1beta2")
mapper.Add(api.Scheme, false, "v1beta3")
// list of versions we support on the server
versions := []string{"v1beta1", "v1beta2", "v1beta3"}

// versions that used mixed case URL formats
versionMixedCase := map[string]bool{
"v1beta1": true,
"v1beta2": true,
}

// backwards compatibility, prior to v1beta3, we identified the namespace as a query parameter
versionToNamespaceScope := map[string]meta.RESTScope{
"v1beta1": meta.RESTScopeNamespaceLegacy,
"v1beta2": meta.RESTScopeNamespaceLegacy,
"v1beta3": meta.RESTScopeNamespace,
}

// the list of kinds that are scoped at the root of the api hierarchy
// if a kind is not enumerated here, it is assumed to have a namespace scope
kindToRootScope := map[string]bool{
"Node": true,
"Minion": true,
}

// enumerate all supported versions, get the kinds, and register with the mapper how to address our resources
for _, version := range versions {
for kind := range api.Scheme.KnownTypes(version) {
mixedCase, found := versionMixedCase[version]
if !found {
mixedCase = false
}
scope := versionToNamespaceScope[version]
_, found = kindToRootScope[kind]
if found {
scope = meta.RESTScopeRoot
}
mapper.Add(scope, kind, version, mixedCase)
}
}
RESTMapper = mapper
}
27 changes: 27 additions & 0 deletions pkg/api/meta/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,30 @@ type MetadataAccessor interface {
runtime.ResourceVersioner
}

type RESTScopeName string

const (
RESTScopeNameNamespace RESTScopeName = "namespace"
RESTScopeNameRoot RESTScopeName = "root"
Copy link
Contributor

Choose a reason for hiding this comment

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

You preferred that to "cluster"?

Copy link
Member Author

Choose a reason for hiding this comment

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

My bias at the time was that it should go at the root of a path.

)

// RESTScope contains the information needed to deal with REST resources that are in a resource hierarchy
// TODO After we deprecate v1beta1 and v1beta2, we can look a supporting removing the flexibility of supporting
// either a query or path param, and instead just support path param
type RESTScope interface {
// Name of the scope
Name() RESTScopeName
// ParamName is the optional name of the parameter that should be inserted in the resource url
// If empty, no param will be inserted
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you put a TODO here that once v1beta1 and v1beta2 are removed, we should consider removing this flexibility everywhere?

ParamName() string
// ParamPath is a boolean that controls how the parameter is manifested in resource paths
// If true, this parameter is encoded in path (i.e. /{paramName}/{paramValue})
// If false, this parameter is encoded in query (i.e. ?{paramName}={paramValue})
ParamPath() bool
// ParamDescription is the optional description to use to document the parameter in api documentation
ParamDescription() string
}

// RESTMapping contains the information needed to deal with objects of a specific
// resource and kind in a RESTful manner.
type RESTMapping struct {
Expand All @@ -104,6 +128,9 @@ type RESTMapping struct {
APIVersion string
Kind string

// Scope contains the information needed to deal with REST Resources that are in a resource hierarchy
Scope RESTScope

runtime.Codec
runtime.ObjectConvertor
MetadataAccessor
Expand Down
87 changes: 65 additions & 22 deletions pkg/api/meta/restmapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,47 @@ package meta
import (
"fmt"
"strings"

"github.com/GoogleCloudPlatform/kubernetes/pkg/runtime"
)

// Implements RESTScope interface
type restScope struct {
name RESTScopeName
paramName string
paramPath bool
paramDescription string
}

func (r *restScope) Name() RESTScopeName {
return r.name
}
func (r *restScope) ParamName() string {
return r.paramName
}
func (r *restScope) ParamPath() bool {
return r.paramPath
}
func (r *restScope) ParamDescription() string {
return r.paramDescription
}

var RESTScopeNamespaceLegacy = &restScope{
name: RESTScopeNameNamespace,
paramName: "namespace",
paramPath: false,
paramDescription: "object name and auth scope, such as for teams and projects",
}

var RESTScopeNamespace = &restScope{
name: RESTScopeNameNamespace,
paramName: "ns",
paramPath: true,
paramDescription: "object name and auth scope, such as for teams and projects",
}

var RESTScopeRoot = &restScope{
name: RESTScopeNameRoot,
}

// typeMeta is used as a key for lookup in the mapping between REST path and
// API object.
type typeMeta struct {
Expand All @@ -45,6 +82,7 @@ type typeMeta struct {
type DefaultRESTMapper struct {
mapping map[string]typeMeta
reverse map[typeMeta]string
scopes map[typeMeta]RESTScope
versions []string
interfacesFunc VersionInterfacesFunc
}
Expand All @@ -61,40 +99,38 @@ type VersionInterfacesFunc func(apiVersion string) (*VersionInterfaces, bool)
func NewDefaultRESTMapper(versions []string, f VersionInterfacesFunc) *DefaultRESTMapper {
mapping := make(map[string]typeMeta)
reverse := make(map[typeMeta]string)
scopes := make(map[typeMeta]RESTScope)
// TODO: verify name mappings work correctly when versions differ

return &DefaultRESTMapper{
mapping: mapping,
reverse: reverse,

mapping: mapping,
reverse: reverse,
scopes: scopes,
versions: versions,
interfacesFunc: f,
}
}

// Add adds objects from a runtime.Scheme and its named versions to this map.
// If mixedCase is true, the legacy v1beta1/v1beta2 Kubernetes resource naming convention
// will be applied (camelCase vs lowercase).
func (m *DefaultRESTMapper) Add(scheme *runtime.Scheme, mixedCase bool, versions ...string) {
for _, version := range versions {
for kind := range scheme.KnownTypes(version) {
plural, singular := kindToResource(kind, mixedCase)
meta := typeMeta{APIVersion: version, Kind: kind}
if _, ok := m.mapping[plural]; !ok {
m.mapping[plural] = meta
m.mapping[singular] = meta
if strings.ToLower(plural) != plural {
m.mapping[strings.ToLower(plural)] = meta
m.mapping[strings.ToLower(singular)] = meta
}
}
m.reverse[meta] = plural
func (m *DefaultRESTMapper) Add(scope RESTScope, kind string, version string, mixedCase bool) {
plural, singular := kindToResource(kind, mixedCase)
meta := typeMeta{APIVersion: version, Kind: kind}
if _, ok := m.mapping[plural]; !ok {
m.mapping[plural] = meta
m.mapping[singular] = meta
if strings.ToLower(plural) != plural {
m.mapping[strings.ToLower(plural)] = meta
m.mapping[strings.ToLower(singular)] = meta
}
}
m.reverse[meta] = plural
m.scopes[meta] = scope
}

// kindToResource converts Kind to a resource name.
func kindToResource(kind string, mixedCase bool) (plural, singular string) {
if len(kind) == 0 {
return
}
if mixedCase {
// Legacy support for mixed case names
singular = strings.ToLower(kind[:1]) + kind[1:]
Expand Down Expand Up @@ -167,6 +203,12 @@ func (m *DefaultRESTMapper) RESTMapping(kind string, versions ...string) (*RESTM
return nil, fmt.Errorf("the provided version %q and kind %q cannot be mapped to a supported object", version, kind)
}

// Ensure we have a REST scope
scope, ok := m.scopes[typeMeta{APIVersion: version, Kind: kind}]
if !ok {
return nil, fmt.Errorf("the provided version %q and kind %q cannot be mapped to a supported scope", version, kind)
}

interfaces, ok := m.interfacesFunc(version)
if !ok {
return nil, fmt.Errorf("the provided version %q has no relevant versions", version)
Expand All @@ -176,6 +218,7 @@ func (m *DefaultRESTMapper) RESTMapping(kind string, versions ...string) (*RESTM
Resource: resource,
APIVersion: version,
Kind: kind,
Scope: scope,

Codec: interfaces.Codec,
ObjectConvertor: interfaces.ObjectConvertor,
Expand Down
22 changes: 5 additions & 17 deletions pkg/api/meta/restmapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@ func TestRESTMapperVersionAndKindForResource(t *testing.T) {
}
for i, testCase := range testCases {
mapper := NewDefaultRESTMapper([]string{"test"}, fakeInterfaces)
scheme := runtime.NewScheme()
scheme.AddKnownTypes("test", &InternalObject{})
mapper.Add(scheme, testCase.MixedCase, "test")

mapper.Add(RESTScopeNamespace, testCase.Kind, testCase.APIVersion, testCase.MixedCase)
v, k, err := mapper.VersionAndKindForResource(testCase.Resource)
hasErr := err != nil
if hasErr != testCase.Err {
Expand Down Expand Up @@ -150,10 +147,7 @@ func TestRESTMapperRESTMapping(t *testing.T) {
}
for i, testCase := range testCases {
mapper := NewDefaultRESTMapper(testCase.DefaultVersions, fakeInterfaces)
scheme := runtime.NewScheme()
scheme.AddKnownTypes("test", &InternalObject{})
mapper.Add(scheme, testCase.MixedCase, "test")

mapper.Add(RESTScopeNamespace, "InternalObject", "test", testCase.MixedCase)
mapping, err := mapper.RESTMapping(testCase.Kind, testCase.APIVersions...)
hasErr := err != nil
if hasErr != testCase.Err {
Expand All @@ -180,11 +174,8 @@ func TestRESTMapperRESTMapping(t *testing.T) {

func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {
mapper := NewDefaultRESTMapper([]string{"test1", "test2"}, fakeInterfaces)
scheme := runtime.NewScheme()
scheme.AddKnownTypes("test1", &InternalObject{})
scheme.AddKnownTypeWithName("test2", "OtherObject", &InternalObject{})
scheme.AddKnownTypeWithName("test3", "OtherObject", &InternalObject{})
mapper.Add(scheme, false, "test1", "test2")
mapper.Add(RESTScopeNamespace, "InternalObject", "test1", false)
mapper.Add(RESTScopeNamespace, "OtherObject", "test2", false)

// pick default matching object kind based on search order
mapping, err := mapper.RESTMapping("OtherObject")
Expand Down Expand Up @@ -236,10 +227,7 @@ func TestRESTMapperRESTMappingSelectsVersion(t *testing.T) {

func TestRESTMapperReportsErrorOnBadVersion(t *testing.T) {
mapper := NewDefaultRESTMapper([]string{"test1", "test2"}, unmatchedVersionInterfaces)
scheme := runtime.NewScheme()
scheme.AddKnownTypes("test1", &InternalObject{})
mapper.Add(scheme, false, "test1")

mapper.Add(RESTScopeNamespace, "InternalObject", "test1", false)
_, err := mapper.RESTMapping("InternalObject", "test1")
if err == nil {
t.Errorf("unexpected non-error")
Expand Down