Adding warning headers #19
Conversation
pkg/types/server_types.go
Outdated
| @@ -156,10 +157,16 @@ func (r *APIRequest) Option(key string) string { | |||
| } | |||
|
|
|||
| func (r *APIRequest) WriteResponse(code int, obj APIObject) { | |||
| for _, warning := range obj.Warnings { | |||
| r.Response.Header().Add("Warning", fmt.Sprintf("%d - %s", warning.Code, warning.Text)) | |||
There was a problem hiding this comment.
Looks like the same format k8s uses 👍
| @@ -229,16 +236,24 @@ type APIEvent struct { | |||
| Data interface{} `json:"data,omitempty"` | |||
| } | |||
|
|
|||
| type Warning struct { | |||
| Code int | |||
| Agent string | |||
There was a problem hiding this comment.
I don't see this being used here or in the steve PR, is it necessary?
There was a problem hiding this comment.
Yeah, the steve side does keep a slice of these in the "WarningBuffer"
There was a problem hiding this comment.
I see it adding it to the slice https://github.com/rancher/steve/pull/69/files#diff-82b79befb09b4c7baa9a34e0412389dc2be544a3e56086726419bbf939a9e9b9R74 but where does it actually use it? Only the Code and Text are used in the warning message string.
There was a problem hiding this comment.
Oh...your comment was specifically on "Agent"...sorry...it was early. It might not be necessary, I'll check.
There was a problem hiding this comment.
Upon further review, the "agent" is part of the spec for warning headers, even though it's likely to be set to "-", I think we should keep it. I will add it to what is logged just in case it ever proves to be useful.
crobby
force-pushed
the
warningheaders
branch
from
2d76696 to
97d4cef
Compare
Changes: - Adds warnings as a value of types.APIObject and types.APIObjectList - Adds functionality to write warning headers to responses in a k8s-like fashion
crobby
force-pushed
the
warningheaders
branch
from
97d4cef to
00215d2
Compare
samjustus
requested review from
maxsokolovsky
and removed request for
KevinJoiner
| @@ -156,10 +157,16 @@ func (r *APIRequest) Option(key string) string { | |||
| } | |||
|
|
|||
| func (r *APIRequest) WriteResponse(code int, obj APIObject) { | |||
| for _, warning := range obj.Warnings { | |||
| r.Response.Header().Add("Warning", fmt.Sprintf("%d %s %s", warning.Code, warning.Agent, warning.Text)) | |||
There was a problem hiding this comment.
Nit: looks like the K8s format uses a dash after the code. For example:
Warning: 299 - "policy/v1beta1 PodSecurityPolicy is deprecated in v1.21+, unavailable in v1.25+"
But I am not sure how the agent's string comes into play here.
There was a problem hiding this comment.
See #19 (comment) the agent may itself just be the string "-"
Changes:
rancher/rancher#39413
Required for rancher/steve#69