Skip to content

Commit 5001bfc

Browse files
committed
KEP 1693: extension warnings, GA criteria
1 parent 9b3a152 commit 5001bfc

File tree

1 file changed

+140
-39
lines changed
  • keps/sig-api-machinery/1693-warnings

1 file changed

+140
-39
lines changed

keps/sig-api-machinery/1693-warnings/README.md

Lines changed: 140 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,20 @@
55
- [Summary](#summary)
66
- [Motivation](#motivation)
77
- [Goals](#goals)
8-
- [Non-Goals](#non-goals)
98
- [Proposal](#proposal)
109
- [Server-side changes](#server-side-changes)
1110
- [Client-side changes](#client-side-changes)
1211
- [Design Details](#design-details)
12+
- [Server-side](#server-side)
13+
- [Client-side](#client-side)
1314
- [Test Plan](#test-plan)
1415
- [Risks and Mitigations](#risks-and-mitigations)
1516
- [Graduation Criteria](#graduation-criteria)
16-
- [Beta graduation](#beta-graduation)
17-
- [GA graduation](#ga-graduation)
17+
- [Beta](#beta)
18+
- [GA](#ga)
1819
- [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy)
1920
- [Version Skew Strategy](#version-skew-strategy)
21+
- [References](#references)
2022
- [Implementation History](#implementation-history)
2123
<!-- /toc -->
2224

@@ -40,10 +42,10 @@ checklist items _must_ be updated for the enhancement to be released.
4042
- [x] Design details are appropriately documented
4143
- [x] Test plan is in place, giving consideration to SIG Architecture and SIG Testing input
4244
- [x] Graduation criteria is in place
45+
- [x] Supporting documentation e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes
4346
- [ ] KEP approvers have approved the KEP status as `implementable`
4447
- [ ] "Implementation History" section is up-to-date for milestone
4548
- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io]
46-
- [ ] Supporting documentation e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes
4749

4850
<!--
4951
**Note:** This checklist is iterative and should be reviewed and updated every time this enhancement is being considered for a milestone.
@@ -55,9 +57,9 @@ checklist items _must_ be updated for the enhancement to be released.
5557
[kubernetes/website]: https://git.k8s.io/website
5658

5759
## Summary
58-
60+
5961
This enhancement makes it easier for users and cluster administrators
60-
to recognize and remedy use of deprecated APIs.
62+
to recognize and remedy problematic API use, including use of deprecated APIs.
6163
Users are presented with informative warnings at time of use.
6264
Administrators are given metrics that show deprecated API use,
6365
and audit annotations that can be used to identify particular API clients.
@@ -68,6 +70,11 @@ Kubernetes has many deprecations in flight at any given moment, in various stage
6870
Keeping track of all of them is difficult, and has historically required careful reading of release notes,
6971
and manually sweeping for use of deprecated features.
7072

73+
Gaps in API validation which allow problematic API data have also been discovered post-release
74+
([see examples](https://github.com/kubernetes/kubernetes/issues/64841#issuecomment-395141013)).
75+
Tightening validation to reject previously accepted API requests is generally avoided for backwards compatibility,
76+
but surfacing warnings to clients submitting problematic data would help them discover problems earlier.
77+
7178
### Goals
7279

7380
* When a user makes a request to a deprecated API, present them with a warning
@@ -77,16 +84,8 @@ and manually sweeping for use of deprecated features.
7784
* Filtered to particular operations (e.g. get, list, create, update)
7885
* Filtered to APIs targeting removal in particular releases
7986
* Allow a cluster administrator to programatically identify particular clients using deprecated APIs
80-
81-
### Non-Goals
82-
83-
While the proposed warning mechanism is generic enough to carry arbitrary warnings,
84-
the following items are out of scope for the first iteration of this feature:
85-
86-
* Allowing extension mechanisms like admission webhooks to contribute warnings
87-
* Surfacing warnings about other non-fatal problems
88-
(for example, [problematic API requests](http://issue.k8s.io/64841#issuecomment-395141013)
89-
that cannot be rejected for compatibility reasons)
87+
* Allow in-process validation mechanisms (API validation, admission, etc) to contribute warnings
88+
* Allow extension mechanisms (CustomResourceDefinitions, admission webhooks) to contribute warnings
9089

9190
## Proposal
9291

@@ -98,6 +97,10 @@ When a deprecated API is used:
9897
2. Increment a counter metric with labels for the API group, version, resource, API verb, and target removal major/minor version
9998
3. Record an audit annotation indicating the request used a deprecated API
10099

100+
Allow custom resource definitions to indicate specific versions are deprecated
101+
102+
Allow admission webhooks to contribute warnings that are surfaced to the user
103+
101104
### Client-side changes
102105

103106
In client-go:
@@ -114,43 +117,114 @@ In kubectl, configure the per-process handler to:
114117

115118
1. dedupe warnings (only print a given warning once per invocation)
116119
2. log to stderr with a `Warning:` prefix
117-
3. color the `Warning:` prefix if stderr is a terminal and `$TERM != "dumb"` and `$NO_COLOR` is unset
120+
3. color the `Warning:` prefix if the terminal is capable of displaying color
121+
4. add a `--warnings-as-errors` option to exit with a non-zero exit code after executing the command if warnings were encountered
118122

119123
In kube-apiserver and kube-controller-manager, configure the process-wide handler to ignore warnings
120124

121125
## Design Details
122126

123-
Server-side:
127+
### Server-side
128+
129+
**Warning mechanism:**
124130

125131
* Add a handler chain filter that attaches a WarningRecorder implementation to the request context
126132
* Add a WarningRecorder implementation that de-duplicates the warning message per request and writes a `Warning` header
127133
* the header structure is defined in [RFC2616#14.46](https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.46)
128-
* warnings would be written with code `299` and warn-agent of `-`
129-
* In the endpoint installer, decorate handlers for deprecated APIs:
134+
* warnings are written with code `299` and warn-agent of `-`
135+
* no more than 4K of warning messages are written to avoid exceeding client and proxy header size limits;
136+
if the total warning size exceeds that limit, individual warnings are truncated to 256 characters;
137+
if the total warning size still exceeds that limit, warnings are output in the order added until the limit is reached;
138+
for reference, the longest proposed deprecation warning generated by `kube-apiserver` would be ~170 characters,
139+
so ~24 warning messages of that length could be accommodated
140+
141+
**In-process deprecated API warnings:**
142+
143+
In the endpoint installer, decorate handlers for deprecated APIs:
144+
* add a warning header: `<group>/<version> <kind> is deprecated in v1.X+, unavailable in v1.Y+; use <replacementGroup>/<replacementVersion> <replacementKind>`
145+
* increment the counter metric for the deprecated use
146+
* add an audit annotation indicating the request was made to a deprecated API
147+
148+
**In-process validation and admission warnings:**
149+
150+
Enable adding warnings in validation, strategy, and admission handlers:
151+
* Document when warnings should be surfaced (e.g. when a problematic value is present in a create request,
152+
or is introduced by an update request)
153+
* Define a helper method for adding a field-specific warning that takes a `field.Path` and message
154+
155+
**CustomResourceDefinition version deprecation warnings:**
156+
157+
Add `deprecated bool` and `deprecationWarning string` fields to the `CustomResourceDefinitionVersion` type:
158+
159+
```go
160+
type CustomResourceDefinitionVersion struct {
161+
...
162+
// deprecated indicates this version of the custom resource API is deprecated.
163+
// When set to true, API requests to this version receive a warning header in the server response,
164+
// and an audit annotation indicating a deprecated API was used is added to the audit event for the request.
165+
// Defaults to false.
166+
// +optional
167+
Deprecated bool `json:"deprecated,omitempty"`
168+
// deprecationWarning overrides the default warning returned to API clients.
169+
// May only be set when `deprecated` is true.
170+
// The default warning indicates this version is deprecated and recommends use
171+
// of the newest served version of equal or greater stability, if one exists.
172+
// +optional
173+
DeprecationWarning string `json:"deprecationWarning,omitempty"`
174+
...
175+
```
176+
177+
In the custom resource handler, decorate handlers for deprecated versions:
130178
* add the warning header
131179
* increment the counter metric for the deprecated use
132180
* add an audit annotation indicating the request was made to a deprecated API
133-
134-
Client-side:
181+
182+
**Admission webhook warnings:**
183+
184+
Add `warnings []string` to the `v1` `AdmissionResponse` type:
185+
186+
```go
187+
type AdmissionResponse struct {
188+
...
189+
// warnings is a list of warning messages to return to the API client.
190+
// Warning messages describe a problem the client making the API request should correct or be aware of.
191+
// Limit warnings to 120 characters if possible.
192+
// Longer warnings or large numbers of warnings may be truncated.
193+
// +optional
194+
Warnings []string `json:"warnings,omitempty"`
195+
...
196+
}
197+
```
198+
199+
### Client-side
200+
201+
k8s.io/client-go:
135202
136203
* Parse `Warning` headers in server responses
137204
* Ignore malformed warning headers, ensuring that this enhancement
138205
will not cause any previously successful API request to fail
139-
* Ignore warnings with codes other than `299`
140206
* Add the parsed warning headers to the `rest.Result` struct
141207
* Pass parsed warnings through the per-client or per-process warning handler
142208
143209
### Test Plan
144210
145-
- Unit tests
146-
- `Warning` header generation
147-
- `Warning` header parsing (including tolerating/ignoreing malformed headers)
148-
- per-process / per-client warning handler precedence is honored
149-
- Integration tests
150-
- warning headers are returned when making requests to deprecated APIs
151-
- deprecated metrics are incremented when making requests to deprecated APIs
152-
- audit annotations are added when making requests to deprecated APIs
153-
211+
Warning mechanism unit tests:
212+
213+
- `Warning` header generation and encoding
214+
- `Warning` header parsing (including tolerating/ignoring malformed headers)
215+
- Per-process / per-client warning handler precedence is honored
216+
217+
Deprecated API integration tests:
218+
219+
- Warning headers are returned when making requests to deprecated APIs
220+
- Deprecated metrics are incremented when making requests to deprecated APIs
221+
- Audit annotations are added when making requests to deprecated APIs
222+
223+
Extension mechanism integration tests:
224+
225+
- CustomResourceDefinition deprecated versions result in warning headers and audit annotations
226+
- Admission webhook warnings are included in API responses to users
227+
154228
### Risks and Mitigations
155229
156230
**Metric cardinality**
@@ -169,6 +243,14 @@ Additional warning messages may be unexpected by kubectl or client-go consumers.
169243
However, kubectl and client-go already output warning messages to stderr or via `klog.Warn`.
170244
client-go consumers can programmatically modify or suppress the warning output at a per-process or per-client level.
171245
246+
**Header size**
247+
248+
The possibility of accumulating excessive numbers of warnings increases once extension mechanisms can contribute warnings.
249+
This is mitigated by:
250+
* Documented guidance on what types of issues should be surfaced as warnings
251+
* Documented guidance on message length
252+
* A warning recorder implementation that limits the individual and total length of warnings returned to a client
253+
172254
### Graduation Criteria
173255
174256
The structure of the `Warning` header is RFC-defined and unversioned.
@@ -182,18 +264,25 @@ drive automated action in clients, graduation criteria is primarily oriented
182264
toward the stability level of the administrator metrics, and the ability to
183265
disable the server sending warnings during the beta period.
184266
185-
#### Beta graduation
186-
187-
* Test plan is implemented
267+
#### Beta
268+
269+
* Implement warning mechanism
270+
* Implement in-process deprecated API warnings, metrics, audit annotations
271+
* Complete test plan for implemented items
188272
* API server output of `Warning` headers for deprecated API use is feature-gated and enabled by default
189273
* The metric for deprecated API use is registered at [stability level `ALPHA`](https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/20190404-kubernetes-control-plane-metrics-stability.md#stability-classes)
190-
* client-go logs warnings by default
191-
* kubectl outputs warnings to stderr
274+
* client-go logs warnings with code `199` and `299` by default
275+
* kubectl outputs warnings with code `199` and `299` to stderr by default
192276
193-
#### GA graduation
277+
#### GA
194278
195279
* At least two releases after Beta
196-
* Gathered feedback on metric structure and use from multi-cluster admins
280+
* Gather feedback on metric structure and use from multi-cluster admins
281+
* Implement in-process helpers for field-level validation warnings and admission warnings
282+
* Implement admission webhook warning contributions
283+
* Implement custom resource version deprecation
284+
* Complete test plan for implemented items
285+
* Custom resource and admission webhook documentation is extended to describe appropriate use of warnings
197286
* API server output of `Warning` headers for deprecated API use is unconditionally enabled
198287
* Server metric for deprecated API use is registered at [stability level `STABLE`](https://github.com/kubernetes/enhancements/blob/master/keps/sig-instrumentation/20190404-kubernetes-control-plane-metrics-stability.md#stability-classes)
199288
@@ -208,6 +297,18 @@ Old clients making requests to a new API server ignore `Warning` headers.
208297
209298
New clients making requests to old API servers handle requests without `Warning` headers normally.
210299
300+
## References
301+
302+
* sig-cli discussion
303+
* [notes](https://docs.google.com/document/d/1r0YElcXt6G5mOWxwZiXgGu_X6he3F--wKwg-9UBc29I/edit#bookmark=id.e5451knpumhz)
304+
* [recording](https://www.youtube.com/watch?v=ZDSeDx9XbZg&list=PL69nYSiGNLP28HaTzSlFe6RJVxpFmbUvF&index=2&t=6m50s)
305+
* sig-apimachinery discussion
306+
* [notes](https://docs.google.com/document/d/1x9RNaaysyO0gXHIr1y50QFbiL1x8OWnk2v3XnrdkT5Y/edit#bookmark=id.evxgyhkofnhl)
307+
* [recording](https://www.youtube.com/watch?v=-EZzIk-Ut6o&list=PL69nYSiGNLP21oW3hbLyjjj4XhrwKxH2R&index=2&t=0s)
308+
* sig-architecture discussion
309+
* [notes](https://docs.google.com/document/d/1BlmHq5uPyBUDlppYqAAzslVbAO8hilgjqZUTaNXUhKM/edit#bookmark=id.yppt0g2tjwqw)
310+
211311
## Implementation History
212312
213313
- 2020-04-16: KEP introduced
314+
- 2020-04-27: Updated GA criteria, added extension mechanism warnings, kubectl warnings-as-errors option, size limits

0 commit comments

Comments
 (0)