Merge pull request #8486 from JoelSpeed/update-required-fields-conventions

Update guidance on required field serialization

Author: k8s-ci-robot

contributors/devel/sig-architecture/api-conventions.md

@@ -822,50 +822,92 @@ indicated in the Retry-After header, if it is present).
 
 Fields must be either optional or required.
 
+A field that is required means that the writer must express an opinion about the value of the field.
+Required fields are always present, and readers can rely on the field being present in the object.
+
+An optional field is one where the writer may choose to omit the field entirely.
+Readers should not assume that the field is present, unless the field also has a server-side default value.
+
+Default values can only be set on optional fields.
+
 Optional fields have the following properties:
 
 - They have the `+optional` comment tag in Go.
 - They are a pointer type in the Go definition (e.g. `AwesomeFlag *SomeFlag`) or
 have a built-in `nil` value (e.g. maps and slices).
+- They are marked with the `omitempty` json struct tag in the Go definition.
 - The API server should allow POSTing and PUTing a resource with this field
 unset.
 
-In most cases, optional fields should also have the `omitempty` struct tag (the
-`omitempty` option specifies that the field should be omitted from the json
-encoding if the field has an empty value). However, If you want to have
-different logic for an optional field which is not provided vs. provided with
-empty values, do not use `omitempty` (e.g. https://github.com/kubernetes/kubernetes/issues/34641).
+When the field type has a built-in `nil` value, such as a map or a slice, and
+your use case means that you need to be able to distinguish between
+"field not set" and "field set to an empty value", you should use a pointer to
+the type, even though it has a built-in `nil` value.
+See https://github.com/kubernetes/kubernetes/issues/34641.
 
 Note that for backward compatibility, any field that has the `omitempty` struct
-tag will be considered to be optional, but this may change in the future and
-having the `+optional` comment tag is highly recommended.
+tag, and is not explicitly marked as `+required`, will be considered to be optional.
+This is expected to change in the future, and new fields should explicitly set either
+an `+optional` or `+required` comment tag.
 
-Required fields have the opposite properties, namely:
+Required fields have the following properties:
 
 - They do not have an `+optional` comment tag.
-- They do not have an `omitempty` struct tag.
-- They are not a pointer type in the Go definition (e.g. `AnotherFlag SomeFlag`).
+- They mark themselves as required explicitly with a `+required` comment tag.
 - The API server should not allow POSTing or PUTing a resource with this field
 unset.
+- They _typically_ do not use pointer types in the Go definition (e.g. `AnotherFlag SomeFlag`), though required fields where the zero value is a valid value must use pointer types, paired with an `omitempty` struct tag to avoid spurious null serializations.
+
+For more details on how to use pointers and `omitempty` with fields, see [Serialization of optional/required fields](#serialization-of-optionalrequired-fields).
 
 Using the `+optional` or the `omitempty` tag causes OpenAPI documentation to
 reflect that the field is optional.
 
+### Serialization of optional/required fields
+
 Using a pointer allows distinguishing unset from the zero value for that type.
-There are some cases where, in principle, a pointer is not needed for an
-optional field since the zero value is forbidden, and thus implies unset. There
-are examples of this in the codebase. However:
+There are some cases where, in principle, a pointer is not needed for a
+field since the zero value is forbidden, and thus implies unset.
+There are examples of this in the codebase. However:
+
+- It can be difficult for implementors to anticipate all cases where an empty
+value might need to be distinguished from a zero value.
+- Structs are not omitted from encoder output even where `omitempty` is specified,
+which is messy.
+
+To determine whether a field should be a pointer, consider the following:
+
+```mermaid
+graph TD;
+    A[Start] --> B{Is the zero value a valid user choice?};
+    B -- Yes --> C[Use a pointer and omitempty];
+    B -- No --> D{Is the field optional or required?};
+    D -- Optional --> E["Does the field type have a built-in nil value (map or slice)?"];
+    E -- Yes --> F[Do not use a pointer, use omitempty];
+    E -- No --> C;
+    D -- Required --> F
+```
+There are several implications of the above:
+- For lists and maps where the zero valid is a valid user choice, this means that `[]` and `{}` have a semantically different meaning than unset, in this case it is appropriate to use `*[]T` or `*map[T]S` respectively.
+- For `bool` types, the zero value is `false`, which is always a valid user choice. `bool` types should always be pointers, and should always use the `omitempty` tag.
+- When a field is required, and the zero value is not valid, a structured client who has not expressed an explicit choice will have their request rejected by the API server based on the invalid value, rather than the field being unset.
+  - For example, a string with a minimum length of 1; Validation would not understand if the field was unset, or set to the empty string deliberately, but would still reject the request because it did not meet the length requirements.
+  - Technically, using a pointer in these cases is also acceptable, but not advised as it makes coding more complex, and increases the risk of nil pointer exceptions.
+  - In these cases, not using `omitempty` provides the same result, but pollutes the marshaled object with zero values and is not recommended.
+- For structs, the zero value can only be valid when the struct has no required fields, and does not require at least one property to be set.
+  - Required structs should use `omitzero` to avoid marshalling the zero value.
+
+#### Serialization of custom resources
+
+When custom resources are admitted by the API server, openapi validation is applied to the object _prior_ to any structured client observing the object.
 
-- it can be difficult for implementors to anticipate all cases where an empty
-value might need to be distinguished from a zero value
-- structs are not omitted from encoder output even where omitempty is specified,
-which is messy;
-- having a pointer consistently imply optional is clearer for users of the Go
-language client, and any other clients that use corresponding types
+For a field where the zero value is not valid, the openapi validation will reject the object if the field is present and set to the zero value,
+before a controller or validation webhook could observe the field.
 
-Therefore, we ask that pointers always be used with optional fields that do not
-have a built-in `nil` value.
+This means that there is no need to distinguish in a custom resource, between unset and the zero value for fields where the zero value is not valid.
+In these cases, pointers are not needed, as the zero value indicates to the structured client that the field is unset.
 
+This can be beneficial to API authors, as it reduces the complexity of the API, and reduces the risk of nil pointer exceptions in controllers.
 
 ## Defaulting