When setting the kubebuilder:pruning:PreserveUnknownFields on a struct and a field using that struct, the generated schema is not valid.
For example:
// +kubebuilder:pruning:PreserveUnknownFields
type Preserved struct {
ConcreteField string `json:"concreteField"`
Rest map[string]interface{} `json:"-"`
}
type Spec struct {
// +kubebuilder:pruning:PreserveUnknownFields
Preserved Preserved `json:"preserved"`
}
// +kubebuilder:object:root=true
type Test struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Spec Spec `json:"spec,omitempty"`
}Will generate the following schema:
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
annotations:
controller-gen.kubebuilder.io/version: (devel)
creationTimestamp: null
name: tests.testdata.kubebuilder.io
spec:
group: testdata.kubebuilder.io
names:
kind: Test
listKind: TestList
plural: tests
singular: test
scope: Namespaced
versions:
- name: v1
schema:
openAPIV3Schema:
properties:
apiVersion:
description: 'APIVersion defines the versioned schema of this representation
of an object. Servers should convert recognized schemas to the latest
internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
type: string
kind:
description: 'Kind is a string value representing the REST resource this
object represents. Servers may infer this from the endpoint the client
submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
type: string
metadata:
type: object
spec:
properties:
preserved:
allOf:
- x-kubernetes-preserve-unknown-fields: true
- x-kubernetes-preserve-unknown-fields: true
properties:
concreteField:
type: string
required:
- concreteField
type: object
required:
- preserved
type: object
type: object
served: true
storage: trueThe problematic part being:
preserved:
allOf:
- x-kubernetes-preserve-unknown-fields: true
- x-kubernetes-preserve-unknown-fields: trueFrom what I observed, this happens when flattening the schema.
I managed to fix the bug by adding a case for XPreserveUnknownFields here to prevent hoisting:
|
switch fieldName { |
|
case "Properties": |
|
// merge if possible, use all of otherwise |
|
srcMap := srcInt.(map[string]apiext.JSONSchemaProps) |
|
dstMap := dstInt.(map[string]apiext.JSONSchemaProps) |
|
|
|
for k, v := range srcMap { |
|
dstProp, exists := dstMap[k] |
|
if !exists { |
|
dstMap[k] = v |
|
continue |
|
} |
|
flattenAllOfInto(&dstProp, v, errRec) |
|
dstMap[k] = dstProp |
|
} |
|
case "Required": |
|
// merge |
|
dstField.Set(reflect.AppendSlice(dstField, srcField)) |
|
case "Type": |
|
if srcInt != dstInt { |
|
// TODO(directxman12): figure out how to attach this back to a useful point in the Go source or in the schema |
|
errRec.AddError(fmt.Errorf("conflicting types in allOf branches in schema: %s vs %s", dstInt, srcInt)) |
|
} |
|
// keep the destination value, for now |
|
// TODO(directxman12): Default -- use field? |
|
// TODO(directxman12): |
|
// - Dependencies: if field x is present, then either schema validates or all props are present |
|
// - AdditionalItems: like AdditionalProperties |
|
// - Definitions: common named validation sets that can be references (merge, bail if duplicate) |
|
case "AdditionalProperties": |
|
// as of the time of writing, `allows: false` is not allowed, so we don't have to handle it |
|
srcProps := srcInt.(*apiext.JSONSchemaPropsOrBool) |
|
if srcProps.Schema == nil { |
|
// nothing to merge |
|
continue |
|
} |
|
dstProps := dstInt.(*apiext.JSONSchemaPropsOrBool) |
|
if dstProps.Schema == nil { |
|
dstProps.Schema = &apiext.JSONSchemaProps{} |
|
} |
|
flattenAllOfInto(dstProps.Schema, *srcProps.Schema, errRec) |
|
// NB(directxman12): no need to explicitly handle nullable -- false is considered to be the zero value |
|
// TODO(directxman12): src isn't necessarily the field value -- it's just the most recent allOf entry |
|
default: |
|
// hoist into allOf... |
|
hoisted = true |
|
|
|
srcRemVal.Field(i).Set(srcField) |
|
dstRemVal.Field(i).Set(dstField) |
|
// ...and clear the original |
|
dstField.Set(zeroVal) |
|
} |
While this fixed the bug, I'm not too sure about the fix itself.
I can open a PR but would like to confirm it's a bug first and eventually if the fix I tried makes sense.
cc @akutz @sbueringer (per Alvaro's recommendation).
When setting the
kubebuilder:pruning:PreserveUnknownFieldson a struct and a field using that struct, the generated schema is not valid.For example:
Will generate the following schema:
The problematic part being:
From what I observed, this happens when flattening the schema.
I managed to fix the bug by adding a case for
XPreserveUnknownFieldshere to prevent hoisting:controller-tools/pkg/crd/flatten.go
Lines 105 to 156 in f89071a
While this fixed the bug, I'm not too sure about the fix itself.
I can open a PR but would like to confirm it's a bug first and eventually if the fix I tried makes sense.
cc @akutz @sbueringer (per Alvaro's recommendation).