-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Update guidance on required field serialization #8486
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -822,50 +822,80 @@ 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 having the `+optional` comment tag is | ||||||
now required on new optional fields. | ||||||
|
||||||
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 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. q: should this (openAPI documentation) change now that we can have omitempty on required fields? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will need to make sure that it respects In the long run, we need to make sure all fields are marked as |
||||||
reflect that the field is optional. | ||||||
|
||||||
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: | ||||||
|
||||||
- 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 | ||||||
|
||||||
Therefore, we ask that pointers always be used with optional fields that do not | ||||||
have a built-in `nil` value. | ||||||
### 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 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?}; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
D -- Optional --> E[Does the field have a built-in nil value?]; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would it be simpler to inline the types which do have built-in nil values? "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; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
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 marhsalled object with zero values and is not recommended. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
- 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. | ||||||
|
||||||
## Defaulting | ||||||
|
||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe rephrase this to avoid interleaving
optional ... required ... optional
wording (even though it refers to different contexts)Maybe something like this: