changed mind about oenps

Author: alexzielenski

keps/sig-api-machinery/4153-declarative-validation/README.md

@@ -799,7 +799,7 @@ We will need to add the ability for fields that re-use a type to specify a
 field-level for its nested fields. Kube-openapi gen will inline the custom 
 definition.
 
-#### Representing Go-isms in Schema
+#### Zero values, `null`, and `omitempty`
 
 Our native types are backed by Go and serialized into Go for mutation, defaulting
 and other transformations before being used for validation. The schemas presented
@@ -822,7 +822,7 @@ and propose additional schema annotations to make our native type schemas more
 representative of their implicit constraints to ensure all validations are
 ported correctly.
 
-##### Dropped Fields in Unstructured Conversion
+##### `omitempty` Primitives Have Incorrect Schema
 
 Consider the following omitempty non-pointer field from `DeploymentSpec`:
 
@@ -857,23 +857,25 @@ field not to be considered during validation, even if it was included.
 This would surface as:
 1. `has(self.paused)` in CEL on the Container object always returns `false` if
 that value is `false`. But a CRD with identical schema would return `true`.
+A proof of concept example of how this bug could bite users of ValidatingAdmissionPolicy 
+is [here](https://github.com/kubernetes/kubernetes/pull/120244).
 2. CEL and JSONSchema Validations of the field if they exist also will not run.
+But this is not a concern as long as empty values are assumed to match the schema.
 
-This class of problem exclusively affects `omitempty` non-pointer fields: pointers
-to the openapi-generater are treated as optionals and nil used to disambiguate
-set/unset.
+There are two alternative approaches to address the difference in behavior
+of native types vs CRDs in the schema.
 
-There are two alternative approaches to address this in the schema:
+1. Express in schema that zero values are considered unset: by adding an 
+x-kubernetes extension like `x-kubernetes-zero-is-unset: true` to fields where
+ this is true.
+2. Express in schema that unset values are considered zero: by settings its
+`default` to the empty value for the type.
+
+Option 2 has the drawback of applying a default to fields where the zero
+value is not considered a default. For cases like `Paused` this is correct,
+but for most cases involving `string`, the empty string is treated as `unset`.
 
-1. Express in schema that zero values are considered unset by adding
-an x-kubernetes extension like `x-kubernetes-zero-unset: true` to fields where
-this is true.
-2. Express in schema that unset values are considered zero by settings
-its `default` to the empty value.
 
-Option 2 seems best, since this is already done for `struct` types, there is 
-already support for `default` in OpenAPI, and the solution naturally expresses 
-Go's behavior of `unset` being implicitly defaulted to zero.
 
 An exhaustive list of all 730 omit-empty non-pointer is [found here](https://gist.github.com/alexzielenski/05f594f3974aec9b3e74620b8dc7dde1). Struct types already have their
 defaults populated with the go zero value, so they are excluded from this list.