You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copy file name to clipboardexpand all lines: contributors/design-proposals/support-union-in-api-server.md
+113-79
Original file line number
Diff line number
Diff line change
@@ -1,6 +1,104 @@
1
-
Add Tag Indicating To Replace *Union* Fields in Patch (a.k.a Support Replacing Maps in Strategic Merge Patch)
1
+
Support Unionin API Server
2
2
=============
3
3
4
+
Generalize support for Unions in the API server. Instead of having the unionness of the field
5
+
hard coded into the validation logic on a per-field basic, add first class support for validating unions.
6
+
7
+
## Proposed Changes
8
+
9
+
### APIs
10
+
11
+
**Scope**:
12
+
13
+
| Union Type | Supported |
14
+
|---|---|
15
+
| non-inlined non-discriminated union | Yes |
16
+
| discriminated union | Yes |
17
+
| inlined union with [patchMergeKey](https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md#strategic-merge-patch) only | Yes |
18
+
| other inlined union | No |
19
+
20
+
Add a go struct tag / openAPI value indicating the field is a union and what type of union.
21
+
22
+
- Tag `union:"oneof"` means this is non-inlined non-discriminated union: only one of the fields in this struct can be set.
23
+
24
+
- Tag `union:"discriminator/<discriminatorName>"` means this field is a union with a discriminator in the struct.
25
+
26
+
- Tag `union: "inlined/<patchMergeKey>"` means this is an inlined union with only a `patchMergeKey`
We don't make any changes on other inlined unions.
58
+
59
+
### Server Changes
60
+
61
+
- Validate that neither *PATCH* nor *UPDATE* set multiple values within a Union
62
+
- Add first class support for validating discriminators if there are any.
63
+
-*UPDATE* replaces the entire object, so there should be no need to clear anything in this case
64
+
65
+
For the inlined union that is not supported, we keep the validation code as it is.
66
+
67
+
### kubectl
68
+
69
+
When doing a *PATCH* to set one of the fields in a union, clear any other field in the union that
70
+
was previously set. The client explicitly expresses setting one field and clearing the other fields.
71
+
We provide the users a dry-run mode to let them check what is going to be sent to the server. E.g.
72
+
```yaml
73
+
containerstate:
74
+
waiting:
75
+
...
76
+
running: null
77
+
terminated: null
78
+
```
79
+
80
+
### Open API change
81
+
82
+
We would need to add extensions to the openapi spec we publish. This is something we already need to do for the `patchStrategy` and `mergeKey` struct tags.
83
+
84
+
### Docs
85
+
86
+
Update `API-conventions-md` to include:
87
+
```
88
+
we should avoid adding new inlined unions in the future.
89
+
```
90
+
91
+
## Summary
92
+
93
+
Limitation: We don't support inlined union types. Because the validator doesn't have
94
+
enough metadata to distinguish the inlined union fields and other non-union fields.
95
+
96
+
# Alternatives Considered
97
+
98
+
The proposals below are not mutually exclusive with the proposal above, and maybe can be added at some point in the future.
99
+
100
+
# 1. Add Tag Indicating To Replace *Union* Fields in Patch (a.k.a Support Replacing Maps in Strategic Merge Patch)
101
+
4
102
This approach could be used to fix the issues with `kubectl apply` simply by having apply always replace union fields instead of patching them.
5
103
6
104
## Proposed Changes
@@ -81,7 +179,7 @@ No required change.
81
179
82
180
### Open API
83
181
84
-
We would need to add extensions to the openapi spec we publish. This is something we already need to do for the `patchStrategy` and `mergeKey` struct tags.
182
+
Similar to section [Open API](#open-api-change)
85
183
86
184
### Strategic Merge Patch
87
185
@@ -91,78 +189,14 @@ We need to add additional logic to support:
91
189
- Generate the correct patch respecting the new tag, `patchStrategyForListEntry:"replace"`
92
190
- Replace the entire union respecting the new directive, `$listEntryStrategy: replace`.
93
191
94
-
### Docs
95
-
96
-
Update `API-conventions-md` to include:
97
-
```
98
-
we should avoid adding new inlined unions in the future.
99
-
```
100
192
101
193
## Summary
102
-
Option 1: it may limit the flexibility to add more fields in the parent struct that already has an inlined union.
194
+
Option 1: We lose the ability of *merging* the struct nested in the union field.
195
+
And it may limit the flexibility to add more fields in the parent struct that already has an inlined union.
103
196
**TODO**: we need to examine all the impacted APIs to check if it is safe to replace the entire parent struct.
104
197
105
198
Option 2: The limitation is that the metadata associated with the inlined APIs will not be reflected in the OpenAPI schema, because OpenAPI schemas flatten inlined structs.
106
199
107
-
# Alternatives Considered
108
-
109
-
The proposals below are not mutually exclusive with the proposal above, and maybe can be added at some point in the future.
110
-
111
-
# 1. Add Union Support to the API Server
112
-
113
-
## Proposed Changes
114
-
115
-
### APIs
116
-
117
-
Add a go struct tag / openAPI value telling the field is a union.
118
-
119
-
Tag `union:"oneof"` means only one of the fields in this struct can be set.
120
-
121
-
Tag `union:"discriminator"` means this field is a union with a discriminator in the struct. E.g. `union:"discriminator/type"` indicates the struct has a discriminator with name `type`.
122
-
123
-
Example of non-discriminated union:
124
-
```go
125
-
type ContainerStatus struct {
126
-
...
127
-
State ContainerState `json:"state,omitempty" protobuf:"bytes,2,opt,name=state" union: "oneof"`
Generalize support for Unions in the server. Instead of having the union-ness of the field hard coded into the validation logic on a per-field basic, add first class support for validating unions.
143
-
144
-
- Validate that neither *PATCH* nor *UPDATE* set multiple values within a Union
145
-
-*Maybe* add first class support for validating discriminators if there are any.
146
-
-*UPDATE* replaces the entire object, so there should be no need to clear anything in this case
147
-
148
-
### kubectl
149
-
150
-
When doing a *PATCH* to set one of the fields in a union, clear any other field in the union that was previously set. The client explicitly expresses setting one field and clearing the other fields. We provide the users a dry-run mode to let them check what is going to be sent to the server. E.g.
151
-
```yaml
152
-
containerstate:
153
-
waiting:
154
-
reason: waitingReason
155
-
message: watingMessage
156
-
running: null
157
-
terminated: null
158
-
```
159
-
160
-
### Open API change
161
-
162
-
Similar to section [Open API](#open-api)
163
-
164
-
The limitation is that it will be very hard to make it work for inline union types. the validator doesn't have enough metadata to distinguish the inlined union fields and other non-union fields.
165
-
166
200
# 2. Add Discriminators in All Unions/OneOf APIs
167
201
168
202
Original issue is described in kubernetes/kubernetes#35345
@@ -200,15 +234,15 @@ No change required on kubectl.
200
234
201
235
## Summary
202
236
203
-
Limitation is that automatically clear fields based on discriminator may be unsafe.
237
+
Limitation: Server-side automatically clearing fields based on discriminator may be unsafe.
It is inlined. Besides `VolumeSource`. its parent [Volume](https://github.com/kubernetes/kubernetes/blob/v1.5.2/pkg/api/v1/types.go#L222) has `Name`.
It is inlined. Besides `PersistentVolumeSource`, its parent [PersistentVolumeSpec](https://github.com/kubernetes/kubernetes/blob/v1.5.2/pkg/api/v1/types.go#L442) has the following fields:
It is inlined. Besides `Handler`, its parent struct [`Probe`](https://github.com/kubernetes/kubernetes/blob/v1.5.2/pkg/api/v1/types.go#L1297) also has the following fields:
It says "exactly one of the following must be set". But it has only one field.
255
289
It is inlined. Its parent [`IngressRule`](https://github.com/kubernetes/kubernetes/blob/v1.5.2/pkg/apis/extensions/v1beta1/types.go#L848) also has the following fields:
0 commit comments