Merge pull request #38406 from liggitt/remove-internal-json-annotations

Automatic merge from submit-queue

Remove json serialization annotations from internal types

fixes #3933

Internal types should never be serialized, and including json serialization tags on them makes it possible to accidentally do that without realizing it.

fixes in this PR:

* types
  * [x] remove json tags from internal types
  * [x] fix references from serialized types to internal ObjectMeta
* generation
  * [x] remove generated json codecs for internal types (they should never be used)
* kubectl
  * [x] fix `apply` to operate on versioned object
  * [x] fix sorting by field to operate on versioned object
  * [x] fix `--record` to build annotation patch using versioned object
* hpa
  * [x] fix unmarshaling to internal CustomMetricTargetList in validation
* thirdpartyresources
  * [x] fix encoding API responses using internal ObjectMeta
* tests
  * [x] fix tests to use versioned objects when checking encoded content
  * [x] fix tests passing internal objects to generic printers

follow ups (will open tracking issues or additional PRs):
- [ ] remove json tags from internal kubeconfig types (`kubectl config set` pathfinding needs to work against external type)
- [ ] HPA should version CustomMetricTargetList serialization in annotations
- [ ] revisit how TPR resthandlers encoding objects
- [ ] audit and add tests for printer use (human-readable printer requires internal versions, generic printers require external versions)
- [ ] add static analysis tests preventing new internal types from adding tags
- [ ] add static analysis tests requiring json tags on external types (and enforcing lower-case first letter)
- [ ] add more tests for `kubectl get` exercising known and unknown types with all output options

plugin/pkg/scheduler/api/types.go

@@ -25,34 +25,34 @@ import (
 )
 
 type Policy struct {
-	metav1.TypeMeta `json:",inline"`
+	metav1.TypeMeta
 	// Holds the information to configure the fit predicate functions
-	Predicates []PredicatePolicy `json:"predicates"`
+	Predicates []PredicatePolicy
 	// Holds the information to configure the priority functions
-	Priorities []PriorityPolicy `json:"priorities"`
+	Priorities []PriorityPolicy
 	// Holds the information to communicate with the extender(s)
-	ExtenderConfigs []ExtenderConfig `json:"extenders"`
+	ExtenderConfigs []ExtenderConfig
 }
 
 type PredicatePolicy struct {
 	// Identifier of the predicate policy
 	// For a custom predicate, the name can be user-defined
 	// For the Kubernetes provided predicates, the name is the identifier of the pre-defined predicate
-	Name string `json:"name"`
+	Name string
 	// Holds the parameters to configure the given predicate
-	Argument *PredicateArgument `json:"argument"`
+	Argument *PredicateArgument
 }
 
 type PriorityPolicy struct {
 	// Identifier of the priority policy
 	// For a custom priority, the name can be user-defined
 	// For the Kubernetes provided priority functions, the name is the identifier of the pre-defined priority function
-	Name string `json:"name"`
+	Name string
 	// The numeric multiplier for the node scores that the priority function generates
 	// The weight should be a positive integer
-	Weight int `json:"weight"`
+	Weight int
 	// Holds the parameters to configure the given priority function
-	Argument *PriorityArgument `json:"argument"`
+	Argument *PriorityArgument
 }
 
 // Represents the arguments that the different types of predicates take
