Skip to content
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.

Sign up for GitHub

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

Jump to bottom

apiextensions: forbid false x-kubernetes-preserve-unknown-fields #77750

Merged
merged 2 commits into from
May 14, 2019
Merged

apiextensions: forbid false x-kubernetes-preserve-unknown-fields #77750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d014591
apiextensions: forbid false x-kubernetes-preserve-unknown-fields
sttts May 10, 2019
69c50a7
Update generated files
sttts May 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/openapi-spec/swagger.json
View file
Open in desktop

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

10 changes: 10 additions & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/deepcopy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,5 +258,15 @@ func (in *JSONSchemaProps) DeepCopy() *JSONSchemaProps {
}
}

if in.XPreserveUnknownFields != nil {
in, out := &in.XPreserveUnknownFields, &out.XPreserveUnknownFields
if *in == nil {
*out = nil
} else {
*out = new(bool)
**out = **in
}
}

return out
}
3 changes: 2 additions & 1 deletion staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ type JSONSchemaProps struct {
// in the validation schema. This affects fields recursively,
// but switches back to normal pruning behaviour if nested
// properties or additionalProperties are specified in the schema.
XPreserveUnknownFields bool
// This can either be true or undefined. False is forbidden.
XPreserveUnknownFields *bool

// x-kubernetes-embedded-resource defines that the value is an
// embedded Kubernetes runtime.Object, with TypeMeta and
Expand Down
10 changes: 10 additions & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/deepcopy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,5 +234,15 @@ func (in *JSONSchemaProps) DeepCopy() *JSONSchemaProps {
}
}

if in.XPreserveUnknownFields != nil {
in, out := &in.XPreserveUnknownFields, &out.XPreserveUnknownFields
if *in == nil {
*out = nil
} else {
*out = new(bool)
**out = **in
}
}

return out
}
389 changes: 197 additions & 192 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/generated.proto
View file
Open in desktop

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

3 changes: 2 additions & 1 deletion ...ing/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ type JSONSchemaProps struct {
// in the validation schema. This affects fields recursively,
// but switches back to normal pruning behaviour if nested
// properties or additionalProperties are specified in the schema.
XPreserveUnknownFields bool `json:"x-kubernetes-preserve-unknown-fields,omitempty" protobuf:"bytes,38,opt,name=xKubernetesPreserveUnknownFields"`
// This can either be true or undefined. False is forbidden.
XPreserveUnknownFields *bool `json:"x-kubernetes-preserve-unknown-fields,omitempty" protobuf:"bytes,38,opt,name=xKubernetesPreserveUnknownFields"`

// x-kubernetes-embedded-resource defines that the value is an
// embedded Kubernetes runtime.Object, with TypeMeta and
Expand Down
4 changes: 2 additions & 2 deletions .../k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/zz_generated.conversion.go
View file
Open in desktop

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

1 change: 1 addition & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ go_test(
"//staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
],
)

Expand Down
4 changes: 4 additions & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,10 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
}
}

if schema.XPreserveUnknownFields != nil && *schema.XPreserveUnknownFields == false {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-preserve-unknown-fields"), *schema.XPreserveUnknownFields, "must be true or undefined"))
}

return allErrs
}

Expand Down
36 changes: 36 additions & 0 deletions ...g/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/utils/pointer"
)

type validationMatch struct {
Expand Down Expand Up @@ -935,6 +936,41 @@ func TestValidateCustomResourceDefinition(t *testing.T) {
invalid("spec", "versions"),
},
},
{
name: "x-kubernetes-preserve-unknown-field: false",
resource: &apiextensions.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"},
Spec: apiextensions.CustomResourceDefinitionSpec{
Group: "group.com",
Version: "version",
Validation: &apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
XPreserveUnknownFields: pointer.BoolPtr(false),
},
},
Versions: []apiextensions.CustomResourceDefinitionVersion{
{
Name: "version",
Served: true,
Storage: true,
},
},
Scope: apiextensions.NamespaceScoped,
Names: apiextensions.CustomResourceDefinitionNames{
Plural: "plural",
Singular: "singular",
Kind: "Plural",
ListKind: "PluralList",
},
},
Status: apiextensions.CustomResourceDefinitionStatus{
StoredVersions: []string{"version"},
},
},
errors: []validationMatch{
invalid("spec", "validation", "openAPIV3Schema", "x-kubernetes-preserve-unknown-fields"),
},
},
}

for _, tc := range tests {
Expand Down
18 changes: 13 additions & 5 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,19 @@ func newExtensions(s *apiextensions.JSONSchemaProps) (*Extensions, error) {
return nil, nil
}

return &Extensions{
XPreserveUnknownFields: s.XPreserveUnknownFields,
XEmbeddedResource: s.XEmbeddedResource,
XIntOrString: s.XIntOrString,
}, nil
ret := &Extensions{
XEmbeddedResource: s.XEmbeddedResource,
XIntOrString: s.XIntOrString,
}

if s.XPreserveUnknownFields != nil {
if !*s.XPreserveUnknownFields {
return nil, fmt.Errorf("'x-kubernetes-preserve-unknown-fields' must be true or undefined")
}
ret.XPreserveUnknownFields = true
}

return ret, nil
}

// validateUnsupportedFields checks that those fields rejected by validation are actually unset.
Expand Down
2 changes: 2 additions & 0 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ type Extensions struct {
// in the validation schema. This affects fields recursively,
// but switches back to normal pruning behaviour if nested
// properties or additionalProperties are specified in the schema.
// False means that the pruning behaviour is inherited from the parent.
// False does not mean to activate pruning.
XPreserveUnknownFields bool
Copy link
Contributor Author

@sttts sttts May 10, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am undecided whether this is a good idea. In the structural schema we never want to see the API false case. Hence, I define false here differently and use it for the nil-case in the API.


// x-kubernetes-embedded-resource defines that the value is an
Expand Down
4 changes: 2 additions & 2 deletions staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,8 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou
}
}

if in.XPreserveUnknownFields {
out.VendorExtensible.AddExtension("x-kubernetes-preserve-unknown-fields", true)
if in.XPreserveUnknownFields != nil {
liggitt marked this conversation as resolved.
Show resolved Hide resolved
out.VendorExtensible.AddExtension("x-kubernetes-preserve-unknown-fields", *in.XPreserveUnknownFields)
}
if in.XEmbeddedResource {
out.VendorExtensible.AddExtension("x-kubernetes-embedded-resource", true)
Expand Down