Skip to content

Graduate server side validation to beta #110178

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

Merged
merged 4 commits into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ require (
gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect
gopkg.in/warnings.v0 v0.1.1 // indirect
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.32 // indirect
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
sigs.k8s.io/kustomize/api v0.11.4 // indirect
sigs.k8s.io/kustomize/kustomize/v4 v4.5.4 // indirect
sigs.k8s.io/kustomize/kyaml v0.13.6 // indirect
Expand Down Expand Up @@ -605,7 +605,7 @@ replace (
modernc.org/xc => modernc.org/xc v1.0.0
rsc.io/pdf => rsc.io/pdf v0.1.1
sigs.k8s.io/apiserver-network-proxy/konnectivity-client => sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.32
sigs.k8s.io/json => sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2
sigs.k8s.io/json => sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2
sigs.k8s.io/kustomize/api => sigs.k8s.io/kustomize/api v0.11.4
sigs.k8s.io/kustomize/cmd/config => sigs.k8s.io/kustomize/cmd/config v0.10.6
sigs.k8s.io/kustomize/kustomize/v4 => sigs.k8s.io/kustomize/kustomize/v4 v4.5.4
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -557,8 +557,8 @@ modernc.org/xc v1.0.0/go.mod h1:mRNCo0bvLjGhHO9WsyuKVU4q0ceiDDDoEeWDJHrNx8I=
rsc.io/pdf v0.1.1/go.mod h1:n8OzWcQ6Sp37PL01nO98y4iUCRdTGarVfzxY20ICaU4=
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.32 h1:2WjukG7txtEsbXsSKWtTibCdsyYAhcu6KFnttyDdZOQ=
sigs.k8s.io/apiserver-network-proxy/konnectivity-client v0.0.32/go.mod h1:fEO7lRTdivWO2qYVCVG7dEADOMo/MLDCVr8So2g88Uw=
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 h1:kDi4JBNAsJWfz1aEXhO8Jg87JJaPNLh5tIzYHgStQ9Y=
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2/go.mod h1:B+TnT182UBxE84DiCz4CVE26eOSDAeYCpfDnC2kdKMY=
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 h1:iXTIw73aPyC+oRdyqqvVJuloN1p0AC/kzH07hu3NE+k=
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
sigs.k8s.io/kustomize/api v0.11.4 h1:/0Mr3kfBBNcNPOW5Qwk/3eb8zkswCwnqQxxKtmrTkRo=
sigs.k8s.io/kustomize/api v0.11.4/go.mod h1:k+8RsqYbgpkIrJ4p9jcdPqe8DprLxFUUO0yNOq8C+xI=
sigs.k8s.io/kustomize/cmd/config v0.10.6/go.mod h1:/S4A4nUANUa4bZJ/Edt7ZQTyKOY9WCER0uBS1SW2Rco=
Expand Down
2 changes: 1 addition & 1 deletion pkg/features/kube_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -1045,7 +1045,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS

genericfeatures.ServerSideApply: {Default: true, PreRelease: featuregate.GA},

genericfeatures.ServerSideFieldValidation: {Default: false, PreRelease: featuregate.Alpha},
genericfeatures.ServerSideFieldValidation: {Default: true, PreRelease: featuregate.Beta},

// features that enable backwards compatibility but are scheduled to be removed
// ...
Expand Down
2 changes: 1 addition & 1 deletion staging/src/k8s.io/api/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ require (
gopkg.in/yaml.v3 v3.0.1 // indirect
k8s.io/klog/v2 v2.70.1 // indirect
k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9 // indirect
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2 // indirect
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2 // indirect
sigs.k8s.io/structured-merge-diff/v4 v4.2.1 // indirect
sigs.k8s.io/yaml v1.2.0 // indirect
)
Expand Down
4 changes: 2 additions & 2 deletions staging/src/k8s.io/api/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion staging/src/k8s.io/apiextensions-apiserver/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ require (
k8s.io/klog/v2 v2.70.1
k8s.io/kube-openapi v0.0.0-20220627174259-011e075b9cb8
k8s.io/utils v0.0.0-20220210201930-3a6ce19ff2f9
sigs.k8s.io/json v0.0.0-20211208200746-9f7c6b3444d2
sigs.k8s.io/json v0.0.0-20220713155537-f223a00ba0e2
sigs.k8s.io/structured-merge-diff/v4 v4.2.1
sigs.k8s.io/yaml v1.2.0
)
Expand Down
4 changes: 2 additions & 2 deletions staging/src/k8s.io/apiextensions-apiserver/go.sum

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -1331,10 +1331,14 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) (unknown
if err != nil {
return nil, err
}
objectMeta, foundObjectMeta, err := schemaobjectmeta.GetObjectMeta(u.Object, v.dropInvalidMetadata)
objectMeta, foundObjectMeta, metaUnknownFields, err := schemaobjectmeta.GetObjectMetaWithOptions(u.Object, schemaobjectmeta.ObjectMetaOptions{
DropMalformedFields: v.dropInvalidMetadata,
ReturnUnknownFieldPaths: v.returnUnknownFieldPaths,
})
if err != nil {
return nil, err
}
unknownFieldPaths = append(unknownFieldPaths, metaUnknownFields...)

// compare group and kind because also other object like DeleteCollection options pass through here
gv, err := schema.ParseGroupVersion(apiVersion)
Expand All @@ -1345,17 +1349,23 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) (unknown
if gv.Group == v.structuralSchemaGK.Group && kind == v.structuralSchemaGK.Kind {
if !v.preserveUnknownFields {
// TODO: switch over pruning and coercing at the root to schemaobjectmeta.Coerce too
pruneOpts := structuralpruning.PruneOptions{}
pruneOpts := structuralschema.UnknownFieldPathOptions{}
if v.returnUnknownFieldPaths {
pruneOpts.ReturnPruned = true
pruneOpts.TrackUnknownFieldPaths = true
}
unknownFieldPaths = structuralpruning.PruneWithOptions(u.Object, v.structuralSchemas[gv.Version], true, pruneOpts)
unknownFieldPaths = append(unknownFieldPaths, structuralpruning.PruneWithOptions(u.Object, v.structuralSchemas[gv.Version], true, pruneOpts)...)
structuraldefaulting.PruneNonNullableNullsWithoutDefaults(u.Object, v.structuralSchemas[gv.Version])
}

if err := schemaobjectmeta.Coerce(nil, u.Object, v.structuralSchemas[gv.Version], false, v.dropInvalidMetadata); err != nil {
err, paths := schemaobjectmeta.CoerceWithOptions(nil, u.Object, v.structuralSchemas[gv.Version], false, schemaobjectmeta.CoerceOptions{
DropInvalidFields: v.dropInvalidMetadata,
ReturnUnknownFieldPaths: v.returnUnknownFieldPaths,
})
if err != nil {
return nil, err
}
unknownFieldPaths = append(unknownFieldPaths, paths...)

// fixup missing generation in very old CRs
if v.repairGeneration && objectMeta.Generation == 0 {
objectMeta.Generation = 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,34 @@ limitations under the License.
package objectmeta

import (
"sort"

structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/util/validation/field"
)

// CoerceOptions gives the ability to ReturnUnknownFieldPaths for fields
// unrecognized by the schema or DropInvalidFields for fields that are a part
// of the schema, but are malformed.
type CoerceOptions struct {
// DropInvalidFields discards malformed serialized metadata fields that
// cannot be successfully decoded to the corresponding ObjectMeta field.
// This only applies to fields that are recognized as part of the schema,
// but of an invalid type (i.e. cause an error when unmarshaling, rather
// than being dropped or causing a strictErr).
DropInvalidFields bool
// ReturnUnknownFieldPaths will return the paths to fields that are not
// recognized as part of the schema.
ReturnUnknownFieldPaths bool
}

// Coerce checks types of embedded ObjectMeta and TypeMeta and prunes unknown fields inside the former.
// It does coerce ObjectMeta and TypeMeta at the root if isResourceRoot is true.
// If dropInvalidFields is true, fields of wrong type will be dropped.
func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot, dropInvalidFields bool) *field.Error {
// If opts.ReturnUnknownFieldPaths is true, it will return the paths of any fields that are not a part of the
// schema that are dropped when unmarshaling.
// If opts.DropInvalidFields is true, fields of wrong type will be dropped.
func CoerceWithOptions(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot bool, opts CoerceOptions) (*field.Error, []string) {
if isResourceRoot {
if s == nil {
s = &structuralschema.Structural{}
Expand All @@ -36,36 +55,57 @@ func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, is
s = &clone
}
}
c := coercer{dropInvalidFields: dropInvalidFields}
return c.coerce(pth, obj, s)
c := coercer{DropInvalidFields: opts.DropInvalidFields, ReturnUnknownFieldPaths: opts.ReturnUnknownFieldPaths}
schemaOpts := &structuralschema.UnknownFieldPathOptions{
TrackUnknownFieldPaths: opts.ReturnUnknownFieldPaths,
}
fieldErr := c.coerce(pth, obj, s, schemaOpts)
sort.Strings(schemaOpts.UnknownFieldPaths)
return fieldErr, schemaOpts.UnknownFieldPaths
}

// Coerce calls CoerceWithOptions without returning unknown field paths.
func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot, dropInvalidFields bool) *field.Error {
fieldErr, _ := CoerceWithOptions(pth, obj, s, isResourceRoot, CoerceOptions{DropInvalidFields: dropInvalidFields})
return fieldErr
}

type coercer struct {
dropInvalidFields bool
DropInvalidFields bool
ReturnUnknownFieldPaths bool
}

func (c *coercer) coerce(pth *field.Path, x interface{}, s *structuralschema.Structural) *field.Error {
func (c *coercer) coerce(pth *field.Path, x interface{}, s *structuralschema.Structural, opts *structuralschema.UnknownFieldPathOptions) *field.Error {
if s == nil {
return nil
}
origPathLen := len(opts.ParentPath)
defer func() {
opts.ParentPath = opts.ParentPath[:origPathLen]
}()
switch x := x.(type) {
case map[string]interface{}:
for k, v := range x {
if s.XEmbeddedResource {
switch k {
case "apiVersion", "kind":
if _, ok := v.(string); !ok && c.dropInvalidFields {
if _, ok := v.(string); !ok && c.DropInvalidFields {
delete(x, k)
} else if !ok {
return field.Invalid(pth.Child(k), v, "must be a string")
}
case "metadata":
meta, found, err := GetObjectMeta(x, c.dropInvalidFields)
meta, found, unknownFields, err := GetObjectMetaWithOptions(x, ObjectMetaOptions{
DropMalformedFields: c.DropInvalidFields,
ReturnUnknownFieldPaths: c.ReturnUnknownFieldPaths,
ParentPath: pth,
})
opts.UnknownFieldPaths = append(opts.UnknownFieldPaths, unknownFields...)
if err != nil {
if !c.dropInvalidFields {
if !c.DropInvalidFields {
return field.Invalid(pth.Child("metadata"), v, err.Error())
}
// pass through on error if dropInvalidFields is true
// pass through on error if DropInvalidFields is true
} else if found {
if err := SetObjectMeta(x, meta); err != nil {
return field.Invalid(pth.Child("metadata"), v, err.Error())
Expand All @@ -78,20 +118,26 @@ func (c *coercer) coerce(pth *field.Path, x interface{}, s *structuralschema.Str
}
prop, ok := s.Properties[k]
if ok {
if err := c.coerce(pth.Child(k), v, &prop); err != nil {
opts.AppendKey(k)
if err := c.coerce(pth.Child(k), v, &prop, opts); err != nil {
return err
}
opts.ParentPath = opts.ParentPath[:origPathLen]
} else if s.AdditionalProperties != nil {
if err := c.coerce(pth.Key(k), v, s.AdditionalProperties.Structural); err != nil {
opts.AppendKey(k)
if err := c.coerce(pth.Key(k), v, s.AdditionalProperties.Structural, opts); err != nil {
return err
}
opts.ParentPath = opts.ParentPath[:origPathLen]
}
}
case []interface{}:
for i, v := range x {
if err := c.coerce(pth.Index(i), v, s.Items); err != nil {
opts.AppendIndex(i)
if err := c.coerce(pth.Index(i), v, s.Items, opts); err != nil {
return err
}
opts.ParentPath = opts.ParentPath[:origPathLen]
}
default:
// scalars, do nothing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package objectmeta
import (
"bytes"
"reflect"
"strings"
"testing"

structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
Expand All @@ -28,13 +29,14 @@ import (

func TestCoerce(t *testing.T) {
tests := []struct {
name string
json string
includeRoot bool
dropInvalidFields bool
schema *structuralschema.Structural
expected string
expectedError bool
name string
json string
includeRoot bool
dropInvalidFields bool
schema *structuralschema.Structural
expected string
expectedError bool
expectedUnknownFields []string
}{
{name: "empty", json: "null", schema: nil, expected: "null"},
{name: "scalar", json: "4", schema: &structuralschema.Structural{}, expected: "4"},
Expand Down Expand Up @@ -199,7 +201,12 @@ func TestCoerce(t *testing.T) {
}
}
}
`},
`, expectedUnknownFields: []string{
"nested.metadata.unspecified",
"nested.spec.embedded.metadata.unspecified",
"preserving.metadata.unspecified",
"pruned.metadata.unspecified",
}},
{name: "x-kubernetes-embedded-resource, with includeRoot=true", json: `
{
"apiVersion": "foo/v1",
Expand Down Expand Up @@ -356,8 +363,13 @@ func TestCoerce(t *testing.T) {
}
}
}
}
`},
}`, expectedUnknownFields: []string{
"metadata.unspecified",
"nested.metadata.unspecified",
"nested.spec.embedded.metadata.unspecified",
"preserving.metadata.unspecified",
"pruned.metadata.unspecified",
}},
{name: "without name", json: `
{
"apiVersion": "foo/v1",
Expand Down Expand Up @@ -495,7 +507,10 @@ func TestCoerce(t *testing.T) {
t.Fatal(err)
}

err := Coerce(nil, in, tt.schema, tt.includeRoot, tt.dropInvalidFields)
err, unknownFields := CoerceWithOptions(nil, in, tt.schema, tt.includeRoot, CoerceOptions{
DropInvalidFields: tt.dropInvalidFields,
ReturnUnknownFieldPaths: true,
})
if tt.expectedError && err == nil {
t.Error("expected error, but did not get any")
} else if !tt.expectedError && err != nil {
Expand All @@ -510,6 +525,9 @@ func TestCoerce(t *testing.T) {
}
t.Errorf("expected: %s\ngot: %s\ndiff: %s", tt.expected, buf.String(), diff.ObjectDiff(expected, in))
}
if !reflect.DeepEqual(unknownFields, tt.expectedUnknownFields) {
t.Errorf("expected unknown fields:\n\t%v\ngot:\n\t%v\n", strings.Join(tt.expectedUnknownFields, "\n\t"), strings.Join(unknownFields, "\n\t"))
}
})
}
}
Loading