@@ -60,10 +60,10 @@ type PriorityPolicy struct {
 type PredicateArgument struct {
 	// The predicate that provides affinity for pods belonging to a service
 	// It uses a label to identify nodes that belong to the same "group"
-	ServiceAffinity *ServiceAffinity `json:"serviceAffinity"`
+	ServiceAffinity *ServiceAffinity
 	// The predicate that checks whether a particular node has a certain label
 	// defined or not, regardless of value
-	LabelsPresence *LabelsPresence `json:"labelsPresence"`
+	LabelsPresence *LabelsPresence
 }
 
 // Represents the arguments that the different types of priorities take.
@@ -71,72 +71,72 @@ type PredicateArgument struct {
 type PriorityArgument struct {
 	// The priority function that ensures a good spread (anti-affinity) for pods belonging to a service
 	// It uses a label to identify nodes that belong to the same "group"
-	ServiceAntiAffinity *ServiceAntiAffinity `json:"serviceAntiAffinity"`
+	ServiceAntiAffinity *ServiceAntiAffinity
 	// The priority function that checks whether a particular node has a certain label
 	// defined or not, regardless of value
-	LabelPreference *LabelPreference `json:"labelPreference"`
+	LabelPreference *LabelPreference
 }
 
 // Holds the parameters that are used to configure the corresponding predicate
 type ServiceAffinity struct {
 	// The list of labels that identify node "groups"
 	// All of the labels should match for the node to be considered a fit for hosting the pod
-	Labels []string `json:"labels"`
+	Labels []string
 }
 
 // Holds the parameters that are used to configure the corresponding predicate
 type LabelsPresence struct {
 	// The list of labels that identify node "groups"
 	// All of the labels should be either present (or absent) for the node to be considered a fit for hosting the pod
-	Labels []string `json:"labels"`
+	Labels []string
 	// The boolean flag that indicates whether the labels should be present or absent from the node
-	Presence bool `json:"presence"`
+	Presence bool
 }
 
 // Holds the parameters that are used to configure the corresponding priority function
 type ServiceAntiAffinity struct {
 	// Used to identify node "groups"
-	Label string `json:"label"`
+	Label string
 }
 
 // Holds the parameters that are used to configure the corresponding priority function
 type LabelPreference struct {
 	// Used to identify node "groups"
-	Label string `json:"label"`
+	Label string
 	// This is a boolean flag
 	// If true, higher priority is given to nodes that have the label
 	// If false, higher priority is given to nodes that do not have the label
-	Presence bool `json:"presence"`
+	Presence bool
 }
 
 // Holds the parameters used to communicate with the extender. If a verb is unspecified/empty,
 // it is assumed that the extender chose not to provide that extension.
 type ExtenderConfig struct {
 	// URLPrefix at which the extender is available
-	URLPrefix string `json:"urlPrefix"`
+	URLPrefix string
 	// Verb for the filter call, empty if not supported. This verb is appended to the URLPrefix when issuing the filter call to extender.
-	FilterVerb string `json:"filterVerb,omitempty"`
+	FilterVerb string
 	// Verb for the prioritize call, empty if not supported. This verb is appended to the URLPrefix when issuing the prioritize call to extender.
-	PrioritizeVerb string `json:"prioritizeVerb,omitempty"`
+	PrioritizeVerb string
 	// The numeric multiplier for the node scores that the prioritize call generates.
 	// The weight should be a positive integer
-	Weight int `json:"weight,omitempty"`
+	Weight int
 	// EnableHttps specifies whether https should be used to communicate with the extender
-	EnableHttps bool `json:"enableHttps,omitempty"`
+	EnableHttps bool
 	// TLSConfig specifies the transport layer security config
-	TLSConfig *restclient.TLSClientConfig `json:"tlsConfig,omitempty"`
+	TLSConfig *restclient.TLSClientConfig
 	// HTTPTimeout specifies the timeout duration for a call to the extender. Filter timeout fails the scheduling of the pod. Prioritize
 	// timeout is ignored, k8s/other extenders priorities are used to select the node.
-	HTTPTimeout time.Duration `json:"httpTimeout,omitempty"`
+	HTTPTimeout time.Duration
 }
 
 // ExtenderArgs represents the arguments needed by the extender to filter/prioritize
 // nodes for a pod.
 type ExtenderArgs struct {
 	// Pod being scheduled
-	Pod v1.Pod `json:"pod"`
+	Pod v1.Pod
 	// List of candidate nodes where the pod can be scheduled
-	Nodes v1.NodeList `json:"nodes"`
+	Nodes v1.NodeList
 }
 
 // FailedNodesMap represents the filtered out nodes, with node names and failure messages
@@ -145,19 +145,19 @@ type FailedNodesMap map[string]string
 // ExtenderFilterResult represents the results of a filter call to an extender
 type ExtenderFilterResult struct {
 	// Filtered set of nodes where the pod can be scheduled
-	Nodes v1.NodeList `json:"nodes,omitempty"`
+	Nodes v1.NodeList
 	// Filtered out nodes where the pod can't be scheduled and the failure messages
-	FailedNodes FailedNodesMap `json:"failedNodes,omitempty"`
+	FailedNodes FailedNodesMap
 	// Error message indicating failure
-	Error string `json:"error,omitempty"`
+	Error string
 }
 
 // HostPriority represents the priority of scheduling to a particular host, higher priority is better.
 type HostPriority struct {
 	// Name of the host
-	Host string `json:"host"`
+	Host string
 	// Score associated with the host
-	Score int `json:"score"`
+	Score int
 }
 
 type HostPriorityList []HostPriority