Further address feedback

Author: apelisse

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

@@ -176,9 +176,11 @@ some defaulting that doesn't require to be done as code though.
 It is important to us that the OpenAPI definition of a type should
 result in the same behavior, whether the type is built-in or a CRD.
 
-In order to do that, we need to change how CRDs are defaulted, and
-create a defaulting mechanism for built-in that would behave the same
-way.
+In order to do that, we need to change how CRDs are defaulted, and place
+specific requirements on definition of built-in types (specifically
+around omitempty on non-pointer scalars, and an implicit default of {}
+on non-pointer struct fields), and create a defaulting mechanism for
+built-in that would behave the same way.
 
 ### CRD Normalization
 
@@ -202,8 +204,8 @@ 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. This means that will start accepting inputs that were
-rejected in the past (there is code that would work with a new server
-but not with an old server). We assume that existing APIs that use
+rejected in the past. There is code that would work with a new server
+but not with an old server. 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
@@ -255,13 +257,13 @@ while still allowing more complex expressions:
 // +default=[{"port": 80, "name": "http"}]
 ```
 
-### Built-in Defaulting
+### Defaulting mechanisms
 
 Because built-in types are serialized and deserialized as Go structures,
 there are semantics that are imposed on us. The two main ones are:
 1. We can't differentiate between null, unspecified and zero-value,
 2. non-pointer structs are always present (and hence have to be
-recursively defaulted), they also can't be omitEmpty'd.
+recursively defaulted), they also can't be omitempty'd.
 
 The first point is solved by the removed nulls described in the previous
 section (and invalid zero-values can't be present in objects), and the
@@ -270,9 +272,9 @@ default. To address that, we're proposing that the `default` marker is
 forbidden on non-pointer structs (pointer structs can behave as
 expected) and the default field of the openapi is automatically set to
 `{}`. This only applies to OpenAPI generated from built-in types or from
-kubebuilder. CRDs can still have any defaulting they have for
-structures, but we strongly suggest that OpenAPI generated from Go types
-should follow that semantics.
+kubebuilder. Existing CRD defaults defined for type=object properties
+will continue to be supported, but we strongly suggest that OpenAPI
+generated from Go types should follow that semantics.
 
 Computed defaults (through defaulting functions) will not be supported
 as a declarative marker and will continue being supported through the
@@ -400,14 +402,10 @@ which would have the following behavior, both for CRD types and built-in types:
 {"entry": {"name": "other-name", "number": 0}}
 ```
 
-#### Non-pointer fields
+#### Non-pointer scalar fields
 
-Non-pointer fields are also important, since they have a specific
-unmarshalling mechanism in Go.
-
-For CRDs, invalid zero-values are omitted, otherwise the objects would
-technically be invalid. We can safely default these field if they are
-unspecified.
+Non-pointer scalar fields also have a specific unmarshalling mechanism in Go,
+since they always have a value.
 
 For built-ins, we can differentiate two cases:
 1. The zero-value for the field is invalid (e.g. if empty string is not a valid value)
@@ -418,24 +416,18 @@ Both these cases will be handled properly, here's an example:
 type Object struct {
      // This field has an invalid zero-value, empty string is not a
      // valid string.
-     //
-     // If this is a CRD, there would be a kubebuilder specific tag, and
-     // if this is a built-in, there would be a specific validation function,
-     // but basically:
-     // +minLength=1
-     //
      // +default="default-name"
-     Name string `json:",omitempty"`
+     Name string `json:"name,omitempty"`
 
-     // This field is defaulted to 0 with no defaulter.
+     // This field would be defaulted to 0 without a default.
      // +default=0
      Defaulted int
 }
 ```
 
-Non-pointer fields that have a default must be omitEmpty. Kubebuilder
-and built-in defaulting generators should both prevent these from
-happening.
+Non-pointer scalar fields that have a default must be omitempty (unless
+their default is the zero-value). Kubebuilder and built-in defaulting
+generators should both require this.
 
 This would generate the following OpenAPI:
 ```yaml
@@ -445,26 +437,23 @@ 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
     defaulted:
-      type: int
+      type: integer
       default: false
 ```
 
 Which, for both CRD types and built-in types, would generate the following behavior:
 ```python
 >>> default('{}')
-{"entry": {"name": "default-name", "number": 0}}
->>> default('{name: other-name}')
-{"name": "other-name", "number": 0}
+{"entry": {"invalid": "default-name", "number": 0}}
+>>> default('{invalid: other-name}')
+{"invalid": "other-name", "defaulted": 0}
 # For built-in, additionally, we would have:
->>> default('{name: ""}')
-{"name": "default-name", "number": 0}
+>>> default('{invalid: ""}')
+{"invalid": "default-name", "defaulted": 0}
 ```
 
-Note that a simplified, and non-cached version of the generated code for
+Note that a simplified version, which deserializes on every call, of the generated code for
 built-ins would look like this:
 ```golang
 func GenerateDefault(obj *Object) {