drop Enabled() checks for ServerSideFieldValidation feature gate

This commit is contained in:
Kevin Delgado 2023-01-24 17:48:31 +00:00
parent 3b6c4d307f
commit 2d5ceb9b15
7 changed files with 19 additions and 58 deletions

View File

@ -39,8 +39,6 @@ import (
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/apiserver/pkg/endpoints"
"k8s.io/apiserver/pkg/endpoints/openapi"
"k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
utilopenapi "k8s.io/apiserver/pkg/util/openapi"
openapibuilder "k8s.io/kube-openapi/pkg/builder"
"k8s.io/kube-openapi/pkg/builder3"
@ -347,10 +345,6 @@ func (b *builder) buildRoute(root, path, httpMethod, actionVerb, operationVerb s
route.Consumes(runtime.ContentTypeJSON, runtime.ContentTypeYAML)
}
var disabledParams []string
if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideFieldValidation) {
disabledParams = []string{"fieldValidation"}
}
// Build option parameters
switch actionVerb {
case "get":
@ -360,9 +354,9 @@ func (b *builder) buildRoute(root, path, httpMethod, actionVerb, operationVerb s
endpoints.AddObjectParams(b.ws, route, &metav1.ListOptions{})
case "put", "patch":
// TODO: PatchOption added in feature branch but not in master yet
endpoints.AddObjectParams(b.ws, route, &metav1.UpdateOptions{}, disabledParams...)
endpoints.AddObjectParams(b.ws, route, &metav1.UpdateOptions{})
case "post":
endpoints.AddObjectParams(b.ws, route, &metav1.CreateOptions{}, disabledParams...)
endpoints.AddObjectParams(b.ws, route, &metav1.CreateOptions{})
case "delete":
endpoints.AddObjectParams(b.ws, route, &metav1.DeleteOptions{})
route.Reads(&metav1.DeleteOptions{}).ParameterNamed("body").Required(false)

View File

@ -542,19 +542,16 @@ type CreateOptions struct {
// fieldValidation instructs the server on how to handle
// objects in the request (POST/PUT/PATCH) containing unknown
// or duplicate fields, provided that the `ServerSideFieldValidation`
// feature gate is also enabled. Valid values are:
// or duplicate fields. Valid values are:
// - Ignore: This will ignore any unknown fields that are silently
// dropped from the object, and will ignore all but the last duplicate
// field that the decoder encounters. This is the default behavior
// prior to v1.23 and is the default behavior when the
// `ServerSideFieldValidation` feature gate is disabled.
// prior to v1.23.
// - Warn: This will send a warning via the standard warning response
// header for each unknown field that is dropped from the object, and
// for each duplicate field that is encountered. The request will
// still succeed if there are no other errors, and will only persist
// the last of any duplicate fields. This is the default when the
// `ServerSideFieldValidation` feature gate is enabled.
// the last of any duplicate fields. This is the default in v1.23+
// - Strict: This will fail the request with a BadRequest error if
// any unknown fields would be dropped from the object, or if any
// duplicate fields are present. The error returned from the server
@ -597,19 +594,16 @@ type PatchOptions struct {
// fieldValidation instructs the server on how to handle
// objects in the request (POST/PUT/PATCH) containing unknown
// or duplicate fields, provided that the `ServerSideFieldValidation`
// feature gate is also enabled. Valid values are:
// or duplicate fields. Valid values are:
// - Ignore: This will ignore any unknown fields that are silently
// dropped from the object, and will ignore all but the last duplicate
// field that the decoder encounters. This is the default behavior
// prior to v1.23 and is the default behavior when the
// `ServerSideFieldValidation` feature gate is disabled.
// prior to v1.23.
// - Warn: This will send a warning via the standard warning response
// header for each unknown field that is dropped from the object, and
// for each duplicate field that is encountered. The request will
// still succeed if there are no other errors, and will only persist
// the last of any duplicate fields. This is the default when the
// `ServerSideFieldValidation` feature gate is enabled.
// the last of any duplicate fields. This is the default in v1.23+
// - Strict: This will fail the request with a BadRequest error if
// any unknown fields would be dropped from the object, or if any
// duplicate fields are present. The error returned from the server
@ -674,19 +668,16 @@ type UpdateOptions struct {
// fieldValidation instructs the server on how to handle
// objects in the request (POST/PUT/PATCH) containing unknown
// or duplicate fields, provided that the `ServerSideFieldValidation`
// feature gate is also enabled. Valid values are:
// or duplicate fields. Valid values are:
// - Ignore: This will ignore any unknown fields that are silently
// dropped from the object, and will ignore all but the last duplicate
// field that the decoder encounters. This is the default behavior
// prior to v1.23 and is the default behavior when the
// `ServerSideFieldValidation` feature gate is disabled.
// prior to v1.23.
// - Warn: This will send a warning via the standard warning response
// header for each unknown field that is dropped from the object, and
// for each duplicate field that is encountered. The request will
// still succeed if there are no other errors, and will only persist
// the last of any duplicate fields. This is the default when the
// `ServerSideFieldValidation` feature gate is enabled.
// the last of any duplicate fields. This is the default in v1.23+
// - Strict: This will fail the request with a BadRequest error if
// any unknown fields would be dropped from the object, or if any
// duplicate fields are present. The error returned from the server

View File

@ -72,10 +72,7 @@ import (
"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
"k8s.io/apiserver/pkg/endpoints/request"
genericapitesting "k8s.io/apiserver/pkg/endpoints/testing"
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/registry/rest"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
)
type alwaysMutatingDeny struct{}
@ -4041,7 +4038,6 @@ var (
// TestFieldValidation tests the create, update, and patch handlers for correctness when faced with field validation errors.
func TestFieldValidation(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServerSideFieldValidation, true)()
var (
strictDecodingErr = `strict decoding error: duplicate field \"other\", unknown field \"unknown\"`
strictDecodingWarns = []string{`duplicate field "other"`, `unknown field "unknown"`}
@ -4180,7 +4176,6 @@ unknown: baz`)
// BenchmarkFieldValidation benchmarks the create, update, and patch handlers for performance distinctions between
// strict, warn, and ignore field validation handling.
func BenchmarkFieldValidation(b *testing.B) {
defer featuregatetesting.SetFeatureGateDuringTest(b, utilfeature.DefaultFeatureGate, features.ServerSideFieldValidation, true)()
var (
validJSONDataPost = []byte(`{"kind":"Simple", "apiVersion":"test.group/version", "metadata":{"creationTimestamp":null}, "other":"foo"}`)
validYAMLDataPost = []byte(`apiVersion: test.group/version

View File

@ -45,9 +45,7 @@ import (
"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
"k8s.io/apiserver/pkg/endpoints/metrics"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/features"
"k8s.io/apiserver/pkg/registry/rest"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/apiserver/pkg/warning"
)
@ -406,13 +404,10 @@ func isDryRun(url *url.URL) bool {
// fieldValidation checks that the field validation feature is enabled
// and returns a valid directive of either
// - Ignore (default when feature is disabled)
// - Warn (default when feature is enabled)
// - Ignore
// - Warn (default)
// - Strict
func fieldValidation(directive string) string {
if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideFieldValidation) {
return metav1.FieldValidationIgnore
}
if directive == "" {
return metav1.FieldValidationWarn
}

View File

@ -622,11 +622,6 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
}
}
var disabledParams []string
if !utilfeature.DefaultFeatureGate.Enabled(features.ServerSideFieldValidation) {
disabledParams = []string{"fieldValidation"}
}
// Create Routes for the actions.
// TODO: Add status documentation using Returns()
// Errors (see api/errors/errors.go as well as go-restful router):
@ -857,7 +852,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
Returns(http.StatusCreated, "Created", producedObject).
Reads(defaultVersionedObject).
Writes(producedObject)
if err := AddObjectParams(ws, route, versionedUpdateOptions, disabledParams...); err != nil {
if err := AddObjectParams(ws, route, versionedUpdateOptions); err != nil {
return nil, nil, err
}
addParams(route, action.Params)
@ -886,7 +881,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
Returns(http.StatusCreated, "Created", producedObject).
Reads(metav1.Patch{}).
Writes(producedObject)
if err := AddObjectParams(ws, route, versionedPatchOptions, disabledParams...); err != nil {
if err := AddObjectParams(ws, route, versionedPatchOptions); err != nil {
return nil, nil, err
}
addParams(route, action.Params)
@ -917,7 +912,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
Returns(http.StatusAccepted, "Accepted", producedObject).
Reads(defaultVersionedObject).
Writes(producedObject)
if err := AddObjectParams(ws, route, versionedCreateOptions, disabledParams...); err != nil {
if err := AddObjectParams(ws, route, versionedCreateOptions); err != nil {
return nil, nil, err
}
addParams(route, action.Params)

View File

@ -33,8 +33,6 @@ import (
"k8s.io/apiserver/pkg/authentication/user"
"k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/endpoints/responsewriter"
"k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
compbasemetrics "k8s.io/component-base/metrics"
"k8s.io/component-base/metrics/legacyregistry"
)
@ -136,7 +134,7 @@ var (
fieldValidationRequestLatencies = compbasemetrics.NewHistogramVec(
&compbasemetrics.HistogramOpts{
Name: "field_validation_request_duration_seconds",
Help: "Response latency distribution in seconds for each field validation value and whether field validation is enabled or not",
Help: "Response latency distribution in seconds for each field validation value",
// This metric is supplementary to the requestLatencies metric.
// It measures request durations for the various field validation
// values.
@ -144,7 +142,7 @@ var (
4, 5, 6, 8, 10, 15, 20, 30, 45, 60},
StabilityLevel: compbasemetrics.ALPHA,
},
[]string{"field_validation", "enabled"},
[]string{"field_validation"},
)
responseSizes = compbasemetrics.NewHistogramVec(
&compbasemetrics.HistogramOpts{
@ -543,8 +541,7 @@ func MonitorRequest(req *http.Request, verb, group, version, resource, subresour
}
requestLatencies.WithContext(req.Context()).WithLabelValues(reportedVerb, dryRun, group, version, resource, subresource, scope, component).Observe(elapsedSeconds)
fieldValidation := cleanFieldValidation(req.URL)
fieldValidationEnabled := strconv.FormatBool(utilfeature.DefaultFeatureGate.Enabled(features.ServerSideFieldValidation))
fieldValidationRequestLatencies.WithContext(req.Context()).WithLabelValues(fieldValidation, fieldValidationEnabled)
fieldValidationRequestLatencies.WithContext(req.Context()).WithLabelValues(fieldValidation)
if wd, ok := request.LatencyTrackersFrom(req.Context()); ok {
sliLatency := elapsedSeconds - (wd.MutatingWebhookTracker.GetLatency() + wd.ValidatingWebhookTracker.GetLatency()).Seconds()

View File

@ -32,12 +32,9 @@ import (
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apiserver/pkg/features"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/dynamic"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/klog/v2"
kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing"
@ -517,8 +514,6 @@ spec:
)
func TestFieldValidation(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServerSideFieldValidation, true)()
server, err := kubeapiservertesting.StartTestServer(t, kubeapiservertesting.NewDefaultTestServerOptions(), nil, framework.SharedEtcd())
if err != nil {
t.Fatal(err)
@ -2934,7 +2929,6 @@ func setupCRD(t testing.TB, config *rest.Config, apiGroup string, schemaless boo
}
func BenchmarkFieldValidation(b *testing.B) {
defer featuregatetesting.SetFeatureGateDuringTest(b, utilfeature.DefaultFeatureGate, features.ServerSideFieldValidation, true)()
flag.Lookup("v").Value.Set("0")
server, err := kubeapiservertesting.StartTestServer(b, kubeapiservertesting.NewDefaultTestServerOptions(), nil, framework.SharedEtcd())
if err != nil {