diff --git a/pkg/master/BUILD b/pkg/master/BUILD index c99e505660..fed6ca1c03 100644 --- a/pkg/master/BUILD +++ b/pkg/master/BUILD @@ -152,6 +152,7 @@ go_test( "//pkg/version:go_default_library", "//staging/src/k8s.io/api/certificates/v1beta1:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", diff --git a/pkg/master/import_known_versions_test.go b/pkg/master/import_known_versions_test.go index a0260656d0..a5ecc7638c 100644 --- a/pkg/master/import_known_versions_test.go +++ b/pkg/master/import_known_versions_test.go @@ -17,15 +17,12 @@ limitations under the License. package master import ( - "encoding/json" "reflect" - "strings" "testing" "k8s.io/api/core/v1" + apinamingtest "k8s.io/apimachinery/pkg/api/apitesting/naming" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/kubernetes/pkg/api/legacyscheme" @@ -48,20 +45,8 @@ func TestGroupVersions(t *testing.T) { t.Errorf("No additional unnamespaced groups should be created") } - for _, gv := range legacyscheme.Scheme.PrioritizedVersionsAllGroups() { - if !strings.HasSuffix(gv.Group, ".k8s.io") && !legacyUnsuffixedGroups.Has(gv.Group) { - t.Errorf("Group %s does not have the standard kubernetes API group suffix of .k8s.io", gv.Group) - } - } -} - -func TestTypeTags(t *testing.T) { - for gvk, knownType := range legacyscheme.Scheme.AllKnownTypes() { - if gvk.Version == runtime.APIVersionInternal { - ensureNoTags(t, gvk, knownType, nil) - } else { - ensureTags(t, gvk, knownType, nil) - } + if err := apinamingtest.VerifyGroupNames(legacyscheme.Scheme, legacyUnsuffixedGroups); err != nil { + t.Errorf("%v", err) } } @@ -87,100 +72,14 @@ var typesAllowedTags = map[reflect.Type]bool{ reflect.TypeOf(metav1.Status{}): true, } -func ensureNoTags(t *testing.T, gvk schema.GroupVersionKind, tp reflect.Type, parents []reflect.Type) { - if _, ok := typesAllowedTags[tp]; ok { - return - } - - parents = append(parents, tp) - - switch tp.Kind() { - case reflect.Map, reflect.Slice, reflect.Ptr: - ensureNoTags(t, gvk, tp.Elem(), parents) - - case reflect.String, reflect.Bool, reflect.Float32, reflect.Int32, reflect.Int64, reflect.Uint8, reflect.Uintptr, reflect.Uint32, reflect.Uint64, reflect.Interface: - // no-op - - case reflect.Struct: - for i := 0; i < tp.NumField(); i++ { - f := tp.Field(i) - if f.PkgPath != "" { - continue // Ignore unexported fields - } - jsonTag := f.Tag.Get("json") - protoTag := f.Tag.Get("protobuf") - if len(jsonTag) > 0 || len(protoTag) > 0 { - t.Errorf("Internal types should not have json or protobuf tags. %#v has tag on field %v: %v", gvk, f.Name, f.Tag) - for i, tp := range parents { - t.Logf("%s%v:", strings.Repeat(" ", i), tp) - } - } - - ensureNoTags(t, gvk, f.Type, parents) - } - - default: - t.Errorf("Unexpected type %v in %#v", tp.Kind(), gvk) - for i, tp := range parents { - t.Logf("%s%v:", strings.Repeat(" ", i), tp) - } - } -} - -var ( - marshalerType = reflect.TypeOf((*json.Marshaler)(nil)).Elem() - unmarshalerType = reflect.TypeOf((*json.Unmarshaler)(nil)).Elem() -) - // These fields are limited exceptions to the standard JSON naming structure. // Additions should only be made if a non-standard field name was released and cannot be changed for compatibility reasons. var allowedNonstandardJSONNames = map[reflect.Type]string{ reflect.TypeOf(v1.DaemonEndpoint{}): "Port", } -func ensureTags(t *testing.T, gvk schema.GroupVersionKind, tp reflect.Type, parents []reflect.Type) { - // This type handles its own encoding/decoding and doesn't need json tags - if tp.Implements(marshalerType) && (tp.Implements(unmarshalerType) || reflect.PtrTo(tp).Implements(unmarshalerType)) { - return - } - - parents = append(parents, tp) - - switch tp.Kind() { - case reflect.Map, reflect.Slice, reflect.Ptr: - ensureTags(t, gvk, tp.Elem(), parents) - - case reflect.String, reflect.Bool, reflect.Float32, reflect.Int, reflect.Int32, reflect.Int64, reflect.Uint8, reflect.Uintptr, reflect.Uint32, reflect.Uint64, reflect.Interface: - // no-op - - case reflect.Struct: - for i := 0; i < tp.NumField(); i++ { - f := tp.Field(i) - jsonTag := f.Tag.Get("json") - if len(jsonTag) == 0 { - t.Errorf("External types should have json tags. %#v tags on field %v are: %s", gvk, f.Name, f.Tag) - for i, tp := range parents { - t.Logf("%s%v", strings.Repeat(" ", i), tp) - } - } - - jsonTagName := strings.Split(jsonTag, ",")[0] - if len(jsonTagName) > 0 && (jsonTagName[0] < 'a' || jsonTagName[0] > 'z') && jsonTagName != "-" && allowedNonstandardJSONNames[tp] != jsonTagName { - t.Errorf("External types should have json names starting with lowercase letter. %#v has json tag on field %v with name %s", gvk, f.Name, jsonTagName) - t.Log(tp) - t.Log(allowedNonstandardJSONNames[tp]) - for i, tp := range parents { - t.Logf("%s%v", strings.Repeat(" ", i), tp) - } - } - - ensureTags(t, gvk, f.Type, parents) - } - - default: - t.Errorf("Unexpected type %v in %#v", tp.Kind(), gvk) - for i, tp := range parents { - t.Logf("%s%v:", strings.Repeat(" ", i), tp) - } +func TestTypeTags(t *testing.T) { + if err := apinamingtest.VerifyTagNaming(legacyscheme.Scheme, typesAllowedTags, allowedNonstandardJSONNames); err != nil { + t.Errorf("%v", err) } } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/BUILD b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/BUILD index 40a3072b8c..6144d9be6e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/BUILD +++ b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/BUILD @@ -30,6 +30,7 @@ filegroup( srcs = [ ":package-srcs", "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/fuzzer:all-srcs", + "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming:all-srcs", "//staging/src/k8s.io/apimachinery/pkg/api/apitesting/roundtrip:all-srcs", ], tags = ["automanaged"], diff --git a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming/BUILD b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming/BUILD new file mode 100644 index 0000000000..e5d2a06a1f --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming/BUILD @@ -0,0 +1,29 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["naming.go"], + importmap = "k8s.io/kubernetes/vendor/k8s.io/apimachinery/pkg/api/apitesting/naming", + importpath = "k8s.io/apimachinery/pkg/api/apitesting/naming", + visibility = ["//visibility:public"], + deps = [ + "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming/naming.go b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming/naming.go new file mode 100644 index 0000000000..f65e86b6e1 --- /dev/null +++ b/staging/src/k8s.io/apimachinery/pkg/api/apitesting/naming/naming.go @@ -0,0 +1,146 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package naming + +import ( + "encoding/json" + "fmt" + "reflect" + "strings" + + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/errors" + "k8s.io/apimachinery/pkg/util/sets" +) + +var ( + marshalerType = reflect.TypeOf((*json.Marshaler)(nil)).Elem() + unmarshalerType = reflect.TypeOf((*json.Unmarshaler)(nil)).Elem() +) + +// VerifyGroupNames ensures that all groups in the scheme ends with the k8s.io suffix. +// Exceptions can be tolerated using the legacyUnsuffixedGroups parameter +func VerifyGroupNames(scheme *runtime.Scheme, legacyUnsuffixedGroups sets.String) error { + errs := []error{} + for _, gv := range scheme.PrioritizedVersionsAllGroups() { + if !strings.HasSuffix(gv.Group, ".k8s.io") && !legacyUnsuffixedGroups.Has(gv.Group) { + errs = append(errs, fmt.Errorf("Group %s does not have the standard kubernetes API group suffix of .k8s.io", gv.Group)) + } + } + return errors.NewAggregate(errs) +} + +// VerifyTagNaming ensures that all types in the scheme have JSON tags set on external types, and JSON tags not set on internal types. +// Exceptions can be tolerated using the typesAllowedTags and allowedNonstandardJSONNames parameters +func VerifyTagNaming(scheme *runtime.Scheme, typesAllowedTags map[reflect.Type]bool, allowedNonstandardJSONNames map[reflect.Type]string) error { + errs := []error{} + for gvk, knownType := range scheme.AllKnownTypes() { + var err error + if gvk.Version == runtime.APIVersionInternal { + err = errors.NewAggregate(ensureNoTags(gvk, knownType, nil, typesAllowedTags)) + } else { + err = errors.NewAggregate(ensureTags(gvk, knownType, nil, allowedNonstandardJSONNames)) + } + if err != nil { + errs = append(errs, err) + } + } + return errors.NewAggregate(errs) +} + +func ensureNoTags(gvk schema.GroupVersionKind, tp reflect.Type, parents []reflect.Type, typesAllowedTags map[reflect.Type]bool) []error { + errs := []error{} + if _, ok := typesAllowedTags[tp]; ok { + return errs + } + + parents = append(parents, tp) + + switch tp.Kind() { + case reflect.Map, reflect.Slice, reflect.Ptr: + errs = append(errs, ensureNoTags(gvk, tp.Elem(), parents, typesAllowedTags)...) + + case reflect.String, reflect.Bool, reflect.Float32, reflect.Float64, reflect.Int32, reflect.Int64, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, reflect.Interface: + // no-op + + case reflect.Struct: + for i := 0; i < tp.NumField(); i++ { + f := tp.Field(i) + if f.PkgPath != "" { + continue // Ignore unexported fields + } + jsonTag := f.Tag.Get("json") + protoTag := f.Tag.Get("protobuf") + if len(jsonTag) > 0 || len(protoTag) > 0 { + errs = append(errs, fmt.Errorf("Internal types should not have json or protobuf tags. %#v has tag on field %v: %v.\n%s", gvk, f.Name, f.Tag, fmtParentString(parents))) + } + + errs = append(errs, ensureNoTags(gvk, f.Type, parents, typesAllowedTags)...) + } + + default: + errs = append(errs, fmt.Errorf("Unexpected type %v in %#v.\n%s", tp.Kind(), gvk, fmtParentString(parents))) + } + return errs +} + +func ensureTags(gvk schema.GroupVersionKind, tp reflect.Type, parents []reflect.Type, allowedNonstandardJSONNames map[reflect.Type]string) []error { + errs := []error{} + // This type handles its own encoding/decoding and doesn't need json tags + if tp.Implements(marshalerType) && (tp.Implements(unmarshalerType) || reflect.PtrTo(tp).Implements(unmarshalerType)) { + return errs + } + + parents = append(parents, tp) + + switch tp.Kind() { + case reflect.Map, reflect.Slice, reflect.Ptr: + errs = append(errs, ensureTags(gvk, tp.Elem(), parents, allowedNonstandardJSONNames)...) + + case reflect.String, reflect.Bool, reflect.Float32, reflect.Float64, reflect.Int32, reflect.Int64, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr, reflect.Interface: + // no-op + + case reflect.Struct: + for i := 0; i < tp.NumField(); i++ { + f := tp.Field(i) + jsonTag := f.Tag.Get("json") + if len(jsonTag) == 0 { + errs = append(errs, fmt.Errorf("External types should have json tags. %#v tags on field %v are: %s.\n%s", gvk, f.Name, f.Tag, fmtParentString(parents))) + } + + jsonTagName := strings.Split(jsonTag, ",")[0] + if len(jsonTagName) > 0 && (jsonTagName[0] < 'a' || jsonTagName[0] > 'z') && jsonTagName != "-" && allowedNonstandardJSONNames[tp] != jsonTagName { + errs = append(errs, fmt.Errorf("External types should have json names starting with lowercase letter. %#v has json tag on field %v with name %s.\n%s", gvk, f.Name, jsonTagName, fmtParentString(parents))) + } + + errs = append(errs, ensureTags(gvk, f.Type, parents, allowedNonstandardJSONNames)...) + } + + default: + errs = append(errs, fmt.Errorf("Unexpected type %v in %#v.\n%s", tp.Kind(), gvk, fmtParentString(parents))) + } + return errs +} + +func fmtParentString(parents []reflect.Type) string { + str := "Type parents:\n" + for i, tp := range parents { + str += fmt.Sprintf("%s%v\n", strings.Repeat(" ", i), tp) + } + return str +}