Merge pull request #95423 from apelisse/default-null-crds

CRs: Default non-nullable nulls

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -1222,7 +1222,8 @@ func (v schemaCoercingConverter) ConvertFieldLabel(gvk schema.GroupVersionKind,
 // in addition for native types when decoding into Golang structs:
 //
 // - validating and pruning ObjectMeta
-// - generic pruning of unknown fields following a structural schema.
+// - generic pruning of unknown fields following a structural schema
+// - removal of non-defaulted non-nullable null map values.
 type unstructuredSchemaCoercer struct {
 	dropInvalidMetadata bool
 	repairGeneration    bool
@@ -1256,6 +1257,7 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) error {
 		if !v.preserveUnknownFields {
 			// TODO: switch over pruning and coercing at the root to  schemaobjectmeta.Coerce too
 			structuralpruning.Prune(u.Object, v.structuralSchemas[gv.Version], false)
+			structuraldefaulting.PruneNonNullableNullsWithoutDefaults(u.Object, v.structuralSchemas[gv.Version])
 		}
 		if err := schemaobjectmeta.Coerce(nil, u.Object, v.structuralSchemas[gv.Version], false, v.dropInvalidMetadata); err != nil {
 			return err

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/BUILD

@@ -5,6 +5,7 @@ go_library(
     srcs = [
         "algorithm.go",
         "prune.go",
+        "prunenulls.go",
         "surroundingobject.go",
         "validation.go",
     ],
@@ -26,7 +27,10 @@ go_library(
 
 go_test(
     name = "go_default_test",
-    srcs = ["algorithm_test.go"],
+    srcs = [
+        "algorithm_test.go",
+        "prunenulls_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema:go_default_library",

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/algorithm.go

@@ -21,8 +21,16 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+// isNonNullalbeNull returns true if the item is nil AND it's nullable
+func isNonNullableNull(x interface{}, s *structuralschema.Structural) bool {
+	return x == nil && s != nil && s.Generic.Nullable == false
+}
+
 // Default does defaulting of x depending on default values in s.
 // Default values from s are deep-copied.
+//
+// PruneNonNullableNullsWithoutDefaults has left the non-nullable nulls
+// that have a default here.
 func Default(x interface{}, s *structuralschema.Structural) {
 	if s == nil {
 		return
@@ -34,20 +42,26 @@ func Default(x interface{}, s *structuralschema.Structural) {
 			if prop.Default.Object == nil {
 				continue
 			}
-			if _, found := x[k]; !found {
+			if _, found := x[k]; !found || isNonNullableNull(x[k], &prop) {
 				x[k] = runtime.DeepCopyJSONValue(prop.Default.Object)
 			}
 		}
-		for k, v := range x {
+		for k := range x {
 			if prop, found := s.Properties[k]; found {
-				Default(v, &prop)
+				Default(x[k], &prop)
 			} else if s.AdditionalProperties != nil {
-				Default(v, s.AdditionalProperties.Structural)
+				if isNonNullableNull(x[k], s.AdditionalProperties.Structural) {
+					x[k] = runtime.DeepCopyJSONValue(s.AdditionalProperties.Structural.Default.Object)
+				}
+				Default(x[k], s.AdditionalProperties.Structural)
 			}
 		}
 	case []interface{}:
-		for _, v := range x {
+		for i := range x {
-			Default(v, s.Items)
+			if isNonNullableNull(x[i], s.Items) {
+				x[i] = runtime.DeepCopyJSONValue(s.Items.Default.Object)
+			}
+			Default(x[i], s.Items)
 		}
 	default:
 		// scalars, do nothing

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/algorithm_test.go

@@ -135,7 +135,83 @@ func TestDefault(t *testing.T) {
 					},
 				},
 			},
-		}, `[{"a":"A"},{"a":1},{"a":0},{"a":0.0},{"a":""},{"a":null},{"a":[]},{"a":{}}]`},
+		}, `[{"a":"A"},{"a":1},{"a":0},{"a":0.0},{"a":""},{"a":"A"},{"a":[]},{"a":{}}]`},
+		{"null in nullable list", `[null]`, &structuralschema.Structural{
+			Generic: structuralschema.Generic{
+				Nullable: true,
+			},
+			Items: &structuralschema.Structural{
+				Properties: map[string]structuralschema.Structural{
+					"a": {
+						Generic: structuralschema.Generic{
+							Default: structuralschema.JSON{"A"},
+						},
+					},
+				},
+			},
+		}, `[null]`},
+		{"null in non-nullable list", `[null]`, &structuralschema.Structural{
+			Generic: structuralschema.Generic{
+				Nullable: false,
+			},
+			Items: &structuralschema.Structural{
+				Generic: structuralschema.Generic{
+					Default: structuralschema.JSON{"A"},
+				},
+			},
+		}, `["A"]`},
+		{"null in nullable object", `{"a": null}`, &structuralschema.Structural{
+			Generic: structuralschema.Generic{},
+			Properties: map[string]structuralschema.Structural{
+				"a": {
+					Generic: structuralschema.Generic{
+						Nullable: true,
+						Default:  structuralschema.JSON{"A"},
+					},
+				},
+			},
+		}, `{"a": null}`},
+		{"null in non-nullable object", `{"a": null}`, &structuralschema.Structural{
+			Properties: map[string]structuralschema.Structural{
+				"a": {
+					Generic: structuralschema.Generic{
+						Nullable: false,
+						Default:  structuralschema.JSON{"A"},
+					},
+				},
+			},
+		}, `{"a": "A"}`},
+		{"null in nullable object with additionalProperties", `{"a": null}`, &structuralschema.Structural{
+			Generic: structuralschema.Generic{
+				AdditionalProperties: &structuralschema.StructuralOrBool{
+					Structural: &structuralschema.Structural{
+						Generic: structuralschema.Generic{
+							Nullable: true,
+							Default:  structuralschema.JSON{"A"},
+						},
+					},
+				},
+			},
+		}, `{"a": null}`},
+		{"null in non-nullable object with additionalProperties", `{"a": null}`, &structuralschema.Structural{
+			Generic: structuralschema.Generic{
+				AdditionalProperties: &structuralschema.StructuralOrBool{
+					Structural: &structuralschema.Structural{
+						Generic: structuralschema.Generic{
+							Nullable: false,
+							Default:  structuralschema.JSON{"A"},
+						},
+					},
+				},
+			},
+		}, `{"a": "A"}`},
+		{"null unknown field", `{"a": null}`, &structuralschema.Structural{
+			Generic: structuralschema.Generic{
+				AdditionalProperties: &structuralschema.StructuralOrBool{
+					Bool: true,
+				},
+			},
+		}, `{"a": null}`},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/prunenulls.go

@@ -0,0 +1,66 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package defaulting
+
+import structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
+
+func isNonNullableNonDefaultableNull(x interface{}, s *structuralschema.Structural) bool {
+	return x == nil && s != nil && s.Generic.Nullable == false && s.Default.Object == nil
+}
+
+func getSchemaForField(field string, s *structuralschema.Structural) *structuralschema.Structural {
+	if s == nil {
+		return nil
+	}
+	schema, ok := s.Properties[field]
+	if ok {
+		return &schema
+	}
+	if s.AdditionalProperties != nil {
+		return s.AdditionalProperties.Structural
+	}
+	return nil
+}
+
+// PruneNonNullableNullsWithoutDefaults removes non-nullable
+// non-defaultable null values from object.
+//
+// Non-nullable nulls that have a default are left alone here and will
+// be defaulted later.
+func PruneNonNullableNullsWithoutDefaults(x interface{}, s *structuralschema.Structural) {
+	switch x := x.(type) {
+	case map[string]interface{}:
+		for k, v := range x {
+			schema := getSchemaForField(k, s)
+			if isNonNullableNonDefaultableNull(v, schema) {
+				delete(x, k)
+			} else {
+				PruneNonNullableNullsWithoutDefaults(v, schema)
+			}
+		}
+	case []interface{}:
+		var schema *structuralschema.Structural
+		if s != nil {
+			schema = s.Items
+		}
+		for i := range x {
+			PruneNonNullableNullsWithoutDefaults(x[i], schema)
+		}
+	default:
+		// scalars, do nothing
+	}
+}

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting/prunenulls_test.go

@@ -0,0 +1,93 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package defaulting
+
+import (
+	"bytes"
+	"reflect"
+	"testing"
+
+	structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema"
+	"k8s.io/apimachinery/pkg/util/json"
+)
+
+func TestPruneNonNullableNullsWithoutDefaults(t *testing.T) {
+	tests := []struct {
+		name     string
+		json     string
+		schema   *structuralschema.Structural
+		expected string
+	}{
+		{"empty", "null", nil, "null"},
+		{"scalar", "4", &structuralschema.Structural{
+			Generic: structuralschema.Generic{
+				Default: structuralschema.JSON{"foo"},
+			},
+		}, "4"},
+		{"scalar array", "[1,null]", nil, "[1,null]"},
+		{"object array", `[{"a":null},{"b":null},{"c":null},{"d":null},{"e":null}]`, &structuralschema.Structural{
+			Items: &structuralschema.Structural{
+				Properties: map[string]structuralschema.Structural{
+					"a": {
+						Generic: structuralschema.Generic{
+							Default: structuralschema.JSON{"A"},
+						},
+					},
+					"b": {
+						Generic: structuralschema.Generic{
+							Nullable: true,
+						},
+					},
+					"c": {
+						Generic: structuralschema.Generic{
+							Default:  structuralschema.JSON{"C"},
+							Nullable: true,
+						},
+					},
+					"d": {
+						Generic: structuralschema.Generic{},
+					},
+				},
+			},
+		}, `[{"a":null},{"b":null},{"c":null},{},{"e":null}]`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var in interface{}
+			if err := json.Unmarshal([]byte(tt.json), &in); err != nil {
+				t.Fatal(err)
+			}
+
+			var expected interface{}
+			if err := json.Unmarshal([]byte(tt.expected), &expected); err != nil {
+				t.Fatal(err)
+			}
+
+			PruneNonNullableNullsWithoutDefaults(in, tt.schema)
+			if !reflect.DeepEqual(in, expected) {
+				var buf bytes.Buffer
+				enc := json.NewEncoder(&buf)
+				enc.SetIndent("", "  ")
+				err := enc.Encode(in)
+				if err != nil {
+					t.Fatalf("unexpected result mashalling error: %v", err)
+				}
+				t.Errorf("expected: %s\ngot: %s", tt.expected, buf.String())
+			}
+		})
+	}
+}