More feedback :-)

Author: apelisse

keps/sig-api-machinery/1929-built-in-default/README.md

@@ -197,14 +197,15 @@ meaning, they do not "look" the same and are hard to compare for
 clients. They are also currently treated differently by Kubernetes for
 defaulting purposes.
 
-We propose to start removing fields with a null value, when the openapi
-nullable flag is missing or false. We assume that existing APIs that use
-nullable are happy with null values. Also, people can remove the
-nullable attribute if they want null values to start being removed.
-This will help aligning CRDs and built-in types (there are no ways to
-set nullable to true for built-in types). If a non-nullable defaulted
-field is set to null, the null value will be removed by the apiserver,
-and then the default will be applied.
+Currently, null values trigger an error when nullable is false and don't
+get defaulted if a default is available. We propose to start defaulting
+fields with null values if a default is available, or the field will be
+removed if no default exists, but only when the openapi nullable flag is
+missing or false. We assume that existing APIs that use nullable are
+happy with null values. Also, people can remove the nullable attribute
+if they want null values to start being removed. This will help
+aligning CRDs and built-in types (there are no ways to set nullable to
+true for built-in types).
 
 Note that we still consider `{}` (and 0, 0.0, "", false, []) to be
 different from `null`, so `{"foo": {}}` is different from `{}` or from
@@ -438,9 +439,9 @@ Object:
     invalid:
       type: string
       default: "default-name"
-	  # This field would only be present for CRDs, for built-in,
-	  # the empty value would be accepted and treated as missing.
-	  minLength: 1
+      # This field would only be present for CRDs, for built-in,
+      # the empty value would be accepted and treated as missing.
+      minLength: 1
     defaulted:
       type: int
       default: false
@@ -457,14 +458,19 @@ Which, for both CRD types and built-in types, would generate the following behav
 {"name": "default-name", "number": 0}
 ```
 
-Note that a simplified version of the generated code for built-ins would look like this:
+Note that a simplified, and non-cached version of the generated code for
+built-ins would look like this:
 ```golang
 func GenerateDefault(obj *Object) {
      if obj.Name == "" {
-          obj.Name = "default-name"
+          if err := json.Unmarshal([]byte(`"default-name"`), &obj.Name); err != nil {
+               panic(err)
+          }
      }
      if obj.Number == 0 {
-          obj.Number = 0
+          if err := json.Unmarshal([]byte(`0`), &obj.Number); err != nil {
+               panic(err)
+          }
      }
 }
 ```
@@ -500,8 +506,14 @@ the additional defaulting functions, automatically call them prior to
 the existing defaulting functions
 2. Update kube-openapi to parse the `+default` marker and include it in
 the OpenAPI definition.
-3. Implement the new CRD semantics that clears `null` values from
-objects when nullable is unset or false.
+3. Implement the new CRD semantics for properties with nullable unset or
+false (which previously would have resulted in a validation error that
+null is not a valid value):
+- properties and list items with a default have the default applied,
+replacing the explicit null value
+- list items without a default let the explicit null value remain,
+resulting in a validation error (matches current behavior)
+- properties without a default have the explicit null value removed
 4. Add additional "default" field to sigs.k8s.io/structured-merge-diff
 internal schema, and parse that field from the OpenAPI, use it when the
 map key is missing. The code for this already exists in a