Merge pull request #39152 from liggitt/json-tags

Automatic merge from submit-queue (batch tested with PRs 39152, 39142, 39055)

Add test for json tags on internal and external types

Follow up from https://github.com/kubernetes/kubernetes/pull/38406

- adds static analysis tests preventing internal types from adding new json or protobuf tags
- adds static analysis tests requiring json tags on external types (and enforcing lower-case first letter)
- fixes issues found by the tests
This commit is contained in:
Kubernetes Submit Queue 2016-12-27 17:10:11 -08:00 committed by GitHub
commit 59bd66ce3e
4 changed files with 140 additions and 27 deletions

View File

@ -1933,7 +1933,7 @@ type PodStatus struct {
// +optional
StartTime *metav1.Time
// +optional
QOSClass PodQOSClass `json:"qosClass,omitempty"`
QOSClass PodQOSClass
// The list has one entry per init container in the manifest. The most recent successful
// init container will have ready = true, the most recently started container will have

View File

@ -22,7 +22,7 @@ import (
)
type KubeProxyConfiguration struct {
metav1.TypeMeta
metav1.TypeMeta `json:",inline"`
// bindAddress is the IP address for the proxy server to serve on (set to 0.0.0.0
// for all interfaces)
@ -100,7 +100,7 @@ const (
)
type KubeSchedulerConfiguration struct {
metav1.TypeMeta
metav1.TypeMeta `json:",inline"`
// port is the port that the scheduler's http service runs on.
Port int `json:"port"`
@ -178,7 +178,7 @@ type LeaderElectionConfiguration struct {
}
type KubeletConfiguration struct {
metav1.TypeMeta
metav1.TypeMeta `json:",inline"`
// podManifestPath is the path to the directory containing pod manifests to
// run, or the path to a single manifest file
@ -523,7 +523,7 @@ type KubeletConfiguration struct {
// This flag, if set, enables a check prior to mount operations to verify that the required components
// (binaries, etc.) to mount the volume are available on the underlying node. If the check is enabled
// and fails the mount operation fails.
ExperimentalCheckNodeCapabilitiesBeforeMount bool `json:"ExperimentalCheckNodeCapabilitiesBeforeMount,omitempty"`
ExperimentalCheckNodeCapabilitiesBeforeMount bool `json:"experimentalCheckNodeCapabilitiesBeforeMount,omitempty"`
}
type KubeletAuthorizationMode string

View File

@ -8010,11 +8010,6 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"TypeMeta": {
SchemaProps: spec.SchemaProps{
Ref: spec.MustCreateRef("#/definitions/v1.TypeMeta"),
},
},
"bindAddress": {
SchemaProps: spec.SchemaProps{
Description: "bindAddress is the IP address for the proxy server to serve on (set to 0.0.0.0 for all interfaces)",
@ -8158,21 +8153,16 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
},
},
},
Required: []string{"TypeMeta", "bindAddress", "clusterCIDR", "healthzBindAddress", "healthzPort", "hostnameOverride", "iptablesMasqueradeBit", "iptablesSyncPeriodSeconds", "iptablesMinSyncPeriodSeconds", "kubeconfigPath", "masqueradeAll", "master", "oomScoreAdj", "mode", "portRange", "resourceContainer", "udpTimeoutMilliseconds", "conntrackMax", "conntrackMaxPerCore", "conntrackMin", "conntrackTCPEstablishedTimeout", "conntrackTCPCloseWaitTimeout"},
Required: []string{"bindAddress", "clusterCIDR", "healthzBindAddress", "healthzPort", "hostnameOverride", "iptablesMasqueradeBit", "iptablesSyncPeriodSeconds", "iptablesMinSyncPeriodSeconds", "kubeconfigPath", "masqueradeAll", "master", "oomScoreAdj", "mode", "portRange", "resourceContainer", "udpTimeoutMilliseconds", "conntrackMax", "conntrackMaxPerCore", "conntrackMin", "conntrackTCPEstablishedTimeout", "conntrackTCPCloseWaitTimeout"},
},
},
Dependencies: []string{
"v1.Duration", "v1.TypeMeta"},
"v1.Duration"},
},
"v1alpha1.KubeSchedulerConfiguration": {
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"TypeMeta": {
SchemaProps: spec.SchemaProps{
Ref: spec.MustCreateRef("#/definitions/v1.TypeMeta"),
},
},
"port": {
SchemaProps: spec.SchemaProps{
Description: "port is the port that the scheduler's http service runs on.",
@ -8264,11 +8254,11 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
},
},
},
Required: []string{"TypeMeta", "port", "address", "algorithmProvider", "policyConfigFile", "enableProfiling", "enableContentionProfiling", "contentType", "kubeAPIQPS", "kubeAPIBurst", "schedulerName", "hardPodAffinitySymmetricWeight", "failureDomains", "leaderElection"},
Required: []string{"port", "address", "algorithmProvider", "policyConfigFile", "enableProfiling", "enableContentionProfiling", "contentType", "kubeAPIQPS", "kubeAPIBurst", "schedulerName", "hardPodAffinitySymmetricWeight", "failureDomains", "leaderElection"},
},
},
Dependencies: []string{
"v1.TypeMeta", "v1alpha1.LeaderElectionConfiguration"},
"v1alpha1.LeaderElectionConfiguration"},
},
"v1alpha1.KubeletAnonymousAuthentication": {
Schema: spec.Schema{
@ -8344,11 +8334,6 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
Schema: spec.Schema{
SchemaProps: spec.SchemaProps{
Properties: map[string]spec.Schema{
"TypeMeta": {
SchemaProps: spec.SchemaProps{
Ref: spec.MustCreateRef("#/definitions/v1.TypeMeta"),
},
},
"podManifestPath": {
SchemaProps: spec.SchemaProps{
Description: "podManifestPath is the path to the directory containing pod manifests to run, or the path to a single manifest file",
@ -9181,7 +9166,7 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
Format: "",
},
},
"ExperimentalCheckNodeCapabilitiesBeforeMount": {
"experimentalCheckNodeCapabilitiesBeforeMount": {
SchemaProps: spec.SchemaProps{
Description: "This flag, if set, enables a check prior to mount operations to verify that the required components (binaries, etc.) to mount the volume are available on the underlying node. If the check is enabled and fails the mount operation fails.",
Type: []string{"boolean"},
@ -9189,11 +9174,11 @@ var OpenAPIDefinitions *common.OpenAPIDefinitions = &common.OpenAPIDefinitions{
},
},
},
Required: []string{"TypeMeta", "podManifestPath", "syncFrequency", "fileCheckFrequency", "httpCheckFrequency", "manifestURL", "manifestURLHeader", "enableServer", "address", "port", "readOnlyPort", "tlsCertFile", "tlsPrivateKeyFile", "certDirectory", "authentication", "authorization", "hostnameOverride", "podInfraContainerImage", "dockerEndpoint", "rootDirectory", "seccompProfileRoot", "allowPrivileged", "hostNetworkSources", "hostPIDSources", "hostIPCSources", "registryPullQPS", "registryBurst", "eventRecordQPS", "eventBurst", "enableDebuggingHandlers", "minimumGCAge", "maxPerPodContainerCount", "maxContainerCount", "cAdvisorPort", "healthzPort", "healthzBindAddress", "oomScoreAdj", "registerNode", "clusterDomain", "masterServiceNamespace", "clusterDNS", "streamingConnectionIdleTimeout", "nodeStatusUpdateFrequency", "imageMinimumGCAge", "imageGCHighThresholdPercent", "imageGCLowThresholdPercent", "lowDiskSpaceThresholdMB", "volumeStatsAggPeriod", "networkPluginName", "networkPluginDir", "cniConfDir", "cniBinDir", "networkPluginMTU", "volumePluginDir", "cloudProvider", "cloudConfigFile", "kubeletCgroups", "runtimeCgroups", "systemCgroups", "cgroupRoot", "containerRuntime", "remoteRuntimeEndpoint", "remoteImageEndpoint", "runtimeRequestTimeout", "rktPath", "rktAPIEndpoint", "rktStage1Image", "lockFilePath", "exitOnLockContention", "hairpinMode", "babysitDaemons", "maxPods", "nvidiaGPUs", "dockerExecHandlerName", "podCIDR", "resolvConf", "cpuCFSQuota", "containerized", "maxOpenFiles", "reconcileCIDR", "registerSchedulable", "registerWithTaints", "contentType", "kubeAPIQPS", "kubeAPIBurst", "serializeImagePulls", "outOfDiskTransitionFrequency", "nodeIP", "nodeLabels", "nonMasqueradeCIDR", "enableCustomMetrics", "evictionHard", "evictionSoft", "evictionSoftGracePeriod", "evictionPressureTransitionPeriod", "evictionMaxPodGracePeriod", "evictionMinimumReclaim", "experimentalKernelMemcgNotification", "podsPerCore", "enableControllerAttachDetach", "systemReserved", "kubeReserved", "protectKernelDefaults", "makeIPTablesUtilChains", "iptablesMasqueradeBit", "iptablesDropBit"},
Required: []string{"podManifestPath", "syncFrequency", "fileCheckFrequency", "httpCheckFrequency", "manifestURL", "manifestURLHeader", "enableServer", "address", "port", "readOnlyPort", "tlsCertFile", "tlsPrivateKeyFile", "certDirectory", "authentication", "authorization", "hostnameOverride", "podInfraContainerImage", "dockerEndpoint", "rootDirectory", "seccompProfileRoot", "allowPrivileged", "hostNetworkSources", "hostPIDSources", "hostIPCSources", "registryPullQPS", "registryBurst", "eventRecordQPS", "eventBurst", "enableDebuggingHandlers", "minimumGCAge", "maxPerPodContainerCount", "maxContainerCount", "cAdvisorPort", "healthzPort", "healthzBindAddress", "oomScoreAdj", "registerNode", "clusterDomain", "masterServiceNamespace", "clusterDNS", "streamingConnectionIdleTimeout", "nodeStatusUpdateFrequency", "imageMinimumGCAge", "imageGCHighThresholdPercent", "imageGCLowThresholdPercent", "lowDiskSpaceThresholdMB", "volumeStatsAggPeriod", "networkPluginName", "networkPluginDir", "cniConfDir", "cniBinDir", "networkPluginMTU", "volumePluginDir", "cloudProvider", "cloudConfigFile", "kubeletCgroups", "runtimeCgroups", "systemCgroups", "cgroupRoot", "containerRuntime", "remoteRuntimeEndpoint", "remoteImageEndpoint", "runtimeRequestTimeout", "rktPath", "rktAPIEndpoint", "rktStage1Image", "lockFilePath", "exitOnLockContention", "hairpinMode", "babysitDaemons", "maxPods", "nvidiaGPUs", "dockerExecHandlerName", "podCIDR", "resolvConf", "cpuCFSQuota", "containerized", "maxOpenFiles", "reconcileCIDR", "registerSchedulable", "registerWithTaints", "contentType", "kubeAPIQPS", "kubeAPIBurst", "serializeImagePulls", "outOfDiskTransitionFrequency", "nodeIP", "nodeLabels", "nonMasqueradeCIDR", "enableCustomMetrics", "evictionHard", "evictionSoft", "evictionSoftGracePeriod", "evictionPressureTransitionPeriod", "evictionMaxPodGracePeriod", "evictionMinimumReclaim", "experimentalKernelMemcgNotification", "podsPerCore", "enableControllerAttachDetach", "systemReserved", "kubeReserved", "protectKernelDefaults", "makeIPTablesUtilChains", "iptablesMasqueradeBit", "iptablesDropBit"},
},
},
Dependencies: []string{
"v1.Duration", "v1.Taint", "v1.TypeMeta", "v1alpha1.KubeletAuthentication", "v1alpha1.KubeletAuthorization"},
"v1.Duration", "v1.Taint", "v1alpha1.KubeletAuthentication", "v1alpha1.KubeletAuthorization"},
},
"v1alpha1.KubeletWebhookAuthentication": {
Schema: spec.Schema{

View File

@ -17,10 +17,18 @@ limitations under the License.
package master
import (
"encoding/json"
"reflect"
"strings"
"testing"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/api/v1"
"k8s.io/kubernetes/pkg/apimachinery/registered"
metav1 "k8s.io/kubernetes/pkg/apis/meta/v1"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/runtime/schema"
"k8s.io/kubernetes/pkg/util/intstr"
"k8s.io/kubernetes/pkg/util/sets"
)
@ -49,3 +57,123 @@ func TestGroupVersions(t *testing.T) {
}
}
}
func TestTypeTags(t *testing.T) {
for gvk, knownType := range api.Scheme.AllKnownTypes() {
if gvk.Version == runtime.APIVersionInternal {
ensureNoTags(t, gvk, knownType, nil)
} else {
ensureTags(t, gvk, knownType, nil)
}
}
}
// These types are registered in external versions, and therefore include json tags,
// but are also registered in internal versions (or referenced from internal types),
// so we explicitly allow tags for them
var typesAllowedTags = map[reflect.Type]bool{
reflect.TypeOf(intstr.IntOrString{}): true,
reflect.TypeOf(metav1.Time{}): true,
reflect.TypeOf(metav1.Duration{}): true,
reflect.TypeOf(metav1.TypeMeta{}): true,
reflect.TypeOf(metav1.ListMeta{}): true,
reflect.TypeOf(metav1.OwnerReference{}): true,
reflect.TypeOf(metav1.LabelSelector{}): true,
reflect.TypeOf(metav1.GetOptions{}): true,
reflect.TypeOf(metav1.ExportOptions{}): 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.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")
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)
}
}
}