Skip to content

Commit 69576fc

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #7507 from sbueringer/pr-validate-unknown-variable-fields
⚠️ ClusterClass: validate unknown fields in variable values
2 parents 4484e6b + fdbac9b commit 69576fc

File tree

6 files changed

+297
-1
lines changed

6 files changed

+297
-1
lines changed

‎api/v1beta1/clusterclass_types.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,12 @@ type JSONSchemaProps struct {
319319
// +optional
320320
ExclusiveMinimum bool `json:"exclusiveMinimum,omitempty"`
321321

322+
// XPreserveUnknownFields allows setting fields in a variable object
323+
// which are not defined in the variable schema. This affects fields recursively,
324+
// except if nested properties or additionalProperties are specified in the schema.
325+
// +optional
326+
XPreserveUnknownFields bool `json:"x-kubernetes-preserve-unknown-fields,omitempty"`
327+
322328
// Enum is the list of valid values of the variable.
323329
// NOTE: Can be set for all types.
324330
// +optional

‎api/v1beta1/zz_generated.openapi.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/cluster.x-k8s.io_clusterclasses.yaml

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎internal/topology/variables/cluster_variable_validation.go

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@ package variables
1919
import (
2020
"encoding/json"
2121
"fmt"
22+
"strings"
2223

2324
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
25+
structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
26+
structuralpruning "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning"
2427
"k8s.io/apiextensions-apiserver/pkg/apiserver/validation"
2528
"k8s.io/apimachinery/pkg/util/validation/field"
2629

@@ -133,7 +136,55 @@ func ValidateClusterVariable(clusterVariable *clusterv1.ClusterVariable, cluster
133136

134137
// Validate variable against the schema.
135138
// NOTE: We're reusing a library func used in CRD validation.
136-
return validation.ValidateCustomResource(fldPath, variableValue, validator)
139+
if err := validation.ValidateCustomResource(fldPath, variableValue, validator); err != nil {
140+
return err
141+
}
142+
143+
return validateUnknownFields(fldPath, clusterVariable, variableValue, apiExtensionsSchema)
144+
}
145+
146+
// validateUnknownFields validates the given variableValue for unknown fields.
147+
// This func returns an error if there are variable fields in variableValue that are not defined in
148+
// variableSchema and if x-kubernetes-preserve-unknown-fields is not set.
149+
func validateUnknownFields(fldPath *field.Path, clusterVariable *clusterv1.ClusterVariable, variableValue interface{}, variableSchema *apiextensions.JSONSchemaProps) field.ErrorList {
150+
// Structural schema pruning does not work with scalar values,
151+
// so we wrap the schema and the variable in objects.
152+
// <variable-name>: <variable-value>
153+
wrappedVariable := map[string]interface{}{
154+
clusterVariable.Name: variableValue,
155+
}
156+
// type: object
157+
// properties:
158+
// <variable-name>: <variable-schema>
159+
wrappedSchema := &apiextensions.JSONSchemaProps{
160+
Type: "object",
161+
Properties: map[string]apiextensions.JSONSchemaProps{
162+
clusterVariable.Name: *variableSchema,
163+
},
164+
}
165+
ss, err := structuralschema.NewStructural(wrappedSchema)
166+
if err != nil {
167+
return field.ErrorList{field.Invalid(fldPath, "",
168+
fmt.Sprintf("failed defaulting variable %q: %v", clusterVariable.Name, err))}
169+
}
170+
171+
// Run Prune to check if it would drop any unknown fields.
172+
opts := structuralschema.UnknownFieldPathOptions{
173+
// TrackUnknownFieldPaths has to be true so PruneWithOptions returns the unknown fields.
174+
TrackUnknownFieldPaths: true,
175+
}
176+
prunedUnknownFields := structuralpruning.PruneWithOptions(wrappedVariable, ss, false, opts)
177+
if len(prunedUnknownFields) > 0 {
178+
// If prune dropped any unknown fields, return an error.
179+
// This means that not all variable fields have been defined in the variable schema and
180+
// x-kubernetes-preserve-unknown-fields was not set.
181+
return field.ErrorList{
182+
field.Invalid(fldPath, "",
183+
fmt.Sprintf("failed validation: %q fields are not specified in the variable schema of variable %q", strings.Join(prunedUnknownFields, ","), clusterVariable.Name)),
184+
}
185+
}
186+
187+
return nil
137188
}
138189

139190
func getClusterVariablesMap(clusterVariables []clusterv1.ClusterVariable) map[string]*clusterv1.ClusterVariable {

‎internal/topology/variables/cluster_variable_validation_test.go

Lines changed: 217 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,223 @@ func Test_ValidateClusterVariable(t *testing.T) {
900900
},
901901
wantErr: true,
902902
},
903+
{
904+
name: "Valid object with x-kubernetes-preserve-unknown-fields",
905+
clusterClassVariable: &clusterv1.ClusterClassVariable{
906+
Name: "testObject",
907+
Required: true,
908+
Schema: clusterv1.VariableSchema{
909+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
910+
Type: "object",
911+
Properties: map[string]clusterv1.JSONSchemaProps{
912+
"knownProperty": {
913+
Type: "boolean",
914+
},
915+
},
916+
// Preserves fields for the current object (in this case unknownProperty).
917+
XPreserveUnknownFields: true,
918+
},
919+
},
920+
},
921+
clusterVariable: &clusterv1.ClusterVariable{
922+
Name: "testObject",
923+
Value: apiextensionsv1.JSON{
924+
Raw: []byte(`{"knownProperty":false,"unknownProperty":true}`),
925+
},
926+
},
927+
},
928+
{
929+
name: "Error if undefined field",
930+
clusterClassVariable: &clusterv1.ClusterClassVariable{
931+
Name: "testObject",
932+
Required: true,
933+
Schema: clusterv1.VariableSchema{
934+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
935+
Type: "object",
936+
Properties: map[string]clusterv1.JSONSchemaProps{
937+
"knownProperty": {
938+
Type: "boolean",
939+
},
940+
},
941+
},
942+
},
943+
},
944+
clusterVariable: &clusterv1.ClusterVariable{
945+
Name: "testObject",
946+
Value: apiextensionsv1.JSON{
947+
// unknownProperty is not defined in the schema.
948+
Raw: []byte(`{"knownProperty":false,"unknownProperty":true}`),
949+
},
950+
},
951+
wantErr: true,
952+
},
953+
{
954+
name: "Error if undefined field with different casing",
955+
clusterClassVariable: &clusterv1.ClusterClassVariable{
956+
Name: "testObject",
957+
Required: true,
958+
Schema: clusterv1.VariableSchema{
959+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
960+
Type: "object",
961+
Properties: map[string]clusterv1.JSONSchemaProps{
962+
"knownProperty": {
963+
Type: "boolean",
964+
},
965+
},
966+
},
967+
},
968+
},
969+
clusterVariable: &clusterv1.ClusterVariable{
970+
Name: "testObject",
971+
Value: apiextensionsv1.JSON{
972+
// KnownProperty is only defined with lower case in the schema.
973+
Raw: []byte(`{"KnownProperty":false}`),
974+
},
975+
},
976+
wantErr: true,
977+
},
978+
{
979+
name: "Valid nested object with x-kubernetes-preserve-unknown-fields",
980+
clusterClassVariable: &clusterv1.ClusterClassVariable{
981+
Name: "testObject",
982+
Required: true,
983+
Schema: clusterv1.VariableSchema{
984+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
985+
Type: "object",
986+
// XPreserveUnknownFields preservers recursively if the object has nested fields
987+
// as no nested Properties are defined.
988+
XPreserveUnknownFields: true,
989+
},
990+
},
991+
},
992+
clusterVariable: &clusterv1.ClusterVariable{
993+
Name: "testObject",
994+
Value: apiextensionsv1.JSON{
995+
Raw: []byte(`{"test": {"unknownProperty":false}}`),
996+
},
997+
},
998+
},
999+
{
1000+
name: "Valid object with nested fields and x-kubernetes-preserve-unknown-fields",
1001+
clusterClassVariable: &clusterv1.ClusterClassVariable{
1002+
Name: "testObject",
1003+
Required: true,
1004+
Schema: clusterv1.VariableSchema{
1005+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
1006+
Type: "object",
1007+
Properties: map[string]clusterv1.JSONSchemaProps{
1008+
"test": {
1009+
Type: "object",
1010+
Properties: map[string]clusterv1.JSONSchemaProps{
1011+
"knownProperty": {
1012+
Type: "boolean",
1013+
},
1014+
},
1015+
// Preserves fields on the current level (in this case unknownProperty).
1016+
XPreserveUnknownFields: true,
1017+
},
1018+
},
1019+
},
1020+
},
1021+
},
1022+
clusterVariable: &clusterv1.ClusterVariable{
1023+
Name: "testObject",
1024+
Value: apiextensionsv1.JSON{
1025+
Raw: []byte(`{"test": {"knownProperty":false,"unknownProperty":true}}`),
1026+
},
1027+
},
1028+
},
1029+
{
1030+
name: "Error if undefined field nested",
1031+
clusterClassVariable: &clusterv1.ClusterClassVariable{
1032+
Name: "testObject",
1033+
Required: true,
1034+
Schema: clusterv1.VariableSchema{
1035+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
1036+
Type: "object",
1037+
Properties: map[string]clusterv1.JSONSchemaProps{
1038+
"test": {
1039+
Type: "object",
1040+
Properties: map[string]clusterv1.JSONSchemaProps{
1041+
"knownProperty": {
1042+
Type: "boolean",
1043+
},
1044+
},
1045+
},
1046+
},
1047+
},
1048+
},
1049+
},
1050+
clusterVariable: &clusterv1.ClusterVariable{
1051+
Name: "testObject",
1052+
Value: apiextensionsv1.JSON{
1053+
// unknownProperty is not defined in the schema.
1054+
Raw: []byte(`{"test": {"knownProperty":false,"unknownProperty":true}}`),
1055+
},
1056+
},
1057+
wantErr: true,
1058+
},
1059+
{
1060+
name: "Error if undefined field nested and x-kubernetes-preserve-unknown-fields one level above",
1061+
clusterClassVariable: &clusterv1.ClusterClassVariable{
1062+
Name: "testObject",
1063+
Required: true,
1064+
Schema: clusterv1.VariableSchema{
1065+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
1066+
Type: "object",
1067+
Properties: map[string]clusterv1.JSONSchemaProps{
1068+
"test": {
1069+
Type: "object",
1070+
Properties: map[string]clusterv1.JSONSchemaProps{
1071+
"knownProperty": {
1072+
Type: "boolean",
1073+
},
1074+
},
1075+
},
1076+
},
1077+
// Preserves only on the current level as nested Properties are defined.
1078+
XPreserveUnknownFields: true,
1079+
},
1080+
},
1081+
},
1082+
clusterVariable: &clusterv1.ClusterVariable{
1083+
Name: "testObject",
1084+
Value: apiextensionsv1.JSON{
1085+
Raw: []byte(`{"test": {"knownProperty":false,"unknownProperty":true}}`),
1086+
},
1087+
},
1088+
wantErr: true,
1089+
},
1090+
{
1091+
name: "Valid object with mid-level unknown fields",
1092+
clusterClassVariable: &clusterv1.ClusterClassVariable{
1093+
Name: "testObject",
1094+
Required: true,
1095+
Schema: clusterv1.VariableSchema{
1096+
OpenAPIV3Schema: clusterv1.JSONSchemaProps{
1097+
Type: "object",
1098+
Properties: map[string]clusterv1.JSONSchemaProps{
1099+
"test": {
1100+
Type: "object",
1101+
Properties: map[string]clusterv1.JSONSchemaProps{
1102+
"knownProperty": {
1103+
Type: "boolean",
1104+
},
1105+
},
1106+
},
1107+
},
1108+
// Preserves only on the current level as nested Properties are defined.
1109+
XPreserveUnknownFields: true,
1110+
},
1111+
},
1112+
},
1113+
clusterVariable: &clusterv1.ClusterVariable{
1114+
Name: "testObject",
1115+
Value: apiextensionsv1.JSON{
1116+
Raw: []byte(`{"test": {"knownProperty":false},"unknownProperty":true}`),
1117+
},
1118+
},
1119+
},
9031120
}
9041121
for _, tt := range tests {
9051122
t.Run(tt.name, func(t *testing.T) {

‎internal/topology/variables/schema.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
2424
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
2525
"k8s.io/apimachinery/pkg/util/validation/field"
26+
"k8s.io/utils/pointer"
2627

2728
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
2829
)
@@ -48,6 +49,13 @@ func convertToAPIExtensionsJSONSchemaProps(schema *clusterv1.JSONSchemaProps, fl
4849
ExclusiveMinimum: schema.ExclusiveMinimum,
4950
}
5051

52+
// Only set XPreserveUnknownFields to true if it's true.
53+
// apiextensions.JSONSchemaProps only allows setting XPreserveUnknownFields
54+
// to true or undefined, false is forbidden.
55+
if schema.XPreserveUnknownFields {
56+
props.XPreserveUnknownFields = pointer.Bool(true)
57+
}
58+
5159
if schema.Default != nil && schema.Default.Raw != nil {
5260
var v interface{}
5361
if err := json.Unmarshal(schema.Default.Raw, &v); err != nil {

0 commit comments

Comments
 (0)